Refactor product orchestration and update mocks for product add and …

[jamalsoueidan]

User description

…destroy operations

• Implement CustomerProductAddOrchestration and CustomerProductDestroyOrchestration
• Update mocks to reflect changes in product add and destroy orchestrations
• Remove unnecessary request parameter from CustomerProductControllerUpdate
• Adjust updateArticle to accept customerId instead of user object
• Add new tests for destroy-product orchestration
• Remove redundant shopifyAdmin requests from product destroy service and tests

---

PR Type

Enhancement, Tests

---

Description

• Implemented CustomerProductAddOrchestration and CustomerProductDestroyOrchestration.
• Updated mocks to reflect changes in product add and destroy orchestrations.
• Removed unnecessary request parameter from CustomerProductControllerUpdate.
• Adjusted updateArticle to accept customerId instead of user object.
• Added new tests for destroy-product orchestration.
• Removed redundant shopifyAdmin requests from product destroy service and tests.

---

Changes walkthrough 📝

	Relevant files
Enhancement	10 files customer-product.function.ts Add extraInputs parameter to customerProductDestroy function. src/functions/customer-product.function.ts Added extraInputs parameter to customerProductDestroy function. +1/-0      add.ts Replace CustomerProductUpdateOrchestration with CustomerProductAddOrchestration. src/functions/customer/controllers/product/add.ts Replaced CustomerProductUpdateOrchestration with CustomerProductAddOrchestration. +2/-2      destroy.ts Integrate CustomerProductDestroyOrchestration in destroy controller. src/functions/customer/controllers/product/destroy.ts Added CustomerProductDestroyOrchestration and CustomerProductServiceGet imports. Updated controller to use CustomerProductDestroyOrchestration. +11/-1    update.ts Remove request parameter from update controller.                  src/functions/customer/controllers/product/update.ts Removed request parameter from CustomerProductControllerUpdate. +3/-8      update.ts Replace user with customerId in updateArticle call.            src/functions/customer/orchestrations/customer/update.ts Replaced user with customerId in updateArticle call. +1/-1      update-article.ts Replace user parameter with customerId in updateArticle.  src/functions/customer/orchestrations/customer/update/update-article.ts Replaced user parameter with customerId. Added call to CustomerServiceGet to fetch user details. +5/-3      add.ts Implement CustomerProductAddOrchestration.                              src/functions/customer/orchestrations/product/add.ts Implemented CustomerProductAddOrchestration. +57/-0    destroy.ts Implement CustomerProductDestroyOrchestration.                      src/functions/customer/orchestrations/product/destroy.ts Implemented CustomerProductDestroyOrchestration. +58/-0    destroy-product.ts Implement destroyProduct function and PRODUCT_DESTROY mutation. src/functions/customer/orchestrations/product/destroy/destroy-product.ts Implemented destroyProduct function and PRODUCT_DESTROY mutation. +20/-0    destroy.ts Remove shopifyAdmin request from destroy service.                src/functions/customer/services/product/destroy.ts Removed shopifyAdmin request from CustomerProductServiceDestroy. +1/-16
Tests	6 files add.spec.ts Update mock to use CustomerProductAddOrchestration.            src/functions/customer/controllers/product/add.spec.ts Updated mock to use CustomerProductAddOrchestration instead of CustomerProductUpdateOrchestration. +2/-2      destroy.spec.ts Update destroy product controller tests and mocks.              src/functions/customer/controllers/product/destroy.spec.ts Removed mock for shopifyAdmin and added mock for CustomerProductDestroyOrchestration. Updated test to remove redundant shopifyAdmin request. +3/-15    update.spec.ts Remove unnecessary request parameter from update controller test. src/functions/customer/controllers/product/update.spec.ts Removed unnecessary request parameter from CustomerProductControllerUpdate test. +0/-1      destroy-product.spec.ts Add tests for destroyProduct function.                                      src/functions/customer/orchestrations/product/destroy/destroy-product.spec.ts Added tests for destroyProduct function. +48/-0    update-product.spec.ts Add handle to variables in update product test.                    src/functions/customer/orchestrations/product/update/update-product.spec.ts Added handle to variables in CustomerProductUpdateOrchestration test. +1/-0      destroy.spec.ts Remove redundant shopifyAdmin requests from destroy service tests. src/functions/customer/services/product/destroy.spec.ts Removed mock for shopifyAdmin and updated test to remove redundant shopifyAdmin request. +0/-19

---

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

[github-actions]

PR Description updated to latest commit (f2491dd)

[github-actions]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves multiple files and significant changes including orchestration logic, service updates, and test modifications. The complexity of the orchestration and the interactions between components require a thorough review to ensure that the logic is correct and that there are no side effects.
🧪 Relevant tests	Yes
⚡ Possible issues	Possible Bug: The CustomerProductDestroyOrchestration might attempt to use product details after the product has been deleted, as indicated by the comment in src/functions/customer/controllers/product/destroy.ts. This could lead to errors if the product details are required after the deletion.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/functions/customer-product.function.ts
suggestion	Consider validating the extraInputs parameter in the customerProductDestroy function to ensure it contains expected values and types. This validation can prevent potential issues with unexpected inputs and improve the robustness of the function. [important]
relevant line	extraInputs: [df.input.durableClient()],

---

relevant file	src/functions/customer/controllers/product/destroy.ts
suggestion	It's important to handle potential errors from CustomerProductServiceGet and CustomerProductDestroyOrchestration calls with try-catch blocks to prevent the function from crashing on exceptions. This will improve the reliability of the product destruction process. [important]
relevant line	const product = await CustomerProductServiceGet(validateQuery);

---

relevant file	src/functions/customer/orchestrations/product/destroy.ts
suggestion	Ensure that the destroyProductOption activity is properly handling cases where product.options might be undefined or empty. This can prevent runtime errors in the orchestration process. [important]
relevant line	for (const productOption of input.product.options || []) {

---

relevant file	src/functions/customer/orchestrations/product/add.ts
suggestion	Consider adding error handling for the orchestration steps in CustomerProductAddOrchestration. This could involve using try-catch blocks around each yield statement to manage exceptions from the activities and ensure the orchestration can handle errors gracefully. [important]
relevant line	yield context.df.callActivity(

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Add error handling for the orchestration process Ensure that error handling is implemented for the orchestration process to manage failures in any of the activity functions called within the orchestrator. src/functions/customer/orchestrations/product/add.ts [17-21] -const productUpdated: Awaited<ReturnType<typeof updateProduct>> = - yield context.df.callActivity( +let productUpdated; +try { + productUpdated = yield context.df.callActivity( updateProductName, activityType<typeof updateProduct>(input) ); +} catch (error) { + context.log.error(`Error updating product: ${error}`); + throw new Error(`Failed to update product with ID ${input.productId}`); +} Suggestion importance[1-10]: 9 Why: Implementing error handling for the orchestration process is crucial for managing failures in any of the activity functions called within the orchestrator. This improves the robustness and reliability of the code.	9
	Add input validation for df.input.durableClient() Consider validating the input from df.input.durableClient() to ensure it meets expected criteria before using it in the extraInputs array. This can prevent potential issues with unexpected input types or values. src/functions/customer-product.function.ts [89] -extraInputs: [df.input.durableClient()] +extraInputs: [validateInput(df.input.durableClient())] Suggestion importance[1-10]: 7 Why: Adding input validation is a good practice to ensure that the input meets expected criteria, which can prevent potential issues with unexpected input types or values. However, the suggestion does not specify how to implement the validateInput function, which is crucial for this change.	7
Best practice	Use a factory function for mocking to prevent state leakage between tests Replace the direct mocking of CustomerProductAddOrchestration with a factory function that returns a new instance each time it's called. This ensures that each test has a clean instance and prevents state leakage between tests. src/functions/customer/controllers/product/add.spec.ts [26-30] -jest.mock("../../orchestrations/product/add", () => ({ - CustomerProductAddOrchestration: () => ({ - request: jest.fn(), - }), -})); +jest.mock("../../orchestrations/product/add", () => { + return { + CustomerProductAddOrchestration: jest.fn().mockImplementation(() => { + return { + request: jest.fn(), + }; + }), + }; +}); Suggestion importance[1-10]: 8 Why: Using a factory function for mocking ensures that each test has a clean instance, preventing state leakage between tests. This is a best practice for writing reliable and maintainable tests.	8
Maintainability	Refactor to separate product retrieval into its own function Refactor the CustomerProductControllerDestroy function to separate concerns by moving the product retrieval logic to a separate function. This enhances readability and maintainability. src/functions/customer/controllers/product/destroy.ts [33-34] -const product = await CustomerProductServiceGet(validateQuery); +const product = getProductDetails(validateQuery); await CustomerProductDestroyOrchestration({ product }, context); +async function getProductDetails(query) { + return await CustomerProductServiceGet(query); +} + Suggestion importance[1-10]: 6 Why: Separating the product retrieval logic into its own function enhances readability and maintainability. However, the improvement is minor and does not address any critical issues.	6