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

[Customer Center] Promotional Offers support #3968

Conversation

vegaro
Copy link
Contributor

@vegaro vegaro commented Jun 17, 2024

Adds displaying a Promotional Offer if the path has an offer id configured

@vegaro
Copy link
Contributor Author

vegaro commented Jun 17, 2024

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @vegaro and the rest of your teammates on Graphite Graphite

@vegaro vegaro changed the title Removes eligibility from reponse [Customer Center] Removes eligibility from reponse Jun 17, 2024
@vegaro vegaro force-pushed the feedback-survey branch from b17129a to 6d0dfbf Compare June 17, 2024 13:58
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch 2 times, most recently from d7ef0b6 to 901e975 Compare June 21, 2024 15:12
@vegaro vegaro changed the title [Customer Center] Removes eligibility from reponse [Customer Center] Removes eligibility from response Jun 21, 2024
@vegaro vegaro changed the title [Customer Center] Removes eligibility from response [Customer Center] Promotional Offers support Jun 21, 2024
@vegaro vegaro force-pushed the feedback-survey branch from 6d0dfbf to 0540b7f Compare June 26, 2024 12:47
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch from 901e975 to 181e73b Compare June 26, 2024 12:47
@vegaro vegaro force-pushed the feedback-survey branch from 0540b7f to 4a16289 Compare June 26, 2024 12:56
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch from 181e73b to 76b9e6a Compare June 26, 2024 12:56
@vegaro vegaro force-pushed the feedback-survey branch from 4a16289 to 2eb43e8 Compare June 26, 2024 14:45
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch from 76b9e6a to f8b3f67 Compare June 26, 2024 14:45
@vegaro vegaro force-pushed the feedback-survey branch from 2eb43e8 to 5e24a1d Compare June 27, 2024 10:15
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch from f8b3f67 to e86e19c Compare June 27, 2024 10:15
@vegaro vegaro force-pushed the feedback-survey branch from 5e24a1d to 65f0138 Compare June 28, 2024 10:19
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch from e86e19c to bf7e1e0 Compare June 28, 2024 10:19
@vegaro vegaro force-pushed the feedback-survey branch from 65f0138 to bd32cdd Compare June 28, 2024 10:33
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch from bf7e1e0 to 9c62db1 Compare June 28, 2024 10:34
@vegaro vegaro force-pushed the feedback-survey branch from bd32cdd to 466858a Compare June 28, 2024 11:06
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch from 9c62db1 to 76decce Compare June 28, 2024 11:06
@vegaro vegaro force-pushed the feedback-survey branch from 466858a to 085e2f5 Compare June 28, 2024 13:24
@vegaro vegaro force-pushed the 06-17-removes_eligibility_from_reponse branch from 8caebf1 to 6028e5d Compare July 18, 2024 15:56
@vegaro
Copy link
Contributor Author

vegaro commented Jul 19, 2024

@JayShortway @tonidero I am changing localization to be an Environment like for appearance. It cleans up the code a lot!

@vegaro vegaro requested review from tonidero and JayShortway July 19, 2024 11:25
Copy link
Contributor

@tonidero tonidero left a comment

Choose a reason for hiding this comment

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

Left some comments but nothing blocking! I think we can merge this 👍

.active
.values
.lazy
.filter { $0.store == .appStore }
Copy link
Contributor

Choose a reason for hiding this comment

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

[not for this PR] I was thinking, if the user has a purchase in a different store, we could probably still get it and display whatever information is available in the "no_active" page I guess but nothing to do here right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good point, will take notes of that

}

do {
let result = try await Purchases.shared.purchase(product: product, promotionalOffer: promotionalOffer)
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking about this, we should move this to the CustomerCenterPurchasesType?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should for sue. I will do it in another PR if you don't mind, since that's going to involve some updates and potentially breaking tests and I need to merge this today

private var viewModel: FeedbackSurveyViewModel

@Environment(\.localization)
private var localization: CustomerCenterConfigData.Localization?
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the back and forth but I'm wondering if we should make this not nullable after all... When these views are created, the configuration should have always loaded so it would be safe to assume this not to be null. That way we don't need checks all over the place for nullability. Or are there any views that require localization before the configuration has loaded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the configuration is the first thing that gets loaded so there should be localization in all screens

@vegaro vegaro merged commit fa06324 into integration/customer_support_workflow Jul 19, 2024
26 checks passed
@vegaro vegaro deleted the 06-17-removes_eligibility_from_reponse branch July 19, 2024 14:30
tonidero added a commit that referenced this pull request Jul 19, 2024
### Description
This is based on the changes in #3968 

This PR:
- Moves the strings currently hardcode into the
`CustomerCenterConfigData.Localization` object.
- Fixes an issue compiling paywall tester introduced in a previous PR
- Modifies how we read backend strings to not account for the `common_`
prefix, which is already removed by the backend.
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.

5 participants