Skip to content
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

[CPDNPQ-2317] change validation and do not destroy declarations #2131

Merged
merged 2 commits into from
Jan 27, 2025

Conversation

alkesh
Copy link
Contributor

@alkesh alkesh commented Jan 21, 2025

Context

Ticket: https://dfedigital.atlassian.net/browse/CPDNPQ-2317

When there are voided statement items and a voided declaration, it is currently not possible to revert an application to pending.
It raises an error in the admin console:

ActiveRecord::InvalidForeignKey in NpqSeparation::Admin::Applications::RevertToPendingController#create`
PG::ForeignKeyViolation: ERROR: update or delete on table "declarations" violates foreign key constraint "fk_rails_19a5cbebb5" on table "statement_items"
DETAIL: Key (id)=(1278) is still referenced from table "statement_items".

Changes proposed in this pull request

  • Change the validation, so that only applications with declaration in revertable states(voided, ineligible, awaiting_clawback, clawed_back) can be reverted to pending
  • Do not delete any declarations
  • If there are declarations with states submitted, eligible, payable or paid - an error will be shown.
  • Previously declarations that were in state submitted were allowed - but this has been removed - the provider will have to void those declarations first if they want the application reverted to pending.

The error you get if the state of the declarations are not revertable is:
Screenshot 2025-01-22 at 13 16 32

Copy link
Contributor

@alkesh alkesh force-pushed the CPDNPQ-2317-revert-to-pending-bug branch from 185782b to fbf930c Compare January 22, 2025 11:26
@alkesh alkesh marked this pull request as ready for review January 22, 2025 13:58
@alkesh alkesh requested a review from a team as a code owner January 22, 2025 13:58
%i[submitted voided ineligible].each do |declaration_state|
context "with #{declaration_state} state" do
context "when application already has declarations" do
%i[voided ineligible awaiting_clawback clawed_back].each do |declaration_state|
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use Applications::RevertToPending::REVERTABLE_DECLARATION_STATES here?

end
end
end

%i[eligible payable paid awaiting_clawback clawed_back].each do |declaration_state|
context "with #{declaration_state} state" do
Declaration.states.keys.excluding(%w[voided ineligible awaiting_clawback clawed_back]).each do |declaration_state|
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use Applications::RevertToPending::REVERTABLE_DECLARATION_STATES here too?

@@ -264,7 +264,7 @@ en:
lead_provider_approval_status:
inclusion: Current lead provider approval status is not Accepted
base:
pending_unremoveable_declarations: There are already declarations for this participant on this course, please ask provider to void and/or clawback any declarations they have made before attempting to reset the application.
pending_unremoveable_declarations: There are already declarations for this participant on this course, please ask the provider to void and/or clawback any declarations they have made before attempting to reset the application.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
pending_unremoveable_declarations: There are already declarations for this participant on this course, please ask the provider to void and/or clawback any declarations they have made before attempting to reset the application.
pending_unremoveable_declarations: There are already declarations for this participant on this course, please ask the provider to void any declarations they have made before attempting to revert the application.

From a provider's perspective, they can only void a declaration. Whether it goes to void or through the clawback states is determined by Declarations::Void#void checking whether money has changed hands, but the provider has no control over that.

Also tiny change for consistent verb revert (could also change i18n key)

@alkesh alkesh requested a review from rwrrll January 24, 2025 15:25
@alkesh alkesh added this pull request to the merge queue Jan 27, 2025
Merged via the queue into main with commit 3d9dc82 Jan 27, 2025
18 checks passed
@alkesh alkesh deleted the CPDNPQ-2317-revert-to-pending-bug branch January 27, 2025 14:09
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