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

Prepare the database for Good Job v4 #840

Merged
merged 3 commits into from
Jul 11, 2024
Merged

Conversation

ollietreend
Copy link
Contributor

Context

We need to perform some database migrations to prepare for the upgrade from Good Job v3 to v4. This will allow us to re-apply the upgrade in #828, which was reverted by #838.

Note

This PR does not upgrade Good Job to v4. It prepares the database for v4, but these changes are still compatible with v3.99.1 which is currently in use.

Changes proposed in this pull request

I've followed the instructions in the upgrade guide. The database migrations were automatically generated by the gem.

I've also updated the DfE Analytics blocklist to exclude the newly added fields.

Guidance to review

Run the migrations in this pull request, then open a Rails console and run:

GoodJob.v4_ready?

It should return true.

Link to Trello card

https://trello.com/c/vneRTcpO/554-review-and-merge-dependabot-prs

We need to perform some database migrations to prepare for the upgrade
from Good Job v3 to v4.

I've followed the instructions in [the upgrade guide][1], and have
confirmed that the database is now ready by running the following in a
Rails console:

```
GoodJob.v4_ready?
```

[1]: https://github.com/bensheldon/good_job?tab=readme-ov-file#upgrading-v3-to-v4
@ollietreend ollietreend requested review from a team as code owners July 11, 2024 11:05
RuboCop complained about the code style of the auto-generated database
migrations.

Most of these issues were autocorrected by RuboCop.

Two were resolved manually.
It wanted me to wrap the `change_table` call in a `safety_assured`
block.
Copy link
Contributor

@Nitemaeric Nitemaeric left a comment

Choose a reason for hiding this comment

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

🚀

@Nitemaeric
Copy link
Contributor

Note:

GoodJob.v4_ready?

is a bit sneaky. If there are no enqueued jobs, it will always return true.

@ollietreend
Copy link
Contributor Author

If there are no enqueued jobs, it will always return true.

🤦🏻‍♂️

@ollietreend ollietreend merged commit 7232e64 into main Jul 11, 2024
8 checks passed
@ollietreend ollietreend deleted the good-job-v4-migrations branch July 11, 2024 11:36
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