Project edit by admins #1757
Merged
Project edit by admins #1757
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
JoeCohen
commented
Dec 29, 2023
JoeCohen
commented
- Lets admins edit Projects
- Turns tests of prior commit green, i.e., admin sees an edit_project link - But unexpectly breaks other tests
Uses a project fixture owned by mary or rolf and for which the other is not an admin. ControllerExtensions expects the request to pass with the first suppied user, and fail with the alternate user.
Commit ab4aaa9 broke some tests in unexpected ways. The current commit fixes those breakages as follows: - Adds a new Project fixture - Adds a NameDescription fixture associated with that Project (Needed in order to turn test_destroy_project_member green) - Updates QueryTest to conform to the new NameDescription fixture
- Gets tubaria_furfuracea_desc source_name directly instead of reflecting on project name. - Hopefully fixes [this CI test failure](https://github.com/MushroomObserver/mushroom-observer/actions/runs/7358762659/job/20032512727) - NOTE: That test passed locally. I wonder why it fails in CI, but don't want to go down that rabbit hole.
- Fixes Naming/PredicateName offense - Conforms to our other uses of `member?` Note: Only Project has `is_member?` method.
JoeCohen
commented
Dec 29, 2023
Comment on lines
+112
to
+115
| def can_edit?(user = User.current) | ||
| admin?(user) | ||
| end | ||
|
|
There was a problem hiding this comment.
This seemed like the simplest change to make Projects editable by admins.
- Adds clarifcation - Fixes typo
JoeCohen
commented
Dec 29, 2023
JoeCohen
commented
Dec 29, 2023
Comment on lines
+453
to
+463
| project = projects(:rolf_project) | ||
| member = users(:katrina) | ||
| assert( | ||
| project.member?(member) && project.user != member && | ||
| !project.admin?(member) && | ||
| NameDescription.where(source_name: project.title).any?, | ||
| "Bad fixtures: member must be project member, but not owner or admin " \ | ||
| "and project must be the source of a NameDescription" | ||
| ) | ||
|
|
||
| destroy_project_helper(project, katrina) |
There was a problem hiding this comment.
destroy_project_helper
- Needs a Project with an associated NameDescription
- Should not destroy the Project, so it must be called with a user who cannot destroy.
|
@mo-nathan: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.