To fix #992, show the Feedback NavBar button only to Admins & Sys-Admins
I chose those approach after seeing that the route triggered by the Feedback button uses the SoftwareFeedbacksController is derived from AdminController, which allows actions only by users with either the admin or sys_admin role. That is, I inferred the UI was incorrectly offering the button.
What
Change NavBarPolicy#visible_buttons to include Feedback only for Users with either the admin or sys_admin.
How
Moved adding Feedback to the allow-list from the line with NavBarPolicy#visible_buttons's simple non-nil check to the line guarded with the call to ApplicationPolicy#can_admin?
Testing
The PR modifies existing examples, removing the expectation for Feedback's presence for neighbor cases.
With modified examples, without modified code - 2 failures
With modified examples & code - 0 failures
Rubocop - no change
Outstanding Questions, Concerns and Other Notes
More about the specs than the bug: in my dev/repro environment, I noticed that my new User had the unset role. This was different from neighbor, but both are "neither admin nor sys_admin", so I simply modified the existing contexts.
Accessibility
None that I'm aware of.
Security
Very slight improvement—now the UI "advertises" one fewer route to non-{,sys}admin users.
Meta
This is my first PR here.
I hope/believe I have read and understood the various guides, policies, etc. (humble apologies if not...)
Constructive feedback welcome, of course.
Pre-Merge Checklist
[ ] Security & accessibility have been considered
[ ] Tests have been added, or an explanation has been given why the features cannot be tested
[ ] Documentation and comments have been added to the codebase where required
[ ] Entry added to CHANGELOG.md if appropriate
[ ] Outstanding questions and concerns have been resolved
[ ] Any next steps have been turned into Issues or Discussions as appropriate
Why
To fix #992, show the
FeedbackNavBar button only to Admins & Sys-AdminsI chose those approach after seeing that the route triggered by the
Feedbackbutton uses theSoftwareFeedbacksControlleris derived fromAdminController, which allows actions only by users with either theadminorsys_adminrole. That is, I inferred the UI was incorrectly offering the button.What
Change
NavBarPolicy#visible_buttonsto includeFeedbackonly forUsers with either theadminorsys_admin.How
Moved adding
Feedbackto the allow-list from the line withNavBarPolicy#visible_buttons's simple non-nilcheck to the line guarded with the call toApplicationPolicy#can_admin?Testing
The PR modifies existing examples, removing the expectation for
Feedback's presence forneighborcases.With modified examples, without modified code - 2 failures
With modified examples & code - 0 failures
Rubocop - no change
Outstanding Questions, Concerns and Other Notes
More about the specs than the bug: in my dev/repro environment, I noticed that my new
Userhad theunsetrole. This was different fromneighbor, but both are "neitheradminnorsys_admin", so I simply modified the existingcontexts.Accessibility
None that I'm aware of.
Security
Very slight improvement—now the UI "advertises" one fewer route to non-{,sys}admin users.
Meta
This is my first PR here. I hope/believe I have read and understood the various guides, policies, etc. (humble apologies if not...) Constructive feedback welcome, of course.
Pre-Merge Checklist