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

[CPDLP-3968] Amend void declarations service to include ineligible statement line items #5487

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

leandroalemao
Copy link
Contributor

@leandroalemao leandroalemao commented Jan 27, 2025

Context

Contract managers are not able to see voids for ineligible declarations in the CSV extracts.

Changes proposed in this pull request

  • Amend void declarations service to include ineligible statement line items, so the record state is updated to voided;
  • Removed submitted line items from the logic as it's not a valid state for StatementLineItem. Checked prod and there's no submitted line itens currently in the db;
  • Improved tests coverage to cover all 3 states in the logic;

Copy link

Review app deployed to https://cpd-ecf-review-5487-web.test.teacherservices.cloud

@@ -62,9 +62,12 @@
)
end

it "can be voided" do
let(:line_item) { participant_declaration.statement_line_items.first }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we group the specs correctly, as the below context for when declaration is attached to a statement will include the line items instead, where we can check all types there perhaps?

@@ -91,37 +75,60 @@
end
end

context "when declaration is submitted" do
Copy link
Collaborator

Choose a reason for hiding this comment

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

can we keep submitted/this test please? as it can be voided but won't be in a statement

Copy link
Collaborator

@ethax-ross ethax-ross left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@leandroalemao leandroalemao added this pull request to the merge queue Jan 31, 2025
Merged via the queue into main with commit 205cff1 Jan 31, 2025
39 checks passed
@leandroalemao leandroalemao deleted the CPDLP-3968 branch January 31, 2025 09:54
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.

4 participants