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

Fix event timestamps when created for bill run #87

Merged
merged 6 commits into from
Jan 17, 2023

Conversation

Cruikshanks
Copy link
Member

https://eaflood.atlassian.net/browse/WATER-3831

We got our migration wrong. For our test DB we have db/migrations/20221209193845_create-water-events.js set the timestamps in the water.events table to default to knex.fn.now() (essentially Date.now()). But when testing creating an SROC bill run we spotted that our event record didn't have its timestamps set.

Double checking the legacy DB we can see water.events timestamps don't have a default value set. So, this change adds a migration to have the test table behave as it does in the legacy DB. It also updates the app/services/supplementary-billing/create-billing-batch-event.service.js to ensure they get set when creating the record.

@Cruikshanks Cruikshanks added the bug Something isn't working label Jan 13, 2023
@Cruikshanks Cruikshanks self-assigned this Jan 13, 2023
@Cruikshanks Cruikshanks marked this pull request as ready for review January 16, 2023 10:00
https://eaflood.atlassian.net/browse/WATER-3831

We got our migration wrong. For our test DB we have `db/migrations/20221209193845_create-water-events.js` set the timestamps in the `water.events` table to default to `knex.fn.now()` (essentially `Date.now()`). But when testing creating an SROC bill run we spotted that our `event` record didn't have it's timestamps set.

Double checking the legacy DB we can see `water.events` timestamps don't have a default value set. So, this change adds a migration to have the test table behave as it does in the legacy DB. It also updates the `app/services/supplementary-billing/create-billing-batch-event.service.js` to ensure they get when creating the record.
We tested both migration and rollback to confirm the changes are as expected. Another thing we spotted was that the legacy DB allows nulls in the `water.events` timestamps. So, when altering the columns we made that change as well.
We need to get the current date as a ISOString to pass to PostgreSQL for reasons explained in the comments.

This doesn't feel like reason enough to build a standalone `TimeLib` or `PostgreSqlLib`. But we do need to keep an eye on it and ensure it doesn't become a 'kitchen-drawer' of methods.
It now sets the timestamps as part of creating the record.
Also spotted that the JSONB field `licences` defaults to an empty array. So, fix that whilst we at it.
@Cruikshanks Cruikshanks force-pushed the fix-bill-run-event-creation branch from 67766d6 to d700f19 Compare January 16, 2023 11:34
Copy link
Contributor

@Jozzey Jozzey left a comment

Choose a reason for hiding this comment

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

@Cruikshanks Cruikshanks merged commit a54ff6e into main Jan 17, 2023
@Cruikshanks Cruikshanks deleted the fix-bill-run-event-creation branch January 17, 2023 11:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants