feat: add observer role access control with migration and tests

[WaelAlahamdi]

Description

The Observer Role is a new property added to unit-level roles (unit_roles table) 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.

• Added observer:boolean column to the unit_roles table via migration (20250803223033_add_observer_to_unit_roles.rb).
• Updated app/helpers/authorisation_helpers.rb to enforce read-only access when observer is true.
• Created unit tests in test/helpers/authorisation_helpers_test.rb to validate observer behavior.
• Updated db/schema.rb to reflect the migration changes.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

• Added unit tests verifying:
  • Observer can perform :get actions.
  • Observer cannot perform non-GET actions.
• Ran local tests:
  2 runs, 2 assertions, 0 failures, 0 errors

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation if appropriate
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have created or extended unit tests to address my new additions
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

If you have any questions, please contact @macite or @jakerenzella.

[ibi420]

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.

Thank you for the opportunity to review your work, and good job on this task.

[chelaz1234]

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!

[ibi420]

@WaelAlahamdi, on going through this a second time, you opened the pull request against the wrong base. It should be to thothtech and not to doubtfire-lms

[JosephKS10]

Hi Wael,

I've reviewed your PR for adding the Observer role. I have a clear understanding of the task, which was to implement a read-only access level for tutors and convenors, and I can see you've nailed the core requirement. The implementation looks great. Adding the observer flag directly to the unit_roles table is the perfect place for it, and the logic in the authorisation helper is clean, efficient, and placed exactly where it needs to be to control access across the application. It's excellent that you included the migration and updated the schema as well.

Thank you for the opportunity to review your work, and good job on this task.

[JosephKS10]

@WaelAlahamdi, on going through this a second time, you opened the pull request against the wrong base. It should be to thothtech and not to doubtfire-lms

I agree with @ibi420

[JosephKS10]

you've opened the pull request against the wrong base branch. It should be thothtech instead of doubtfire-lms. Please change the base, and it’ll be good to go.