Skip to content

Comments

feat: Add Shopify refund to return mapping logic#250

Draft
puru-khedre wants to merge 25 commits intorefunds-mega-queryfrom
refactoring-xml-service
Draft

feat: Add Shopify refund to return mapping logic#250
puru-khedre wants to merge 25 commits intorefunds-mega-queryfrom
refactoring-xml-service

Conversation

@puru-khedre
Copy link
Contributor

@puru-khedre puru-khedre commented Jan 11, 2026

Changes:

  • Added MapShopifyRefundToReturn.groovy to map Shopify refund JSON to internal Return payload.

@gemini-code-assist
Copy link

Summary of Changes

Hello @puru-khedre, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the integration with Shopify by implementing a robust and detailed mechanism for processing refunds and returns. The core objective is to accurately translate complex Shopify refund data, including item-level details, financial adjustments, and payment information, into the internal system's return payload. This ensures consistent data representation, supports various refund and exchange scenarios, and provides comprehensive tracking and auditing capabilities, ultimately improving the system's ability to manage post-purchase operations from Shopify.

Highlights

  • Shopify Refund to Internal Return Mapping: A new Groovy script, MapShopifyRefundToReturn.groovy, has been introduced to accurately map Shopify refund JSON data into the internal Return payload structure. This script handles refund line items, prorates discounts and taxes based on quantity ratios, and maps various adjustments (shipping, tax, order) and payments.
  • Enhanced Shopify Refund Processing Services: Several new services have been added and existing ones updated in ShopifyReturnServices.xml to orchestrate the complete Shopify refund attribution flow. This includes services for fetching comprehensive order, refund, and return details using a 'Mega Query', retrieving specific refund components (line items, shipping lines, transactions, order adjustments), and managing return details.
  • New System Message Types and Webhook Integration: New SystemMessageType and ServiceJob entries have been added to handle the queuing and generation of exchanged order IDs and to integrate with Shopify's refunds/create webhook. The consume#ShopifyRefundWebhook service now processes incoming refund webhooks, triggering the new refund attribution logic.
  • Comprehensive Documentation for Refund Logic: Extensive documentation has been added, including a 'Master Specification' for refund attribution and reconciliation, a 'Decision Tree' for returns and refunds, and a detailed breakdown of business scenarios and data elements. These documents outline the algorithms, data retrieval strategies, and decision-making processes for handling various refund and exchange scenarios.
  • Refund History Tracking: A new entity, ShopifyRefundHistory, has been added to ShopifyEntities.xml to enable historical tracking and auditing of Shopify refund attribution decisions within the system.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@puru-khedre puru-khedre changed the base branch from main to refunds-mega-query January 11, 2026 20:35
Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a new Groovy script for mapping Shopify refund data to an internal return payload and refactors the create#ShopifyRefunds service to utilize this script. This is a good move towards modularity. My review has identified a critical bug in the updated service that would cause it to fail, a potential critical runtime error in the new script, and some areas for code improvement and cleanup. Please see the detailed comments for suggestions on how to address these issues.

grandTotal: refund.totalRefundedSet.shopMoney.amount,
items: refund.refundLineItems.collect { rli ->
def lineItem = rli.lineItem
def qtyRatio = rli.quantity / lineItem.quantity

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

There's a risk of a DivisionByZero exception here if lineItem.quantity is 0. It's crucial to add a guard to prevent the service from crashing.

            def qtyRatio = lineItem.quantity > 0 ? (rli.quantity / lineItem.quantity) : 0

<parameter name="shopifyOrderId" required="true"/>
</in-parameters>
<actions>
<service-call name="co.hotwax.shopify.return.ShopifyReturnServices.get#CompleteOrderRefundDetails" in-map="[systemMessageRemoteId:systemMessageRemoteId? systemMessageRemoteId:systemMessageRemote.systemMessageRemoteId , shopifyOrderId:shopifyOrderId]" out-map="orderRefundDetails"/>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

This service call will fail because the systemMessageRemote variable is used in the in-map attribute but is not defined within the service's actions. The logic to fetch the SystemMessageRemote entity using the shopId (which was present in the old version of the service) is missing. You need to re-introduce this logic before the service call to prevent a runtime error.

Comment on lines 47 to 56
refund.refundShippingLines.collect {
[type: "RET_SHIPPING_ADJ", amount: it.subtotalAmountSet?.shopMoney?.amount, description: "Shipping Refund"]
} +
refund.refundShippingLines.collect {
[type: "RET_SALES_TAX_ADJ", amount: it.taxAmountSet?.shopMoney?.amount, description: "Shipping Tax Refund"]
} +
refund.orderAdjustments.collect {
[type: "RET_ORDER_ADJ", amount: it.amountSet?.shopMoney?.amount, description: "Order Adjustment"]
}
).findAll { it.amount > 0 },

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation iterates over refund.refundShippingLines twice, which is inefficient. You can refactor this to use flatMap for a cleaner and more performant single-pass operation. Additionally, it's safer to explicitly check for null on the amount field in the findAll clause, as the safe-navigation operator (?.) can result in null values.

            refund.refundShippingLines.flatMap { rsl ->
                [
                    [type: "RET_SHIPPING_ADJ", amount: rsl.subtotalAmountSet?.shopMoney?.amount, description: "Shipping Refund"],
                    [type: "RET_SALES_TAX_ADJ", amount: rsl.taxAmountSet?.shopMoney?.amount, description: "Shipping Tax Refund"]
                ]
            } +
            refund.orderAdjustments.collect { 
                [type: "RET_ORDER_ADJ", amount: it.amountSet?.shopMoney?.amount, description: "Order Adjustment"] 
            }
        ).findAll { it.amount != null && it.amount > 0 },

<iterate list="order.refunds" entry="refund">
<service-call name="co.hotwax.shopify.return.ShopifyReturnServices.map#ShopifyRefundToReturn" in-map="[order:order, refund:refund]" out-map="result"/>

<log message="======= \n ${groovy.json.JsonOutput.prettyPrint(groovy.json.JsonOutput.toJson(result))} \n ======"/>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This log statement appears to be for debugging. It's very verbose and could expose sensitive data in production logs. It should be removed before this change is merged.

)
]
},
returnAdjustment: (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is flatMap a method on collection ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated with the correct method

def prorate = { val -> (val as BigDecimal * qtyRatio).setScale(2, RoundingMode.HALF_UP) }

[
id: ShopifyHelper.resolveShopifyGid(lineItem.variant?.id), // TODO: Requires internal productId resolution
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we must have fallback condition based on sku and isGiftCard.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the fallback, but the custom gift card is not handled.

@puru-khedre puru-khedre force-pushed the refactoring-xml-service branch 2 times, most recently from 77e1f04 to 5c60d45 Compare January 15, 2026 09:10
@puru-khedre puru-khedre force-pushed the refactoring-xml-service branch from 5c60d45 to 9acc9ed Compare January 15, 2026 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants