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

[CAPT-1797] Ops reports #3284

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

[CAPT-1797] Ops reports #3284

wants to merge 13 commits into from

Conversation

alkesh
Copy link
Contributor

@alkesh alkesh commented Oct 8, 2024

  • new nav bar entry on the admin site: Reports
  • reports page shows previously ran reports
  • reports stored in database
  • ReportsJob set to run on second Tuesday of each month
  • Report classes based on the existing Claim::DataReportRequest class

@alkesh alkesh force-pushed the CAPT-1797-monthly-reports branch from d00e400 to 5247399 Compare October 8, 2024 09:46
Copy link
Contributor

@rjlynch rjlynch 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 to me - would check with @slorek and see if this fits with his design for the app

app/models/reports/duplicate_claims.rb Outdated Show resolved Hide resolved
app/models/reports/failed_provider_check_claims.rb Outdated Show resolved Hide resolved
app/models/reports/failed_qualification_claims.rb Outdated Show resolved Hide resolved
db/migrate/20241007141638_create_reports.rb Show resolved Hide resolved
@rjlynch rjlynch force-pushed the CAPT-1797-monthly-reports branch 2 times, most recently from 5f9a97f to ccf6dc2 Compare January 14, 2025 16:55
alkesh and others added 2 commits January 16, 2025 12:11
Getting this spike pr in shape, first step is adding some feature specs.
@rjlynch rjlynch force-pushed the CAPT-1797-monthly-reports branch from 8f20409 to f5f6dbc Compare January 16, 2025 12:11
@rjlynch rjlynch changed the title [CAPT-1797] Ops reports spike [CAPT-1797] Ops reports Jan 16, 2025
Some tweaks to the spike code to get this working, we need to handle the
eligibility not having an eligibile itt subject, and we also want to
scope the claims we're looking at to the current academic year.
Some tweaks to the spike code for this report and adds a test.
Adds a test and fixes up the csv code as not all claims will have a
teacher_reference_number
Pull out a claim presenter to match how the other reports work. This
will also give us somewhere to store presentation logic if we need any
more.
Slightly reduce the number of claims we need to check!
Don't need to stub the duplciates, pretty straightforward to set up a
basic duplicate scenario.
Even if this claim doesn't have duplicates we still access the
eligibility as part of the matching attribute checking code (to grab
thhe policy), so it's worth eager loading.
Adds a button to the report page
Greatly speeds up the duplicate checking
We don't need to keep old reports around so there's no need to generate
a new report each time the job runs.
Limiting the duplicate claims checking to unpaid claims only the
reporting can be run everyday as it's pretty quick (sub one minute).
@rjlynch rjlynch force-pushed the CAPT-1797-monthly-reports branch from f5f6dbc to 1690825 Compare January 16, 2025 14:26
@rjlynch
Copy link
Contributor

rjlynch commented Jan 16, 2025

I've decided to use the background job based approach in this spike pr and have cleaned up this PR so it's good to review.
The reports don't need to be live so a background job generating them daily is fine (can be less frequent).
I think the Report model / storing the CSV in a column on the model (the approach taken in the spike) is a good idea, v simple and does the job.

I've profiled the duplicate claim report against the production database.
Checking all approved claims for duplicates takes ~9minutes to run, which isn't bad for a background job.
Checking only approved unpaid claims takes ~11 seconds which is fine for a background job task.
Speaking with Dan, the main use of the duplicate claims report will be a final sense check before payroll so filtering out paid claims from this report seems fine. (The unpaid claims we're checking are checked against paid claims for duplicates)

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.

3 participants