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

Task/migrate operation reg1 statuses/2770 #2868

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

andrea-williams
Copy link
Contributor

@andrea-williams andrea-williams commented Feb 19, 2025

Card: #2770

How to Test/Run:

  1. Follow the steps listed in bc_obps/common/management/commands/check_migrations_with_prod_data.py (regarding making changes to your local bc_obps/.env file.
  2. You'll also likely want to comment out lines 73-74 and 80-81 in check_migrations_with_prod_data, so that you can retain a copy of the prod data locally while you test.
  3. Sign in to Openshift with the CLI tool
  4. Run poetry run python manage.py check_migrations_with_prod_data --pod <pod_name>, where pod_name is the name of a postgres pod in our Openshift PROD env
  5. Once you've successfully pulled the prod data into your local "obps" database, run poetry run python manage.py migrate registration 0078 and poetry run python manage.py migrate reporting to apply all Django migrations except for the migration included in this PR
  6. Now run poetry run python manage.py migrate registration, which will run the migration included in this PR (registration/0079). You'll be able to see the print statements describing what data was affected by the 0079 migration.

For testing purposes, the migration in this PR contains assertions and print statements for testing purposes and to demonstrate that the migration is working as expected. These assertions and print statements will be deleted from the migration before merging (it's also why the pre-commit hook is failing).

REMEMBER TO DELETE YOUR LOCAL COPY OF THE PROD DATA ONCE YOU'VE FINISHED TESTING!

@andrea-williams andrea-williams force-pushed the task/migrate-operation-reg1-statuses/2770 branch 2 times, most recently from ca3e675 to fef7181 Compare February 19, 2025 22:05
@andrea-williams andrea-williams marked this pull request as ready for review February 19, 2025 22:05
@andrea-williams andrea-williams force-pushed the task/migrate-operation-reg1-statuses/2770 branch from fef7181 to c492e2a Compare February 20, 2025 17:09
@andrea-williams andrea-williams force-pushed the task/migrate-operation-reg1-statuses/2770 branch from c492e2a to db431f5 Compare February 20, 2025 17:10
Copy link
Contributor

@BCerki BCerki left a comment

Choose a reason for hiding this comment

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

Looks good! A couple thoughts:

  • running poetry run python manage.py check_migrations_with_prod_data, runs all the migrations so in the testing instructions, you have to additionally comment out line 69-70 of check_migrations_with_prod_data if you want to apply the migrations up to 0078 and play around with the before data
  • if there's anything we'll have to do when we do the migration for real (e.g. remove reg1 statuses from the model choices if we're doing that), we can add to the real migration card so we remember
  • Thank you for providing a detailed example I can steal for mine >:)

approved_operations_updated = Operation.objects.filter(status="Approved").update(status="Draft")
changes_requested_operations_updated = Operation.objects.filter(status="Changes Requested").update(status="Draft")
declined_operations_archived = Operation.objects.filter(status="Declined").update(
archived_by_id='c3bc1b69-15de-44ac-b03b-982bf3163406', archived_at=datetime.datetime.now(datetime.timezone.utc)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it OK to have a real uuid in a public repo? (I know we have some in our mock data too, but I think we were trying to avoid where possible.) Possibly we could select it from the User model by some other field?

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