Skip to content

Resolve source CE referral on enrollment - #5999

Merged
martha merged 17 commits into
release-190from
mke/8283-src-enr-ref
Nov 14, 2025
Merged

Resolve source CE referral on enrollment#5999
martha merged 17 commits into
release-190from
mke/8283-src-enr-ref

Conversation

@martha

@martha martha commented Nov 12, 2025

Copy link
Copy Markdown
Contributor

Merging this PR

  • use the squash-merge strategy for PRs targeting a release-X branch

Description

  • Repurpose the existing can_view_summary? referral policy for users who can view the target enrollment, even if they can't necessarily view details of the referral. (This policy was originally designed to display some limited data about the referral to a user who can send direct referrals from the source project, but can't view referrals in the target project.)
    • I thought this was ergonomically nice because the frontend can then resolve the CeReferral schema object and use its access field to determine whether to link to the referral from the enrollment.
    • I reviewed the list of summary fields on CeReferral that this would give the user access to, and they all seem okay to expose, especially since under this new use case, the user already can view enrollments in the target project.
    • Alternative, if we are concerned that this repurposing exposes too much data: We could just resolve the referral-related fields needed (sourceCeReferralId, sourceCeReferralProjectName, etc.) directly on the enrollment.
  • Add source_ce_referral assoc on enrollment and resolve it on the graphql type

Frontend PR: greenriver/hmis-frontend#1319

Type of change

New feature

Checklist before requesting review

  • I have performed a self-review of my code
  • I have run the code that is being changed under ideal conditions, and it doesn't fail
  • If adding a new endpoint / exposing data in a new way, I have:
    • ensured the API can't leak data from other data sources
    • ensured this does not introduce N+1s
    • ensured permissions and visibility checks are performed in the right places
  • Any major architectural changes are supported by an approved ADR (Architectural Decision Record)
  • I have updated the documentation (or not applicable)
  • I have added spec tests (or not applicable)
  • I have provided testing instructions in this PR or the related issue (or not applicable)

//: # NOTE: system tests may fail if there is no branch on the hmis-frontend that matches the Source or Target branch of this PR. This is expected

@martha martha changed the title Mke/8283 src enr ref Resolve source CE referral on enrollment Nov 12, 2025
summary_field :created_at, GraphQL::Types::ISO8601DateTime, null: false
summary_field :source_enrollment_id, ID, null: false
# source_project_name is resolved separately from source_enrollment as a summary field to minimize data exposure
summary_field :source_project_name, String, null: false

@martha martha Nov 12, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm proposing to add source_project_name as a new summary-level field, even though it is also resolvable through the sourceEnrollment field. As we currently have it set up, sourceEnrollment is not a summary-level field, so it's only resolvable by those who can view the referral. I thought about elevating it to a summary field, but it contains a lot of data (like assessments and household members) that are not needed to solve the current requirement.

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.

I think this makes sense! I needs to be nullable though, because source_enrollment_id is not required on the referral table. (source_enrollment_id graphql field also should be nullable, which I incorporated into wip #6000 , but you can make the change here too).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

summary_field :custom_status, HmisSchema::CeCustomReferralStatus, null: true
summary_field :client_id, ID, null: false
summary_field :client_name, String, null: true, description: 'The name of the referred client. Always available to those who can view the full referral, even without full client record access.'
# Special case: Client is a "summary field" because it doesn't require referral visibility, but resolving it does require permission to view the client record.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Removing unrelated - I think this comment was outdated, since client is not a summary-level field.

@martha
martha requested a review from gigxz November 13, 2025 15:25
@martha
martha marked this pull request as ready for review November 13, 2025 15:25
Comment thread drivers/hmis/app/graphql/types/hmis_schema/ce_referral.rb

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

Looks good!

Comment thread drivers/hmis/app/graphql/types/hmis_schema/enrollment.rb
# All referrals where this enrollment is the source enrollment. NOT only 'direct' referrals
has_many :outgoing_ce_referrals, class_name: 'Hmis::Ce::Referral', foreign_key: :source_enrollment_id
# The referral that created this enrollment (if any)
has_one :source_ce_referral, class_name: 'Hmis::Ce::Referral', foreign_key: :target_enrollment_id

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.

Thanks for adding this. This is a bit unrelated, but we have the same "dangling reference" problem as we do with the outgoing_ce_referrals association. If the target Enrollment is destroyed, the source enrollment will continue to have a target_enrollment_id that doesn't resolve to anything.

I will address this in #6000 since you're just adding the reference and not changing the default behavior here, it will need more tests etc. Just a note to self! I think we want dependent: :nullify In other words if the Target Enrollment is deleted, the referral should remain but have the target_enrollment_id nullified.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree, that makes sense!

@@ -39,7 +39,13 @@ def can_view?
def can_view_summary?
return false unless Hmis::Ce.configuration.enabled?

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.

Can we add a return true if can_view?
It doesn't feel logical to have scenarios where the use can view the full referral, but they can't view the summary of it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines +45 to +46
# Users who can view the target enrollment. Bakes in the assumption that the target enrollment is in the referral's project.
return true if referral.target_enrollment_id.present? && project_permissions.include?(:can_view_enrollment_details)

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.

If we had an Enrollment Policy, it would be nice if we could check against that here. Maybe we can add a TODO referencing https://github.com/open-path/Green-River/issues/8549 for that.

Full Enrollment visibility currently requires both can_view_enrollment_details and can_view_project so we should check that both of those are included in project_permissions to be comprehensive. (This is the thing that would be ideal to encapsulate in the policy!)

@martha martha Nov 14, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Cool thanks for tracking down the open ticket! I'll add a todo and also add an extra check that the user can_view_project. b855f95

end
end

context 'when referral has a target enrollment' do

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.

Thanks for these tests! I think we are missing coverage for the case where the referral doesn't have a target enrollment. We should test that because I think we are wanting to limit summary visibility to only referrals that have resulted in an enrollment at the target project.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oh yup makes sense, added test coverage for that 77b4f03

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was beginning to type up the question, Do you think we need coverage for the case where the target enrollment ID points at an enrollment that isn't in the target project? (referring back to my comment here)

But, thinking about this more, that really sounds like it should be a validation on the referral model. I'm taking a stab at that, but wanted to split it out into a separate diff, in case it causes a bunch of tests to fail, so it doesn't block this work. #6007

@martha
martha changed the base branch from release-189 to release-190 November 14, 2025 12:58

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

Looks great! Thanks!

@martha
martha merged commit 12f64f4 into release-190 Nov 14, 2025
18 checks passed
@martha
martha deleted the mke/8283-src-enr-ref branch November 14, 2025 22:17
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.

2 participants