Skip to content

add supervisor check - #144

Open
sbagg wants to merge 6 commits into
sandboxfrom
addSupervisor
Open

add supervisor check#144
sbagg wants to merge 6 commits into
sandboxfrom
addSupervisor

Conversation

@sbagg

@sbagg sbagg commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Supervisor added to check if they can read separation request. #141

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds supervisor-based authorization to the separation-record read endpoint so supervisors can view separation requests for their direct reports (Issue #141).

Changes:

  • Introduces models.admin.isSupervisor(iamId, separationRecordId) to determine whether a user supervises the employee tied to a separation request.
  • Updates GET /separation/:id authorization to allow access when the requester is the employee’s supervisor (in addition to admin/HR access).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
services/lib/models/admin.js Adds isSupervisor helper to check supervisor/direct-report relationship against a separation record.
services/app/api/separation.js Extends GET /separation/:id authorization to allow supervisor access.
Comments suppressed due to low confidence (1)

services/lib/models/admin.js:633

  • Not having any direct reports is an expected condition and shouldn’t be logged as an error. Also, keeping this if/else block formatted consistently improves readability.
    if( !directReports.res.rows.length ){
      console.error("No direct reports found for IAM ID:", iamId);
      return false;
    } else {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread services/lib/models/admin.js Outdated
Comment thread services/app/api/separation.js Outdated
@sbagg
sbagg requested a review from spelkey-ucd July 27, 2026 18:11

@spelkey-ucd spelkey-ucd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach has one big problem - it only works if the user is the current supervisor of the separating employee.

So, the supervisor will not be able to see 1. their historical separation records, 2. an active separation record in which the employee has been removed from our database, but still needs deprovisioning from other systems.

Check the supervisorId on the separation record itself.

@sbagg
sbagg requested a review from spelkey-ucd July 29, 2026 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants