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

LUPEYALPHA 1096/fix claim started date 2 #3222

Merged
merged 1 commit into from
Sep 25, 2024

Conversation

rjlynch
Copy link
Contributor

@rjlynch rjlynch commented Sep 24, 2024

Back fill started at

If the claim has a journey session we try and set the started_at to
that, if it doesn't have a journey session we'll just use the
created_at timestamp, which for claims pre the introduction of journey
sessions will be the started at time.

PR 2 of 3
1 #3221
3 #3224

@rjlynch rjlynch force-pushed the LUPEYALPHA-1096/fix-claim-started-date-2 branch 3 times, most recently from c8cbcb0 to 6a44b2f Compare September 24, 2024 10:52
@rjlynch rjlynch changed the base branch from master to LUPEYALPHA-1096/fix-claim-started-date-1 September 24, 2024 10:53
@rjlynch rjlynch marked this pull request as ready for review September 24, 2024 11:18
AND claims.started_at IS NULL
SQL

change_column_null :claims, :started_at, false
Copy link
Contributor

Choose a reason for hiding this comment

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

feels risky without a prior run? might save you more time if we do this change after confirmation? or happy to keep in if you're feeling confident, i don't think there's any material risk

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're setting this on new claims in the previous PR and we're using the created_at timestamp which should be present on all records as rails is setting it. But yeah I think for avoiding risk we'll split this out into a separate pr (and also because I forgot to remove the not null constraint in the down migration 🙈)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Have moved the migration to make the column non nullable to this pr https://github.com/DFE-Digital/claim-additional-payments-for-teaching/pull/3224/files

@rjlynch rjlynch force-pushed the LUPEYALPHA-1096/fix-claim-started-date-2 branch 2 times, most recently from 9526105 to e281157 Compare September 24, 2024 11:35
@rjlynch rjlynch force-pushed the LUPEYALPHA-1096/fix-claim-started-date-1 branch from 08eef0f to c656ee5 Compare September 25, 2024 09:09
Base automatically changed from LUPEYALPHA-1096/fix-claim-started-date-1 to master September 25, 2024 09:32
If the claim has a journey session we try and set the `started_at` to
that, if it doesn't have a journey session we'll just use the
`created_at` timestamp, which for claims pre the introduction of journey
sessions will be the started at time.
@rjlynch rjlynch force-pushed the LUPEYALPHA-1096/fix-claim-started-date-2 branch from e281157 to bb99389 Compare September 25, 2024 09:52
@rjlynch rjlynch merged commit 5451ece into master Sep 25, 2024
14 checks passed
@rjlynch rjlynch deleted the LUPEYALPHA-1096/fix-claim-started-date-2 branch September 25, 2024 09:56
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.

2 participants