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: use customer id when present whilst creating a premium order #2715

Merged
merged 4 commits into from
Nov 21, 2024

Conversation

pavelbrm
Copy link
Contributor

Summary

This PR makes it possible for the known customer to be passed when creating a premium order. Additionally, this makes use of the customer information obtained from the old session when recreating one (mainly as part of the set trial days call).

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 Nov 19, 2024
@pavelbrm pavelbrm marked this pull request as ready for review November 19, 2024 08:45
@pavelbrm pavelbrm requested a review from clD11 November 19, 2024 08:45
Copy link
Contributor

@clD11 clD11 left a comment

Choose a reason for hiding this comment

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

LGTM approved.

@pavelbrm pavelbrm merged commit 2e8f6a4 into master Nov 21, 2024
13 checks passed
@pavelbrm pavelbrm deleted the handle-checkout-cust-01 branch November 21, 2024 10:38
Copy link
Member

@kdenhartog kdenhartog left a comment

Choose a reason for hiding this comment

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

LGTM - reviewed this as a part of https://github.com/brave/reviews/issues/1822

As long as I'm understanding the flow of this properly, I think we are introducing a correlating tracker here that we don't want. I need to keep reviewing to be sure, but definitely want to call it out early so we can revert this here and find a different approach to this if it is the case.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for adding these tests in here - it made it much simpler to understand what this change was doing.

@pavelbrm
Copy link
Contributor Author

pavelbrm commented Jan 5, 2025

As long as I'm understanding the flow of this properly, I think we are introducing a correlating tracker here that we don't want.

I don't think so. This has always been here in a slightly different form. This data is not kept in SKUs. SKUs used to figure out the customer id itself in some flows, but now it accepts it.

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.

3 participants