Skip to content

Conversation

@ealeksandrov
Copy link
Contributor

Closes: #7112

Description

In OrderDetailsDataSource refunds logic is relying on directly accessing array by index. Since refunds list is sharing section with other cells (see let payment: Section), index is calculated by subtracting 1 for "Payment" and "Paid by Customer" cells.

The crash appeared because we added condition to hide "Paid by Customer" when there are no payments yet.
Because index constants are static and not directly connected with section logic - we still subtract "-2" when there is only 1 extra cell in a section.

Fix

Static index replaced by a func (in the same Constants namespace) that requires boolean for its state to remind about the condition.
Maybe there is a better way without padding constants? I decided against logic refactor for now.

Testing

  1. Prepare order without payments, but with refunds. Easy to do on the web, not sure about mobile.
  2. Go to the Order tab, open order.
  3. Confirm it doesn't crash.
  4. Open "Refunded Products" list, confirm correct data.

Screenshots


  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@ealeksandrov ealeksandrov added feature: order details Related to order details. type: crash The worst kind of bug. feature: refunds Related to order refunds. labels Jun 20, 2022
@ealeksandrov ealeksandrov requested review from Ecarrion and toupper June 20, 2022 19:06
@peril-woocommerce
Copy link

Warnings
⚠️ PR is not assigned to a milestone.

Generated by 🚫 dangerJS

@ealeksandrov ealeksandrov requested review from a team and removed request for Ecarrion and toupper June 20, 2022 19:09
@wpmobilebot
Copy link
Collaborator

You can test the changes from this Pull Request by:
  • Clicking here or scanning the QR code below to access App Center
  • Then installing the build number pr7113-6f5095e on your iPhone

If you need access to App Center, please ask a maintainer to add you.

@ealeksandrov ealeksandrov removed the request for review from a team June 21, 2022 04:22
@ealeksandrov ealeksandrov changed the base branch from trunk to release/9.3.1 June 21, 2022 04:24
@toupper toupper self-assigned this Jun 21, 2022
@toupper
Copy link
Contributor

toupper commented Jun 21, 2022

LGTM! The code looks good and tests fine, thanks a lot @ealeksandrov 🚀

I decided against logic refactor for now.
I agree 👍

@oguzkocer oguzkocer merged commit 0e4f0f5 into release/9.3.1 Jun 21, 2022
@oguzkocer oguzkocer deleted the issue/7112-fix-refunds-crash branch June 21, 2022 19:33
@ealeksandrov ealeksandrov mentioned this pull request Aug 16, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: order details Related to order details. feature: refunds Related to order refunds. type: crash The worst kind of bug.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Crash in OrderDetailsDataSource.configureRefund

5 participants