Merge branch 'release/v1.2.0'

8 files changed, 68 insertions, 24 deletions

diff --git a/server/lib/emailer.ts b/server/lib/emailer.ts
index f384a254e..99010f473 100644
--- a/server/lib/emailer.ts
+++ b/server/lib/emailer.ts
@@ -361,7 +361,8 @@ class Emailer {
       'PeerTube.'
 
     const emailPayload: EmailPayload = {
-      from: fromEmail,
+      fromDisplayName: fromEmail,
+      replyTo: fromEmail,
       to: [ CONFIG.ADMIN.EMAIL ],
       subject: '[PeerTube] Contact form submitted',
       text
@@ -370,16 +371,21 @@ class Emailer {
     return JobQueue.Instance.createJob({ type: 'email', payload: emailPayload })
   }
 
-  sendMail (to: string[], subject: string, text: string, from?: string) {
+  sendMail (options: EmailPayload) {
     if (!Emailer.isEnabled()) {
       throw new Error('Cannot send mail because SMTP is not configured.')
     }
 
+    const fromDisplayName = options.fromDisplayName
+      ? options.fromDisplayName
+      : CONFIG.WEBSERVER.HOST
+
     return this.transporter.sendMail({
-      from: from || CONFIG.SMTP.FROM_ADDRESS,
-      to: to.join(','),
-      subject,
-      text
+      from: `"${fromDisplayName}" <${CONFIG.SMTP.FROM_ADDRESS}>`,
+      replyTo: options.replyTo,
+      to: options.to.join(','),
+      subject: options.subject,
+      text: options.text
     })
   }
 
diff --git a/server/lib/job-queue/handlers/email.ts b/server/lib/job-queue/handlers/email.ts
index 220d0af32..2ba39a156 100644
--- a/server/lib/job-queue/handlers/email.ts
+++ b/server/lib/job-queue/handlers/email.ts
@@ -6,14 +6,16 @@ export type EmailPayload = {
   to: string[]
   subject: string
   text: string
-  from?: string
+
+  fromDisplayName?: string
+  replyTo?: string
 }
 
 async function processEmail (job: Bull.Job) {
   const payload = job.data as EmailPayload
   logger.info('Processing email in job %d.', job.id)
 
-  return Emailer.Instance.sendMail(payload.to, payload.subject, payload.text, payload.from)
+  return Emailer.Instance.sendMail(payload)
 }
 
 // ---------------------------------------------------------------------------
diff --git a/server/lib/notifier.ts b/server/lib/notifier.ts
index d1b331346..2fa320cd7 100644
--- a/server/lib/notifier.ts
+++ b/server/lib/notifier.ts
@@ -148,6 +148,8 @@ class Notifier {
 
   private async notifyOfCommentMention (comment: VideoCommentModel) {
     const usernames = comment.extractMentions()
+    logger.debug('Extracted %d username from comment %s.', usernames.length, comment.url, { usernames, text: comment.text })
+
     let users = await UserModel.listByUsernames(usernames)
 
     if (comment.Video.isOwned()) {
diff --git a/server/models/video/video-comment.ts b/server/models/video/video-comment.ts
index cf6278da7..1163f9a0e 100644
--- a/server/models/video/video-comment.ts
+++ b/server/models/video/video-comment.ts
@@ -466,31 +466,41 @@ export class VideoCommentModel extends Model<VideoCommentModel> {
   }
 
   extractMentions () {
-    if (!this.text) return []
+    let result: string[] = []
 
     const localMention = `@(${actorNameAlphabet}+)`
     const remoteMention = `${localMention}@${CONFIG.WEBSERVER.HOST}`
 
+    const mentionRegex = this.isOwned()
+      ? '(?:(?:' + remoteMention + ')|(?:' + localMention + '))' // Include local mentions?
+      : '(?:' + remoteMention + ')'
+
+    const firstMentionRegex = new RegExp(`^${mentionRegex} `, 'g')
+    const endMentionRegex = new RegExp(` ${mentionRegex}$`, 'g')
     const remoteMentionsRegex = new RegExp(' ' + remoteMention + ' ', 'g')
-    const localMentionsRegex = new RegExp(' ' + localMention + ' ', 'g')
-    const firstMentionRegex = new RegExp('^(?:(?:' + remoteMention + ')|(?:' + localMention + ')) ', 'g')
-    const endMentionRegex = new RegExp(' (?:(?:' + remoteMention + ')|(?:' + localMention + '))$', 'g')
 
-    return uniq(
-      [].concat(
-        regexpCapture(this.text, remoteMentionsRegex)
-          .map(([ , username ]) => username),
+    result = result.concat(
+      regexpCapture(this.text, firstMentionRegex)
+        .map(([ , username1, username2 ]) => username1 || username2),
 
-        regexpCapture(this.text, localMentionsRegex)
-          .map(([ , username ]) => username),
+      regexpCapture(this.text, endMentionRegex)
+        .map(([ , username1, username2 ]) => username1 || username2),
+
+      regexpCapture(this.text, remoteMentionsRegex)
+        .map(([ , username ]) => username)
+    )
 
-        regexpCapture(this.text, firstMentionRegex)
-          .map(([ , username1, username2 ]) => username1 || username2),
+    // Include local mentions
+    if (this.isOwned()) {
+      const localMentionsRegex = new RegExp(' ' + localMention + ' ', 'g')
 
-        regexpCapture(this.text, endMentionRegex)
-          .map(([ , username1, username2 ]) => username1 || username2)
+      result = result.concat(
+        regexpCapture(this.text, localMentionsRegex)
+          .map(([ , username ]) => username)
       )
-    )
+    }
+
+    return uniq(result)
   }
 
   toFormattedJSON () {
diff --git a/server/tests/api/server/contact-form.ts b/server/tests/api/server/contact-form.ts
index 93221d0a3..06a2f89b0 100644
--- a/server/tests/api/server/contact-form.ts
+++ b/server/tests/api/server/contact-form.ts
@@ -45,7 +45,8 @@ describe('Test contact form', function () {
 
     const email = emails[0]
 
-    expect(email['from'][0]['address']).equal('toto@example.com')
+    expect(email['from'][0]['address']).equal('test-admin@localhost')
+    expect(email['from'][0]['name']).equal('toto@example.com')
     expect(email['to'][0]['address']).equal('admin1@example.com')
     expect(email['subject']).contains('Contact form')
     expect(email['text']).contains('my super message')
diff --git a/server/tests/api/server/email.ts b/server/tests/api/server/email.ts
index f96c57b66..f8f16f54f 100644
--- a/server/tests/api/server/email.ts
+++ b/server/tests/api/server/email.ts
@@ -89,6 +89,7 @@ describe('Test emails', function () {
 
       const email = emails[0]
 
+      expect(email['from'][0]['name']).equal('localhost:9001')
       expect(email['from'][0]['address']).equal('test-admin@localhost')
       expect(email['to'][0]['address']).equal('user_1@example.com')
       expect(email['subject']).contains('password')
@@ -133,6 +134,7 @@ describe('Test emails', function () {
 
       const email = emails[1]
 
+      expect(email['from'][0]['name']).equal('localhost:9001')
       expect(email['from'][0]['address']).equal('test-admin@localhost')
       expect(email['to'][0]['address']).equal('admin1@example.com')
       expect(email['subject']).contains('abuse')
@@ -152,6 +154,7 @@ describe('Test emails', function () {
 
       const email = emails[2]
 
+      expect(email['from'][0]['name']).equal('localhost:9001')
       expect(email['from'][0]['address']).equal('test-admin@localhost')
       expect(email['to'][0]['address']).equal('user_1@example.com')
       expect(email['subject']).contains(' blocked')
@@ -169,6 +172,7 @@ describe('Test emails', function () {
 
       const email = emails[3]
 
+      expect(email['from'][0]['name']).equal('localhost:9001')
       expect(email['from'][0]['address']).equal('test-admin@localhost')
       expect(email['to'][0]['address']).equal('user_1@example.com')
       expect(email['subject']).contains(' unblocked')
@@ -188,6 +192,7 @@ describe('Test emails', function () {
 
       const email = emails[4]
 
+      expect(email['from'][0]['name']).equal('localhost:9001')
       expect(email['from'][0]['address']).equal('test-admin@localhost')
       expect(email['to'][0]['address']).equal('user_1@example.com')
       expect(email['subject']).contains(' blacklisted')
@@ -205,6 +210,7 @@ describe('Test emails', function () {
 
       const email = emails[5]
 
+      expect(email['from'][0]['name']).equal('localhost:9001')
       expect(email['from'][0]['address']).equal('test-admin@localhost')
       expect(email['to'][0]['address']).equal('user_1@example.com')
       expect(email['subject']).contains(' unblacklisted')
@@ -224,6 +230,7 @@ describe('Test emails', function () {
 
       const email = emails[6]
 
+      expect(email['from'][0]['name']).equal('localhost:9001')
       expect(email['from'][0]['address']).equal('test-admin@localhost')
       expect(email['to'][0]['address']).equal('user_1@example.com')
       expect(email['subject']).contains('Verify')
diff --git a/server/tests/api/users/user-notifications.ts b/server/tests/api/users/user-notifications.ts
index 5260d64cc..72b6a0aa2 100644
--- a/server/tests/api/users/user-notifications.ts
+++ b/server/tests/api/users/user-notifications.ts
@@ -506,6 +506,20 @@ describe('Test users notifications', function () {
       await removeAccountFromAccountBlocklist(servers[ 0 ].url, userAccessToken, 'root')
     })
 
+    it('Should not send a new mention notification if the remote account mention a local account', async function () {
+      this.timeout(20000)
+
+      const resVideo = await uploadVideo(servers[0].url, servers[0].accessToken, { name: 'super video' })
+      const uuid = resVideo.body.video.uuid
+
+      await waitJobs(servers)
+      const resThread = await addVideoCommentThread(servers[1].url, servers[1].accessToken, uuid, '@user_1 hello')
+      const threadId = resThread.body.comment.id
+
+      await waitJobs(servers)
+      await checkCommentMention(baseParams, uuid, threadId, threadId, 'super root 2 name', 'absence')
+    })
+
     it('Should send a new mention notification after local comments', async function () {
       this.timeout(10000)
 
diff --git a/server/tests/helpers/comment-model.ts b/server/tests/helpers/comment-model.ts
index 76bb0f212..ebfd779e1 100644
--- a/server/tests/helpers/comment-model.ts
+++ b/server/tests/helpers/comment-model.ts
@@ -10,6 +10,8 @@ class CommentMock {
   text: string
 
   extractMentions = VideoCommentModel.prototype.extractMentions
+
+  isOwned = () => true
 }
 
 describe('Comment model', function () {