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

Update categories and eligibilities for DCYF. #744

Merged
merged 4 commits into from
Jun 13, 2024

Conversation

richardxia
Copy link
Member

This adds a migration that makes a large number of changes to categories and eligibilities, to match the changes requested by DCYF. Each group of categories and elgibilities is handled a little bit differently, but they roughly fall into one of the following patterns:

  • Create a new category/eligibility
  • Rename a category/eligibility
  • Mass migrate a number of resources and services from a set of old categories/eligibilities to a new one
  • Sometimes delete the old categories/eligibilities when doing said mass migration, though in some cases preserve the old ones

This was implemented as a pure data migration that does not change the schema. Since data migrations must be agnostic to the actual models, we cannot use any of the model code, and instead we construct a number of raw SQL queries. These queries have been factored into independent methods so that the main migration code can be read top to bottom with high-level method calls describing the exact changes being made.

This is supposed to follow the instructions in https://docs.google.com/spreadsheets/d/1rCvcITtUdmjOz3IzjNClbgY4UO53xGV8ix7oeeXDtKc/edit#gid=688234094, and I'd appreciate it if someone could double check my work. I've been testing locally on a dump of the production database, and I'm planning to spend some time later tonight to sanity check all my queries.

This adds a migration that makes a large number of changes to categories
and eligibilities, to match the changes requested by DCYF. Each group of
categories and elgibilities is handled a little bit differently, but
they roughly fall into one of the following patterns:

- Create a new category/eligibility
- Rename a category/eligibility
- Mass migrate a number of resources and services from a set of old
  categories/eligibilities to a new one
- Sometimes delete the old categories/eligibilities when doing said mass
  migration, though in some cases preserve the old ones

This was implemented as a pure data migration that does not change the
schema. Since data migrations must be agnostic to the actual models, we
cannot use any of the model code, and instead we construct a number of
raw SQL queries. These queries have been factored into independent
methods so that the main migration code can be read top to bottom with
high-level method calls describing the exact changes being made.
@richardxia
Copy link
Member Author

One bug I discovered by manually examining the DB after applying this migration is that this will result in duplicate entries in the many-to-many join tables, since we don't have any uniqueness constraints on them. For example, because categories_services doesn't have a unique key on category_id and service_id, it's possible to insert a second row with the same category and service.

I need to alter the logic to avoid creating duplicates when possible.

@richardxia
Copy link
Member Author

One other oddity I noticed is that the data in Algolia will be stale after running this migration, so we'll probably want to retrigger a mass reindexing after applying this migration.

Copy link
Contributor

@schroerbrian schroerbrian left a comment

Choose a reason for hiding this comment

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

Wow this is great - really well-thought out how you managed the changes and check for a category’s existence or non-existence first. I don’t currently have my personal computer with me for testing locally but this code LGTM. Thanks!

@schroerbrian
Copy link
Contributor

One other oddity I noticed is that the data in Algolia will be stale after running this migration, so we'll probably want to retrigger a mass reindexing after applying this migration.

Good thought. I suppose this maybe be since these records are created outside of the Resource/Service.rb models, which is where the indexing code lives

Copy link
Contributor

@katerina-kossler katerina-kossler left a comment

Choose a reason for hiding this comment

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

lgtm! had one smalll q but seemed like this would all do what we want!

@richardxia
Copy link
Member Author

I'm going to do one last set of sanity checking the data resulting from this migration with my local DB, specifically focusing on the last set of changes I pushed up, but otherwise I'm planning to merge this later tonight and maybe doing a deploy tomorrow.

@richardxia
Copy link
Member Author

Just finished spot-checking the data, and I think the new changes I made are working correctly. I'll leave this open for a few more hours, in case if @lgarofalo wanted to check it out, but I'll merge it regardless by the end of tonight.

@richardxia richardxia merged commit c812f84 into master Jun 13, 2024
4 checks passed
@richardxia richardxia deleted the migrate-dcyf-categories-and-eligibilities branch June 13, 2024 06: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.

None yet

3 participants