Fixes 500 error when viewing the "Tag Mods" page without having the role on the database #10515
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
The current implementation of the query would yield an exception when recieving an invalid state parameter. As per the [GH issue discussion](#10060 (comment)), Zhao recommended to change its behaviour and not trigger an exception in this condition. This commit does just that. If the state argument is invalid, the query now returns an empty result set.
diogoosorio
requested review from
a team and
michael-tharrington
and removed request for
a team
fdocr
requested review from
a team,
juliannatetreault and
benhalpern
and removed request for
michael-tharrington and
a team
fdocr
approved these changes
Oct 1, 2020
There was a problem hiding this comment.
Thank you for the fix @diogoosorio. Tested it locally and seems to be working well
| @@ -24,7 +24,7 @@ def self.call(relation: User.all, options: {}) | |||
| end | |||
|
|
|||
| def self.role_id_for(role) | |||
| Role.find_by!(name: role).id | |||
| Role.find_by(name: role)&.id | |||
There was a problem hiding this comment.
I always forget about this operator, nice 🙌
pr-triage
bot
added
PR: partially-approved
juliannatetreault
approved these changes
Oct 1, 2020
pr-triage
bot
added
PR: merged
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.
What type of PR is this? (check all applicable)
Description
I think that the issue (#10060) is self-explanatory. What was happening with @msarit was that she was visiting the
/admin/mods?state=tag_moderatorwithout having a role namedtag_moderatoron the database.The query class explicitly "blew up" in this condition. I tried to follow the approach proposed by @Zhao-Andy:
The query class now doesn't raise an exception in the scenario above. Instead returns an empty result set
The mods page now returns a generic "there are mods matching your search" warning if there are no mods matching the query
Related Tickets & Documents
#10060
QA Instructions, Screenshots, Recordings
Go to the
/admin/modspageSearch by a random text string which yields no results and see that a "no results" message is rendered
Delete the
tag_moderatorrole from your database (Role.find_by(name: "tag_moderator").delete)Go to
/admin/mods?state=tag_moderatorSee that a "no results" message is rendered
Added tests?
Added to documentation?