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

docs: changed details related to new method signatures #2876

Merged
merged 7 commits into from
Jan 5, 2023

Conversation

shahednasser
Copy link
Member

Changed details in the Payment Provider docs regarding new method signature.

Closes DOCS-390

@shahednasser shahednasser requested a review from a team as a code owner December 22, 2022 11:52
@changeset-bot
Copy link

changeset-bot bot commented Dec 22, 2022

⚠️ No Changeset found

Latest commit: 12b98e9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Member

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

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

Here are few changes 💪

Cart, Data, Payment, PaymentSession,
PaymentSessionStatus, TransactionBaseService,
} from "@medusajs/medusa"
import { AbstractPaymentService, Context, Data, Payment, PaymentSession, PaymentSessionStatus, TransactionBaseService } from "@medusajs/medusa"
import { EntityManager } from "typeorm"

class MyPaymentService extends AbstractPaymentService<TransactionBaseService> {
Copy link
Member

Choose a reason for hiding this comment

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

todo: The class is not template, rm TransactionBaseService

Cart, Data, Payment, PaymentSession,
PaymentSessionStatus, TransactionBaseService,
} from "@medusajs/medusa"
import { AbstractPaymentService, Context, Data, Payment, PaymentSession, PaymentSessionStatus, TransactionBaseService } from "@medusajs/medusa"
import { EntityManager } from "typeorm"

class MyPaymentService extends AbstractPaymentService<TransactionBaseService> {
protected manager_: EntityManager
protected transactionManager_: EntityManager
Copy link
Member

Choose a reason for hiding this comment

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

todo: | undefined

throw new Error("Method not implemented.")
}
updatePaymentData(paymentSessionData: Data, data: Data): Promise<Data> {
async updatePaymentData(paymentSessionData: Data, data: Data): Promise<Data> {
Copy link
Member

Choose a reason for hiding this comment

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

todo: async updatePaymentData(paymentSessionData: PaymentSessionData, data: Data): Promise<PaymentSessionData>

throw new Error("Method not implemented.")
}
createPayment(cart: Cart): Promise<Data> {
async createPayment(context: Context): Promise<PaymentSessionResponse> {
Copy link
Member

Choose a reason for hiding this comment

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

todo: Cart & PaymentContext)

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we include Cart though? Because we're moving to PaymentContext and Cart is used for backwards compatibility (from my understanding)

Copy link
Member

@adrien2p adrien2p Jan 4, 2023

Choose a reason for hiding this comment

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

The thing is that TS will error out because the typings are mismatched. PaymentContext can't be assigned to Cart & PaymentContext

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ok I didn't notice that. Will make the change!

throw new Error("Method not implemented.")
}
updatePayment(paymentSessionData: Data, cart: Cart): Promise<Data> {
async updatePayment(paymentSessionData: Data, context: Context): Promise<PaymentSessionResponse> {
Copy link
Member

Choose a reason for hiding this comment

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

todo:
paymentSessionData: PaymentSessionData, context: Cart & PaymentContext

@@ -137,8 +130,8 @@ Additionally, if you’re creating your Payment Provider as an external plugin t
```ts
class MyPaymentService extends AbstractPaymentService<TransactionBaseService> {
Copy link
Member

Choose a reason for hiding this comment

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

todo: rm template


For example, in Stripe this method is used to initialize a Payment Intent for the customer.

The method receives a context object as a first parameter. This object is of type `PaymentContext` and has the following properties:
Copy link
Member

Choose a reason for hiding this comment

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

todo: is of type Cart & PaymentContext.
This is for the backward compatibility. do you think it should be mentioned? plus it will change soon with the new payment processor API. I let you choose


class MyPaymentService extends AbstractPaymentService<TransactionBaseService> {
// ...
async createPayment(cart: Cart): Promise<Data> {
async createPayment(context: PaymentContext): Promise<PaymentSessionResponse> {
Copy link
Member

Choose a reason for hiding this comment

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

todo: Cart & PaymentContext

@@ -244,24 +267,56 @@ A line item refers to a product in the cart.

:::

It accepts the `data` field of the Payment Session as the first parameter and the cart as an object for the second parameter.
It accepts the `data` field of the Payment Session as the first parameter and a context object as a second parameter. This object is of type `PaymentContext` and has the following properties:
Copy link
Member

Choose a reason for hiding this comment

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

todo: same as the previous one


<!-- eslint-disable max-len -->

```ts
import { Cart, Data } from "@medusajs/medusa"
import { PaymentContext, Data } from "@medusajs/medusa"
Copy link
Member

Choose a reason for hiding this comment

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

todo: To update according to previous todo's

@shahednasser shahednasser requested a review from adrien2p January 4, 2023 14:28
Copy link
Member

@adrien2p adrien2p left a comment

Choose a reason for hiding this comment

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

LGTM 🎉 just some description still showing PaymentContext only but I suppose it is the final choice is that right?

@shahednasser
Copy link
Member Author

@adrien2p yes I added a note about how before v1.7.2 Cart was allowed but moving forward it's recommended to use PaymentContext even though it's still backwards compatible at the moment

@shahednasser shahednasser merged commit 8ba27a2 into master Jan 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants