Refactor invite controller/view layer

- clean up code duplication

app/controllers/invites_controller.rb

@@ -4,6 +4,7 @@ class InvitesController < ApplicationController
   include Gitlab::Utils::StrongMemoize
 
   before_action :member
+  before_action :invite_details
   skip_before_action :authenticate_user!, only: :decline
 
   helper_method :member?, :current_user_matches_invite?
@@ -16,9 +17,8 @@ class InvitesController < ApplicationController
 
   def accept
     if member.accept_invite!(current_user)
-      label, path = source_info(member.source)
-
-      redirect_to path, notice: _("You have been granted %{member_human_access} access to %{label}.") % { member_human_access: member.human_access, label: label }
+      redirect_to invite_details[:path], notice: _("You have been granted %{member_human_access} access to %{title} %{name}.") %
+        { member_human_access: member.human_access, title: invite_details[:title], name: invite_details[:name] }
     else
       redirect_back_or_default(options: { alert: _("The invitation could not be accepted.") })
     end
@@ -26,8 +26,6 @@ class InvitesController < ApplicationController
 
   def decline
     if member.decline_invite!
-      label, _ = source_info(member.source)
-
       path =
         if current_user
           dashboard_projects_path
@@ -35,7 +33,8 @@ class InvitesController < ApplicationController
           new_user_session_path
         end
 
-      redirect_to path, notice: _("You have declined the invitation to join %{label}.") % { label: label }
+      redirect_to path, notice: _("You have declined the invitation to join %{title} %{name}.") %
+        { title: invite_details[:title], name: invite_details[:name] }
     else
       redirect_back_or_default(options: { alert: _("The invitation could not be declined.") })
     end
@@ -79,21 +78,22 @@ class InvitesController < ApplicationController
     redirect_to new_user_session_path, notice: notice
   end
 
-  def source_info(source)
-    case source
+  def invite_details
+    @invite_details ||= case @member.source
                         when Project
-      project = member.source
-      label = "project #{project.full_name}"
-      path = project_path(project)
+                          {
+                            name: @member.source.full_name,
+                            url: project_url(@member.source),
+                            title: _("project"),
+                            path: project_path(@member.source)
+                          }
                         when Group
-      group = member.source
-      label = "group #{group.name}"
-      path = group_path(group)
-    else
-      label = "who knows what"
-      path = dashboard_projects_path
+                          {
+                            name: @member.source.name,
+                            url: group_url(@member.source),
+                            title: _("group"),
+                            path: group_path(@member.source)
+                          }
                         end
-
-    [label, path]
   end
 end

app/views/invites/show.html.haml

@@ -2,28 +2,19 @@
 %h3.page-title= _("Invitation")
 
 %p
-  You have been invited
-  - if inviter = @member.created_by
-    by
+  = _("You have been invited")
+  - inviter = @member.created_by
+  - if inviter
+    = _("by")
     = link_to inviter.name, user_url(inviter)
-  to join
-  - case @member.source
-  - when Project
-    - project = @member.source
-    project
+  = _("to join %{source_name}") % { source_name: @invite_details[:title] }
   %strong
-      = link_to project.full_name, project_url(project)
-  - when Group
-    - group = @member.source
-    group
-    %strong
-      = link_to group.name, group_url(group)
-  as #{@member.human_access}.
+    = link_to @invite_details[:name], @invite_details[:url]
+  = _("as %{role}.") % { role: @member.human_access }
 
 - if member?
   %p
-    - member_source = @member.source.is_a?(Group) ? _("group") : _("project")
-    = _("However, you are already a member of this %{member_source}. Sign in using a different account to accept the invitation.") % { member_source: member_source }
+    = _("However, you are already a member of this %{member_source}. Sign in using a different account to accept the invitation.") % { member_source: @invite_details[:title] }
 
 - if !current_user_matches_invite?
   %p
@@ -32,7 +23,7 @@
     - link_to_current_user = link_to(current_user.to_reference, user_url(current_user))
     = _("Note that this invitation was sent to %{mail_to_invite_email}, but you are signed in as %{link_to_current_user} with email %{mail_to_current_user}.").html_safe % { mail_to_invite_email: mail_to_invite_email, mail_to_current_user: mail_to_current_user, link_to_current_user: link_to_current_user }
 
-- unless member?
+- if !member?
   .actions
     = link_to _("Accept invitation"), accept_invite_url(@token), method: :post, class: "btn btn-success"
     = link_to _("Decline"), decline_invite_url(@token), method: :post, class: "btn btn-danger gl-ml-3"

locale/gitlab.pot

@@ -27477,13 +27477,16 @@ msgstr ""
 msgid "You have been granted %{access_level} access to the %{source_name} %{source_type}."
 msgstr ""
 
-msgid "You have been granted %{member_human_access} access to %{label}."
+msgid "You have been granted %{member_human_access} access to %{title} %{name}."
+msgstr ""
+
+msgid "You have been invited"
 msgstr ""
 
 msgid "You have been unsubscribed from this thread."
 msgstr ""
 
-msgid "You have declined the invitation to join %{label}."
+msgid "You have declined the invitation to join %{title} %{name}."
 msgstr ""
 
 msgid "You have imported from this project %{numberOfPreviousImports} times before. Each new import will create duplicate issues."
@@ -27953,6 +27956,9 @@ msgstr ""
 msgid "archived:"
 msgstr ""
 
+msgid "as %{role}."
+msgstr ""
+
 msgid "assign yourself"
 msgstr ""
 
@@ -29232,6 +29238,9 @@ msgstr ""
 msgid "to help your contributors communicate effectively!"
 msgstr ""
 
+msgid "to join %{source_name}"
+msgstr ""
+
 msgid "to list"
 msgstr ""
 

spec/controllers/invites_controller_spec.rb

@@ -34,25 +34,4 @@ RSpec.describe InvitesController do
       expect(flash[:notice]).to be_nil
     end
   end
-
-  describe 'POST #accept' do
-    it 'accepts user' do
-      expect do
-        post :accept, params: { id: token }
-      end.to change { project_members.include?(user) }.from(false).to(true)
-
-      expect(response).to have_gitlab_http_status(:found)
-      expect(flash[:notice]).to include 'You have been granted'
-    end
-  end
-
-  describe 'GET #decline' do
-    it 'declines user' do
-      get :decline, params: { id: token }
-
-      expect { member.reload }.to raise_error ActiveRecord::RecordNotFound
-      expect(response).to have_gitlab_http_status(:found)
-      expect(flash[:notice]).to include 'You have declined the invitation to join'
-    end
-  end
 end

spec/features/invites_spec.rb

@@ -63,6 +63,8 @@ RSpec.describe 'Invites', :aggregate_failures do
 
     it 'shows message user already a member' do
       visit invite_path(group_invite.raw_invite_token)
+
+      expect(page).to have_link(owner.name, href: user_url(owner))
       expect(page).to have_content('However, you are already a member of this group.')
     end
   end
@@ -197,8 +199,10 @@ RSpec.describe 'Invites', :aggregate_failures do
 
         it 'declines application and redirects to dashboard' do
           page.click_link 'Decline'
+
           expect(current_path).to eq(dashboard_projects_path)
           expect(page).to have_content('You have declined the invitation to join group Owned.')
+          expect { group_invite.reload }.to raise_error ActiveRecord::RecordNotFound
         end
       end
 
@@ -209,7 +213,9 @@ RSpec.describe 'Invites', :aggregate_failures do
 
         it 'declines application and redirects to sign in page' do
           expect(current_path).to eq(new_user_session_path)
+
           expect(page).to have_content('You have declined the invitation to join group Owned.')
+          expect { group_invite.reload }.to raise_error ActiveRecord::RecordNotFound
         end
       end
     end
@@ -223,9 +229,13 @@ RSpec.describe 'Invites', :aggregate_failures do
       end
 
       it 'grants access and redirects to group page' do
+        expect(group.users.include?(user)).to be false
+
         page.click_link 'Accept invitation'
+
         expect(current_path).to eq(group_path(group))
         expect(page).to have_content('You have been granted Owner access to group Owned.')
+        expect(group.users.include?(user)).to be true
       end
     end
   end