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.

Sign up for GitHub

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

Jump to bottom

Introduce stripe integration for 111 NFT #147

Merged
merged 8 commits into from
Jan 22, 2024
Merged

Introduce stripe integration for 111 NFT #147

merged 8 commits into from
Jan 22, 2024

Conversation

thisisommore
Copy link
Member

fix #146

@thisisommore thisisommore linked an issue Jan 7, 2024 that may be closed by this pull request
Stripe integration for 111 NFT #146
Closed
Shachindra
Shachindra approved these changes Jan 11, 2024
View reviewed changes
Copy link
Member

@Shachindra Shachindra left a comment

Choose a reason for hiding this comment

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

LGTM

Shachindra
Shachindra approved these changes Jan 11, 2024
View reviewed changes
api/v1/account/subscription/subscription.go
Copy link
Member

Choose a reason for hiding this comment

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

Rename?

stripe-webhook -> payment-webhook
111-nft -> erebrus

api/v1/account/subscription/type.go
Copy link
Member

Choose a reason for hiding this comment

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

Buy111NFTResponse -> BuyErebrusNFTResponse

migrations/000013_users_add_stripe_session_id.up.sql
Copy link
Member

Choose a reason for hiding this comment

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

Why this is inserted on USERS table?

Copy link
Member Author

@thisisommore thisisommore Jan 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

So we can track for which users wallet to mint nft to after payment success

Copy link
Member

Choose a reason for hiding this comment

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

But doesn't this restricts the user to only be able to buy only 1 subscription? When we launch another subscription does this get updated? Also what'll happen in the case of failed/abandoned payment requests?

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated to support multiple subscriptions, also abandoned/failed payments are current not handled since stripe should just stop that and have time limit but we can keep record of the payment intents in db so in case of any issues we can always access it in db

Shachindra
Shachindra requested changes Jan 19, 2024
View reviewed changes
Copy link
Member

@Shachindra Shachindra left a comment

Choose a reason for hiding this comment

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

Please check the comments

migrations/000013_users_add_stripe_session_id.up.sql
Copy link
Member

Choose a reason for hiding this comment

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

But doesn't this restricts the user to only be able to buy only 1 subscription? When we launch another subscription does this get updated? Also what'll happen in the case of failed/abandoned payment requests?

Shachindra
Shachindra approved these changes Jan 22, 2024
View reviewed changes
Copy link
Member

@Shachindra Shachindra left a comment

Choose a reason for hiding this comment

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

LGTM

@thisisommore thisisommore merged commit 1b7b32a into main Jan 22, 2024
2 of 5 checks passed
@thisisommore thisisommore deleted the 146-stripe-integration-for-111-nft branch January 22, 2024 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shachindra Shachindra Shachindra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Stripe integration for 111 NFT
2 participants
@thisisommore @Shachindra