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.

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

E2E: Fix the dispute save draft challenge failing due to redirect race condition #9284

Merged
merged 3 commits into from
Aug 21, 2024

Conversation

Jinksi
Copy link
Member

@Jinksi Jinksi commented Aug 20, 2024

Fixes #9281

Changes proposed in this Pull Request

This PR fixes the failing e2e test Disputes > Merchant can save and resume draft dispute challenge which is failing due to a redirect to the dispute list screen.

Previously, this redirect was slow enough to allow the dispute details page refresh to occur beforehand.

I suspect an improvement in e2e environment performance is now causing the redirect to occur before the page refresh can, causing the test to check against the wrong screen (disputes list rather than dispute details). This e2e test first started failing alongside #9109, but it may just be a coincidence.

The fix in this PR will wait for the redirect to occur before navigating to the dispute details screen.

Testing instructions

  • Ensure GH action E2E tests all Disputes > Merchant can save and resume draft dispute challenge tests pass.
    • Note this does not run in PR checks.
    • See this run, view wcpay - merchant workflows and search for merchant-disputes-save-draft-challenge.spec.js
  • Optionally, run locally
    • npm run test:e2e-setup or npm run test:e2e-up if already setup
    • npm run test:e2e -- tests/e2e/specs/wcpay/merchant/merchant-disputes-* to run all merchant disputes tests

  • Run npm run changelog to add a changelog file, choose patch to 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.
  • Covered with tests (or have a good reason not to test in description ☝️)
  • Tested on mobile (or does not apply) N/A

Post merge

@Jinksi Jinksi requested a review from a team August 20, 2024 00:35
@botwoo
Copy link
Collaborator

botwoo commented Aug 20, 2024

Test the build

Option 1. Jetpack Beta

  • Install and activate Jetpack Beta.
  • Use this build by searching for PR number 9284 or branch name fix/9281-e2e-dispute-save-challenge in your-test.site/wp-admin/admin.php?page=jetpack-beta&plugin=woocommerce-payments

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:

  • Latest commit: 25cf98c
  • Build time: 2024-08-20 23:47:44 UTC

Note: the build is updated when a new commit is pushed to this PR.

Copy link
Contributor

github-actions bot commented Aug 20, 2024

Size Change: 0 B

Total Size: 1.32 MB

ℹ️ View Unchanged
Filename Size
release/woocommerce-payments/assets/css/admin.css 1.08 kB
release/woocommerce-payments/assets/css/admin.rtl.css 1.08 kB
release/woocommerce-payments/assets/css/success.css 173 B
release/woocommerce-payments/assets/css/success.rtl.css 173 B
release/woocommerce-payments/dist/blocks-checkout-rtl.css 2.36 kB
release/woocommerce-payments/dist/blocks-checkout.css 2.36 kB
release/woocommerce-payments/dist/blocks-checkout.js 65.4 kB
release/woocommerce-payments/dist/cart-block.js 16.3 kB
release/woocommerce-payments/dist/cart.js 5.73 kB
release/woocommerce-payments/dist/checkout-rtl.css 708 B
release/woocommerce-payments/dist/checkout.css 710 B
release/woocommerce-payments/dist/checkout.js 32 kB
release/woocommerce-payments/dist/express-checkout-rtl.css 240 B
release/woocommerce-payments/dist/express-checkout.css 240 B
release/woocommerce-payments/dist/express-checkout.js 14.1 kB
release/woocommerce-payments/dist/frontend-tracks.js 858 B
release/woocommerce-payments/dist/index-rtl.css 39.2 kB
release/woocommerce-payments/dist/index.css 39.2 kB
release/woocommerce-payments/dist/index.js 298 kB
release/woocommerce-payments/dist/multi-currency-analytics.js 1.08 kB
release/woocommerce-payments/dist/multi-currency-rtl.css 3.41 kB
release/woocommerce-payments/dist/multi-currency-switcher-block.js 60.5 kB
release/woocommerce-payments/dist/multi-currency.css 3.41 kB
release/woocommerce-payments/dist/multi-currency.js 55.5 kB
release/woocommerce-payments/dist/order-rtl.css 730 B
release/woocommerce-payments/dist/order.css 730 B
release/woocommerce-payments/dist/order.js 42.7 kB
release/woocommerce-payments/dist/payment-gateways-rtl.css 1.35 kB
release/woocommerce-payments/dist/payment-gateways.css 1.35 kB
release/woocommerce-payments/dist/payment-gateways.js 39.3 kB
release/woocommerce-payments/dist/payment-request-rtl.css 240 B
release/woocommerce-payments/dist/payment-request.css 240 B
release/woocommerce-payments/dist/payment-request.js 13.6 kB
release/woocommerce-payments/dist/plugins-page-rtl.css 386 B
release/woocommerce-payments/dist/plugins-page.css 386 B
release/woocommerce-payments/dist/plugins-page.js 20.1 kB
release/woocommerce-payments/dist/product-details-rtl.css 433 B
release/woocommerce-payments/dist/product-details.css 436 B
release/woocommerce-payments/dist/product-details.js 11.4 kB
release/woocommerce-payments/dist/settings-rtl.css 11.6 kB
release/woocommerce-payments/dist/settings.css 11.5 kB
release/woocommerce-payments/dist/settings.js 224 kB
release/woocommerce-payments/dist/subscription-edit-page.js 703 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal-rtl.css 524 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal.css 524 B
release/woocommerce-payments/dist/subscription-product-onboarding-modal.js 20.2 kB
release/woocommerce-payments/dist/subscription-product-onboarding-toast.js 730 B
release/woocommerce-payments/dist/subscriptions-empty-state-rtl.css 120 B
release/woocommerce-payments/dist/subscriptions-empty-state.css 120 B
release/woocommerce-payments/dist/subscriptions-empty-state.js 19.3 kB
release/woocommerce-payments/dist/tokenized-payment-request-rtl.css 240 B
release/woocommerce-payments/dist/tokenized-payment-request.css 240 B
release/woocommerce-payments/dist/tokenized-payment-request.js 14.4 kB
release/woocommerce-payments/dist/tos-rtl.css 235 B
release/woocommerce-payments/dist/tos.css 235 B
release/woocommerce-payments/dist/tos.js 21.8 kB
release/woocommerce-payments/dist/woopay-direct-checkout.js 6.14 kB
release/woocommerce-payments/dist/woopay-express-button.js 24 kB
release/woocommerce-payments/dist/woopay-rtl.css 4.54 kB
release/woocommerce-payments/dist/woopay.css 4.51 kB
release/woocommerce-payments/dist/woopay.js 71.2 kB
release/woocommerce-payments/includes/subscriptions/assets/css/plugin-page.css 625 B
release/woocommerce-payments/includes/subscriptions/assets/js/plugin-page.js 814 B
release/woocommerce-payments/vendor/automattic/jetpack-assets/build/i18n-loader.js 2.46 kB
release/woocommerce-payments/vendor/automattic/jetpack-assets/src/js/i18n-loader.js 1.02 kB
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.css 198 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.js 20 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-admin-create-user.rtl.css 198 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.css 625 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.js 20 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-login.rtl.css 626 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/jetpack-sso-users.js 392 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/tracks-ajax.js 521 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/dist/tracks-callables.js 584 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-admin-create-user.css 215 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-admin-create-user.js 521 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-login.css 721 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-login.js 412 B
release/woocommerce-payments/vendor/automattic/jetpack-connection/src/sso/jetpack-sso-users.js 520 B
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/babel.config.js 159 B
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.css 2.36 kB
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.js 13.6 kB
release/woocommerce-payments/vendor/automattic/jetpack-identity-crisis/build/index.rtl.css 2.36 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/about.css 1.04 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin-empty-state.css 294 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin-order-statuses.css 408 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/admin.css 3.59 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/checkout.css 301 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/modal.css 746 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/view-subscription.css 574 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/css/wcs-upgrade.css 414 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/admin-pointers.js 543 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/admin.js 9.4 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/jstz.js 6.78 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/jstz.min.js 3.84 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/meta-boxes-coupon.js 545 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/meta-boxes-subscription.js 2.52 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/moment.js 22.2 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/moment.min.js 11.7 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/payment-method-restrictions.js 1.29 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/admin/wcs-meta-boxes-order.js 507 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/payment-methods.js 358 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/single-product.js 428 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/view-subscription.js 1.38 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/frontend/wcs-cart.js 782 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/modal.js 1.09 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/assets/js/wcs-upgrade.js 1.26 kB
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/build/index.css 391 B
release/woocommerce-payments/vendor/woocommerce/subscriptions-core/build/index.js 3.04 kB

compressed-size-action

Copy link
Contributor

@timur27 timur27 left a comment

Choose a reason for hiding this comment

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

Hey @Jinksi, I've been reviewing this PR and wanted to leave my feedback here as well. I think both PRs address the issue effectively, with your approach incorporating a bit more navigation, which adds some robustness, though it might take a few seconds longer. I'm happy to approve your PR—please share your thoughts so we can decide whether to merge Daniel's PR or yours. Thanks for fixing this 🚀

Copy link
Contributor

@zmaglica zmaglica left a comment

Choose a reason for hiding this comment

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

:shipit:

@danielmx-dev
Copy link
Contributor

Hey @Jinksi, I've been reviewing this PR and wanted to leave my feedback here as well. I think both PRs address the issue effectively, with your approach incorporating a bit more navigation, which adds some robustness, though it might take a few seconds longer. I'm happy to approve your PR—please share your thoughts so we can decide whether to merge Daniel's PR or yours. Thanks for fixing this 🚀

I like this solution better over what I implemented on #9275 as it seems more explicit on what it's doing. I'll close mine in favor of this.

@Jinksi
Copy link
Member Author

Jinksi commented Aug 20, 2024

I like this solution better over what I implemented on #9275 as it seems more explicit on what it's doing. I'll close mine in favor of this.

Thanks @danielmx-dev – My apologies for not spotting your PR earlier.

It's a small difference either way, but I also prefer this PR's solution since it is closer to the merchant's flow: click save button → follow page redirect → navigate back to dispute details (in the future).

@Jinksi
Copy link
Member Author

Jinksi commented Aug 21, 2024

Failing E2E Playwright tests are not related to this PR – checkout failures.

@Jinksi
Copy link
Member Author

Jinksi commented Aug 21, 2024

All tests are now passing on E2E Tests - All for this branch – merging.

@Jinksi Jinksi added this pull request to the merge queue Aug 21, 2024
Merged via the queue into develop with commit d70a50f Aug 21, 2024
39 of 40 checks passed
@Jinksi Jinksi deleted the fix/9281-e2e-dispute-save-challenge branch August 21, 2024 01:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Disputes > Merchant can save and resume draft dispute challenge E2E test failure
5 participants