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

fix: ignore 404 when canceling subscription in Stripe #2347

Merged
merged 8 commits into from
Feb 13, 2024

Conversation

pavelbrm
Copy link
Contributor

@pavelbrm pavelbrm commented Feb 7, 2024

Summary

This PR updates the processing of an iOS webhook so that it only cancels an order in SKUs, and does not attempt to cancel it in Stripe, because there is obviously no such subscription.

There are many issues with the existing code for handling iOS webhook, but for now this is only to fix the cancelation. Other improvements will be made at a different time.

Type of Change

  • Product feature
  • Bug fix
  • Performance improvement
  • Refactor
  • Other

Tested Environments

  • Development
  • Staging
  • Production

Before Requesting Review

  • Does your code build cleanly without any errors or warnings?
  • Have you used auto closing keywords?
  • Have you added tests for new functionality?
  • Have validated query efficiency for new database queries?
  • Have documented new functionality in README or in comments?
  • Have you squashed all intermediate commits?
  • Is there a clear title that explains what the PR does?
  • Have you used intuitive function, variable and other naming?
  • Have you requested security and/or privacy review if needed
  • Have you performed a self review of this PR?

Manual Test Plan

@pavelbrm pavelbrm self-assigned this Feb 7, 2024
@pavelbrm pavelbrm requested review from husobee and clD11 February 7, 2024 06:38
@pavelbrm pavelbrm marked this pull request as ready for review February 7, 2024 06:38
Copy link

github-actions bot commented Feb 9, 2024

[puLL-Merge] - brave-intl/bat-go@2347

Description

This pull request makes changes to the controllers.go, controllers_test.go, input.go, model.go, model_test.go, order.go, service.go, service_nonint_test.go files in the services/skus directory. The changes involve refactoring, fixing errors, adding new functions, and improving code readability.

Changes

Changes

  • services/skus/controllers.go
    • Refactor HandleIOSWebhook to handleIOSWebhook to follow naming conventions.
  • services/skus/controllers_test.go
    • Update the test case to use handleIOSWebhook instead of HandleIOSWebhook.
  • services/skus/input.go
    • Remove redundant logging messages in IOSNotification related functions.
  • services/skus/model/model.go
    • Add new functions StripeSubID, IsIOS, IsAndroid, PaymentProc, Vendor in the Order struct for checking order details.
  • services/skus/model/model_test.go
    • Add test cases for the newly added functions in the Order struct.
  • services/skus/order.go
    • Refactor CancelOrder method to handle Stripe subscription cancellation based on order details and improve error handling.
  • services/skus/service.go
    • Add new functions shouldCancelOrderIOS, appStoreTransaction, isErrStripeNotFound to improve code readability and error handling.
  • services/skus/service_nonint_test.go
    • Add test cases for the new functions added in service.go.

Security Hotspots

  1. The handleIOSWebhook function in controllers.go may not handle errors properly when verifying iOS subscription notifications. Ensure that error states and potential vulnerabilities are handled securely in this function.

@pavelbrm
Copy link
Contributor Author

pavelbrm commented Feb 9, 2024

  1. The handleIOSWebhook function in controllers.go may not handle errors properly when verifying iOS subscription notifications. Ensure that error states and potential vulnerabilities are handled securely in this function.

Oh, what age we are living in. 🤦‍♀️ . Go away, robot.

params := &stripe.SubscriptionSearchParams{}
params.Query = *stripe.String(fmt.Sprintf("status:'active' AND metadata['orderID']:'%s'", orderID.String()))
params.Query = fmt.Sprintf("status:'active' AND metadata['orderID']:'%s'", orderID.String())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@clD11 Decided to keep this 'active' here for now as we are not really interested in canceling already canceled subs.

What do you think?

@pavelbrm pavelbrm requested a review from clD11 February 9, 2024 12:21
@pavelbrm pavelbrm changed the title fix: don't attempt to cancel sub in Stripe for ios orders fix: don't attempt to cancel sub in Stripe for iOS orders Feb 12, 2024
@pavelbrm pavelbrm merged commit ba81407 into master Feb 13, 2024
12 checks passed
@pavelbrm pavelbrm deleted the fix-ios-cancellation branch February 13, 2024 03:45
@pavelbrm pavelbrm changed the title fix: don't attempt to cancel sub in Stripe for iOS orders fix: ignore 404 when canceling subscription in Stripe Feb 13, 2024
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