Skip to content

Commit

Permalink
Merge pull request #2344 from alphagov/remove-abbr-tag
Browse files Browse the repository at this point in the history
Remove <abbr> tag from API Users table
  • Loading branch information
mike29736 committed Sep 7, 2023
2 parents efd7ea8 + 3d4fa47 commit 22e7ca6
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 10 deletions.
6 changes: 2 additions & 4 deletions app/helpers/api_users_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,13 @@ def api_user_name(user)
user.suspended? ? content_tag(:del, anchor_tag) : anchor_tag
end

def permissions_by_application(user)
def application_list(user)
content_tag(:ul, class: "govuk-list") do
safe_join(
visible_applications(user).map do |application|
next unless user.permissions_for(application).any?

content_tag(:li) do
content_tag(:abbr, application.name, title: "Permissions: #{user.permissions_for(application).to_sentence}")
end
content_tag(:li, application.name)
end,
)
end
Expand Down
2 changes: 1 addition & 1 deletion app/views/api_users/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
text: user.email,
},
{
text: permissions_by_application(user),
text: application_list(user),
},
{
text: user.suspended? ? "Yes" : "No",
Expand Down
24 changes: 19 additions & 5 deletions test/integration/manage_api_users_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class ManageApiUsersTest < ActionDispatch::IntegrationTest
assert page.has_selector?("td", text: @api_user.name)
assert page.has_selector?("td", text: @api_user.email)

assert page.has_selector?("abbr", text: @application.name)
assert page.has_selector?("td", text: @application.name)
assert page.has_selector?("td:last-child", text: "No") # suspended?
end

Expand Down Expand Up @@ -54,16 +54,18 @@ class ManageApiUsersTest < ActionDispatch::IntegrationTest
select "Managing Editor", from: "Permissions for Whitehall"
click_button "Update API user"

assert page.has_selector?("abbr[title='Permissions: Managing Editor and signin']", text: "Whitehall")

click_link @api_user.name

assert_has_signin_permission_for("Whitehall")
assert_has_other_permissions("Whitehall", ["Managing Editor"])

unselect "Managing Editor", from: "Permissions for Whitehall"
click_button "Update API user"

assert page.has_selector?("abbr[title='Permissions: signin']", text: "Whitehall")

click_link @api_user.name

assert_has_signin_permission_for("Whitehall")

click_link "Account access log"
assert page.has_text?("Access token generated for Whitehall by #{@superadmin.name}")
end
Expand Down Expand Up @@ -111,4 +113,16 @@ class ManageApiUsersTest < ActionDispatch::IntegrationTest
assert page.has_selector?(".alert-success", text: "#{@api_user.email} is now active.")
end
end

def assert_has_signin_permission_for(application_name)
within "table#editable-permissions" do
# The existence of the <tr> indicates that the API User has "singin"
# permission for the application
assert has_selector?("tr", text: application_name)
end
end

def assert_has_other_permissions(application_name, permission_names)
assert has_select?("Permissions for #{application_name}", selected: permission_names)
end
end

0 comments on commit 22e7ca6

Please sign in to comment.