From 4cabe6944d627d500eb4800ba029c62b1b503639 Mon Sep 17 00:00:00 2001 From: George Eaton Date: Wed, 18 Sep 2024 10:52:59 +0100 Subject: [PATCH 1/5] Rename helper for updating permissions success banner This is set to the Success Alert's param called `description` rather than `message`, so calling it `message` which is used for the header to the banner is misleading. --- app/helpers/application_permissions_helper.rb | 2 +- app/views/account/applications/index.html.erb | 2 +- app/views/api_users/applications/index.html.erb | 2 +- app/views/users/applications/index.html.erb | 2 +- .../application_permissions_helper_test.rb | 16 ++++++++-------- 5 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/helpers/application_permissions_helper.rb b/app/helpers/application_permissions_helper.rb index 4be9e8351..ac1f6cc31 100644 --- a/app/helpers/application_permissions_helper.rb +++ b/app/helpers/application_permissions_helper.rb @@ -1,5 +1,5 @@ module ApplicationPermissionsHelper - def message_for_success(application_id, user = current_user) + def permissions_updated_description(application_id, user = current_user) application = Doorkeeper::Application.find_by(id: application_id) return nil unless application diff --git a/app/views/account/applications/index.html.erb b/app/views/account/applications/index.html.erb index e8623ec64..72802502b 100644 --- a/app/views/account/applications/index.html.erb +++ b/app/views/account/applications/index.html.erb @@ -20,7 +20,7 @@ <% content_for(:custom_alerts) do %> <%= render "govuk_publishing_components/components/success_alert", { message: "Permissions updated", - description: message_for_success(flash[:application_id]), + description: permissions_updated_description(flash[:application_id]), } %> <% end %> <% end %> diff --git a/app/views/api_users/applications/index.html.erb b/app/views/api_users/applications/index.html.erb index 44d36ae89..6f654957a 100644 --- a/app/views/api_users/applications/index.html.erb +++ b/app/views/api_users/applications/index.html.erb @@ -25,7 +25,7 @@ <% content_for(:custom_alerts) do %> <%= render "govuk_publishing_components/components/success_alert", { message: "Permissions updated", - description: message_for_success(flash[:application_id], @api_user), + description: permissions_updated_description(flash[:application_id], @api_user), } %> <% end %> <% end %> diff --git a/app/views/users/applications/index.html.erb b/app/views/users/applications/index.html.erb index 5f63b19d9..e83509afc 100644 --- a/app/views/users/applications/index.html.erb +++ b/app/views/users/applications/index.html.erb @@ -25,7 +25,7 @@ <% content_for(:custom_alerts) do %> <%= render "govuk_publishing_components/components/success_alert", { message: "Permissions updated", - description: message_for_success(flash[:application_id], @user), + description: permissions_updated_description(flash[:application_id], @user), } %> <% end %> <% end %> diff --git a/test/helpers/application_permissions_helper_test.rb b/test/helpers/application_permissions_helper_test.rb index fc0cdc8d4..2fdb3f5fe 100644 --- a/test/helpers/application_permissions_helper_test.rb +++ b/test/helpers/application_permissions_helper_test.rb @@ -1,7 +1,7 @@ require "test_helper" class ApplicationPermissionsHelperTest < ActionView::TestCase - context "#message_for_success" do + context "#permissions_updated_description" do setup do @application = create(:application, name: "Whitehall", with_non_delegatable_supported_permissions: ["Permission 1"]) user = create(:user, with_permissions: { @application => ["Permission 1", SupportedPermission::SIGNIN_NAME] }) @@ -9,20 +9,20 @@ class ApplicationPermissionsHelperTest < ActionView::TestCase end should "include the application name in the message" do - assert_includes message_for_success(@application.id), "You now have the following permissions for Whitehall" + assert_includes permissions_updated_description(@application.id), "You now have the following permissions for Whitehall" end should "include the users permissions in the message" do - assert_includes message_for_success(@application.id), "Permission 1" + assert_includes permissions_updated_description(@application.id), "Permission 1" end should "not include the signin permission in the message" do - assert_not_includes message_for_success(@application.id), "signin" + assert_not_includes permissions_updated_description(@application.id), "signin" end context "when the application does not exist" do should "return nil" do - assert_nil message_for_success(:made_up_id) + assert_nil permissions_updated_description(:made_up_id) end end @@ -33,7 +33,7 @@ class ApplicationPermissionsHelperTest < ActionView::TestCase end should "indicate that the user has no additional permissions" do - assert_includes message_for_success(@application.id), "You can access Whitehall but you do not have any additional permissions." + assert_includes permissions_updated_description(@application.id), "You can access Whitehall but you do not have any additional permissions." end end @@ -43,7 +43,7 @@ class ApplicationPermissionsHelperTest < ActionView::TestCase end should "include the application name in the message" do - assert_includes message_for_success(@application.id, @user), "user-name now has the following permissions for Whitehall" + assert_includes permissions_updated_description(@application.id, @user), "user-name now has the following permissions for Whitehall" end end @@ -53,7 +53,7 @@ class ApplicationPermissionsHelperTest < ActionView::TestCase end should "indicate that the user has no additional permissions" do - assert_includes message_for_success(@application.id, @user), "user-name can access Whitehall but does not have any additional permissions." + assert_includes permissions_updated_description(@application.id, @user), "user-name can access Whitehall but does not have any additional permissions." end end end From d725624245f7f28256b09985a6165a24c0d6cfcc Mon Sep 17 00:00:00 2001 From: George Eaton Date: Wed, 18 Sep 2024 12:02:04 +0100 Subject: [PATCH 2/5] Simplify `permissions_updated_description` slightly This felt a bit much to take in, I find it a little more readable now. --- app/helpers/application_permissions_helper.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/helpers/application_permissions_helper.rb b/app/helpers/application_permissions_helper.rb index ac1f6cc31..cb9f82c98 100644 --- a/app/helpers/application_permissions_helper.rb +++ b/app/helpers/application_permissions_helper.rb @@ -6,21 +6,24 @@ def permissions_updated_description(application_id, user = current_user) additional_permissions = user.permissions_for(application).reject { |permission| permission == SupportedPermission::SIGNIN_NAME } if additional_permissions.any? - prefix = user == current_user ? "You now have" : "#{user.name} now has" - paragraph = tag.p("#{prefix} the following permissions for #{application.name}:", class: "govuk-body") + paragraph = tag.p( + (user == current_user ? "You now have" : "#{user.name} now has") + " the following permissions for #{application.name}:", + class: "govuk-body", + ) + list = tag.ul(class: "govuk-list govuk-list--bullet") additional_permissions.map { |permission| list << tag.li(permission) } + + paragraph + list else string = if user == current_user "You can access #{application.name} but you do not have any additional permissions." else "#{user.name} can access #{application.name} but does not have any additional permissions." end - paragraph = tag.p(string, class: "govuk-body") - list = nil - end - paragraph + list + tag.p(string, class: "govuk-body") + end end def notice_about_non_delegatable_permissions(current_user, application, other_grantee = nil) From fbcf5400035344faa30a93695457d08407b216b2 Mon Sep 17 00:00:00 2001 From: George Eaton Date: Wed, 18 Sep 2024 12:24:46 +0100 Subject: [PATCH 3/5] Refactor success alert for updating permissions We'll shortly be adding success alerts for adding and removing access, so being able to pass in message and description in the controller makes this simple. We need to include `ActionView::Helpers::TagHelper` in the helper here to make the `tag` method available to be called in controllers. We could include it directly in the controller but including it in the helper itself means we're less likely to forget this. Unforunately we can't call `serialize` in the helper/controller as the escaping is stripped out when it's serialized as JSON when stored as a browser cookie. [1] This same serialization is what converts symbols to strings, meaning we also can't pass `flash[:success_alert]` to the Success Alert, as the `message` and `description` keys are converted to strings to store in the cookie, while the component expected an object with symbol keys.[2] [1]: https://groups.google.com/g/rubyonrails-core/c/z52zgDgUmbs [2]: bigbinary.com/blog/flash-access-changes-rails-4-1 --- app/controllers/account/permissions_controller.rb | 5 ++++- app/controllers/users/permissions_controller.rb | 5 ++++- app/helpers/application_permissions_helper.rb | 2 ++ app/views/account/applications/index.html.erb | 6 +++--- app/views/users/applications/index.html.erb | 6 +++--- .../controllers/account/permissions_controller_test.rb | 10 ++++++++-- test/controllers/users/permissions_controller_test.rb | 10 ++++++++-- test/support/updating_permissions_helpers.rb | 1 + 8 files changed, 33 insertions(+), 12 deletions(-) diff --git a/app/controllers/account/permissions_controller.rb b/app/controllers/account/permissions_controller.rb index 1bf11d95c..207541c16 100644 --- a/app/controllers/account/permissions_controller.rb +++ b/app/controllers/account/permissions_controller.rb @@ -70,7 +70,10 @@ def update flash[:new_permission_name] = SupportedPermission.find(update_params[:new_permission_id]).name redirect_to edit_account_application_permissions_path(@application) else - flash[:application_id] = @application.id + flash[:success_alert] = { + message: "Permissions updated", + description: permissions_updated_description(@application.id), + } redirect_to account_applications_path end end diff --git a/app/controllers/users/permissions_controller.rb b/app/controllers/users/permissions_controller.rb index 562b9607a..509b47061 100644 --- a/app/controllers/users/permissions_controller.rb +++ b/app/controllers/users/permissions_controller.rb @@ -71,7 +71,10 @@ def update flash[:new_permission_name] = SupportedPermission.find(update_params[:new_permission_id]).name redirect_to edit_user_application_permissions_path(@user, @application) else - flash[:application_id] = @application.id + flash[:success_alert] = { + message: "Permissions updated", + description: permissions_updated_description(@application.id, @user), + } redirect_to user_applications_path(@user) end end diff --git a/app/helpers/application_permissions_helper.rb b/app/helpers/application_permissions_helper.rb index cb9f82c98..b8d75625d 100644 --- a/app/helpers/application_permissions_helper.rb +++ b/app/helpers/application_permissions_helper.rb @@ -1,4 +1,6 @@ module ApplicationPermissionsHelper + include ActionView::Helpers::TagHelper + def permissions_updated_description(application_id, user = current_user) application = Doorkeeper::Application.find_by(id: application_id) return nil unless application diff --git a/app/views/account/applications/index.html.erb b/app/views/account/applications/index.html.erb index 72802502b..334572e10 100644 --- a/app/views/account/applications/index.html.erb +++ b/app/views/account/applications/index.html.erb @@ -16,11 +16,11 @@ }) %> -<% if flash[:application_id] %> +<% if flash[:success_alert] %> <% content_for(:custom_alerts) do %> <%= render "govuk_publishing_components/components/success_alert", { - message: "Permissions updated", - description: permissions_updated_description(flash[:application_id]), + message: flash[:success_alert]["message"], + description: sanitize(flash[:success_alert]["description"]), } %> <% end %> <% end %> diff --git a/app/views/users/applications/index.html.erb b/app/views/users/applications/index.html.erb index e83509afc..d230a79c0 100644 --- a/app/views/users/applications/index.html.erb +++ b/app/views/users/applications/index.html.erb @@ -21,11 +21,11 @@ }) %> -<% if flash[:application_id] %> +<% if flash[:success_alert] %> <% content_for(:custom_alerts) do %> <%= render "govuk_publishing_components/components/success_alert", { - message: "Permissions updated", - description: permissions_updated_description(flash[:application_id], @user), + message: flash[:success_alert]["message"], + description: sanitize(flash[:success_alert]["description"]), } %> <% end %> <% end %> diff --git a/test/controllers/account/permissions_controller_test.rb b/test/controllers/account/permissions_controller_test.rb index 8ae4a9fa9..9c3076a3b 100644 --- a/test/controllers/account/permissions_controller_test.rb +++ b/test/controllers/account/permissions_controller_test.rb @@ -269,7 +269,7 @@ class Account::PermissionsControllerTest < ActionController::TestCase assert_same_elements [application.signin_permission, new_permission], current_user.supported_permissions end - should "assign the application id to the application_id flash" do + should "assign the success alert hash to flash" do application = create(:application) old_permission = create(:supported_permission, application:) new_permission = create(:supported_permission, application:) @@ -281,9 +281,15 @@ class Account::PermissionsControllerTest < ActionController::TestCase ) sign_in user + Account::PermissionsController + .any_instance + .expects(:permissions_updated_description) + .with(application.id).returns("Updated some permissions for myself") + patch :update, params: { application_id: application, application: { supported_permission_ids: [new_permission.id] } } - assert_equal application.id, flash[:application_id] + expected = { message: "Permissions updated", description: "Updated some permissions for myself" } + assert_equal expected, flash[:success_alert] end context "when current_permission_ids and new_permission_id are provided instead of supported_permission_ids" do diff --git a/test/controllers/users/permissions_controller_test.rb b/test/controllers/users/permissions_controller_test.rb index 387df6538..f3f59d93b 100644 --- a/test/controllers/users/permissions_controller_test.rb +++ b/test/controllers/users/permissions_controller_test.rb @@ -364,7 +364,7 @@ class Users::PermissionsControllerTest < ActionController::TestCase assert_same_elements [new_permission, application.signin_permission], user.supported_permissions end - should "assign the application id to the application_id flash" do + should "assign the success alert message to flash" do application = create(:application) old_permission = create(:supported_permission, application:) new_permission = create(:supported_permission, application:) @@ -385,9 +385,15 @@ class Users::PermissionsControllerTest < ActionController::TestCase edit_permissions?: true, ) + Users::PermissionsController + .any_instance + .expects(:permissions_updated_description) + .with(application.id, user).returns("Updated some permissions for another user") + patch :update, params: { user_id: user, application_id: application, application: { supported_permission_ids: [new_permission.id] } } - assert_equal application.id, flash[:application_id] + expected = { message: "Permissions updated", description: "Updated some permissions for another user" } + assert_equal expected, flash[:success_alert] end context "when current_permission_ids and new_permission_id are provided instead of supported_permission_ids" do diff --git a/test/support/updating_permissions_helpers.rb b/test/support/updating_permissions_helpers.rb index 4052a94bf..29bc1994b 100644 --- a/test/support/updating_permissions_helpers.rb +++ b/test/support/updating_permissions_helpers.rb @@ -35,6 +35,7 @@ def assert_update_permissions(application, grantee, grant: [], revoke: []) click_button "Update permissions" + assert_flash_content("Permissions updated") assert_flash_content(grant.map(&:name)) grant.each { |new_permission| assert grantee.has_permission?(new_permission) } From d8ceb8a9488f74971eff02465d524c5fc14a34a6 Mon Sep 17 00:00:00 2001 From: George Eaton Date: Wed, 18 Sep 2024 14:47:24 +0100 Subject: [PATCH 4/5] Show success banner when granting access to an application --- .../account/signin_permissions_controller.rb | 3 ++ .../users/signin_permissions_controller.rb | 3 ++ app/helpers/application_access_helper.rb | 10 +++++++ .../signin_permissions_controller_test.rb | 24 +++++++++++++++ .../signin_permissions_controller_test.rb | 25 ++++++++++++++++ .../helpers/application_access_helper_test.rb | 29 +++++++++++++++++++ test/support/granting_access_helpers.rb | 4 +++ 7 files changed, 98 insertions(+) create mode 100644 app/helpers/application_access_helper.rb create mode 100644 test/helpers/application_access_helper_test.rb diff --git a/app/controllers/account/signin_permissions_controller.rb b/app/controllers/account/signin_permissions_controller.rb index 968f7af87..44fc8ffb7 100644 --- a/app/controllers/account/signin_permissions_controller.rb +++ b/app/controllers/account/signin_permissions_controller.rb @@ -1,12 +1,15 @@ class Account::SigninPermissionsController < ApplicationController before_action :authenticate_user! + include ApplicationAccessHelper + def create authorize [:account, Doorkeeper::Application], :grant_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 + flash[:success_alert] = { message: "Access granted", description: access_granted_description(application.id) } 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 2bca2d42b..09085b727 100644 --- a/app/controllers/users/signin_permissions_controller.rb +++ b/app/controllers/users/signin_permissions_controller.rb @@ -3,6 +3,8 @@ class Users::SigninPermissionsController < ApplicationController before_action :set_user before_action :set_application, except: [:create] + include ApplicationAccessHelper + def create application = Doorkeeper::Application.not_api_only.find(params[:application_id]) authorize [{ application:, user: @user }], :grant_signin_permission?, policy_class: Users::ApplicationPolicy @@ -10,6 +12,7 @@ def create params = { supported_permission_ids: @user.supported_permissions.map(&:id) + [application.signin_permission.id] } UserUpdate.new(@user, params, current_user, user_ip_address).call + flash[:success_alert] = { message: "Access granted", description: access_granted_description(application.id, @user) } redirect_to user_applications_path(@user) end diff --git a/app/helpers/application_access_helper.rb b/app/helpers/application_access_helper.rb new file mode 100644 index 000000000..b15796543 --- /dev/null +++ b/app/helpers/application_access_helper.rb @@ -0,0 +1,10 @@ +module ApplicationAccessHelper + def access_granted_description(application_id, user = current_user) + application = Doorkeeper::Application.find_by(id: application_id) + return nil unless application + + return "You have been granted access to #{application.name}." if user == current_user + + "#{user.name} has been granted access to #{application.name}." + end +end diff --git a/test/controllers/account/signin_permissions_controller_test.rb b/test/controllers/account/signin_permissions_controller_test.rb index 91c7e893c..426ac64d0 100644 --- a/test/controllers/account/signin_permissions_controller_test.rb +++ b/test/controllers/account/signin_permissions_controller_test.rb @@ -19,6 +19,30 @@ class Account::SigninPermissionsControllerTest < ActionController::TestCase post :create, params: { application_id: application.id } end end + + should "assign the success alert hash to flash" do + current_user = create(:admin_user) + sign_in current_user + + application = create(:application) + + stub_policy( + current_user, + Doorkeeper::Application, + policy_class: Account::ApplicationPolicy, + grant_signin_permission?: true, + ) + + Account::SigninPermissionsController + .any_instance + .expects(:access_granted_description) + .with(application.id).returns("Granted access to myself") + + post :create, params: { application_id: application.id } + + expected = { message: "Access granted", description: "Granted access to myself" } + assert_equal expected, flash[:success_alert] + end end context "#delete" do diff --git a/test/controllers/users/signin_permissions_controller_test.rb b/test/controllers/users/signin_permissions_controller_test.rb index f4c1b4989..270352b0b 100644 --- a/test/controllers/users/signin_permissions_controller_test.rb +++ b/test/controllers/users/signin_permissions_controller_test.rb @@ -43,6 +43,31 @@ class Users::SigninPermissionsControllerTest < ActionController::TestCase assert_redirected_to user_applications_path(user) end + should "assign the success alert hash to flash" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + + stub_policy( + current_user, + { application:, user: }, + policy_class: Users::ApplicationPolicy, + grant_signin_permission?: true, + ) + + Users::SigninPermissionsController + .any_instance + .expects(:access_granted_description) + .with(application.id, user).returns("Granted access to another user") + + post :create, params: { user_id: user, application_id: application.id } + + expected = { message: "Access granted", description: "Granted access to another user" } + assert_equal expected, flash[:success_alert] + end + should "prevent unauthenticated users" do user = create(:user) application = create(:application) diff --git a/test/helpers/application_access_helper_test.rb b/test/helpers/application_access_helper_test.rb new file mode 100644 index 000000000..159f86eef --- /dev/null +++ b/test/helpers/application_access_helper_test.rb @@ -0,0 +1,29 @@ +require "test_helper" + +class ApplicationAccessHelperTest < ActionView::TestCase + setup do + @application = create(:application, name: "Whitehall") + stubs(:current_user).returns(create(:user)) + end + + context "#access_granted_description" do + context "when the user is granting themself access" do + should "return a message informing them that they have access to an application" do + assert_equal "You have been granted access to Whitehall.", access_granted_description(@application) + end + end + + context "when the user is granting another access" do + should "return a message informing them that the other user has access to an application" do + user = create(:user, name: "Gerald") + assert_equal "Gerald has been granted access to Whitehall.", access_granted_description(@application, user) + end + end + + context "when the application does not exist" do + should "return nil" do + assert_nil access_granted_description(:made_up_id) + end + end + end +end diff --git a/test/support/granting_access_helpers.rb b/test/support/granting_access_helpers.rb index 063446fb3..c55a6473f 100644 --- a/test/support/granting_access_helpers.rb +++ b/test/support/granting_access_helpers.rb @@ -29,6 +29,10 @@ def assert_grant_access(application, grantee, grantee_is_self: false) assert app_with_access_table.has_content?(application.name) assert grantee.has_access_to?(application) + + success_alert_description = grantee_is_self ? "You have been granted access to #{application.name}." : "#{grantee.name} has been granted access to #{application.name}." + assert_flash_content("Access granted") + assert_flash_content(success_alert_description) end def refute_grant_access(application) From 658aa6da4f63c528d75a6e86c86b9c9ba4b60681 Mon Sep 17 00:00:00 2001 From: George Eaton Date: Wed, 18 Sep 2024 17:28:34 +0100 Subject: [PATCH 5/5] Show success banner when removing access from an application --- .../account/signin_permissions_controller.rb | 1 + .../users/signin_permissions_controller.rb | 1 + app/helpers/application_access_helper.rb | 9 +++++++ .../signin_permissions_controller_test.rb | 25 ++++++++++++++++++ .../signin_permissions_controller_test.rb | 26 +++++++++++++++++++ .../helpers/application_access_helper_test.rb | 21 +++++++++++++++ test/support/removing_access_helpers.rb | 4 +++ 7 files changed, 87 insertions(+) diff --git a/app/controllers/account/signin_permissions_controller.rb b/app/controllers/account/signin_permissions_controller.rb index 44fc8ffb7..47f78dd7a 100644 --- a/app/controllers/account/signin_permissions_controller.rb +++ b/app/controllers/account/signin_permissions_controller.rb @@ -23,6 +23,7 @@ def destroy 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 + flash[:success_alert] = { message: "Access removed", description: access_removed_description(application.id) } 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 09085b727..39fc9a2ea 100644 --- a/app/controllers/users/signin_permissions_controller.rb +++ b/app/controllers/users/signin_permissions_controller.rb @@ -26,6 +26,7 @@ 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[:success_alert] = { message: "Access removed", description: access_removed_description(@application.id, @user) } redirect_to user_applications_path(@user) end diff --git a/app/helpers/application_access_helper.rb b/app/helpers/application_access_helper.rb index b15796543..1dc1b7d42 100644 --- a/app/helpers/application_access_helper.rb +++ b/app/helpers/application_access_helper.rb @@ -7,4 +7,13 @@ def access_granted_description(application_id, user = current_user) "#{user.name} has been granted access to #{application.name}." end + + def access_removed_description(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/test/controllers/account/signin_permissions_controller_test.rb b/test/controllers/account/signin_permissions_controller_test.rb index 426ac64d0..eef99fd50 100644 --- a/test/controllers/account/signin_permissions_controller_test.rb +++ b/test/controllers/account/signin_permissions_controller_test.rb @@ -66,6 +66,31 @@ class Account::SigninPermissionsControllerTest < ActionController::TestCase end context "#destroy" do + should "assign the success alert hash to flash" do + current_user = create(:admin_user) + sign_in current_user + + application = create(:application) + current_user.grant_application_signin_permission(application) + + stub_policy( + current_user, + application, + policy_class: Account::ApplicationPolicy, + remove_signin_permission?: true, + ) + + Account::SigninPermissionsController + .any_instance + .expects(:access_removed_description) + .with(application.id).returns("Removed access from myself") + + delete :destroy, params: { application_id: application.id } + + expected = { message: "Access removed", description: "Removed access from myself" } + assert_equal expected, flash[:success_alert] + end + should "prevent unauthenticated users" do application = create(:application) diff --git a/test/controllers/users/signin_permissions_controller_test.rb b/test/controllers/users/signin_permissions_controller_test.rb index 270352b0b..3c7eb140d 100644 --- a/test/controllers/users/signin_permissions_controller_test.rb +++ b/test/controllers/users/signin_permissions_controller_test.rb @@ -274,6 +274,32 @@ class Users::SigninPermissionsControllerTest < ActionController::TestCase assert_redirected_to user_applications_path(user) end + should "assign the success alert hash to flash" do + current_user = create(:admin_user) + sign_in current_user + + user = create(:user) + application = create(:application) + user.grant_application_signin_permission(application) + + stub_policy( + current_user, + { application:, user: }, + policy_class: Users::ApplicationPolicy, + remove_signin_permission?: true, + ) + + Users::SigninPermissionsController + .any_instance + .expects(:access_removed_description) + .with(application.id, user).returns("Removed access from another user") + + delete :destroy, params: { user_id: user, application_id: application.id } + + expected = { message: "Access removed", description: "Removed access from another user" } + assert_equal expected, flash[:success_alert] + end + should "prevent unauthenticated users" do user = create(:user) application = create(:application) diff --git a/test/helpers/application_access_helper_test.rb b/test/helpers/application_access_helper_test.rb index 159f86eef..0b1356d46 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_description" do + context "when the user is removing their own access" do + should "return a message informing them that they no longer have access to the application" do + assert_equal "Your access to Whitehall has been removed.", access_removed_description(@application) + end + end + + context "when the user is removing another's access" do + should "return a message informing them that the other user no longer has access to the application" do + user = create(:user, name: "Gerald") + assert_equal "Gerald's access to Whitehall has been removed.", access_removed_description(@application, user) + end + end + + context "when the application does not exist" do + should "return nil" do + assert_nil access_removed_description(:made_up_id) + end + end + end end diff --git a/test/support/removing_access_helpers.rb b/test/support/removing_access_helpers.rb index a8b674788..228d2fa3b 100644 --- a/test/support/removing_access_helpers.rb +++ b/test/support/removing_access_helpers.rb @@ -30,6 +30,10 @@ 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_alert_description = 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_alert_description) end def refute_remove_access(application)