From c61a54f7fe932b9b76ce930aaccb04f897c4093b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 16 Sep 2016 17:08:09 +0200 Subject: Fix initial implementation to actually render the unsubscribe page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- app/controllers/sent_notifications_controller.rb | 7 ++++-- app/views/sent_notifications/unsubscribe.html.haml | 17 ++++++++++--- .../sent_notifications_controller_spec.rb | 14 ++++++++--- spec/features/unsubscribe_links_spec.rb | 29 +++++++++++++++++++--- 4 files changed, 54 insertions(+), 13 deletions(-) diff --git a/app/controllers/sent_notifications_controller.rb b/app/controllers/sent_notifications_controller.rb index c4abc597cf1..3085ff33aba 100644 --- a/app/controllers/sent_notifications_controller.rb +++ b/app/controllers/sent_notifications_controller.rb @@ -2,12 +2,15 @@ class SentNotificationsController < ApplicationController skip_before_action :authenticate_user! def unsubscribe - return redirect_to new_user_session_path unless current_user || params[:force] - @sent_notification = SentNotification.for(params[:id]) return render_404 unless @sent_notification && @sent_notification.unsubscribable? + return unsubscribe_and_redirect if current_user || params[:force] + end + + private + def unsubscribe_and_redirect noteable = @sent_notification.noteable noteable.unsubscribe(@sent_notification.recipient) diff --git a/app/views/sent_notifications/unsubscribe.html.haml b/app/views/sent_notifications/unsubscribe.html.haml index 568d2ac3af1..9baadf8a0c5 100644 --- a/app/views/sent_notifications/unsubscribe.html.haml +++ b/app/views/sent_notifications/unsubscribe.html.haml @@ -1,4 +1,15 @@ -%h3.page-title - Are you sure you want to unsubscribe from this thread? +- noteable = @sent_notification.noteable +- noteable_type = @sent_notification.noteable_type.humanize(capitalize: false) +- noteable_text = %(#{noteable_type} "#{noteable.title}" (#{noteable.to_reference})) +- title = "Unsubscribe from #{noteable_text}" - = link_to "Unsubscribe", unsubscribe_sent_notification_path(@sent_notification, force: true), class: 'btn btn-primary wide' +- page_title title + +%h3.page-title= title + +%p= "Are you sure you want to unsubscribe from #{noteable_text}?" + +%p + = link_to 'Unsubscribe', unsubscribe_sent_notification_path(@sent_notification, force: true), + class: 'btn btn-primary append-right-10' + = link_to 'Cancel', new_user_session_path, class: 'btn append-right-10' diff --git a/spec/controllers/sent_notifications_controller_spec.rb b/spec/controllers/sent_notifications_controller_spec.rb index 4e75372ffb2..191e290a118 100644 --- a/spec/controllers/sent_notifications_controller_spec.rb +++ b/spec/controllers/sent_notifications_controller_spec.rb @@ -41,7 +41,7 @@ describe SentNotificationsController, type: :controller do end it 'redirects to the login page' do - expect(response).to redirect_to(new_user_session_path) + expect(response).to render_template :unsubscribe end end end @@ -83,19 +83,25 @@ describe SentNotificationsController, type: :controller do end context 'when the force param is not passed' do + let(:merge_request) do + create(:merge_request, source_project: project, author: user) do |merge_request| + merge_request.subscriptions.create(user: user, subscribed: true) + end + end + let(:sent_notification) { create(:sent_notification, noteable: merge_request, recipient: user) } before { get(:unsubscribe, id: sent_notification.reply_key) } it 'unsubscribes the user' do - expect(issue.subscribed?(user)).to be_falsey + expect(merge_request.subscribed?(user)).to be_falsey end it 'sets the flash message' do expect(controller).to set_flash[:notice].to(/unsubscribed/).now end - it 'redirects to the issue page' do + it 'redirects to the merge request page' do expect(response). - to redirect_to(namespace_project_issue_path(project.namespace, project, issue)) + to redirect_to(namespace_project_merge_request_path(project.namespace, project, merge_request)) end end end diff --git a/spec/features/unsubscribe_links_spec.rb b/spec/features/unsubscribe_links_spec.rb index eafad1a726d..3802f5f1335 100644 --- a/spec/features/unsubscribe_links_spec.rb +++ b/spec/features/unsubscribe_links_spec.rb @@ -19,11 +19,32 @@ describe 'Unsubscribe links', feature: true do end context 'when logged out' do - it 'redirects to the login page when visiting the link from the body' do - visit body_link + context 'when visiting the link from the body' do + it 'shows the unsubscribe confirmation page and redirects to root path when confirming' do + visit body_link + + expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last) + expect(page).to have_text(%(Unsubscribe from issue "#{issue.title}" (#{issue.to_reference}))) + expect(page).to have_text(%(Are you sure you want to unsubscribe from issue "#{issue.title}" (#{issue.to_reference})?)) + expect(issue.subscribed?(recipient)).to be_truthy + + click_link 'Unsubscribe' + + expect(issue.subscribed?(recipient)).to be_falsey + expect(current_path).to eq new_user_session_path + end + + it 'shows the unsubscribe confirmation page and redirects to root path when canceling' do + visit body_link + + expect(current_path).to eq unsubscribe_sent_notification_path(SentNotification.last) + expect(issue.subscribed?(recipient)).to be_truthy + + click_link 'Cancel' - expect(current_path).to eq new_user_session_path - expect(issue.subscribed?(recipient)).to be_truthy + expect(issue.subscribed?(recipient)).to be_truthy + expect(current_path).to eq new_user_session_path + end end it 'unsubscribes from the issue when visiting the link from the header' do -- cgit v1.2.1