Commit 58867eff authored by Zeger-Jan van de Weg's avatar Zeger-Jan van de Weg

Unsubscribe from thread through link in email footer

parent c81647ae
...@@ -59,6 +59,7 @@ v 8.3.0 ...@@ -59,6 +59,7 @@ v 8.3.0
- Add open_issues_count to project API (Stan Hu) - Add open_issues_count to project API (Stan Hu)
- Expand character set of usernames created by Omniauth (Corey Hinshaw) - Expand character set of usernames created by Omniauth (Corey Hinshaw)
- Add button to automatically merge a merge request when the build succeeds (Zeger-Jan van de Weg) - Add button to automatically merge a merge request when the build succeeds (Zeger-Jan van de Weg)
- Add unsubscribe link in the email footer (Zeger-Jan van de Weg)
- Provide better diagnostic message upon project creation errors (Stan Hu) - Provide better diagnostic message upon project creation errors (Stan Hu)
- Bump devise to 3.5.3 to fix reset token expiring after account creation (Stan Hu) - Bump devise to 3.5.3 to fix reset token expiring after account creation (Stan Hu)
- Remove api credentials from link to build_page - Remove api credentials from link to build_page
......
class SentNotificationsController < ApplicationController
skip_before_action :authenticate_user!
def unsubscribe
@sent_notification = SentNotification.for(params[:id])
return render_404 unless @sent_notification && !@sent_notification.for_commit?
noteable = @sent_notification.noteable
noteable.unsubscribe(@sent_notification.recipient)
flash[:notice] = "You have been unsubscribed from this thread."
if current_user
case @sent_notification.noteable
when Issue
redirect_to issue_path(noteable)
when MergeRequest
redirect_to merge_request_path(noteable)
else
redirect_to root_path
end
else
redirect_to new_user_session_path
end
end
end
module Emails module Emails
module Issues module Issues
def new_issue_email(recipient_id, issue_id) def new_issue_email(recipient_id, issue_id)
issue_mail_with_notification(issue_id, recipient_id) do setup_issue_mail(issue_id, recipient_id)
mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id))
end mail_new_thread(@issue, issue_thread_options(@issue.author_id, recipient_id))
end end
def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id) def reassigned_issue_email(recipient_id, issue_id, previous_assignee_id, updated_by_user_id)
issue_mail_with_notification(issue_id, recipient_id) do setup_issue_mail(issue_id, recipient_id)
@previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id)) @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
end mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
end end
def closed_issue_email(recipient_id, issue_id, updated_by_user_id) def closed_issue_email(recipient_id, issue_id, updated_by_user_id)
issue_mail_with_notification(issue_id, recipient_id) do setup_issue_mail(issue_id, recipient_id)
@updated_by = User.find updated_by_user_id
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id)) @updated_by = User.find updated_by_user_id
end mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
end end
def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id) def issue_status_changed_email(recipient_id, issue_id, status, updated_by_user_id)
issue_mail_with_notification(issue_id, recipient_id) do setup_issue_mail(issue_id, recipient_id)
@issue_status = status
@updated_by = User.find updated_by_user_id @issue_status = status
mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id)) @updated_by = User.find updated_by_user_id
end mail_answer_thread(@issue, issue_thread_options(updated_by_user_id, recipient_id))
end end
private private
...@@ -38,14 +38,12 @@ module Emails ...@@ -38,14 +38,12 @@ module Emails
} }
end end
def issue_mail_with_notification(issue_id, recipient_id) def setup_issue_mail(issue_id, recipient_id)
@issue = Issue.find(issue_id) @issue = Issue.find(issue_id)
@project = @issue.project @project = @issue.project
@target_url = namespace_project_issue_url(@project.namespace, @project, @issue) @target_url = namespace_project_issue_url(@project.namespace, @project, @issue)
yield @sent_notification = SentNotification.record(@issue, recipient_id, reply_key)
SentNotification.record(@issue, recipient_id, reply_key)
end end
end end
end end
module Emails module Emails
module MergeRequests module MergeRequests
def new_merge_request_email(recipient_id, merge_request_id) def new_merge_request_email(recipient_id, merge_request_id)
@merge_request = MergeRequest.find(merge_request_id) setup_merge_request_mail(merge_request_id, recipient_id)
@project = @merge_request.project
@target_url = namespace_project_merge_request_url(@project.namespace,
@project,
@merge_request)
mail_new_thread(@merge_request, mail_new_thread(@merge_request,
from: sender(@merge_request.author_id), from: sender(@merge_request.author_id),
to: recipient(recipient_id), to: recipient(recipient_id),
subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
SentNotification.record(@merge_request, recipient_id, reply_key)
end end
def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id) def reassigned_merge_request_email(recipient_id, merge_request_id, previous_assignee_id, updated_by_user_id)
@merge_request = MergeRequest.find(merge_request_id) setup_merge_request_mail(merge_request_id, recipient_id)
@previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id @previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id
@project = @merge_request.project
@target_url = namespace_project_merge_request_url(@project.namespace,
@project,
@merge_request)
mail_answer_thread(@merge_request, mail_answer_thread(@merge_request,
from: sender(updated_by_user_id), from: sender(updated_by_user_id),
to: recipient(recipient_id), to: recipient(recipient_id),
subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
SentNotification.record(@merge_request, recipient_id, reply_key)
end end
def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id) def closed_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
@merge_request = MergeRequest.find(merge_request_id) setup_merge_request_mail(merge_request_id, recipient_id)
@updated_by = User.find updated_by_user_id @updated_by = User.find updated_by_user_id
@project = @merge_request.project
@target_url = namespace_project_merge_request_url(@project.namespace,
@project,
@merge_request)
mail_answer_thread(@merge_request, mail_answer_thread(@merge_request,
from: sender(updated_by_user_id), from: sender(updated_by_user_id),
to: recipient(recipient_id), to: recipient(recipient_id),
subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
SentNotification.record(@merge_request, recipient_id, reply_key)
end end
def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id) def merged_merge_request_email(recipient_id, merge_request_id, updated_by_user_id)
@merge_request = MergeRequest.find(merge_request_id) setup_merge_request_mail(merge_request_id, recipient_id)
@project = @merge_request.project
@target_url = namespace_project_merge_request_url(@project.namespace,
@project,
@merge_request)
mail_answer_thread(@merge_request, mail_answer_thread(@merge_request,
from: sender(updated_by_user_id), from: sender(updated_by_user_id),
to: recipient(recipient_id), to: recipient(recipient_id),
subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
SentNotification.record(@merge_request, recipient_id, reply_key)
end end
def merge_request_status_email(recipient_id, merge_request_id, status, updated_by_user_id) def merge_request_status_email(recipient_id, merge_request_id, status, updated_by_user_id)
@merge_request = MergeRequest.find(merge_request_id) setup_merge_request_mail(merge_request_id, recipient_id)
@mr_status = status @mr_status = status
@project = @merge_request.project
@updated_by = User.find updated_by_user_id @updated_by = User.find updated_by_user_id
@target_url = namespace_project_merge_request_url(@project.namespace,
@project,
@merge_request)
mail_answer_thread(@merge_request, mail_answer_thread(@merge_request,
from: sender(updated_by_user_id), from: sender(updated_by_user_id),
to: recipient(recipient_id), to: recipient(recipient_id),
subject: subject("#{@merge_request.title} (##{@merge_request.iid})")) subject: subject("#{@merge_request.title} (##{@merge_request.iid})"))
end
private
def setup_merge_request_mail(merge_request_id, recipient_id)
@merge_request = MergeRequest.find(merge_request_id)
@project = @merge_request.project
@target_url = namespace_project_merge_request_url(@project.namespace,
@project,
@merge_request)
SentNotification.record(@merge_request, recipient_id, reply_key) @sent_notification = SentNotification.record(@merge_request, recipient_id, reply_key)
end end
end end
end end
module Emails module Emails
module Notes module Notes
def note_commit_email(recipient_id, note_id) def note_commit_email(recipient_id, note_id)
note_mail_with_notification(note_id, recipient_id) do note_mail_with_notification(note_id, recipient_id)
@commit = @note.noteable
@target_url = namespace_project_commit_url(*note_target_url_options) @commit = @note.noteable
@target_url = namespace_project_commit_url(*note_target_url_options)
mail_answer_thread(@commit,
from: sender(@note.author_id), mail_answer_thread(@commit,
to: recipient(recipient_id), from: sender(@note.author_id),
subject: subject("#{@commit.title} (#{@commit.short_id})")) to: recipient(recipient_id),
end subject: subject("#{@commit.title} (#{@commit.short_id})"))
end end
def note_issue_email(recipient_id, note_id) def note_issue_email(recipient_id, note_id)
note_mail_with_notification(note_id, recipient_id) do note_mail_with_notification(note_id, recipient_id)
@issue = @note.noteable
@target_url = namespace_project_issue_url(*note_target_url_options) @issue = @note.noteable
mail_answer_thread(@issue, note_thread_options(recipient_id)) @target_url = namespace_project_issue_url(*note_target_url_options)
end mail_answer_thread(@issue, note_thread_options(recipient_id))
end end
def note_merge_request_email(recipient_id, note_id) def note_merge_request_email(recipient_id, note_id)
note_mail_with_notification(note_id, recipient_id) do note_mail_with_notification(note_id, recipient_id)
@merge_request = @note.noteable
@target_url = namespace_project_merge_request_url(*note_target_url_options) @merge_request = @note.noteable
mail_answer_thread(@merge_request, note_thread_options(recipient_id)) @target_url = namespace_project_merge_request_url(*note_target_url_options)
end mail_answer_thread(@merge_request, note_thread_options(recipient_id))
end end
private private
...@@ -46,9 +46,7 @@ module Emails ...@@ -46,9 +46,7 @@ module Emails
@note = Note.find(note_id) @note = Note.find(note_id)
@project = @note.project @project = @note.project
yield @sent_notification = SentNotification.record_note(@note, recipient_id, reply_key)
SentNotification.record_note(@note, recipient_id, reply_key)
end end
end end
end end
...@@ -107,10 +107,9 @@ class Notify < BaseMailer ...@@ -107,10 +107,9 @@ class Notify < BaseMailer
end end
headers["X-GitLab-#{model.class.name}-ID"] = model.id headers["X-GitLab-#{model.class.name}-ID"] = model.id
headers['X-GitLab-Reply-Key'] = reply_key
if reply_key if Gitlab::IncomingEmail.enabled?
headers['X-GitLab-Reply-Key'] = reply_key
address = Mail::Address.new(Gitlab::IncomingEmail.reply_address(reply_key)) address = Mail::Address.new(Gitlab::IncomingEmail.reply_address(reply_key))
address.display_name = @project.name_with_namespace address.display_name = @project.name_with_namespace
......
...@@ -119,6 +119,12 @@ module Issuable ...@@ -119,6 +119,12 @@ module Issuable
update(subscribed: !subscribed?(user)) update(subscribed: !subscribed?(user))
end end
def unsubscribe(user)
subscriptions.
find_or_initialize_by(user_id: user.id).
update(subscribed: false)
end
def to_hook_data(user) def to_hook_data(user)
{ {
object_kind: self.class.name.underscore, object_kind: self.class.name.underscore,
......
...@@ -25,8 +25,6 @@ class SentNotification < ActiveRecord::Base ...@@ -25,8 +25,6 @@ class SentNotification < ActiveRecord::Base
class << self class << self
def reply_key def reply_key
return nil unless Gitlab::IncomingEmail.enabled?
SecureRandom.hex(16) SecureRandom.hex(16)
end end
...@@ -59,7 +57,7 @@ class SentNotification < ActiveRecord::Base ...@@ -59,7 +57,7 @@ class SentNotification < ActiveRecord::Base
def record_note(note, recipient_id, reply_key, params = {}) def record_note(note, recipient_id, reply_key, params = {})
params[:line_code] = note.line_code params[:line_code] = note.line_code
record(note.noteable, recipient_id, reply_key, params) record(note.noteable, recipient_id, reply_key, params)
end end
end end
...@@ -75,4 +73,8 @@ class SentNotification < ActiveRecord::Base ...@@ -75,4 +73,8 @@ class SentNotification < ActiveRecord::Base
super super
end end
end end
def to_param
self.reply_key
end
end end
...@@ -44,6 +44,10 @@ ...@@ -44,6 +44,10 @@
%br %br
-# Don't link the host is the line below, one link in the email is easier to quickly click than two. -# Don't link the host is the line below, one link in the email is easier to quickly click than two.
You're receiving this email because of your account on #{Gitlab.config.gitlab.host}. You're receiving this email because of your account on #{Gitlab.config.gitlab.host}.
If you'd like to receive fewer emails, you can adjust your notification settings. If you'd like to receive fewer emails, you can
- if @sent_notification && !@sent_notification.for_commit?
= link_to "unsubscribe", unsubscribe_sent_notification_url(@sent_notification)
from this thread or
adjust your notification settings.
= email_action @target_url = email_action @target_url
\ No newline at end of file
...@@ -88,6 +88,12 @@ Rails.application.routes.draw do ...@@ -88,6 +88,12 @@ Rails.application.routes.draw do
end end
end end
resources :sent_notifications, only: [], constraints: { id: /[0-9a-f]{32}/ } do
member do
get :unsubscribe
end
end
# Spam reports # Spam reports
resources :abuse_reports, only: [:new, :create] resources :abuse_reports, only: [:new, :create]
......
require 'rails_helper'
describe SentNotificationsController, type: :controller do
let(:user) { create(:user) }
let(:issue) { create(:issue, author: user) }
let(:sent_notification) { create(:sent_notification, noteable: issue) }
describe 'GET #unsubscribe' do
it 'returns a 404 when calling without existing id' do
get(:unsubscribe, id: '0' * 32)
expect(response.status).to be 404
end
context 'calling with id' do
it 'shows a flash message to the user' do
get(:unsubscribe, id: sent_notification.reply_key)
expect(response.status).to be 302
expect(response).to redirect_to new_user_session_path
expect(controller).to set_flash[:notice].to(/unsubscribed/).now
end
end
end
end
...@@ -212,4 +212,11 @@ FactoryGirl.define do ...@@ -212,4 +212,11 @@ FactoryGirl.define do
provider 'ldapmain' provider 'ldapmain'
extern_uid 'my-ldap-id' extern_uid 'my-ldap-id'
end end
factory :sent_notification do
project
recipient factory: :user
noteable factory: :issue
reply_key "0123456789abcdef" * 2
end
end end
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment