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

feat(targets): Add Payment target #2817

Closed
wants to merge 14 commits into from
Closed

feat(targets): Add Payment target #2817

wants to merge 14 commits into from

Conversation

larsroettig
Copy link
Member

@larsroettig larsroettig commented Oct 23, 2020

Description

As developer, we want to build our own Payment methods as Extension for that reasons.
Payments should be extendable via Targetables

Related Issue

Closes #2816

Acceptance

Verification Stakeholders

Specification

Verification Steps

  1. Checkout should work as before
  2. Scaffolding
DEBUG_PROJECT_CREATION=true ./pwa-studio/packages/pwa-buildpack/bin/buildpack create-project payment-test-1 --template "venia-concept" --name "payment-test-1" --author "larsroettig" --backend-url "https://master-7rqtwti-mfwmkrjfqvbjk.us-4.magentosite.cloud/" --braintree-token "sandbox_8yrzsvtm_s2bg8fs563crhqzk" --npm-client "yarn"
  1. Payment addable via
    payment-test-1/local-intercept.js
function localIntercept(targets) {
    targets.of("@magento/venia-ui").payments.tap((payments) =>
        payments.add({
            paymentCode: "custome",
            importPath: "src/components/MyPayment/payment.js",
        })
    );
}

Screenshots / Screen Captures (if appropriate)

Screenshot at 11-50-24

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Oct 23, 2020

Fails
🚫 A version label is required. A maintainer must add one.
Messages
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).

Generated by 🚫 dangerJS against 4a24865

@m2-community-project m2-community-project bot moved this from Ready for Review to Review in Progress in Pull Request Progress Oct 23, 2020
@sirugh
Copy link
Contributor

sirugh commented Oct 23, 2020

I'm unable to view the deployment, but it says it passed. Not sure why.

Copy link
Contributor

@revanth0212 revanth0212 left a comment

Choose a reason for hiding this comment

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

Beautiful, love the way this small piece of code opens up payment methods for extensions. I have a couple of suggestions for you, I'll leave it up to you if you wanna handle them.

Thanks for the contribution @larsroettig.

@@ -11,7 +11,7 @@ test('normalizes JSX without brackets or closing elements', () => {
['wat', '<wat />'],
[' oh no="crap"', '<oh no="crap" />']
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change this 🤣 ?

* This file is augmented at build time using the @magento/venia-ui build
* target "payments", which allows third-party modules to add new payment component mappings.
*/
export default {};
Copy link
Contributor

Choose a reason for hiding this comment

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

I would add to the docs the potential type of the default object.

{
   [paymentCode]: paymentCodeComponent
}

Or the post build representation of this file:

import braintree from '@magento/venia-ui/lib/components/CheckoutPage/paymentMethods/braintree`;
import paypal from '@third_party_package/path/to/the/component';

export default {
   braintree: braintree,
   paypal: paypal
}

const paymentMethodList = new PaymentMethodList(venia);
paymentMethodList.add({
paymentCode: 'braintree',
importPath: './creditCard'
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be clear if you can either mention in a comment here that the path reference is from the paymentMetodCollection.js file or just change this to '@magento/venia-ui/lib/components/CheckoutPage/PaymentInformation/creditCard`.

@dpatil-magento dpatil-magento deleted the branch magento:zetlen/targetables October 27, 2020 04:04
@m2-community-project m2-community-project bot removed this from Review in Progress in Pull Request Progress Oct 27, 2020
@sirugh
Copy link
Contributor

sirugh commented Oct 27, 2020

@dpatil-magento did you mean to close this? Maybe the base just needs to be updated now that the original base was merged.

@larsroettig
Copy link
Member Author

@sirugh I will resubmit in new PR today because I don't want to rebase from a force rebased branch i saved as diff.

@dpatil-magento
Copy link
Contributor

@sirugh I merged #2765 and closed branch zetlen/targetables that might have caused this pr to close automatically. I have restored branch zetlen/targetables just incase needed.

dpatil-magento added a commit that referenced this pull request Oct 28, 2020
* #2817: Add Payment Targets

* #2816: Review changes

* Prettier changes.

Co-authored-by: Devagouda <40405790+dpatil-magento@users.noreply.github.com>
Co-authored-by: Revanth Kumar <revanth0212@gmail.com>
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.

None yet

7 participants