-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CPDLP-3899] Ensure Void/Show declarations are scoped to ECF declarations #5402
Conversation
Review app deployed to https://cpd-ecf-review-5402-web.test.teacherservices.cloud |
@@ -71,7 +71,7 @@ def participant_declaration_query_scope | |||
end | |||
|
|||
def participant_declaration_for_lead_provider | |||
@participant_declaration_for_lead_provider ||= ParticipantDeclaration.for_lead_provider(cpd_lead_provider).find(params[:id]) | |||
@participant_declaration_for_lead_provider ||= ParticipantDeclaration::ECF.for_lead_provider(cpd_lead_provider).find(params[:id]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we add specs for this please?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's already specs for this scenario.. any other scenario you have in mind so I can add specs?
https://github.com/DFE-Digital/early-careers-framework/blob/main/spec/requests/api/v1/participant_declarations_spec.rb#L922
https://github.com/DFE-Digital/early-careers-framework/blob/main/spec/requests/api/v2/participant_declarations_spec.rb#L640
https://github.com/DFE-Digital/early-careers-framework/blob/main/spec/requests/api/v3/participant_declarations_spec.rb#L776
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it to ensure NPQ declarations are no longer returned maybe? 🤔 I'm not sue its worth adding specs for those given we're nuking the data (or have already removed it I'm not sure?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea fair enough if NPQ is going we can't really test it
Context
As part of our clean up of NPQ in ECF, we are scoping all endpoints and queries to ECF only. We need to also scope void/show declaration to ECF as well.
Changes proposed in this pull request
participant_declaration_for_lead_provider
byparticipant_declaration_query_scope
but the queries of the second are slightly different taking in considerationinduction records
&partnerships
as well. That would allow past lead providers to void declarations currently with other lead providers;