Skip to content

CAPT 2121/expand topup functionality #3527

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

Merged
merged 6 commits into from
Feb 12, 2025

Conversation

rjlynch
Copy link
Contributor

@rjlynch rjlynch commented Jan 22, 2025

Allow topups for all policies

  • Updates the app to support topups for all policies
  • Adds a note to the case when a topup is created / removed
  • Fixes an edge case in displaying the payroll run.

Probably easiest to review each commit separately, or at least the bug fix on
the payroll run.

Now we need to check the number of topups for each policy we make a few extra
trips to the db when showing the payroll run. I investigated caching the line
items and performing the policy filtering in Ruby which saves ~200ms but makes
the code harder to follow. Opted against including this in the pr as the page
loads in about 500ms, but something to bear in mind if we want a bit of extra
performance.

@rjlynch rjlynch force-pushed the CAPT-2121/expand-topup-functionality branch 4 times, most recently from d8529d0 to a4af10f Compare January 24, 2025 13:27
@rjlynch rjlynch added the deploy Deploy a review app for this PR label Jan 24, 2025
@rjlynch rjlynch marked this pull request as ready for review January 27, 2025 14:52
Copy link
Contributor

@asmega asmega left a comment

Choose a reason for hiding this comment

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

looks fine but i'm not familiar with any of the payroll stuff tbh

@rjlynch
Copy link
Contributor Author

rjlynch commented Jan 27, 2025

looks fine but i'm not familiar with any of the payroll stuff tbh

Thanks, I checked that it doesn't change any of the old payrolls!

@rjlynch rjlynch force-pushed the CAPT-2121/expand-topup-functionality branch from ce5a221 to b5bb92c Compare January 30, 2025 15:33
@rjlynch rjlynch force-pushed the CAPT-2121/expand-topup-functionality branch from b5bb92c to 7d356df Compare February 10, 2025 11:31
We'll be making topups availble on all polices so we don't want any
policy specific logic in the topup.
There was a bug in the previous payroll logic that couldn't handle the
following scenario

* Claimant paid for a claim (Claim A) in Jan payroll
* Claimant paid a topup on Claim A in Feb payroll
* Claimant paid for a claim (Claim B) in Feb payroll

In that scenario the claimant would receive the topup payment twice as
both Claim A and Claim B have the same payment and so when we joined
claims to payments and left joined payments to topup both rows would
have a topup, causing the coalesce to pick the topup amount.
Thankfully the original topup code used `Claim#topupable?` in most
places so supporting additional policies was straightforward.
A requirement is to add a note to the claim when we add or remove a
topup. We've introduced two form objects to handle generating the note
and amending the claim's topups.
For policies that don't define a max award amount set £10K as the limit
as per the comments on CAPT-2121.
Makes the topup form multi step with a confirmation screen.
Once an admin has entered a topup amount we need to display a page to
confirm the amount before we create the topup.
We handle tracking the state by passing around a step param and using
contextual validations. If all validations pass we consider the form
complete and create the records.
We use `dup` in the `complete?` method to avoid rendering errors for
steps other than the one we're currently on.
@rjlynch rjlynch force-pushed the CAPT-2121/expand-topup-functionality branch from 7d356df to 4f17404 Compare February 12, 2025 10:23
@rjlynch rjlynch merged commit 6fd461d into master Feb 12, 2025
15 checks passed
@rjlynch rjlynch deleted the CAPT-2121/expand-topup-functionality branch February 12, 2025 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy Deploy a review app for this PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants