Skip to content

fix: scope eligible residents query to class facility (ID-686)#1161

Merged
carddev81 merged 1 commit into
mainfrom
cpride/enroll_residents_fix
Jun 9, 2026
Merged

fix: scope eligible residents query to class facility (ID-686)#1161
carddev81 merged 1 commit into
mainfrom
cpride/enroll_residents_fix

Conversation

@corypride

@corypride corypride commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Fix

Made the backend authoritative: changed the WHERE clause in GetEligibleResidentsForClass to filter on c.facility_id (the class's own facility via the already-joined program_classes table) instead of args.FacilityID.

This holds regardless of caller role or what facility_id is passed in the request.

Files Changed

backend/src/database/users.go

  • One-line filter change in GetEligibleResidentsForClass

backend/tests/integration/class_enrollments_test.go

  • Added TestEligibleResidentsFilteredByClassFacility covering:
    • Only unenrolled facility-A students are returned
    • Already-enrolled residents are excluded
    • Deactivated residents are excluded
    • Facility-B residents are excluded even when a department admin passes facility_id=B

Not Changed

frontend/src/pages/class-detail/EnrollResidentsModal.tsx

  • Already passes facility_id=${classFacilityId}
  • Kept as-is, but it is no longer the security boundary

getQueryContext facility logic

  • Other endpoints depend on it
  • Left unchanged

Testing

Integration Test

  • Verifies facility isolation is enforced independently of the request's facility_id

Manual Verification

  1. Log in as a department admin
  2. Open a class assigned to facility A
  3. Click Add Resident
  4. Verify that only facility-A residents are shown

@corypride corypride requested a review from a team as a code owner June 5, 2026 14:30
@corypride corypride requested review from calisio and removed request for a team June 5, 2026 14:30
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 604e768b-3874-46f1-86bc-fcabd2e74e6e

📥 Commits

Reviewing files that changed from the base of the PR and between 5f9c489 and d06c336.

📒 Files selected for processing (2)
  • backend/src/database/users.go
  • backend/tests/integration/class_enrollments_test.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Fixed resident eligibility filtering for classes to correctly associate students with their assigned facility, ensuring only residents from the appropriate facility are shown as eligible for enrollment.
  • Tests

    • Added integration test to verify proper facility-based filtering of eligible unenrolled residents.

Walkthrough

The PR modifies how the eligible residents query determines facility scope in GetEligibleResidentsForClass. Rather than filtering by the request's facility context, residents are now filtered by the class's facility via a database join, then validated with a multi-facility integration test.

Changes

Class-based facility filtering

Layer / File(s) Summary
Class facility filter implementation and test
backend/src/database/users.go, backend/tests/integration/class_enrollments_test.go
GetEligibleResidentsForClass now filters eligible residents by the class's facility (users.facility_id = c.facility_id) instead of the request facility (args.FacilityID). A new integration test validates this behavior by requesting unenrolled residents for a facility-A class while supplying a facility-B context, asserting only active, unenrolled, facility-A residents are returned.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: scoping the eligible residents query to the class facility instead of the request context facility.
Description check ✅ Passed The description is directly related to the changeset, explaining the backend fix and providing context about the facility scoping change with appropriate test coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@corypride corypride requested review from CK-7vn and removed request for calisio June 8, 2026 21:20

@carddev81 carddev81 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.

Tested Good and code looks great

@carddev81 carddev81 merged commit 1f3440b into main Jun 9, 2026
12 checks passed
@carddev81 carddev81 deleted the cpride/enroll_residents_fix branch June 9, 2026 15:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants