Refactor product location updates and remove metafieldId dependency

[jamalsoueidan]

User description

• Update Jest test path for product schedule locations field
• Remove metafieldId from product locations schema and related functions
• Change logic to determine if shipping is required based on locationType
• Update product and schedule update functions to adapt to schema changes
• Ensure locations are fetched from LocationModel and handle potential errors
• Update Shopify REST API version to 2024-04

---

PR Type

Enhancement, Bug fix

---

Description

• Refactored logic for determining if shipping is required based on location type in update-price.ts.
• Updated product and schedule update functions to adapt to schema changes, removing metafieldId.
• Ensured locations are fetched from LocationModel and handled potential errors in update-schedule-locations-field.ts.
• Updated Shopify REST API version to 2024-04.
• Updated Jest test paths and product location metafield tests.

---

Changes walkthrough 📝

	Relevant files
Enhancement	6 files update-price.ts Refactor shipping requirement logic in price update            src/functions/customer/orchestrations/product/update/update-price.ts Renamed variable isDestination to requiresShipping Simplified logic for setting requiresShipping in inventoryItem +4/-8      update-product.ts Refactor product location metafield handling                          src/functions/customer/orchestrations/product/update/update-product.ts Updated logic for handling product location metafields +1/-1      update-schedule-locations-field.ts Refactor schedule locations field update logic                      src/functions/customer/orchestrations/product/update/update-schedule-locations-field.ts Added import for LocationModel and NotFoundError Refactored logic to fetch and handle locations from LocationModel +22/-9    schedule.types.ts Remove metafieldId from location schema                                    src/functions/schedule/schedule.types.ts Removed metafieldId from LocationZodSchema +0/-1      product.schema.ts Remove metafieldId from product schema locations                  src/functions/schedule/schemas/product.schema.ts Removed metafieldId from product schema locations +0/-1      product-locations.yaml Remove metafieldId from product locations schema                  openapi/paths/customer/schedule/_types/product-locations.yaml Removed metafieldId from product locations schema +0/-3
Tests	1 files update-product.spec.ts Update product location metafield test                                      src/functions/customer/orchestrations/product/update/update-product.spec.ts Updated test to reflect changes in product location metafield handling +1/-1
Configuration changes	2 files rest.ts Update Shopify REST API version                                                    src/library/shopify/rest.ts Updated Shopify REST API version to 2024-04 +1/-1      launch.json Update Jest test path for schedule locations field              .vscode/launch.json Updated Jest test path for schedule locations field +1/-1

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves multiple files and changes across different aspects of the application including schema updates, logic refactoring, and API version updates. The changes are interconnected and require a good understanding of the existing system to ensure nothing breaks.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The refactored logic in update-price.ts assumes all locations require shipping if any location is of type DESTINATION. This might not always be correct depending on business rules.
	Data Consistency: The removal of metafieldId in various schemas and functions needs thorough testing to ensure that no other parts of the system rely on this field.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/functions/customer/orchestrations/product/update/update-price.ts
suggestion	Consider adding a fallback or default value for requiresShipping in case the locations.some() function returns undefined. This ensures that requiresShipping is always a boolean and prevents potential runtime errors. [important]
relevant line	inventoryItem: {

---

relevant file	src/functions/customer/orchestrations/product/update/update-schedule-locations-field.ts
suggestion	Add error handling for the LocationModel.find() operation. Currently, if the find operation fails or returns an error, it might not be handled properly, leading to uncaught exceptions in the system. [important]
relevant line	const locations = await LocationModel.find({

---

relevant file	src/functions/customer/orchestrations/product/update/update-product.spec.ts
suggestion	Update the test to reflect the removal of metafieldId and ensure that the new logic is correctly tested with updated mock data. This is crucial to ensure that the changes do not break existing functionalities. [important]
relevant line	value: JSON.stringify([location.metafieldId]),

---

relevant file	src/functions/customer/orchestrations/product/update/update-product.ts
suggestion	Ensure that the updated logic correctly handles cases where locations might be empty or undefined, to prevent runtime errors when trying to map over undefined. [important]
relevant line	value: JSON.stringify(locations.map((p) => p.metafieldId)),

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Error handling	Add error logging for better diagnostics before throwing exceptions Ensure error handling for the LocationModel.find operation is robust by adding specific error logging before throwing the exception, which can help in diagnosing issues more effectively. src/functions/customer/orchestrations/product/update/update-schedule-locations-field.ts [36-45] const locations = await LocationModel.find({ _id: { $in: locationIds }, -}).orFail( - new NotFoundError([ +}).catch(error => { + logger.error("Failed to find locations", { error, locationIds }); + throw new NotFoundError([ { path: ["customerId", "locations"], message: "LOCATIONS_ERROR", code: "custom", }, - ]) -); + ]); +}); Suggestion importance[1-10]: 9 Why: Adding error logging before throwing exceptions is a crucial improvement for diagnosing issues effectively. This enhances the robustness and maintainability of the code, making it a highly valuable suggestion.	9
Best practice	Replace basic logging with a more advanced logging approach Replace the console.log statement with a more robust logging mechanism that includes error handling and context-specific information, to enhance debugging and maintainability. src/functions/customer/orchestrations/product/update/update-schedule-locations-field.ts [34] -console.log(locationIds); +logger.debug("Location IDs", { locationIds }); Suggestion importance[1-10]: 8 Why: Replacing console.log with a more advanced logging mechanism improves debugging and maintainability, which is a best practice. This change enhances the quality of the code significantly.	8
	Validate the Shopify API version to ensure compatibility Validate the apiVersion to ensure it matches the expected format and is supported by the Shopify API, to prevent runtime errors due to unsupported API versions. src/library/shopify/rest.ts [21] -apiVersion: "2024-04", +apiVersion: validateApiVersion("2024-04"), Suggestion importance[1-10]: 8 Why: Validating the apiVersion ensures compatibility with the Shopify API and prevents potential runtime errors. This is a best practice that enhances the reliability of the code.	8
Enhancement	Improve variable naming for clarity and context specificity Consider using a more descriptive variable name than requiresShipping for clarity, especially since the previous code used isDestination which is more specific to the context of locations being destinations. src/functions/customer/orchestrations/product/update/update-price.ts [19-21] -const requiresShipping = locations.some( +const isDestination = locations.some( (l) => l.locationType === LocationTypes.DESTINATION ); Suggestion importance[1-10]: 7 Why: The suggestion improves code readability and context specificity by using a more descriptive variable name. However, it is not a critical change and does not address any functional issues.	7