-
Notifications
You must be signed in to change notification settings - Fork 2
Feature/new taxonomies #142
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces two new migrations that add taxonomies and associate services with those taxonomies.
- Adds a migration that creates service taxonomy mappings for “Mental Health”, “Employment”, and “Legal Services”
- Adds a migration that inserts new taxonomy records with appropriate parent relationships
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| sequelize/migrations/20250618083606-new_taxonomy_services.js | Inserts service taxonomy mappings for new taxonomies and clears any existing mappings for the related services |
| sequelize/migrations/20250609090556-add_new_taxonomies.js | Inserts new taxonomy records including parent relationships for Employment, Mental Health, and Legal Services |
|
@shakilhossain1 Can you modify the migration so that it updates the metadata table? Thanks. |
| field_name: 'taxonomy_id', | ||
| replacement_value: record.taxonomy_id, | ||
| created_at: new Date(), | ||
| updated_at: new Date(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
update_by
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
set updated_by '<System>'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added the updated_by
| resource_id: record.id, | ||
| resource_table: 'service_taxonomy', | ||
| last_action_date: new Date(), | ||
| last_action_type: 'create', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to capture delete operations as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| resource_id: t.id, | ||
| resource_table: 'taxonomies', | ||
| last_action_date: new Date(), | ||
| last_action_type: 'create', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we are supposed to create create actions for each field (id, name, parent_name, updated_at, parent_id)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added (name, parent_name, parent_id).
|
@shakilhossain1 As an alternative to writing to the metadata table directly, as I described in the above comments, we could try using these functions instead, which are well-tested and are what the back-end is currently using to write to the metadata table: Could you please do a POC of using these functions inside the migration? If this works well, then we can adopt this for future bulk update/migrations going forward. If it would be helpful, we could walk through how these functions are currently used. Thanks for your help with this. |
I've create a new pr for this #145 |
|
@shakilhossain1 I was checking GoGetta and seeing new taxonomies there - I don't know what's been merged since it's surprising Gogetta prod has new taxonomies rather than test.gogetta. Anyhow, the new categories include Legal Services, however Advocates/Legal is still available as a selection. They are the same thing, we want to make sure we're not leaving duplicate categories on GoGetta. All services previously categorized as legal services should be recategorized as Legal Services. |
No description provided.