Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Show view permissions link even when updatable #3019

Merged
merged 1 commit into from
Jul 18, 2024
Merged

Show view permissions link even when updatable #3019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 13 additions & 12 deletions app/helpers/application_table_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,22 +37,23 @@ def users_applications_remove_access_link(application, user)
end
end

def account_applications_permissions_link(application)
if policy([:account, application]).edit_permissions?
update_permissions_link(application)
elsif policy([:account, application]).view_permissions?
view_permissions_link(application)
else
""
end
def account_applications_permissions_links(application)
links = []

links << view_permissions_link(application) if policy([:account, application]).view_permissions?
links << update_permissions_link(application) if policy([:account, application]).edit_permissions?

safe_join(links)
end

def users_applications_permissions_link(application, user)
def users_applications_permissions_links(application, user)
links = [view_permissions_link(application, user)]

if policy(UserApplicationPermission.for(user:, supported_permission: application.signin_permission)).edit?
update_permissions_link(application, user)
else
view_permissions_link(application, user)
links << update_permissions_link(application, user)
end

safe_join(links)
end

def api_users_applications_permissions_link(application, user)
Expand Down
2 changes: 1 addition & 1 deletion app/views/account/applications/index.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
{ text: application.name },
{ text: application.description },
{ text: wrap_links_in_actions_markup([
account_applications_permissions_link(application),
account_applications_permissions_links(application),
account_applications_remove_access_link(application)
])
},
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/applications/index.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
{ text: application.name },
{ text: application.description },
{ text: wrap_links_in_actions_markup([
users_applications_permissions_link(application, @user),
users_applications_permissions_links(application, @user),
users_applications_remove_access_link(application, @user)
])
},
Expand Down
22 changes: 11 additions & 11 deletions docs/access_and_permissions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,16 +29,16 @@ In this section, the granter and grantee are the same user: this is about managi

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
|-------------------------|--------------|---------------|------------------|------------------|
| None | ✅ | ✅ | ✅ | ❌ |
| `signin` | ✅ | ✅ | ✅ | ❌ |
| Another permission | ✅ | ✅ | ✅ | ❌ |
| None | ✅ | ✅ | ✅ | ✅ |
| `signin` | ✅ | ✅ | ✅ | ✅ |
| Another permission | ✅ | ✅ | ✅ | ✅ |

#### As a publishing manager

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
|-------------------------|--------------|---------------|------------------|------------------|
| None | ❌ | ❌ | ❌ | ✅ |
| `signin` | ❌ | ✅ | ✅ | ❌ |
| `signin` | ❌ | ✅ | ✅ | ✅ |
| Another permission | ❌ | ❌ | ❌ | ✅ |

### Dependencies by route
Expand Down Expand Up @@ -131,17 +131,17 @@ In this section, the granter and grantee are different users: this is about mana

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
|-------------------------|--------------|---------------|------------------|------------------|
| None | ✅ | ✅ | ✅ | ❌ |
| `signin` | ✅ | ✅ | ✅ | ❌ |
| Another permission | ✅ | ✅ | ✅ | ❌ |
| None | ✅ | ✅ | ✅ | ✅ |
| `signin` | ✅ | ✅ | ✅ | ✅ |
| Another permission | ✅ | ✅ | ✅ | ✅ |

##### Without access to the app

| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
|-------------------------|--------------|---------------|------------------|------------------|
| None | ✅ | ✅ | ✅ | ❌ |
| `signin` | ✅ | ✅ | ✅ | ❌ |
| Another permission | ✅ | ✅ | ✅ | ❌ |
| None | ✅ | ✅ | ✅ | ✅ |
| `signin` | ✅ | ✅ | ✅ | ✅ |
| Another permission | ✅ | ✅ | ✅ | ✅ |

#### As a publishing manager

Expand All @@ -150,7 +150,7 @@ In this section, the granter and grantee are different users: this is about mana
| Delegatable permissions | Grant access | Revoke access | Edit permissions | View permissions |
|-------------------------|--------------|---------------|------------------|------------------|
| None | ❌ | ❌ | ❌ | ✅ |
| `signin` | ✅ | ✅ | ✅ | ❌ |
| `signin` | ✅ | ✅ | ✅ | ✅ |
| Another permission | ❌ | ❌ | ❌ | ✅ |

##### Without access to the app
Expand Down
34 changes: 27 additions & 7 deletions test/controllers/account/applications_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,28 +126,48 @@ class Account::ApplicationsControllerTest < ActionController::TestCase
end

context "editing permissions" do
should "not display any permissions links when the app only has the signin permission" do
stub_policy @user, [:account, @application], view_permissions?: true, edit_permissions?: true
context "when the app only has the signin permission" do
should "only display a link to view permissions when authorised to view or edit permissions" do
stub_policy @user, [:account, @application], view_permissions?: true, edit_permissions?: true

get :index
get :index

assert_select "a[href='#{edit_account_application_permissions_path(@application)}']", count: 0
assert_select "a[href='#{account_application_permissions_path(@application)}']"
end

should "not display links to view or edit permissions when not authorised to view permissions" do
stub_policy @user, [:account, @application], view_permissions?: false, edit_permissions?: true

get :index

assert_select "a[href='#{edit_account_application_permissions_path(@application)}']", count: 0
assert_select "a[href='#{account_application_permissions_path(@application)}']", count: 0
assert_select "a[href='#{edit_account_application_permissions_path(@application)}']", count: 0
assert_select "a[href='#{account_application_permissions_path(@application)}']", count: 0
end
end

context "when the app has non-signin permissions" do
setup { create(:supported_permission, application: @application) }

should "display a link to edit permissions when authorised to edit permissions" do
should "display links to view and edit permissions when authorised to view and edit permissions" do
stub_policy @user, [:account, @application], view_permissions?: true, edit_permissions?: true

get :index

assert_select "a[href='#{edit_account_application_permissions_path(@application)}']"
assert_select "a[href='#{account_application_permissions_path(@application)}']"
end

should "only display a link to edit permissions when authorised to edit but not view permissions" do
stub_policy @user, [:account, @application], view_permissions?: false, edit_permissions?: true

get :index

assert_select "a[href='#{edit_account_application_permissions_path(@application)}']"
assert_select "a[href='#{account_application_permissions_path(@application)}']", count: 0
end

should "display a link to view permissions when not authorised to view but not edit permissions" do
should "only display a link to view permissions when not authorised to edit permissions" do
stub_policy @user, [:account, @application], view_permissions?: true, edit_permissions?: false

get :index
Expand Down
13 changes: 7 additions & 6 deletions test/controllers/users/applications_controller_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,33 +136,34 @@ class Users::ApplicationsControllerTest < ActionController::TestCase
context "editing permissions" do
setup { @user_application_permission = stub_user_application_permission(@user, @application) }

should "not display any permissions links when the app only has the signin permission" do
should "only display a link to view permissions when the app only has the signin permission" do
stub_policy @current_user, @user_application_permission, edit?: true

get :index, params: { user_id: @user }

assert_select "a[href='#{edit_user_application_permissions_path(@user, @application)}']", count: 0
assert_select "a[href='#{edit_user_application_permissions_path(@user, @application)}']", text: "Update permissions for app-name", count: 0
assert_select "a[href='#{user_application_permissions_path(@user, @application)}']", text: "View permissions for app-name"
end

context "when the app has non-signin permissions" do
setup { create(:supported_permission, application: @application) }

should "display a link to edit permissions when authorised to edit permissions" do
should "display links to view and edit permissions when authorised to edit permissions" do
stub_policy @current_user, @user_application_permission, edit?: true

get :index, params: { user_id: @user }

assert_select "a[href='#{edit_user_application_permissions_path(@user, @application)}']", text: "Update permissions for app-name"
assert_select "a[href='#{user_application_permissions_path(@user, @application)}']", text: "View permissions for app-name", count: 0
assert_select "a[href='#{user_application_permissions_path(@user, @application)}']", text: "View permissions for app-name"
end

should "display a link to view permissions when not authorised to edit permissions" do
should "only display a link to view permissions when not authorised to edit permissions" do
stub_policy @current_user, @user_application_permission, edit?: false

get :index, params: { user_id: @user }

assert_select "a[href='#{user_application_permissions_path(@user, @application)}']", text: "View permissions for app-name"
assert_select "a[href='#{edit_user_application_permissions_path(@user, @application)}']", text: "Update permissions for app-name", count: 0
assert_select "a[href='#{user_application_permissions_path(@user, @application)}']", text: "View permissions for app-name"
end
end
end
Expand Down
47 changes: 35 additions & 12 deletions test/helpers/application_table_helper_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,47 +97,70 @@ class ApplicationTableHelperTest < ActionView::TestCase
end
end

context "#account_applications_permissions_link" do
context "#account_applications_permissions_links" do
setup do
@user = build(:user)
stubs(:current_user).returns(@user)
@application = create(:application, with_supported_permissions: %w[permission])
end

should "generate an update link when the user can edit permissions" do
stub_policy @user, [:account, @application], edit_permissions?: true
assert_includes account_applications_permissions_link(@application), "Update permissions"
should "generate both view and update links when the user can both view and edit permissions" do
stub_policy @user, [:account, @application], view_permissions?: true, edit_permissions?: true

result = account_applications_permissions_links(@application)

assert_includes result, "View permissions"
assert_includes result, "Update permissions"
end

should "generate a view link when the user can view permissions" do
should "only generate a view link when the user can only view permissions" do
stub_policy @user, [:account, @application], view_permissions?: true
assert_includes account_applications_permissions_link(@application), "View permissions"

result = account_applications_permissions_links(@application)

assert_includes result, "View permissions"
assert_not_includes result, "Update permissions"
end

should "only generate an update link when the user can only edit permissions" do
stub_policy @user, [:account, @application], edit_permissions?: true

result = account_applications_permissions_links(@application)

assert_not_includes result, "View permissions"
assert_includes result, "Update permissions"
end

should "return an empty string when the user can do neither" do
stub_policy @user, [:account, @application]
assert account_applications_permissions_link(@application).empty?
assert account_applications_permissions_links(@application).empty?
end
end

context "#users_applications_permissions_link" do
context "#users_applications_permissions_links" do
setup do
@application = create(:application, with_supported_permissions: %w[permission])
@grantee = create(:user)
end

should "generate an update link when the user can edit permissions" do
should "generate both view and update links when the user can edit permissions" do
granter = create(:superadmin_user)
stubs(:current_user).returns(granter)

assert_includes users_applications_permissions_link(@application, @grantee), "Update permissions"
result = users_applications_permissions_links(@application, @grantee)

assert_includes result, "View permissions"
assert_includes result, "Update permissions"
end

should "generate a view link when the user cannot edit permissions" do
should "only generate a view link when the user cannot edit permissions" do
granter = create(:user)
stubs(:current_user).returns(granter)

assert_includes users_applications_permissions_link(@application, @grantee), "View permissions"
result = users_applications_permissions_links(@application, @grantee)

assert_includes result, "View permissions"
assert_not_includes result, "Update permissions"
end
end

Expand Down