From 787ce5f8593c08bb33defcfa3841e1b114af83d6 Mon Sep 17 00:00:00 2001 From: George Eaton Date: Mon, 16 Sep 2024 18:26:08 +0100 Subject: [PATCH] Add banner for removing access to an application This takes a similar approach to the previous banner. However, I've had to perform a little more logic in the view here to allow us to re-use the success alert component. --- .../account/signin_permissions_controller.rb | 4 +++- .../users/signin_permissions_controller.rb | 3 +++ app/helpers/application_access_helper.rb | 9 ++++++++ app/helpers/success_alert_helper.rb | 7 +++++++ app/views/account/applications/index.html.erb | 10 ++++++--- app/views/users/applications/index.html.erb | 11 +++++++--- .../helpers/application_access_helper_test.rb | 21 +++++++++++++++++++ test/helpers/success_alert_helper_test.rb | 16 ++++++++++++++ test/support/removing_access_helpers.rb | 3 +++ 9 files changed, 77 insertions(+), 7 deletions(-) diff --git a/app/controllers/account/signin_permissions_controller.rb b/app/controllers/account/signin_permissions_controller.rb index 17e031e22..772d55307 100644 --- a/app/controllers/account/signin_permissions_controller.rb +++ b/app/controllers/account/signin_permissions_controller.rb @@ -20,8 +20,10 @@ def destroy authorize [:account, application], :remove_signin_permission? params = { supported_permission_ids: current_user.supported_permissions.map(&:id) - [application.signin_permission.id] } - UserUpdate.new(current_user, params, current_user, user_ip_address).call + UserUpdate.new(current_user, params, current_user, user_ip_address).call + flash[:application_id] = application.id + flash[:removing_access] = true redirect_to account_applications_path end diff --git a/app/controllers/users/signin_permissions_controller.rb b/app/controllers/users/signin_permissions_controller.rb index 85201cce5..9559e4788 100644 --- a/app/controllers/users/signin_permissions_controller.rb +++ b/app/controllers/users/signin_permissions_controller.rb @@ -25,6 +25,9 @@ def destroy params = { supported_permission_ids: @user.supported_permissions.map(&:id) - [@application.signin_permission.id] } UserUpdate.new(@user, params, current_user, user_ip_address).call + flash[:application_id] = @application.id + flash[:removing_access] = true + redirect_to user_applications_path(@user) end diff --git a/app/helpers/application_access_helper.rb b/app/helpers/application_access_helper.rb index a557b430d..439f375ee 100644 --- a/app/helpers/application_access_helper.rb +++ b/app/helpers/application_access_helper.rb @@ -7,4 +7,13 @@ def access_granted_message(application_id, user = current_user) "#{user.name} has been granted access to #{application.name}." end + + def access_removed_message(application_id, user = current_user) + application = Doorkeeper::Application.find_by(id: application_id) + return nil unless application + + return "Your access to #{application.name} has been removed." if user == current_user + + "#{user.name}'s access to #{application.name} has been removed." + end end diff --git a/app/helpers/success_alert_helper.rb b/app/helpers/success_alert_helper.rb index 21c4c8eaf..0aa12d86c 100644 --- a/app/helpers/success_alert_helper.rb +++ b/app/helpers/success_alert_helper.rb @@ -12,4 +12,11 @@ def access_and_permissions_granted_params(application_id, granting_access:, user } end end + + def access_removed_params(application_id, user: current_user) + { + message: "Access removed", + description: access_removed_message(application_id, user), + } + end end diff --git a/app/views/account/applications/index.html.erb b/app/views/account/applications/index.html.erb index 4b1d27c0f..63a99e414 100644 --- a/app/views/account/applications/index.html.erb +++ b/app/views/account/applications/index.html.erb @@ -17,10 +17,14 @@ %> <% if flash[:application_id] %> + <% alert_params = if flash[:removing_access] + access_removed_params(flash[:application_id]) + else + access_and_permissions_granted_params(flash[:application_id], granting_access: flash[:granting_access]) + end + %> <% content_for(:custom_alerts) do %> - <%= render "govuk_publishing_components/components/success_alert", - access_and_permissions_granted_params(flash[:application_id], granting_access: flash[:granting_access]) - %> + <%= render "govuk_publishing_components/components/success_alert", alert_params %> <% end %> <% end %> diff --git a/app/views/users/applications/index.html.erb b/app/views/users/applications/index.html.erb index b5ada0b9a..ca66fbc4c 100644 --- a/app/views/users/applications/index.html.erb +++ b/app/views/users/applications/index.html.erb @@ -22,10 +22,15 @@ %> <% if flash[:application_id] %> + <% alert_params = if flash[:removing_access] + access_removed_params(flash[:application_id], user: @user) + else + access_and_permissions_granted_params(flash[:application_id], granting_access: flash[:granting_access], user: @user) + end + %> <% content_for(:custom_alerts) do %> - <%= render "govuk_publishing_components/components/success_alert", - access_and_permissions_granted_params(flash[:application_id], granting_access: flash[:granting_access], user: @user) - %> <% end %> + <%= render "govuk_publishing_components/components/success_alert", alert_params %> + <% end %> <% end %> <%= render "components/table", { diff --git a/test/helpers/application_access_helper_test.rb b/test/helpers/application_access_helper_test.rb index 38ad11bac..c17a824ed 100644 --- a/test/helpers/application_access_helper_test.rb +++ b/test/helpers/application_access_helper_test.rb @@ -26,4 +26,25 @@ class ApplicationAccessHelperTest < ActionView::TestCase end end end + + context "#access_removed_message" do + context "when the user is setting their own permissions" do + should "return a message informing them that they have access to an application" do + assert_equal "Your access to Whitehall has been removed.", access_removed_message(@application) + end + end + + context "when the user is setting another's permissions" do + should "return a message informing them that the other user have access to an application" do + user = create(:user, name: "Gerald") + assert_equal "Gerald's access to Whitehall has been removed.", access_removed_message(@application, user) + end + end + + context "when the application does not exist" do + should "return nil" do + assert_nil access_removed_message(:made_up_id) + end + end + end end diff --git a/test/helpers/success_alert_helper_test.rb b/test/helpers/success_alert_helper_test.rb index 75437d16f..3e5907ae2 100644 --- a/test/helpers/success_alert_helper_test.rb +++ b/test/helpers/success_alert_helper_test.rb @@ -25,4 +25,20 @@ class SuccessAlertHelperTest < ActionView::TestCase end end end + + context "#access_removed_params" do + setup do + @application = create(:application) + stubs(:current_user).returns(create(:user)) + end + + context "when removing access" do + should "return success alert params with the `access_removed_message` text" do + stubs(:access_removed_message).returns("Removed access") + + expected = { message: "Removed access", description: "Access removed" } + assert_equal expected, access_removed_params(@application.id, granting_access: true) + end + end + end end diff --git a/test/support/removing_access_helpers.rb b/test/support/removing_access_helpers.rb index a8b674788..018427915 100644 --- a/test/support/removing_access_helpers.rb +++ b/test/support/removing_access_helpers.rb @@ -30,6 +30,9 @@ def assert_remove_access(application, grantee, grantee_is_self: false) assert apps_without_access_table.has_content?(application.name) assert_not grantee.has_access_to?(application) + success_banner_caption = grantee_is_self ? "Your access to #{application.name} has been removed." : "#{grantee.name}'s access to #{application.name} has been removed." + assert_flash_content("Access removed") + assert_flash_content(success_banner_caption) end def refute_remove_access(application)