UPA : Resolved Issued and Improvements

[pratik-bharodiya]

Description

• Sync Order status functionality not working. While place the UPA order with web-hook enable. Given Error Message : "Charge Id is not found".
  This PR contain the solution for the above mention scenario.
• Remove session_id display from transaction table. As we are using session_id for our internal logic.
• Display only charge_id in transaction table. Same like legacy flow.
• Remove QR code from order success page only for UPA order.

Rollback procedure

default rollback procedure

[Copilot]

🟢 Approval recommended

The change is small, localized, and directly addresses the missing charge_id persistence in the webhook-enabled callback path.

Pull request overview

This PR fixes UPA order status sync when webhooks are enabled by ensuring the Omise charge_id is persisted onto the Magento payment record before the webhook-enabled early return, preventing downstream flows (e.g., order status sync) from failing with “Charge Id is not found”.

Changes:

• Persist transaction_id, last_trans_id, and charge_id on the order payment before the “webhook enabled” short-circuit.
• Minor formatting/spacing cleanup in a conditional and method call.

File summaries

File	Description
Controller/Callback/UPACallback.php	Persists charge identifiers on the payment earlier so webhook-enabled flows still have charge_id available for later sync logic.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

The webhook flow must persist payment IDs before redirecting, and obsolete callback tests and dependency wiring must be updated.

Get a fresh assessment by requesting another Copilot review.

Review details

Suppressed comments (1)

Controller/Callback/UPACallback.php:176

• This removal eliminates the only use of $transactionBuilder in UPACallback, but the property, constructor parameter, and assignment remain. The controller now requires an unused dependency and its tests remain coupled to obsolete transaction-building behavior; remove the dead dependency and update the constructor fixture.

            if ($this->config->isWebhookEnabled()) {
                return $this->redirect(self::PATH_SUCCESS);

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

Copilot review overview

🟡 Changes recommended

Critical callback persistence and convenience-store success-page issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 4

Open (4)

• Success page block calls undefined isUpaPayment method · New
• Webhook mode returns before persisting payment and order identifiers · New
• With webhooks enabled, this returns before the payment is assigned charge_id or last_trans_id,…
• This branch no longer invokes transactionBuilder, but the existing…

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[rosle]

question: ‏Can we use $this->isUpaPayment() here?

[pratik-bharodiya]

No. Because this class is extending Magento\Framework\View\Element\Template. and other block class like propmptpay, tesco extending Omise\Payment\Block\Checkout\Onepage\Success\AdditionalInformation.
so not possible to use the same method here.

[rosle]

Seems a bit weird that only this class that does not extend from AdditionalInformation like other classes do. However since this is the existing implementation. I'm ok to keep it like this.