destroy schedule metaobject when deleting the document

[jamalsoueidan]

PR Type

Enhancement, Tests

---

Description

• Added tests for destroying schedule metafield in both controller and service.
• Implemented metafield deletion in schedule service using shopifyAdmin request.
• Added ONLINE to LocationTypes enum in both schema and types.
• Ensured async request for product destruction.
• Added types and GraphQL mutation for destroying schedule metafield.

---

Changes walkthrough 📝

	Relevant files
Tests	destroy.spec.ts Add tests for destroying schedule metafield in controller src/functions/customer/controllers/schedule/destroy.spec.ts Added mock for shopifyAdmin request. Ensured type for DestroyScheduleMetafieldMutation. Mocked response for metafield deletion. +19/-0    destroy.spec.ts Add tests for destroying schedule metafield in service      src/functions/customer/services/schedule/destroy.spec.ts Added mock for shopifyAdmin request. Ensured type for DestroyScheduleMetafieldMutation. Mocked response for metafield deletion. +20/-0
Enhancement	destroy.ts Ensure async request for product destruction                          src/functions/customer/services/product/destroy.ts Added await to shopifyAdmin.request call. +1/-1      destroy.ts Implement metafield deletion in schedule service                  src/functions/customer/services/schedule/destroy.ts Added shopifyAdmin request for metafield deletion. Defined GraphQL mutation for destroying schedule metafield. +22/-0    location.schema.ts Add online location type to schema                                              src/functions/location/location.schema.ts Added ONLINE to LocationTypes enum. +5/-1      location.types.ts Add online location type to types                                                src/functions/location/location.types.ts Added ONLINE to LocationTypes enum. +2/-0      admin.generated.d.ts Add types and mutation for destroying schedule metafield  src/types/admin.generated.d.ts Added types for DestroyScheduleMetafieldMutation and its variables. Added GraphQL mutation for destroying schedule metafield. +8/-0

---

💡 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 functionalities including tests, service logic, and type definitions. The changes are moderate in complexity, involving async operations and interactions with external APIs.
🧪 Relevant tests	Yes
⚡ Possible issues	Possible Bug: The deletion of the schedule metafield in CustomerScheduleServiceDestroy does not handle the case where the shopifyAdmin.request might fail. This could lead to inconsistencies where the schedule is deleted from the database but the metafield remains in Shopify.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/functions/customer/services/schedule/destroy.ts
suggestion	Consider adding error handling for the shopifyAdmin.request call within the CustomerScheduleServiceDestroy function. This could involve wrapping the request in a try-catch block and handling the error appropriately, possibly by logging the error or even preventing the deletion of the schedule from the database if the metafield deletion fails. [important]
relevant line	await shopifyAdmin.request(DESTROY_SCHEDULE_METAFIELD, {

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add error handling for the metafield deletion request Consider adding error handling for the shopifyAdmin.request call when attempting to delete a metafield. This will ensure that any failures in the deletion process are caught and handled appropriately, preventing potential issues in the application flow. src/functions/customer/services/schedule/destroy.ts [18-22] -await shopifyAdmin.request(DESTROY_SCHEDULE_METAFIELD, { - variables: { - metafieldId: schedule.metafieldId, - }, -}); +try { + await shopifyAdmin.request(DESTROY_SCHEDULE_METAFIELD, { + variables: { + metafieldId: schedule.metafieldId, + }, + }); +} catch (error) { + console.error('Failed to delete schedule metafield:', error); + // Handle error appropriately +} Suggestion importance[1-10]: 9 Why: Adding error handling for the shopifyAdmin.request call is crucial for ensuring that any failures in the deletion process are caught and handled appropriately. This improves the robustness and reliability of the application.	9
Enhancement	Use await with proper error handling for asynchronous operations Ensure that the await keyword is used with the shopifyAdmin.request to handle asynchronous operations properly. This change is crucial to ensure that the function execution waits for the request to complete before moving on. src/functions/customer/services/product/destroy.ts [12-15] -await shopifyAdmin.request(PRODUCT_DESTROY, { - variables: { - productId: `gid://shopify/Product/${filter.productId}`, - }, -}); +try { + await shopifyAdmin.request(PRODUCT_DESTROY, { + variables: { + productId: `gid://shopify/Product/${filter.productId}`, + }, + }); +} catch (error) { + console.error('Error destroying product:', error); + // Handle error appropriately +} Suggestion importance[1-10]: 8 Why: The suggestion to add error handling around the await keyword ensures that the function execution waits for the request to complete and handles any potential errors, improving the reliability of the code.	8
Best practice	Use dynamic enum values from the LocationTypes to ensure consistency When defining enums in Mongoose schemas, it's a good practice to ensure that all possible values are covered. In this case, adding 'ONLINE' to the enum in the schema ensures data integrity and consistency with the updated LocationTypes enum. src/functions/location/location.schema.ts [25-28] -enum: [ - LocationTypes.DESTINATION, - LocationTypes.ORIGIN, - LocationTypes.ONLINE, -], +enum: Object.values(LocationTypes), Suggestion importance[1-10]: 7 Why: Using dynamic enum values from LocationTypes ensures consistency and reduces the risk of errors if the enum values are updated in the future. This is a good practice for maintaining data integrity.	7
Maintainability	Add assertions to verify the parameters with which mock functions are called It's important to verify the behavior of mocked functions in tests to ensure they behave as expected. Adding assertions to check the call parameters of mockRequest would enhance the test's reliability and maintainability. src/functions/customer/controllers/schedule/destroy.spec.ts [41-46] mockRequest.mockResolvedValueOnce({ data: ensureType<DestroyScheduleMetafieldMutation>({ metafieldDelete: { deletedId: "gid://shopify/Metaobject/77850968391", }, }), }); +expect(mockRequest).toHaveBeenCalledWith({ + variables: { + metafieldId: expect.any(String), + }, +}); Suggestion importance[1-10]: 6 Why: Adding assertions to check the call parameters of mockRequest enhances the test's reliability and maintainability by ensuring that the mocked functions behave as expected.	6