Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix issue with displaying refunded items #13261

Merged
merged 3 commits into from
Jan 9, 2025
Merged

Fix issue with displaying refunded items #13261

merged 3 commits into from
Jan 9, 2025

Conversation

hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented Jan 8, 2025 •
edited by hafizrahman
Loading

Closes: #13258

Description

This PR fixes the linked issue, the cause of the bug was that with the custom fields, we had to refactor how WCMetaData values are represented, and with it, the behavior of value.toString() changed, leading to an issue with retrieving the item's ID.
This PR updates all of the usages that were missed during the refactoring to start using stringValue instead.

Steps to reproduce

  1. Create an order with some products.
  2. Refund the order.
  3. Open the order details.
  4. Tap on the "X items" button.

Testing information

Confirm the refunded items are shown.

The tests that have been performed

^

Images/gif

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@hichamboushaba hichamboushaba added type: bug A confirmed bug. priority: high Affects lots of customers substantially, but not critically. labels Jan 8, 2025
@dangermattic
Copy link
Collaborator

dangermattic commented Jan 8, 2025 •
edited
Loading

1 Warning
⚠️ This PR is assigned to the milestone 21.3 ❄️. The due date for this milestone has already passed.
Please assign it to a milestone with a later deadline or check whether the release for this milestone has already been finished.
1 Message
📖 This PR contains changes to RELEASE-NOTES.txt.
Note that these changes won't affect the final version of the release notes as this version is in code freeze.
Please, get in touch with a release manager if you want to update the final release notes.

Generated by 🚫 Danger

@hichamboushaba hichamboushaba force-pushed the issue/13258-fix-refunded-items branch from 3cbac30 to 41de9c9 Compare January 8, 2025 15:15
@hichamboushaba hichamboushaba added this to the 21.3 ❄️ milestone Jan 8, 2025
@hichamboushaba hichamboushaba added the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Jan 8, 2025
@hichamboushaba hichamboushaba marked this pull request as ready for review January 8, 2025 15:21
@hichamboushaba hichamboushaba changed the base branch from trunk to release/21.3 January 8, 2025 15:22
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 8, 2025 •
edited
Loading

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit0fed58c
Direct Downloadwoocommerce-wear-prototype-build-pr13261-0fed58c.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 8, 2025 •
edited
Loading

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit0fed58c
Direct Downloadwoocommerce-prototype-build-pr13261-0fed58c.apk

@hichamboushaba hichamboushaba removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Jan 9, 2025
@hafizrahman hafizrahman self-assigned this Jan 9, 2025
hafizrahman
hafizrahman approved these changes Jan 9, 2025
View reviewed changes
Copy link
Contributor

@hafizrahman hafizrahman left a comment

Choose a reason for hiding this comment

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

Works correctly in my test store, I tested both partial and full refund and there's no issue. The fix explanation makes sense and great that you're adding unit test, too 👏🏼

@hafizrahman hafizrahman enabled auto-merge January 9, 2025 09:17
@hafizrahman hafizrahman merged commit b834846 into release/21.3 Jan 9, 2025
17 of 19 checks passed
@hafizrahman hafizrahman deleted the issue/13258-fix-refunded-items branch January 9, 2025 09:24
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Project coverage is 40.56%. Comparing base (9949e8f) to head (0fed58c).
Report is 4 commits behind head on release/21.3.

Files with missing lines Patch % Lines
...ain/kotlin/com/woocommerce/android/model/Refund.kt 0.00% 0 Missing and 3 partials ⚠️
.../java/com/woocommerce/android/wear/model/Refund.kt 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@                Coverage Diff                 @@
##             release/21.3   #13261      +/-   ##
==================================================
+ Coverage           40.55%   40.56%   +0.01%     
  Complexity           6367     6367              
==================================================
  Files                1344     1344              
  Lines               77229    77229              
  Branches            10597    10597              
==================================================
+ Hits                31318    31330      +12     
+ Misses              43165    43142      -23     
- Partials             2746     2757      +11

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hafizrahman hafizrahman hafizrahman approved these changes

@irfano irfano Awaiting requested review from irfano

@kidinov kidinov Awaiting requested review from kidinov

Assignees

@hafizrahman hafizrahman

Labels
priority: high Affects lots of customers substantially, but not critically. type: bug A confirmed bug.
Projects
None yet
Milestone
21.3
Development

Successfully merging this pull request may close these issues.

Refunded items are not displayed
5 participants
@hichamboushaba @dangermattic @wpmobilebot @codecov-commenter @hafizrahman