Shopify return location on refunds #6
Conversation
WalkthroughThis update introduces new enum values and expands the functionality for handling reverse fulfillment orders and return locations in Shopify integrations. It enhances the codebase with new GraphQL codeunits for query handling and additional procedures to manage return and refund locations effectively. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (1)
Apps/W1/Shopify/app/src/Order Refunds/Codeunits/ShpfyRefundsAPI.Codeunit.al (1)
Line range hint
104-143: Enhanced error handling and data integrity checks needed inFillInRefundLine.The procedure
FillInRefundLinenow includes logic to handle return locations, which is a significant addition. However, it lacks error handling for cases whereRefundLinerecords might not be found or created successfully. Also, consider using a transactional approach to ensure data integrity throughout the procedure.+ if not RefundLine.Get(RefundId, Id) then begin + Error('Refund Line not found.'); + end; ... + if not RefundLine.Insert() then begin + Error('Failed to insert Refund Line.'); + end;
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (15)
- Apps/W1/Shopify/app/src/Base/Enums/ShpfyReturnLocationPriority.Enum.al (1 hunks)
- Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al (1 hunks)
- Apps/W1/Shopify/app/src/Base/Tables/ShpfyShop.Table.al (3 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRefundLines.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrdLns.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrders.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRefundLines.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrderLines.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (1 hunks)
- Apps/W1/Shopify/app/src/Order Refunds/Codeunits/ShpfyRefundsAPI.Codeunit.al (4 hunks)
- Apps/W1/Shopify/app/src/Order Refunds/Tables/ShpfyRefundLine.Table.al (1 hunks)
- Apps/W1/Shopify/app/src/Order Return Refund Processing/Codeunits/ShpfyCreateSalesDocRefund.Codeunit.al (4 hunks)
- Apps/W1/Shopify/app/src/Order Returns/Codeunits/ShpfyReturnsAPI.Codeunit.al (4 hunks)
- Apps/W1/Shopify/app/src/Order Returns/Tables/ShpfyReturnLine.Table.al (1 hunks)
Files not reviewed due to errors (2)
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (no review received)
- Apps/W1/Shopify/app/src/Order Return Refund Processing/Codeunits/ShpfyCreateSalesDocRefund.Codeunit.al (no review received)
Files skipped from review due to trivial changes (1)
- Apps/W1/Shopify/app/src/Base/Enums/ShpfyReturnLocationPriority.Enum.al
Additional comments not posted (13)
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRefundLines.Codeunit.al (1)
8-8: Updated GraphQL Query:The addition of
location { legacyResourceId }in the GraphQL query is appropriate and aligns with the PR's objectives to handle return locations more effectively.Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRefundLines.Codeunit.al (1)
8-8: Updated GraphQL Query with Pagination:The addition of
location { legacyResourceId }and pagination support via theafterargument is well-integrated into the GraphQL query. This ensures that the system can handle larger datasets efficiently.Verification successful
Updated GraphQL Query with Pagination:
The addition of
location { legacyResourceId }and pagination support via theafterargument is well-integrated into the GraphQL query. This ensures that the system can handle larger datasets efficiently.
- File:
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRefundLines.Codeunit.al- Lines: 8
The verification process indicates that the changes are consistent with the rest of the codebase, and the pagination and the
legacyResourceIdfield are correctly utilized.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that pagination and 'legacyResourceId' are being correctly utilized in the system. # Test: Search for usage of 'after' and 'legacyResourceId'. rg --type al $'after|legacyResourceId'Length of output: 66
Script:
#!/bin/bash # Description: Verify that pagination and 'legacyResourceId' are being correctly utilized in the system. # Step 1: Find all AL files. fd -e al > al_files.txt # Step 2: Search for usage of 'after' and 'legacyResourceId' in AL files. xargs rg 'after|legacyResourceId' < al_files.txtLength of output: 360404
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.al (1)
16-16: New GraphQL Query for Reverse Fulfillment Orders:The introduction of this new GraphQL query specifically for reverse fulfillment orders is well-executed. The structure and parameters used are appropriate for the intended functionality.
Verification successful
Integration of 'Shpfy GQL RevFulfillOrders' Verified
The new codeunit 'Shpfy GQL RevFulfillOrders' is correctly integrated into the system. References to this codeunit are found in the following files:
Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.alApps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.alApps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrderLines.Codeunit.alScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new codeunit 'Shpfy GQL RevFulfillOrders' into the system. # Test: Search for references to 'Shpfy GQL RevFulfillOrders'. rg --type al $'Shpfy GQL RevFulfillOrders'Length of output: 70
Script:
#!/bin/bash # Description: Verify the integration of the new codeunit 'Shpfy GQL RevFulfillOrders' into the system. # Test: Search for references to 'Shpfy GQL RevFulfillOrders' across the entire codebase. rg 'Shpfy GQL RevFulfillOrders'Length of output: 659
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrders.Codeunit.al (1)
16-16: New GraphQL Query for Paginated Reverse Fulfillment Orders:The addition of pagination to the GraphQL query for reverse fulfillment orders is a crucial enhancement for handling larger data sets efficiently. The implementation is consistent and aligns with the system's requirements.
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrderLines.Codeunit.al (2)
14-16: Well-defined GraphQL query structure.The
GetGraphQLprocedure correctly constructs a GraphQL query to fetch reverse fulfillment order lines. This query includes nested objects and fields, which are crucial for the integration's functionality. The query syntax appears correct and should function as expected.
23-25: Consistent expected cost return.The
GetExpectedCostprocedure returns a fixed cost, which is straightforward and consistent. Ensure that this cost aligns with the expected resource consumption of the GraphQL query.Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrdLns.Codeunit.al (2)
14-16: Effective pagination handling in GraphQL query.The
GetGraphQLprocedure effectively handles pagination for the reverse fulfillment order lines query. The inclusion ofafterand pagination controls such aspageInfoensures that the integration can handle large sets of data efficiently.
23-25: Consistent expected cost return.Similar to the previous codeunit, this procedure returns a consistent cost. Verify that the cost is appropriate for the paginated queries, as they might involve different resource consumption compared to non-paginated queries.
Apps/W1/Shopify/app/src/Order Refunds/Tables/ShpfyRefundLine.Table.al (1)
118-122: Addition ofLocation Idfield to track refund locations.The new field
Location Idis appropriately added to theShpfy Refund Linetable with the correct properties. This field is crucial for tracking the location associated with each refund, enhancing the system's tracking capabilities and aligning with the new integration requirements.Apps/W1/Shopify/app/src/Order Returns/Tables/ShpfyReturnLine.Table.al (1)
115-119: Addition ofLocation Idfield to enhance return tracking.The
Location Idfield has been added to theShpfy Return Linetable, which is essential for accurately tracking the location of returned items. This addition supports the new functionality of handling return locations and is consistent with the changes made across other related tables.Apps/W1/Shopify/app/src/Base/Tables/ShpfyShop.Table.al (2)
514-514: Field Caption Update: "Return Location" to "Default Return Location"The caption change for the "Return Location" field to "Default Return Location" is consistent with the PR's objective to handle return locations more explicitly. This should help clarify the purpose of the field in the user interface.
781-785: New Field Added: "Return Location Priority"A new field "Return Location Priority" has been added, which is essential for specifying the hierarchy of return locations. This addition aligns with the PR objectives to enhance the handling of return locations. Ensure that the necessary UI elements and backend logic are updated to support this new field.
Apps/W1/Shopify/app/src/Base/Pages/ShpfyShopCard.Page.al (1)
543-547: Field Addition: "Return Location Priority"The newly added field "Return Location Priority" is correctly placed and configured. The properties such as
ApplicationArea,Caption, andToolTipare appropriately set, ensuring the field is visible and understandable to users. The addition aligns with the PR's objective to handle return location priorities in Shopify refunds.
| JResponse: JsonToken; | ||
| JLines: JsonArray; | ||
| JLine: JsonToken; | ||
| begin | ||
| GetRefundHeader(RefundId, UpdatedAt, RefundHeader); | ||
| ReturnLocations := CollectReturnLocations(RefundHeader."Return Id"); |
There was a problem hiding this comment.
Refactor suggestion for repeated code in GetRefund procedure.
The GetRefund procedure has been updated to handle return locations. However, the repeated call to JsonHelper.GetValueAsText for the 'After' cursor in the GraphQL pagination can be refactored into a helper procedure to reduce duplication and improve maintainability.
+ private procedure GetCursor(JResponse: JsonToken): Text
+ begin
+ exit(JsonHelper.GetValueAsText(JResponse, 'data.refund.refundLineItems.pageInfo.endCursor'));
+ end;
...
- Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.refund.refundLineItems.pageInfo.endCursor'));
+ Parameters.Set('After', GetCursor(JResponse));Also applies to: 54-54
| internal procedure GetReturnLocations(ReturnId: BigInteger) ReturnLocations: Dictionary of [BigInteger, BigInteger] | ||
| var | ||
| GraphQLType: Enum "Shpfy GraphQL Type"; | ||
| LineParameters: Dictionary of [text, Text]; | ||
| JResponse: JsonToken; | ||
| JOrders: JsonArray; | ||
| JOrder: JsonToken; | ||
| begin | ||
| LineParameters.Add('ReturnId', Format(ReturnId)); | ||
| GraphQLType := "Shpfy GraphQL Type"::GetReverseFulfillmentOrders; | ||
| repeat | ||
| JResponse := CommunicationMgt.ExecuteGraphQL(GraphQLType, LineParameters); | ||
| GraphQLType := "Shpfy GraphQL Type"::GetNextReverseFulfillmentOrders; | ||
| JOrders := JsonHelper.GetJsonArray(JResponse, 'data.return.reverseFulfillmentOrders.nodes'); | ||
| if Parameters.ContainsKey('After') then | ||
| Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.endCursor')) | ||
| else | ||
| Parameters.Add('After', JsonHelper.GetValueAsText(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.endCursor')); | ||
| foreach JOrder in JOrders do | ||
| GetReturnLocationsFromReturnFulfillOrder(JsonHelper.GetValueAsText(JOrder, 'id'), ReturnLocations); | ||
| until not JsonHelper.GetValueAsBoolean(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.hasNextPage'); | ||
| end; |
There was a problem hiding this comment.
Enhanced error handling needed in GetReturnLocations.
The GetReturnLocations procedure is critical for fetching and managing return locations. It correctly iterates through GraphQL responses to build a dictionary of locations. However, it lacks error handling for GraphQL query failures and empty responses, which could lead to incomplete or incorrect data being processed.
+ if JResponse.HasError() then
+ Error('GraphQL query failed: ' + JResponse.GetError());
...
+ if JOrders.Count = 0 then
+ Error('No orders found for the given return ID.');Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| internal procedure GetReturnLocations(ReturnId: BigInteger) ReturnLocations: Dictionary of [BigInteger, BigInteger] | |
| var | |
| GraphQLType: Enum "Shpfy GraphQL Type"; | |
| LineParameters: Dictionary of [text, Text]; | |
| JResponse: JsonToken; | |
| JOrders: JsonArray; | |
| JOrder: JsonToken; | |
| begin | |
| LineParameters.Add('ReturnId', Format(ReturnId)); | |
| GraphQLType := "Shpfy GraphQL Type"::GetReverseFulfillmentOrders; | |
| repeat | |
| JResponse := CommunicationMgt.ExecuteGraphQL(GraphQLType, LineParameters); | |
| GraphQLType := "Shpfy GraphQL Type"::GetNextReverseFulfillmentOrders; | |
| JOrders := JsonHelper.GetJsonArray(JResponse, 'data.return.reverseFulfillmentOrders.nodes'); | |
| if Parameters.ContainsKey('After') then | |
| Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.endCursor')) | |
| else | |
| Parameters.Add('After', JsonHelper.GetValueAsText(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.endCursor')); | |
| foreach JOrder in JOrders do | |
| GetReturnLocationsFromReturnFulfillOrder(JsonHelper.GetValueAsText(JOrder, 'id'), ReturnLocations); | |
| until not JsonHelper.GetValueAsBoolean(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.hasNextPage'); | |
| end; | |
| internal procedure GetReturnLocations(ReturnId: BigInteger) ReturnLocations: Dictionary of [BigInteger, BigInteger] | |
| var | |
| GraphQLType: Enum "Shpfy GraphQL Type"; | |
| LineParameters: Dictionary of [text, Text]; | |
| JResponse: JsonToken; | |
| JOrders: JsonArray; | |
| JOrder: JsonToken; | |
| begin | |
| LineParameters.Add('ReturnId', Format(ReturnId)); | |
| GraphQLType := "Shpfy GraphQL Type"::GetReverseFulfillmentOrders; | |
| repeat | |
| JResponse := CommunicationMgt.ExecuteGraphQL(GraphQLType, LineParameters); | |
| if JResponse.HasError() then | |
| Error('GraphQL query failed: ' + JResponse.GetError()); | |
| GraphQLType := "Shpfy GraphQL Type"::GetNextReverseFulfillmentOrders; | |
| JOrders := JsonHelper.GetJsonArray(JResponse, 'data.return.reverseFulfillmentOrders.nodes'); | |
| if JOrders.Count = 0 then | |
| Error('No orders found for the given return ID.'); | |
| if Parameters.ContainsKey('After') then | |
| Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.endCursor')) | |
| else | |
| Parameters.Add('After', JsonHelper.GetValueAsText(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.endCursor')); | |
| foreach JOrder in JOrders do | |
| GetReturnLocationsFromReturnFulfillOrder(JsonHelper.GetValueAsText(JOrder, 'id'), ReturnLocations); | |
| until not JsonHelper.GetValueAsBoolean(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.hasNextPage'); | |
| end; |
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- Apps/W1/Shopify/app/src/Base/Enums/ShpfyReturnLocationPriority.Enum.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrderLines.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/Order Returns/Codeunits/ShpfyReturnsAPI.Codeunit.al (4 hunks)
Files skipped from review as they are similar to previous changes (3)
- Apps/W1/Shopify/app/src/Base/Enums/ShpfyReturnLocationPriority.Enum.al
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrderLines.Codeunit.al
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.al
Additional comments not posted (1)
Apps/W1/Shopify/app/src/Order Returns/Codeunits/ShpfyReturnsAPI.Codeunit.al (1)
54-54: Ensure all necessary fields are captured inFillInReturnLine.The
FillInReturnLinemethod has been modified to include return locations, which is crucial for accurate return processing. Ensure all necessary fields related to the return line are being captured and updated correctly.Verification successful
Ensure all necessary fields are captured in
FillInReturnLine.The
FillInReturnLinemethod captures several fields, includingReturn Line Id,Return Id,Fulfillment Line Id,Order Line Id,Return Reason, andLocation Id. This indicates that the method is designed to capture all necessary fields for processing return lines, including the recently added return locations.
Return Line IdReturn IdFulfillment Line IdOrder Line IdReturn ReasonLocation IdScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if all necessary fields are captured in `FillInReturnLine`. # Test: Search for the function usage. Expect: All necessary fields are captured. rg --type al $'FillInReturnLine'Length of output: 58
Script:
#!/bin/bash # Step 1: Find the definition of `FillInReturnLine`. ast-grep --lang al --pattern 'procedure FillInReturnLine($$$)' # Step 2: Find all instances where `FillInReturnLine` is called. rg 'FillInReturnLine'Length of output: 557
Script:
#!/bin/bash # Extract the full implementation of `FillInReturnLine` from the identified file. rg -A 20 -B 10 'local procedure FillInReturnLine' Apps/W1/Shopify/app/src/Order\ Returns/Codeunits/ShpfyReturnsAPI.Codeunit.alLength of output: 1848
| local procedure CollectLocationsFromLineDispositions(JLine: JsonToken; ReturnLocations: Dictionary of [BigInteger, BigInteger]) | ||
| var | ||
| OrderLineId: BigInteger; | ||
| LocationId: BigInteger; | ||
| Dispositions: JsonArray; | ||
| Disposition: JsonToken; | ||
| begin | ||
| OrderLineId := CommunicationMgt.GetIdOfGId(JsonHelper.GetValueAsText(JLine, 'fulfillmentLineItem.lineItem.id')); | ||
|
|
||
| Dispositions := JsonHelper.GetJsonArray(JLine, 'dispositions'); | ||
| if Dispositions.Count = 0 then | ||
| exit; | ||
|
|
||
| // If dispositions have different locations (Item was restocked to multiple locations), | ||
| // we cannot determine the return location for the line | ||
| Dispositions.Get(0, Disposition); | ||
| LocationId := JsonHelper.GetValueAsBigInteger(Disposition, 'location.legacyResourceId'); | ||
| foreach Disposition in Dispositions do | ||
| if LocationId <> JsonHelper.GetValueAsBigInteger(Disposition, 'location.legacyResourceId') then | ||
| exit; | ||
|
|
||
| ReturnLocations.Add(OrderLineId, LocationId); | ||
| end; |
There was a problem hiding this comment.
Clarify logic for handling multiple restock locations in CollectLocationsFromLineDispositions.
The procedure attempts to handle cases where items are restocked to multiple locations. However, the logic for exiting the loop when locations differ could be clarified or documented to ensure it behaves as expected under all conditions.
+ // Exit if multiple different locations are found, as we cannot determine a single return location.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local procedure CollectLocationsFromLineDispositions(JLine: JsonToken; ReturnLocations: Dictionary of [BigInteger, BigInteger]) | |
| var | |
| OrderLineId: BigInteger; | |
| LocationId: BigInteger; | |
| Dispositions: JsonArray; | |
| Disposition: JsonToken; | |
| begin | |
| OrderLineId := CommunicationMgt.GetIdOfGId(JsonHelper.GetValueAsText(JLine, 'fulfillmentLineItem.lineItem.id')); | |
| Dispositions := JsonHelper.GetJsonArray(JLine, 'dispositions'); | |
| if Dispositions.Count = 0 then | |
| exit; | |
| // If dispositions have different locations (Item was restocked to multiple locations), | |
| // we cannot determine the return location for the line | |
| Dispositions.Get(0, Disposition); | |
| LocationId := JsonHelper.GetValueAsBigInteger(Disposition, 'location.legacyResourceId'); | |
| foreach Disposition in Dispositions do | |
| if LocationId <> JsonHelper.GetValueAsBigInteger(Disposition, 'location.legacyResourceId') then | |
| exit; | |
| ReturnLocations.Add(OrderLineId, LocationId); | |
| end; | |
| // If dispositions have different locations (Item was restocked to multiple locations), | |
| // we cannot determine the return location for the line | |
| + // Exit if multiple different locations are found, as we cannot determine a single return location. | |
| Dispositions.Get(0, Disposition); | |
| LocationId := JsonHelper.GetValueAsBigInteger(Disposition, 'location.legacyResourceId'); | |
| foreach Disposition in Dispositions do | |
| if LocationId <> JsonHelper.GetValueAsBigInteger(Disposition, 'location.legacyResourceId') then | |
| exit; |
| ReturnLocations: Dictionary of [BigInteger, BigInteger]; | ||
| JResponse: JsonToken; | ||
| JLines: JsonArray; | ||
| JLine: JsonToken; | ||
| begin | ||
| GetReturnHeader(ReturnId); | ||
| ReturnLocations := GetReturnLocations(ReturnId); | ||
|
|
There was a problem hiding this comment.
Refactor suggestion for repeated code in GetReturn procedure.
The GetReturn procedure has been updated to handle return locations. Similar to the previous file, consider refactoring the repeated GraphQL pagination code to improve code maintainability.
+ private procedure GetCursor(JResponse: JsonToken): Text
+ begin
+ exit(JsonHelper.GetValueAsText(JResponse, 'data.return.returnLineItems.pageInfo.endCursor'));
+ end;
...
- Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.return.returnLineItems.pageInfo.endCursor'));
+ Parameters.Set('After', GetCursor(JResponse));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ReturnLocations: Dictionary of [BigInteger, BigInteger]; | |
| JResponse: JsonToken; | |
| JLines: JsonArray; | |
| JLine: JsonToken; | |
| begin | |
| GetReturnHeader(ReturnId); | |
| ReturnLocations := GetReturnLocations(ReturnId); | |
| ReturnLocations: Dictionary of [BigInteger, BigInteger]; | |
| JResponse: JsonToken; | |
| JLines: JsonArray; | |
| JLine: JsonToken; | |
| + private procedure GetCursor(JResponse: JsonToken): Text | |
| + begin | |
| + exit(JsonHelper.GetValueAsText(JResponse, 'data.return.returnLineItems.pageInfo.endCursor')); | |
| + end; | |
| begin | |
| GetReturnHeader(ReturnId); | |
| ReturnLocations := GetReturnLocations(ReturnId); | |
| ... | |
| - Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.return.returnLineItems.pageInfo.endCursor')); | |
| + Parameters.Set('After', GetCursor(JResponse)); |
Fixes [AB#420000](https://dynamicssmb2.visualstudio.com/1fcb79e7-ab07-432a-a3c6-6cf5a88ba4a5/_workitems/edit/420000) --------- Co-authored-by: Alexander Holstrup <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (1)
Apps/W1/Shopify/app/src/Order Returns/Codeunits/ShpfyReturnsAPI.Codeunit.al (1)
146-168: Clarify logic for handling multiple restock locations inCollectLocationsFromLineDispositions.The procedure attempts to handle cases where items are restocked to multiple locations. However, the logic for exiting the loop when locations differ could be clarified or documented to ensure it behaves as expected under all conditions.
+ // Exit if multiple different locations are found, as we cannot determine a single return location.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (1 hunks)
- Apps/W1/Shopify/app/src/Order Refunds/Codeunits/ShpfyRefundsAPI.Codeunit.al (4 hunks)
- Apps/W1/Shopify/app/src/Order Returns/Codeunits/ShpfyReturnsAPI.Codeunit.al (4 hunks)
Additional comments not posted (8)
Apps/W1/Shopify/app/src/Order Refunds/Codeunits/ShpfyRefundsAPI.Codeunit.al (2)
Line range hint
110-141:
LGTM!The integration of return locations handling in
FillInRefundLineis well-implemented and documented.
40-40: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
GetRefundare updated to handle the new return locations functionality.Verification successful
Verified: The
GetRefundfunction is updated to handle the new return locations functionality.The new return locations functionality is integrated within the
GetRefundfunction. The lineReturnLocations := CollectReturnLocations(RefundHeader."Return Id");confirms this update.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `GetRefund` are updated to handle the new return locations functionality. # Test: Search for the function usage. Expect: Only occurrences of the new usage. rg --type al -A 5 $'GetRefund'Length of output: 56
Script:
#!/bin/bash # Description: Verify all function calls to `GetRefund` are updated to handle the new return locations functionality. # Find all .al files and search for the GetRefund function usage within those files. fd -e al -x rg 'GetRefund' {}Length of output: 790
Script:
#!/bin/bash # Description: Extract more context around the GetRefund calls to verify if the new return locations functionality is handled. # Find all .al files and search for the GetRefund function usage within those files, including more context around each match. fd -e al -x rg -A 10 -B 10 'GetRefund' {}Length of output: 6171
Apps/W1/Shopify/app/src/Order Returns/Codeunits/ShpfyReturnsAPI.Codeunit.al (2)
Line range hint
176-190:
LGTM!The integration of return locations handling in
FillInReturnLineis well-implemented and documented.
35-42: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
GetReturnare updated to handle the new return locations functionality.Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (4)
408-412: LGTM!The enum value
GetReverseFulfillmentOrdersis correctly defined and includes appropriate captions and implementation.
413-417: LGTM!The enum value
GetNextReverseFulfillmentOrdersis correctly defined and includes appropriate captions and implementation.
418-422: LGTM!The enum value
GetReverseFulfillmentOrderLinesis correctly defined and includes appropriate captions and implementation.
423-427: LGTM!The enum value
GetNextReverseFulfillmentOrderLinesis correctly defined and includes appropriate captions and implementation.
| @@ -31,11 +31,14 @@ codeunit 30228 "Shpfy Refunds API" | |||
| RefundHeader: Record "Shpfy Refund Header"; | |||
| GraphQLType: Enum "Shpfy GraphQL Type"; | |||
| Parameters: Dictionary of [text, Text]; | |||
| ReturnLocations: Dictionary of [BigInteger, BigInteger]; | |||
There was a problem hiding this comment.
Ensure proper error handling for CollectReturnLocations.
The CollectReturnLocations method should handle cases where the ReturnId is zero.
+ if ReturnId = 0 then
+ Error('Invalid Return ID provided.');Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ReturnLocations: Dictionary of [BigInteger, BigInteger]; | |
| ReturnLocations: Dictionary of [BigInteger, BigInteger]; | |
| + if ReturnId = 0 then | |
| + Error('Invalid Return ID provided.'); |
| local procedure CollectReturnLocations(ReturnId: BigInteger): Dictionary of [BigInteger, BigInteger] | ||
| var | ||
| ReturnsAPI: Codeunit "Shpfy Returns API"; | ||
| begin | ||
| if ReturnId <> 0 then | ||
| exit(ReturnsAPI.GetReturnLocations(ReturnId)); | ||
| end; |
There was a problem hiding this comment.
Add error handling for invalid ReturnId.
The CollectReturnLocations method should handle cases where the ReturnId is zero to prevent potential issues.
+ if ReturnId = 0 then
+ Error('Invalid Return ID provided.');Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local procedure CollectReturnLocations(ReturnId: BigInteger): Dictionary of [BigInteger, BigInteger] | |
| var | |
| ReturnsAPI: Codeunit "Shpfy Returns API"; | |
| begin | |
| if ReturnId <> 0 then | |
| exit(ReturnsAPI.GetReturnLocations(ReturnId)); | |
| end; | |
| local procedure CollectReturnLocations(ReturnId: BigInteger): Dictionary of [BigInteger, BigInteger] | |
| var | |
| ReturnsAPI: Codeunit "Shpfy Returns API"; | |
| begin | |
| if ReturnId = 0 then | |
| Error('Invalid Return ID provided.'); | |
| if ReturnId <> 0 then | |
| exit(ReturnsAPI.GetReturnLocations(ReturnId)); | |
| end; |
| foreach JLine in JLines do | ||
| FillInRefundLine(RefundId, JLine.AsObject(), IsNonZeroOrReturnRefund(RefundHeader)); | ||
| FillInRefundLine(RefundId, JLine.AsObject(), IsNonZeroOrReturnRefund(RefundHeader), ReturnLocations); |
There was a problem hiding this comment.
Refactor suggestion for repeated code in GetRefund.
The repeated call to JsonHelper.GetValueAsText for the 'After' cursor in the GraphQL pagination can be refactored into a helper procedure to reduce duplication and improve maintainability.
+ private procedure GetCursor(JResponse: JsonToken): Text
+ begin
+ exit(JsonHelper.GetValueAsText(JResponse, 'data.refund.refundLineItems.pageInfo.endCursor'));
+ end;
...
- Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.refund.refundLineItems.pageInfo.endCursor'));
+ Parameters.Set('After', GetCursor(JResponse));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| FillInRefundLine(RefundId, JLine.AsObject(), IsNonZeroOrReturnRefund(RefundHeader), ReturnLocations); | |
| FillInRefundLine(RefundId, JLine.AsObject(), IsNonZeroOrReturnRefund(RefundHeader), ReturnLocations); | |
| + private procedure GetCursor(JResponse: JsonToken): Text | |
| + begin | |
| + exit(JsonHelper.GetValueAsText(JResponse, 'data.refund.refundLineItems.pageInfo.endCursor')); | |
| + end; | |
| ... | |
| - Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.refund.refundLineItems.pageInfo.endCursor')); | |
| + Parameters.Set('After', GetCursor(JResponse)); |
| local procedure GetReturnLocationsFromReturnFulfillOrder(FulfillOrderId: Text; var ReturnLocations: Dictionary of [BigInteger, BigInteger]) | ||
| var | ||
| GraphQLType: Enum "Shpfy GraphQL Type"; | ||
| LineParameters: Dictionary of [text, Text]; | ||
| JResponse: JsonToken; | ||
| JLines: JsonArray; | ||
| JLine: JsonToken; | ||
| begin | ||
| LineParameters.Add('FulfillOrderId', FulfillOrderId); | ||
| GraphQLType := "Shpfy GraphQL Type"::GetReverseFulfillmentOrderLines; | ||
| repeat | ||
| JResponse := CommunicationMgt.ExecuteGraphQL(GraphQLType, LineParameters); | ||
|
|
||
| GraphQLType := "Shpfy GraphQL Type"::GetNextReverseFulfillmentOrders; | ||
| JLines := JsonHelper.GetJsonArray(JResponse, 'data.reverseFulfillmentOrder.lineItems.nodes'); | ||
| if Parameters.ContainsKey('After') then | ||
| Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.endCursor')) | ||
| else | ||
| Parameters.Add('After', JsonHelper.GetValueAsText(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.endCursor')); | ||
|
|
||
| foreach JLine in JLines do | ||
| CollectLocationsFromLineDispositions(JLine, ReturnLocations); | ||
| until not JsonHelper.GetValueAsBoolean(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.hasNextPage'); | ||
| end; |
There was a problem hiding this comment.
Refactor and improve error handling in GetReturnLocationsFromReturnFulfillOrder.
The GetReturnLocationsFromReturnFulfillOrder procedure should include error handling for GraphQL query failures and empty responses to ensure robust operation.
+ if JResponse.HasError() then
+ Error('GraphQL query failed: ' + JResponse.GetError());
...
+ private procedure GetCursor(JResponse: JsonToken): Text
+ begin
+ exit(JsonHelper.GetValueAsText(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.endCursor'));
+ end;
...
- LineParameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.endCursor'));
+ LineParameters.Set('After', GetCursor(JResponse));Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| local procedure GetReturnLocationsFromReturnFulfillOrder(FulfillOrderId: Text; var ReturnLocations: Dictionary of [BigInteger, BigInteger]) | |
| var | |
| GraphQLType: Enum "Shpfy GraphQL Type"; | |
| LineParameters: Dictionary of [text, Text]; | |
| JResponse: JsonToken; | |
| JLines: JsonArray; | |
| JLine: JsonToken; | |
| begin | |
| LineParameters.Add('FulfillOrderId', FulfillOrderId); | |
| GraphQLType := "Shpfy GraphQL Type"::GetReverseFulfillmentOrderLines; | |
| repeat | |
| JResponse := CommunicationMgt.ExecuteGraphQL(GraphQLType, LineParameters); | |
| GraphQLType := "Shpfy GraphQL Type"::GetNextReverseFulfillmentOrders; | |
| JLines := JsonHelper.GetJsonArray(JResponse, 'data.reverseFulfillmentOrder.lineItems.nodes'); | |
| if Parameters.ContainsKey('After') then | |
| Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.endCursor')) | |
| else | |
| Parameters.Add('After', JsonHelper.GetValueAsText(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.endCursor')); | |
| foreach JLine in JLines do | |
| CollectLocationsFromLineDispositions(JLine, ReturnLocations); | |
| until not JsonHelper.GetValueAsBoolean(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.hasNextPage'); | |
| end; | |
| local procedure GetReturnLocationsFromReturnFulfillOrder(FulfillOrderId: Text; var ReturnLocations: Dictionary of [BigInteger, BigInteger]) | |
| var | |
| GraphQLType: Enum "Shpfy GraphQL Type"; | |
| LineParameters: Dictionary of [text, Text]; | |
| JResponse: JsonToken; | |
| JLines: JsonArray; | |
| JLine: JsonToken; | |
| begin | |
| LineParameters.Add('FulfillOrderId', FulfillOrderId); | |
| GraphQLType := "Shpfy GraphQL Type"::GetReverseFulfillmentOrderLines; | |
| repeat | |
| JResponse := CommunicationMgt.ExecuteGraphQL(GraphQLType, LineParameters); | |
| if JResponse.HasError() then | |
| Error('GraphQL query failed: ' + JResponse.GetError()); | |
| GraphQLType := "Shpfy GraphQL Type"::GetNextReverseFulfillmentOrders; | |
| JLines := JsonHelper.GetJsonArray(JResponse, 'data.reverseFulfillmentOrder.lineItems.nodes'); | |
| if Parameters.ContainsKey('After') then | |
| Parameters.Set('After', GetCursor(JResponse)) | |
| else | |
| Parameters.Add('After', GetCursor(JResponse)); | |
| foreach JLine in JLines do | |
| CollectLocationsFromLineDispositions(JLine, ReturnLocations); | |
| until not JsonHelper.GetValueAsBoolean(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.hasNextPage'); | |
| end; | |
| private procedure GetCursor(JResponse: JsonToken): Text | |
| begin | |
| exit(JsonHelper.GetValueAsText(JResponse, 'data.reverseFulfillmentOrder.lineItems.pageInfo.endCursor')); | |
| end; |
| /// <summary> | ||
| /// Get the return locations for return lines. | ||
| /// </summary> | ||
| /// <param name="ReturnId">Id of the return.</param> | ||
| /// <remarks> | ||
| /// If item was restocked to multiple locations, we cannot determine the return location for the return line, | ||
| /// and the order line id will not be included in the return locations. | ||
| /// </remarks> | ||
| /// <returns>Dictionary of Order Line Id and Location Id.</returns> | ||
| internal procedure GetReturnLocations(ReturnId: BigInteger) ReturnLocations: Dictionary of [BigInteger, BigInteger] | ||
| var | ||
| GraphQLType: Enum "Shpfy GraphQL Type"; | ||
| LineParameters: Dictionary of [text, Text]; | ||
| JResponse: JsonToken; | ||
| JOrders: JsonArray; | ||
| JOrder: JsonToken; | ||
| begin | ||
| LineParameters.Add('ReturnId', Format(ReturnId)); | ||
| GraphQLType := "Shpfy GraphQL Type"::GetReverseFulfillmentOrders; | ||
| repeat | ||
| JResponse := CommunicationMgt.ExecuteGraphQL(GraphQLType, LineParameters); | ||
|
|
||
| GraphQLType := "Shpfy GraphQL Type"::GetNextReverseFulfillmentOrders; | ||
| JOrders := JsonHelper.GetJsonArray(JResponse, 'data.return.reverseFulfillmentOrders.nodes'); | ||
| if Parameters.ContainsKey('After') then | ||
| Parameters.Set('After', JsonHelper.GetValueAsText(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.endCursor')) | ||
| else | ||
| Parameters.Add('After', JsonHelper.GetValueAsText(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.endCursor')); | ||
|
|
||
| foreach JOrder in JOrders do | ||
| GetReturnLocationsFromReturnFulfillOrder(JsonHelper.GetValueAsText(JOrder, 'id'), ReturnLocations); | ||
| until not JsonHelper.GetValueAsBoolean(JResponse, 'data.return.reverseFulfillmentOrders.pageInfo.hasNextPage'); | ||
| end; |
There was a problem hiding this comment.
Enhanced error handling needed in GetReturnLocations.
The GetReturnLocations procedure should include error handling for GraphQL query failures and empty responses to ensure robust operation.
+ if JResponse.HasError() then
+ Error('GraphQL query failed: ' + JResponse.GetError());
...
+ if JOrders.Count = 0 then
+ Error('No orders found for the given return ID.');Committable suggestion was skipped due to low confidence.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.al (1)
24-27: LGTM! But document or externalize the hardcoded value.The method correctly returns a hardcoded integer value. For better maintainability, consider documenting the value or externalizing it.
- exit(7); + // Consider documenting the purpose of this value or externalizing it as a constant. + exit(7);
| internal procedure GetGraphQL(): Text | ||
| begin | ||
| exit('{"query":"{ return(id: \"gid://shopify/Return/{{ReturnId}}\") { reverseFulfillmentOrders(first: 10) { pageInfo { endCursor hasNextPage } nodes { id } } } }"}'); | ||
|
|
||
| end; |
There was a problem hiding this comment.
LGTM! But consider improving readability of the query string.
The method correctly returns a GraphQL query string. For better readability and maintainability, consider formatting the query string.
- exit('{"query":"{ return(id: \"gid://shopify/Return/{{ReturnId}}\") { reverseFulfillmentOrders(first: 10) { pageInfo { endCursor hasNextPage } nodes { id } } } }"}');
+ exit(
+ '{
+ "query": "{
+ return(id: \"gid://shopify/Return/{{ReturnId}}\") {
+ reverseFulfillmentOrders(first: 10) {
+ pageInfo { endCursor hasNextPage }
+ nodes { id }
+ }
+ }
+ }"
+ }'
+ );Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| internal procedure GetGraphQL(): Text | |
| begin | |
| exit('{"query":"{ return(id: \"gid://shopify/Return/{{ReturnId}}\") { reverseFulfillmentOrders(first: 10) { pageInfo { endCursor hasNextPage } nodes { id } } } }"}'); | |
| end; | |
| internal procedure GetGraphQL(): Text | |
| begin | |
| exit( | |
| '{ | |
| "query": "{ | |
| return(id: \"gid://shopify/Return/{{ReturnId}}\") { | |
| reverseFulfillmentOrders(first: 10) { | |
| pageInfo { endCursor hasNextPage } | |
| nodes { id } | |
| } | |
| } | |
| }" | |
| }' | |
| ); | |
| end; |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- Apps/W1/Shopify/app/src/Base/Enums/ShpfyReturnLocationPriority.Enum.al (1 hunks)
Files skipped from review due to trivial changes (1)
- Apps/W1/Shopify/app/src/Base/Enums/ShpfyReturnLocationPriority.Enum.al
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (4)
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.al (1)
14-18: Consider improving the readability of the query string.The method correctly returns a GraphQL query string. For better readability and maintainability, consider formatting the query string.
- exit('{"query":"{ return(id: \"gid://shopify/Return/{{ReturnId}}\") { reverseFulfillmentOrders(first: 10) { pageInfo { endCursor hasNextPage } nodes { id } } } }"}'); + exit( + '{ + "query": "{ + return(id: \"gid://shopify/Return/{{ReturnId}}\") { + reverseFulfillmentOrders(first: 10) { + pageInfo { endCursor hasNextPage } + nodes { id } + } + } + }" + }' + );Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrders.Codeunit.al (1)
14-18: Consider improving the readability of the query string.The method correctly returns a GraphQL query string. For better readability and maintainability, consider formatting the query string.
- exit('{"query":"{ return(id: \"gid://shopify/Return/{{ReturnId}}\") { reverseFulfillmentOrders(first: 10, after:\"{{After}}\") { pageInfo { endCursor hasNextPage } nodes { id } } } }"}'); + exit( + '{ + "query": "{ + return(id: \"gid://shopify/Return/{{ReturnId}}\") { + reverseFulfillmentOrders(first: 10, after:\"{{After}}\") { + pageInfo { endCursor hasNextPage } + nodes { id } + } + } + }" + }' + );Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrderLines.Codeunit.al (1)
14-17: Consider improving the readability of the query string.The method correctly returns a GraphQL query string. For better readability and maintainability, consider formatting the query string.
- exit('{"query":"{ reverseFulfillmentOrder(id: \"{{FulfillOrderId}}\") { id lineItems(first: 10) { nodes { id fulfillmentLineItem { id lineItem { id name } } dispositions { id quantity type location { id legacyResourceId } } } } } }"}'); + exit( + '{ + "query": "{ + reverseFulfillmentOrder(id: \"{{FulfillOrderId}}\") { + id + lineItems(first: 10) { + nodes { + id + fulfillmentLineItem { + id + lineItem { + id + name + } + } + dispositions { + id + quantity + type + location { + id + legacyResourceId + } + } + } + } + } + }" + }' + );Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrdLns.Codeunit.al (1)
14-17: Consider improving the readability of the query string.The method correctly returns a GraphQL query string. For better readability and maintainability, consider formatting the query string.
- exit('{"query":"{ reverseFulfillmentOrder(id: \"{{FulfillOrderId}}\") { lineItems(first: 10, after:\"{{After}}\") { pageInfo { endCursor hasNextPage } nodes { id fulfillmentLineItem { id lineItem { id name } } dispositions { id quantity type location { id legacyResourceId } } } } } }"}'); + exit( + '{ + "query": "{ + reverseFulfillmentOrder(id: \"{{FulfillOrderId}}\") { + lineItems(first: 10, after:\"{{After}}\") { + pageInfo { + endCursor + hasNextPage + } + nodes { + id + fulfillmentLineItem { + id + lineItem { + id + name + } + } + dispositions { + id + quantity + type + location { + id + legacyResourceId + } + } + } + } + } + }" + }' + );
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- Apps/W1/Shopify/app/src/Base/Enums/ShpfyReturnLocationPriority.Enum.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrdLns.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrders.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrderLines.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.al (1 hunks)
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- Apps/W1/Shopify/app/src/Base/Enums/ShpfyReturnLocationPriority.Enum.al
- Apps/W1/Shopify/app/src/GraphQL/Enums/ShpfyGraphQLType.Enum.al
Additional comments not posted (4)
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrders.Codeunit.al (1)
24-27: LGTM!The method correctly returns a hardcoded integer value. No further action needed.
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrders.Codeunit.al (1)
24-27: LGTM!The method correctly returns a hardcoded integer value. No further action needed.
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLRevFulfillOrderLines.Codeunit.al (1)
23-26: LGTM!The method correctly returns a hardcoded integer value. No further action needed.
Apps/W1/Shopify/app/src/GraphQL/Codeunits/ShpfyGQLNextRevFulfillOrdLns.Codeunit.al (1)
23-26: LGTM!The method correctly returns a hardcoded integer value. No further action needed.
This pull request does not have a related issue as it's part of delivery for development agreed directly with @AndreiPanko
Added a setting to the shopify shop to use original return location (From shopify) or a default return location set in the Shopify shop when creating Credit Memos for Refunds.
Modified the return and refund creation to pull original location from shopify refund or return orders. When an item on a single return order in shopify is restocked to multiple locations, we cannot determine a single location to put on a return line, so during Credit Memo creation, a default return location from the Shop settings will be used.
Summary by CodeRabbit