Taxonomy Updates #158
Open
Taxonomy Updates #158
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbeard4
reviewed
Oct 24, 2025
| const service = await models.Service.findByPk(serviceId); | ||
| if (!service) return; | ||
|
|
||
| // NOTE: We need to also check if its already associated to other taxonomy. we probably want to delete this record. for now I'm skipping that. |
Member
There was a problem hiding this comment.
@shakilhossain1 Probably need to address this comment and delete existing record when it exists.
jbeard4
reviewed
Oct 24, 2025
| } | ||
| ); | ||
|
|
||
| taxonomy.services.forEach(async (serviceId) => { |
Member
There was a problem hiding this comment.
Can't use foreach hear. See google's answer below:
While you can pass an async function as a callback to Array.prototype.forEach(), forEach() itself is not designed to handle asynchronous operations and will not wait for the promises returned by the async callback to resolve.
Explanation:
forEach is synchronous:
The forEach method iterates through the array and calls the provided callback function for each element in a synchronous manner. It does not inherently understand or manage asynchronous operations.
async callback returns a Promise:
When you pass an async function as the callback to forEach, that async function will immediately return a Promise.
forEach does not await Promises:
forEach does not wait for these returned Promises to resolve or reject. It simply calls the async function, gets the Promise, and then moves on to the next element in the array. This means that the code following the forEach loop will execute before any of the asynchronous operations within the forEach callback have completed.
Unhandled Promises:
If an error occurs within an async callback inside forEach and the returned Promise is not handled (e.g., with a .catch() or by being awaited in a different context), it can lead to unhandled promise rejections.
Alternatives for Asynchronous Iteration:
for...of loop: For sequential execution of asynchronous operations, for...of loops are the recommended approach as they allow you to await each asynchronous operation within the loop.
JavaScript
async function processItemsSequentially(items) {
for (const item of items) {
await doSomethingAsync(item);
}
console.log("All items processed sequentially.");
}
Promise.all() with Array.prototype.map(): For parallel execution of asynchronous operations, Promise.all() combined with map() is the appropriate choice. map() creates an array of Promises, and Promise.all() waits for all of them to resolve.
JavaScript
async function processItemsInParallel(items) {
const promises = items.map(async (item) => {
await doSomethingAsync(item);
});
await Promise.all(promises);
console.log("All items processed in parallel.");
}
jbeard4
reviewed
Oct 24, 2025
Comment on lines
234
to
241
| name: "Transitional Independent Living", | ||
| parent_name: "Shelter", | ||
| parent_id: "228d5932-634e-48b4-a2bd-d5f0a74730c7", | ||
| services: [ | ||
| "dd733e25-1442-4306-b745-db3c2dc9b2da", | ||
| "e943b886-1005-49a5-9e71-3825ac87877e", | ||
| ], | ||
| }, |
Member
There was a problem hiding this comment.
Suggested change
| name: "Transitional Independent Living", | |
| parent_name: "Shelter", | |
| parent_id: "228d5932-634e-48b4-a2bd-d5f0a74730c7", | |
| services: [ | |
| "dd733e25-1442-4306-b745-db3c2dc9b2da", | |
| "e943b886-1005-49a5-9e71-3825ac87877e", | |
| ], | |
| }, |
Remove duplicate.
jbeard4
reviewed
Oct 24, 2025
| ], | ||
| }, | ||
| { | ||
| name: "Transitional Independent Living", |
Member
There was a problem hiding this comment.
Suggested change
| name: "Transitional Independent Living", | |
| name: "Transitional Independent Living (TIL)", |
jbeard4
reviewed
Oct 24, 2025
|
|
||
| taxonomy.services.forEach(async (serviceId) => { | ||
| const service = await models.Service.findByPk(serviceId); | ||
| if (!service) return; |
Member
There was a problem hiding this comment.
You will want to migrate the .forEach to a for...of loop, and change this return statement to a continue statement in order skip the service if it does not exist.
Member
|
It will be fine to run this in dev. We want to QA the following service ids that are available in both dev and prod: |
jbeard4
reviewed
Oct 27, 2025
Co-authored-by: Jacob Beard <[email protected]>
jbeard4
previously approved these changes
Oct 27, 2025
Member
|
@adambard1 I ran the migration on dev. Let's QA this in the test environment, and if it looks good, then merge it to master and run the migration in PROD. |
Collaborator
Author
|
after this we also have merge this pr to add youth in the sidebar. |
shakilhossain1
commented
Dec 10, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.