-
Notifications
You must be signed in to change notification settings - Fork 78
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.
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
Remove edit option from collection menu if not available to user. #2567
Remove edit option from collection menu if not available to user. #2567
Conversation
</button> | ||
<ul class="dropdown-menu dropdown-menu-right"> | ||
<% if current_user && document.edit_people.include?(current_user.email) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way we check for permissions is by using the ability class. So can? :edit, document
. This way it will check the edit groups as well as the edit users.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. Thanks.
Updated test using allow(user).to receive versus stubbing in access to document. |
</button> | ||
<ul class="dropdown-menu dropdown-menu-right"> | ||
<% if current_user && current_user.can?(:edit, document) %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need for the current_user
just can? ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't think that was possible as test failed without current_user. Looking closer, I found the issue was in the test not the execution. Added let(:ability) { instance_double("Ability") }
and allow(controller).to receive(:current_ability).and_return(ability)
to fix.
Once you've made that change can you squash all your commits into a single commit (and then force push to this branch)? |
…t_people. Added tests. Remove testing stuff. Rubocop fix. Use ability class to check for edit rights. Refactored test. Use can? without current_user.
2979a63
to
8f0bd06
Compare
Fixes #2400
Removes edit option when unavailable.
This should remove the edit option when a user clicks on the "Select an Action" for an individual work being viewed from a collection page if that user does not have edit rights to the work.
@projecthydra/sufia-code-reviewers