Skip to content

Commit

Permalink
Remove <abbr> permissions list on app names
Browse files Browse the repository at this point in the history
This tooltip was on our list of things that don't seem like they'd
comply with the design system.

I've now spoken to a few recent and frequent users of the API Users
index page and of the ones who knew this feature existed, none have used
it, so I'm happy to remove it.

The tests that used this feature are now using the API User edit page to
get that information.
  • Loading branch information
mike29736 committed Sep 6, 2023
1 parent 35621cd commit 0637a92
Show file tree
Hide file tree
Showing 3 changed files with 18 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
20 changes: 15 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,26 @@ 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

within "table#editable-permissions" do
# The existence of the <tr> indicates that the API User has "singin"
# permission for Whitehall
assert has_selector?("tr", text: "Whitehall")
end
assert has_select?("Permissions for Whitehall", selected: ["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

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

click_link "Account access log"
assert page.has_text?("Access token generated for Whitehall by #{@superadmin.name}")
end
Expand Down

0 comments on commit 0637a92

Please sign in to comment.