Ops reports #3457
Open
Ops reports #3457
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rjlynch
force-pushed
the
CAPT-1797-ops-queries
branch
from
dc8caee to
c84088e
Compare
rjlynch
force-pushed
the
CAPT-1797-ops-queries
branch
from
c84088e to
57cd3ea
Compare
rjlynch
force-pushed
the
CAPT-1797-ops-queries
branch
from
57cd3ea to
ff79ef8
Compare
rjlynch
force-pushed
the
CAPT-1797-ops-queries
branch
from
54d7f75 to
9df634c
Compare
rjlynch
force-pushed
the
CAPT-1797-ops-queries
branch
from
9df634c to
a397db1
Compare
rjlynch
force-pushed
the
CAPT-1797-ops-queries
branch
from
a397db1 to
cd9d3b3
Compare
rjlynch
commented
Dec 17, 2024
| end | ||
| end | ||
| end | ||
| end |
There was a problem hiding this comment.
@kenfodder @slorek instead of doing this gnarly query we could:
- add a new claim verifier for detecting duplicates
- when the verifier runs use the existing matching attributes finder code to find any duplciates
- create a record associating the new claim with these duplicates (
Duplicate belongs_to :source_claim, belongs_to :duplicate_claim) - replace the use of matching attribute finder in the app with looking at the new duplicates record for the given claim.
- this report can be simplified by simply joining claims to the new duplicate record.
Might need this query logic for the initial back fill on production but it can then be deleted.
What do you guys think?
There was a problem hiding this comment.
Something along these lines 07013ac
Adds the model to handle generating the reporting code for further education claims that have been approved but with a failing provider verification.
Adds the controller and views for downloading the ops reports.
One gotcha that we might need to deal with in this report is the `dqt_teacher_status`. When the qualification claim verifier runs is uses the dqt record to set some notes on the claim. If the claim has a populated `dqt_teacher_status` field it uses that to build the dqt teacher record object, if not the status is fetched from the api, however this information from the api is not persisted to the claim. When generating the report we don't want to hit the dqt api for potentially many claims, so if we're missing this dqt status we don't include it in the report. Checking the current academic year's claims there don't seem to be any claims that would be returned from this report that are missing their `dqt_teacher_status`. If missing `dqt_teacher_status` in this report is causing issues for the ops team, we could consider parsing the claim notes to get this information.
Adds the query to identify duplicate approved claims. As per the MatchingAttributeFinder claims can either be duplicates on claim attributes or on eligibility attributes. Certain eligibilities can only be compared with other eligibilities determined by the policy's `SomePolicy.policies_claimable` method. Eligibilities define the attributes they should be compared with other eligibilities by in their `SomePolicy.eligibility_matching_attributes` method, however some policies that are comparable don't implement the attributes used for finding matches (eg `EarlyYearsPayments` is in `EarlyCareerPayments` `policies_claimable` list, but it doesn't implement `teacher_reference_number` which is what `EarlyCareerPayments` uses to determine duplicates). We have to do some fairly gnarl joins and unions to compare eligibilities! When comparing claims we search all approved claims in the current academic year for duplicates, ignoring the claim's policy's `policies_claimable` list, this is how MatchingAttributeFinder works so we've kept that behaviour. For each group in the `CLAIM_ATTRIBUTE_GROUPS_TO_MATCH` list we AND the conditions together, normalising strings, we then use UNION to handle the OR conditions, this performs significantly better than complicating the join condition. When adding new attributes to the duplicate detection list we _really_ need to make sure we add the appropriate indexes.
Return the duplicate claim ids from the query and introduce a struct to wrap the result. This change will make it easier to reuse this query if trying to find duplicates of a specific claim.
Noticed we were missing the csv extension on the report
Ignores the sql injection warning in duplicate claims. Non of the interpolated values in this query are supplable by a user, and using bind params would make this a lot harder to read! Anyone modifying this query needs to be careful not to introduce a sql injection attack!
rjlynch
force-pushed
the
CAPT-1797-ops-queries
branch
from
cd9d3b3 to
4f7832b
Compare
|
Part of the work in CAPT-2065 (#3468) is to show the matching attribute alert for all |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ops reports
This PR introduces some new reporting functionality for the ops team.
We've added a new button on the admin claims index page that links to the
/admin/reportspage where the ops team can download various reports.Initially there are three reports:
Reports 2 and 3 are fairly straight forward, we just need to join to the
appropriate tasks and present the data. Report 1 is involved! For report 1 we
need to duplicate the functionality of the
Claim::MatchingAttributeFinderbutrather than checking for duplicates of a single claim, it needs to work across
all claims. Running the claim matching attribute finder in a loop over all
claims takes a long time (6 minuets to check 1000 claims), even with moving
generating the report into a background job it would be tanning the DB and who
wants to wait for an 1hr 30 for a report to generate! To avoid this we've
re implemented all the duplicate checking in SQL.
Having the report in SQL feels a little less maintainable, so something we
might want to consider is:
using claim matching attribute finder
new join model
However we'd need to back fill the duplicate records for existing claims on
production using something like the logic we've written here for finding the
duplicates.
If we do decide to keep the version of the claim duplicate checking from this
pr then we probably want to update the
Claim::MatchingAttributeFinderto usethe logic introduced in this PR so we don't have two definitions of what
constitutes a duplicate claim.
Open for suggestions of the namespacing of these reports.
NOTE for reviewers: probably worth reviewing each commit separately.