feat: add observer role access control with migration and tests#75
feat: add observer role access control with migration and tests#75WaelAlahamdi wants to merge 1 commit intothoth-tech:9.xfrom
Conversation
|
Hello Wael, I’ve reviewed your changes and can confirm they’re stable and well-written. From my understanding, this task was to create a role with read-only access in OnTrack. I tested this using Postman and verified that change attempts are correctly rejected when the role is set to true, which aligns with the expected behaviour. The tests you’ve written also confirm these findings. Good work on changing the base branch! Thank you for the opportunity to review your work, and good job on this task. |
chelaz1234
left a comment
There was a problem hiding this comment.
Hi Wael,
As Ibi mentioned, I also validated this through Postman and confirmed that modification attempts are properly blocked when the observer role is enabled. That means the observer logic is indeed being triggered as expected, which is great to see.
I also want to highlight some positives in this work:
Preventing observer users from making write actions is an important access-control improvement and aligns well with least-privilege principles.
You included a migration that’s clean and safe, with a sensible default.
Tests were added alongside the change rather than leaving it untested. Even if the coverage doesn’t fully exercise every path, pairing implementation with tests is a very good practice.
Overall, this is a solid step forward in tightening role-based permissions. Nice work!
Description
The Observer Role is a new property added to unit-level roles (
unit_rolestable) in the Doubtfire API. It enables read-only access for tutors or convenors who are granted observer-level privileges. Observers can view information but cannot perform any actions that modify data.observer:booleancolumn to theunit_rolestable via migration (20250803223033_add_observer_to_unit_roles.rb).app/helpers/authorisation_helpers.rbto enforce read-only access whenobserveris true.test/helpers/authorisation_helpers_test.rbto validate observer behavior.db/schema.rbto reflect the migration changes.Type of change
How Has This Been Tested?
:getactions.2 runs, 2 assertions, 0 failures, 0 errorsChecklist:
If you have any questions, please contact @macite or @jakerenzella.