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

Migrate manage income evidence stored procedure #130

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

karlbaker02
Copy link
Contributor

@karlbaker02 karlbaker02 commented Oct 7, 2024

This PR migrates the income evidence stored procedure to the Evidence Service, refactoring the existing logic at the same time as migrating and operating on the following directions and assumptions:

  • Do not migrate role checks from the stored procedure
  • Do not migrate check_values from the stored procedure as this has already been front-loaded into the service
  • Creating default income evidence is another story and therefore not part of the migration in this PR
  • Do not migrate the updating of reminders (as this is taken care of by a hub job)

As part of migrating the logic from the stored procedure, it is now necessary to call out to the CMA service to find the means assessment that is requested to be updated, before then calling back later with the rest to update that assessment with the evidence passed in.

This PR also updates crime-commons mod schemas from version 1.8.0 to 1.16.0.

Link to story

@karlbaker02 karlbaker02 changed the title Karlb/lcam 1404 migrate manage income evidence Migrate manage income evidence stored procedure Oct 11, 2024
@karlbaker02 karlbaker02 marked this pull request as ready for review October 11, 2024 14:16
@karlbaker02 karlbaker02 requested a review from a team as a code owner October 11, 2024 14:16
@karlbaker02 karlbaker02 force-pushed the karlb/lcam-1404-migrate-manage-income-evidence branch 11 times, most recently from a125d28 to 3d08441 Compare October 25, 2024 15:47
karlbaker02 and others added 2 commits October 25, 2024 16:48
…ervice

This commit implements and refactors the logic from _manage_income_evidence_
stored procedure from the legacy database into the Evidence Service. Some of
the logic in the stored procedure has not been implemented as it is no longer
applicable or necessary, or is defined as part of a separate story, such as
migrating the create_default_income_evidence stored procedure.
@karlbaker02 karlbaker02 force-pushed the karlb/lcam-1404-migrate-manage-income-evidence branch 2 times, most recently from cd6e9e4 to f8def0f Compare October 25, 2024 15:50
This commit makes changes in line with a new decision to update the
income evidence not within the Evidence Service as initially thought
but instead to do this within the Orchestration Service.

As such, the CMA client and calls to it are removed, with
new fields instead introduced via an update of the mod schemas
to version `1.16.0` to replace the information previously
obtained from the CMA Service.
@karlbaker02 karlbaker02 force-pushed the karlb/lcam-1404-migrate-manage-income-evidence branch from af801f8 to 72cb0da Compare October 28, 2024 11:11
Copy link

Copy link
Contributor

@venkat980 venkat980 left a comment

Choose a reason for hiding this comment

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

Lg2m

@karlbaker02 karlbaker02 merged commit aabf258 into main Oct 28, 2024
9 of 12 checks passed
@karlbaker02 karlbaker02 deleted the karlb/lcam-1404-migrate-manage-income-evidence branch October 28, 2024 11:26
and ((i2.partner_emst_code is null and ?3 is null) or (i2.partner_emst_code = ?3))
and I2.APPLICANT_PARTNER = ?4
and i2.ANNUAL_PENSION_AMOUNT <= nvl(?5,0)
and i2.ANNUAL_PENSION_AMOUNT > I.ANNUAL_PENSION_AMOUNT)
Copy link
Contributor

Choose a reason for hiding this comment

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

This would be a good candidate for adding some indexes to the DB for - imagine it's slow to run!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could be, I didn't check if it has any indexes already but this is the same query that's currently in one of the nested stored procedures, so should at least be no worse than it currently is.

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