Credit card management updates #14670

andesol · 2025-01-22T16:59:08Z

Done

Update "payment methods" page to allow removing the payment method
If removed, show warning notification on Pro dashboard and disable renewal and resize features
Use singular: "Payment method", as there's only one

QA

Open demo or run site locally
Go to /account/payment-methods
Remove the credit card
- The card goes away and it shows an empty state type of page
Go to /pro/dashboard
- There's a warning notification stating there's no payment method
- You can't change renewal settings
- You can't edit/resize any subscription
Compare with design

Issue

https://warthogs.atlassian.net/browse/WD-18327

webteam-app · 2025-01-22T16:59:12Z

Demo

Jenkins

demos.haus

codecov · 2025-01-22T17:00:43Z

Codecov Report

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

Project coverage is 72.40%. Comparing base (a9423d8) to head (eb79b76).
Report is 42 commits behind head on main.

Files with missing lines	Patch %	Lines
static/js/src/advantage/api/contracts.js	0.00%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #14670      +/-   ##
==========================================
+ Coverage   72.32%   72.40%   +0.07%     
==========================================
  Files         120      121       +1     
  Lines        3404     3424      +20     
  Branches     1172     1179       +7     
==========================================
+ Hits         2462     2479      +17     
- Misses        917      920       +3     
  Partials       25       25

Files with missing lines	Coverage Δ
...ents/Subscriptions/Notifications/Notifications.tsx	`87.87% <100.00%> (+1.67%)`	⬆️
...ubscriptions/SubscriptionEdit/SubscriptionEdit.tsx	`83.21% <100.00%> (+0.12%)`	⬆️
...bscriptionList/RenewalSettings/RenewalSettings.tsx	`96.10% <100.00%> (+0.15%)`	⬆️
...s/src/advantage/react/hooks/useHasPaymentMethod.ts	`100.00% <100.00%> (ø)`
.../subscribe/checkout/components/Summary/Summary.tsx	`76.92% <ø> (ø)`
.../checkout/components/UserInfoForm/UserInfoForm.tsx	`68.38% <ø> (ø)`
static/js/src/advantage/api/contracts.js	`23.58% <0.00%> (+0.05%)`	⬆️

minkyngkm · 2025-02-07T09:13:10Z

If no card presented, all auto renewal should be switched off on the dashboard, right?
but it seems it's not switched off

minkyngkm · 2025-02-07T12:11:38Z

templates/account/payment-methods/index.html

          </div>
-          <div class="col-8">
+          <div class="u-align--left col-8 col-medium-4 col-small-3">
+            <p class="p-heading--4 u-no-margin--bottom">You have no payment method saved</p>


I think margin-bottom should be 0. margin-bottom is -0.45rem !important; because of this class u-no-margin--bottom

I always though it was the same... Done, thanks!

minkyngkm · 2025-02-07T12:18:32Z

templates/account/payment-methods/index.html

+        </div>
+        <div class="col-8"></div>
+      </div>
+      <hr class="{% if not default_payment_method.id %}u-hide{% endif %}" />


I think the line should always show?

This is the line at the bottom of the card form. I think it should not be there when there's no card?

minkyngkm · 2025-02-07T12:21:38Z

templates/account/payment-methods/index.html

+        </div>
+        <div class="col-4 u-align--right">
+          <button class="p-button" id="cancel-payment-details">Cancel</button>
+          <button class="p-button--positive" id="update-payment-details" disabled>Update</button>


Should this be "Save" or "Update"?

Good 👀 Let's use "Save" that works for both editing and adding.

minkyngkm · 2025-02-07T12:27:09Z

templates/account/payment-methods/index.html

+           style="margin-top: 2rem">
+        <div class="col-8">
+          <div id="card-element"></div>
+          <span id="card-errors" class="p-form-validation__message u-hide"></span>


should this work the same as the check out page? (the red part)

The styles are different across pages. I tried to keep that part as is.

minkyngkm · 2025-02-07T12:31:16Z

templates/account/payment-methods/index.html

-          <div class="col-4 u-align--right">
-            {% if default_payment_method["id"] %}<button class="p-button" id="cancel-payment-details">Cancel</button>{% endif %}
-            <button class="p-button--positive" id="update-payment-details" disabled>Update</button>
+      <div id="edit-payment-method-section"


it's minor but in design, there seems to be more space between the title "Payment method" and the input area.

True. I can wrap the h1 in a section to give the layout a bit more vertical space. But having tried that, I think it looks a bit strange, and it's inconsistent with other pages. So I would suggest keeping it as it is.

minkyngkm · 2025-02-07T12:40:24Z

static/js/src/advantage/react/components/Subscriptions/Notifications/Notifications.tsx

@@ -36,6 +41,16 @@ const Notifications = () => {

  return (
    <>
+      {hasPaymentMethod === false ? (


Does the no payment method notification display on top of the other notifications?
for example,

That's a good question for @polgarp

minkyngkm · 2025-02-07T12:57:04Z

static/js/src/advantage/subscribe/checkout/components/UserInfoForm/UserInfoForm.tsx

+          <p>
+            We will save your card information. You can change it in your Ubuntu
+            Pro Dashboard.
+          </p>


Distributor users don't have access to the Pro dashboard, but this message is also showing in the checkout for them. Should we display this differently for distributor users?

cc: @polgarp

minkyngkm · 2025-02-07T13:05:02Z

static/js/src/advantage/react/components/Subscriptions/Notifications/Notifications.tsx

@@ -36,6 +41,16 @@ const Notifications = () => {

  return (
    <>
+      {hasPaymentMethod === false ? (


can we simply use !hasPaymentMethod && instead of the ternary operator here?

Using && for conditional rendering is a debated topic in react. I don't have a strong opinion and I use both depending on the situation. This time I went with the ternary because that's what the rest of the file is using.

As for the === false, it's a way to make sure that the notification doesn't show up when hasPayment method is undefined (since it's returned by a hook, it's going to be undefined at first). As an alternative (and maybe more common) I can check for when the query is done, like:

isHasPaymentMethodSuccess && !hasPaymentMethod ? <Notificacion /> : null

Let's try that.

minkyngkm · 2025-02-07T13:07:47Z

static/js/src/advantage/react/components/Subscriptions/SubscriptionEdit/SubscriptionEdit.tsx

@@ -75,7 +76,7 @@ const generateError = (error: Error | null) => {
    return (
      <span data-test="has-pending-purchase-error">
        You already have a pending purchase. Please go to{" "}
-        <a href="/account/payment-methods">payment methods</a> to retry.
+        <a href="/account/payment-methods">payment method</a> to retry.


we can use {urls.account.paymentMethods} instead of /account/payment-methods here as well

True, I didn't realise

minkyngkm · 2025-02-07T13:29:12Z

Looking good @andesol I've just made some comments for what I found 👍

andesol

Thanks @minkyngkm! I addressed some of your comments. As discussed before, we want to keep the autorenewal independent from this change.

Seems like we have something to clarify before this can land, plus a back-end issue. So let's keep it on hold for now.

andesol · 2025-02-03T17:49:16Z

templates/advantage/index.html

+            <div>
+              {% if not is_in_maintenance %}<a class="p-button--positive" href="/pro/subscribe">Buy new subscription</a>{% endif %}
+              <a class="p-button" href="/account/invoices">Invoices</a>
+              <a class="p-button" href="/account/payment-methods">Payment method</a>


This is the only real change. Rest is formatting

andesol · 2025-02-07T16:32:37Z

static/js/src/advantage/react/components/Subscriptions/Notifications/Notifications.tsx

@@ -36,6 +41,16 @@ const Notifications = () => {

  return (
    <>
+      {hasPaymentMethod === false ? (


That's a good question for @polgarp

andesol · 2025-02-07T16:33:20Z

static/js/src/advantage/subscribe/checkout/components/UserInfoForm/UserInfoForm.tsx

+          <p>
+            We will save your card information. You can change it in your Ubuntu
+            Pro Dashboard.
+          </p>


cc: @polgarp

andesol · 2025-02-07T17:00:16Z

templates/account/payment-methods/index.html

-          <div class="col-4 u-align--right">
-            {% if default_payment_method["id"] %}<button class="p-button" id="cancel-payment-details">Cancel</button>{% endif %}
-            <button class="p-button--positive" id="update-payment-details" disabled>Update</button>
+      <div id="edit-payment-method-section"


True. I can wrap the h1 in a section to give the layout a bit more vertical space. But having tried that, I think it looks a bit strange, and it's inconsistent with other pages. So I would suggest keeping it as it is.

andesol · 2025-02-07T17:26:01Z

static/js/src/advantage/react/components/Subscriptions/Notifications/Notifications.tsx

@@ -36,6 +41,16 @@ const Notifications = () => {

  return (
    <>
+      {hasPaymentMethod === false ? (


Using && for conditional rendering is a debated topic in react. I don't have a strong opinion and I use both depending on the situation. This time I went with the ternary because that's what the rest of the file is using.

As for the === false, it's a way to make sure that the notification doesn't show up when hasPayment method is undefined (since it's returned by a hook, it's going to be undefined at first). As an alternative (and maybe more common) I can check for when the query is done, like:

isHasPaymentMethodSuccess && !hasPaymentMethod ? <Notificacion /> : null

Let's try that.

andesol · 2025-02-07T17:31:55Z

templates/account/payment-methods/index.html

+           style="margin-top: 2rem">
+        <div class="col-8">
+          <div id="card-element"></div>
+          <span id="card-errors" class="p-form-validation__message u-hide"></span>


The styles are different across pages. I tried to keep that part as is.

andesol · 2025-02-07T17:32:30Z

templates/account/payment-methods/index.html

+        </div>
+        <div class="col-4 u-align--right">
+          <button class="p-button" id="cancel-payment-details">Cancel</button>
+          <button class="p-button--positive" id="update-payment-details" disabled>Update</button>


Good 👀 Let's use "Save" that works for both editing and adding.

andesol · 2025-02-07T17:38:20Z

templates/account/payment-methods/index.html

+        </div>
+        <div class="col-8"></div>
+      </div>
+      <hr class="{% if not default_payment_method.id %}u-hide{% endif %}" />


This is the line at the bottom of the card form. I think it should not be there when there's no card?

andesol · 2025-02-07T17:45:45Z

templates/account/payment-methods/index.html

          </div>
-          <div class="col-8">
+          <div class="u-align--left col-8 col-medium-4 col-small-3">
+            <p class="p-heading--4 u-no-margin--bottom">You have no payment method saved</p>


I always though it was the same... Done, thanks!

andesol force-pushed the credit-card-mgmt branch 5 times, most recently from cb9ec35 to 4ef9df2 Compare January 30, 2025 16:05

andesol force-pushed the credit-card-mgmt branch 3 times, most recently from 36dfd12 to d54aff8 Compare February 3, 2025 08:35

andesol added Review: QA needed Review: Code needed Review: UX needed labels Feb 3, 2025

andesol marked this pull request as ready for review February 3, 2025 09:27

andesol force-pushed the credit-card-mgmt branch 3 times, most recently from 8a3e0e1 to e6672de Compare February 6, 2025 14:55

andesol requested a review from minkyngkm February 6, 2025 15:22

minkyngkm added Review: UX +1 and removed Review: UX needed labels Feb 6, 2025

minkyngkm added Review: QA -1 and removed Review: QA needed labels Feb 7, 2025

minkyngkm reviewed Feb 7, 2025

View reviewed changes

minkyngkm added Review: QA needed and removed Review: QA -1 labels Feb 7, 2025

minkyngkm added the Commercial 🛒 label Feb 7, 2025

andesol added Don't merge and removed Review: UX +1 labels Feb 7, 2025

andesol added 5 commits February 7, 2025 18:47

feat: delete payment method

7063951

fix: paymentMethod hook

d8e82d0

hide modal on first render

fa9c3b4

remove console log

2dc02ba

address review comments

2d0b0dd

andesol force-pushed the credit-card-mgmt branch from f4a1b73 to 2d0b0dd Compare February 7, 2025 17:47

andesol commented Feb 7, 2025

View reviewed changes

format js

eb79b76

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Credit card management updates #14670

Credit card management updates #14670

andesol commented Jan 22, 2025 •

edited

Loading

webteam-app commented Jan 22, 2025

codecov bot commented Jan 22, 2025 •

edited

Loading

minkyngkm commented Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm Feb 7, 2025

andesol Feb 7, 2025

minkyngkm commented Feb 7, 2025

andesol left a comment •

edited

Loading

andesol Feb 3, 2025

andesol Feb 7, 2025

andesol Feb 7, 2025

andesol Feb 7, 2025

andesol Feb 7, 2025

andesol Feb 7, 2025

andesol Feb 7, 2025

andesol Feb 7, 2025

andesol Feb 7, 2025

Credit card management updates #14670

Are you sure you want to change the base?

Credit card management updates #14670

Conversation

andesol commented Jan 22, 2025 • edited Loading

Done

QA

Issue

webteam-app commented Jan 22, 2025

codecov bot commented Jan 22, 2025 • edited Loading

Codecov Report

minkyngkm commented Feb 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minkyngkm commented Feb 7, 2025

andesol left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andesol commented Jan 22, 2025 •

edited

Loading

codecov bot commented Jan 22, 2025 •

edited

Loading

andesol left a comment •

edited

Loading