Hide the transaction details refund menu for non-refundable disputes #7962
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Size Change: +22 B (0%) Total Size: 1.27 MB
ℹ️ View Unchanged
|
Jinksi
commented
Jan 3, 2024
| // Refund menu is not rendered | ||
| expect( | ||
| screen.queryByRole( 'button', { | ||
| name: /Translation actions/i, |
There was a problem hiding this comment.
Note that the label for the refund menu button is Translation actions – I've opened #7964 to correct this label.
6 tasks
kalessil
approved these changes
Jan 3, 2024
KarlisJ
approved these changes
Jan 3, 2024
There was a problem hiding this comment.
Thank you @Jinksi for handling this!
Functionally the changes are looking good, hence I'm approving the PR.
However, as Vlad already mentioned there's a slight misconception that the menu is meant only for refund related actions.
While it's true for now, the idea is that we'll be able to add more transaction related actions to the menu in the future. I didn't want to over-engineer things initially but now seeing that that some confusion is already there, I'm re-thinking that decision.
Ideally, we should have extract an array for refund actions and show the menu only if any action is available. Something like this:
const refundActions = [];
// populate refund actions based on conditions.
// this is where we would add more actions in the future.
const controlActions = [...refundActions];
const showControlMenu = !! refundActions.length;This way it's clear for the future selves how to add more actions to the controls and the visibility of controls menu will be handled automatically. And we we'll be able to keep the logic in single place out from the template.
…ther actions in the future
Ah, yes I did assume this was for refunds only. I think your initial approach was correct – it's OK for the small refactor to occur if or when new transaction actions are added. I've renamed the var |
|
@Jinksi Thank you for addressing this. I like the simplicity of your last commit. You address the confusion without any over-engineering. |
|
Failing shopper e2e tests are unrelated (see failing tests on all PRs https://github.com/Automattic/woocommerce-payments/actions/workflows/e2e-pull-request.yml). Adding to merge queue |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #7960
Changes proposed in this Pull Request
This PR hides the refund menu on the transaction details screen when it has a non-refundable dispute.
It introduces a new dispute utility function
isRefundable()to determine if a dispute status is eligible or not. Dispute statuses that are refundable and should show the refund dropdown:won,warning_needs_response,warning_under_review,warning_closed.woocommerce-payments/client/disputes/utils.ts
Lines 75 to 78 in b864c62
Transaction with a dispute not eligible for refund – refund menu is not rendered
Transaction with a dispute eligible for refund – refund menu is rendered
Testing instructions
Ensure the refund menu does not appear on the transaction details screen:
4000000000000259at checkout.Ensure the refund menu appears on the transaction details screen:
4000000000001976at checkout.winning_evidencein the “Additional details” field.npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge