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

One Time Checkout - validation message #6442

Merged
merged 3 commits into from
Oct 28, 2024

Conversation

andrewHEguardian
Copy link
Contributor

What are you doing in this PR?

Only show "Something went wrong" message when payment has been attempted. Also move this message to the right place.

Unset payment method validation on selection of a payment method.

Trello Card

Why are you doing this?

Fixing some bugs found in end to end testing

Screenshots

Something went wrong:

Before (when should not show)
image

After (when should not show)
image

After (when should show)
image

Payment Method Validation unset

Before
image

After
image

Copy link
Contributor

github-actions bot commented Oct 25, 2024

Size Change: +7 B (0%)

Total Size: 2.29 MB

ℹ️ View Unchanged
Filename Size Change
./public/compiled-assets/javascripts/[countryGroupId]/events/router.js 112 kB 0 B
./public/compiled-assets/javascripts/[countryGroupId]/router.js 259 kB +7 B (0%)
./public/compiled-assets/javascripts/ausMomentMap.js 108 kB 0 B
./public/compiled-assets/javascripts/contributionsRedirectStyles.js 20 B 0 B
./public/compiled-assets/javascripts/digitalSubscriptionLandingPage.js 241 kB 0 B
./public/compiled-assets/javascripts/downForMaintenancePage.js 69.3 kB 0 B
./public/compiled-assets/javascripts/error404Page.js 69.3 kB 0 B
./public/compiled-assets/javascripts/error500Page.js 69.2 kB 0 B
./public/compiled-assets/javascripts/favicons.js 617 B 0 B
./public/compiled-assets/javascripts/paperSubscriptionCheckoutPage.js 194 kB 0 B
./public/compiled-assets/javascripts/paperSubscriptionLandingPage.js 85.9 kB 0 B
./public/compiled-assets/javascripts/payPalErrorPage.js 67.7 kB 0 B
./public/compiled-assets/javascripts/payPalErrorPageStyles.js 20 B 0 B
./public/compiled-assets/javascripts/promotionTerms.js 72.2 kB 0 B
./public/compiled-assets/javascripts/subscriptionsLandingPage.js 71.5 kB 0 B
./public/compiled-assets/javascripts/subscriptionsRedemptionPage.js 126 kB 0 B
./public/compiled-assets/javascripts/supporterPlusLandingPage.js 312 kB 0 B
./public/compiled-assets/javascripts/unsupportedBrowserStyles.js 20 B 0 B
./public/compiled-assets/javascripts/weeklySubscriptionCheckoutPage.js 191 kB 0 B
./public/compiled-assets/javascripts/weeklySubscriptionLandingPage.js 85.8 kB 0 B
./public/compiled-assets/webpack/136.js 2.17 kB 0 B
./public/compiled-assets/webpack/187.js 21.6 kB 0 B
./public/compiled-assets/webpack/344.js 2.01 kB 0 B
./public/compiled-assets/webpack/671.js 21.8 kB 0 B
./public/compiled-assets/webpack/706.js 107 kB 0 B

compressed-size-action

paymentMethod === 'Stripe' &&
stripe &&
cardElement &&
recaptchaToken
Copy link
Contributor Author

@andrewHEguardian andrewHEguardian Oct 25, 2024

Choose a reason for hiding this comment

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

This doesn't actually produce a validation message but does require the token to proceed with card payment. This puts us at parity with the generic checkout. Let's try add a message as part of this ticket: https://trello.com/c/jcYuEiiR/1112-investigate-validation-on-generic-checkout

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, looks like it comes up with Please check the 'Im not a robot' checkbox with this anyway

@andrewHEguardian andrewHEguardian requested a review from a team October 25, 2024 11:19
paymentResult.paymentStatus === 'failure'
) {
setErrorContext(appropriateErrorMessage(paymentResult.error ?? ''));
if (paymentResult) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Yep undefined is assumed failure

Copy link
Contributor

@paul-daniel-dempsey paul-daniel-dempsey left a comment

Choose a reason for hiding this comment

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

Looks good

@andrewHEguardian andrewHEguardian merged commit c2a4fdc into main Oct 28, 2024
18 checks passed
@andrewHEguardian andrewHEguardian deleted the ahe/one-time-validation-message branch October 28, 2024 14:02
@prout-bot
Copy link

Seen on PROD (merged by @andrewHEguardian 8 minutes and 45 seconds ago)

Sentry Release: support-client-side, support

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.

3 participants