-
Notifications
You must be signed in to change notification settings - Fork 190
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
Cred customer purchasing #12868
Cred customer purchasing #12868
Conversation
Demo starting at https://ubuntu-com-12868.demos.haus |
Codecov Report
@@ Coverage Diff @@
## main #12868 +/- ##
==========================================
- Coverage 74.90% 74.80% -0.11%
==========================================
Files 107 107
Lines 2873 2881 +8
Branches 928 936 +8
==========================================
+ Hits 2152 2155 +3
- Misses 699 704 +5
Partials 22 22
|
quantity: quantity, | ||
}; | ||
|
||
const CredCheckout = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you duplicate the whole Checkout app? You should not do that.
Your credential shop should set the product data in the localstorage and reuse the checkout app. You needed a custom redirect and you added an extra parameter for that. That's all correct.
When you add that parameter you have to make sure on all other places the Checkout
component is being used, those would still work.
What else is the Checkout
missing that requires you to duplicate this whole logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not duplicating the Checkout here, I am duplicating the static/js/src/advantage/subscribe/checkout/app.tsx. app.tsx is not an exported component so it won't work outside your application, hence I have to duplicate it. The thing is that you are initializing the product in the base app itself but I have to do it in a separate checkout component which then references your original checkout component. I think the component could be renamed to something like CheckoutWrapper to avoid confusion.
...s/src/advantage/credentials/components/CredPurchaseConfirmation/CredPurchaseConfirmation.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! 🚀
static/js/src/advantage/subscribe/checkout/components/BuyButton/BuyButton.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Revoking the code and QA checks as specs for this have changed |
3c6c6dc
to
e847a86
Compare
@abhigyanghosh30 can you please rebase? |
@abhigyanghosh30 prettier and linting are failing, can you fix that? |
Yes. But webhook responses have stopped working. Fixing that first |
948977d
to
690e909
Compare
@abhigyanghosh30 check-prettier still red. |
690e909
to
60168cd
Compare
Done
QA
./run serve
ordotrun
Issue / Card
Fixes #WD-3821
Help
QA steps - Commit guidelines