Skip to content

[Customer Center] Fix checking eligibility#4138

Merged
vegaro merged 2 commits into
integration/customer_support_workflowfrom
fix-checking-eligibility
Jul 31, 2024
Merged

[Customer Center] Fix checking eligibility#4138
vegaro merged 2 commits into
integration/customer_support_workflowfrom
fix-checking-eligibility

Conversation

@vegaro

@vegaro vegaro commented Jul 31, 2024

Copy link
Copy Markdown
Member

We were not checking if the user is eligible for the promotional offer before trying to load it

@vegaro vegaro requested a review from a team July 31, 2024 10:39

@tonidero tonidero left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this ready for review? LGTM so far

@@ -127,15 +127,20 @@ class ManageSubscriptionsViewModel: ObservableObject {
}
}
case let .promotionalOffer(promotionalOffer):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've wondering whether it would make sense to filter out the promotional offers before this... But that would require some preprocessing of the config data, so I think this is ok.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thought about it as well, but figured this would be the easiest for now

case .success(let promotionalOfferData):
self.promotionalOfferData = promotionalOfferData
case .failure:
if promotionalOffer.eligible {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Probably good to add some tests once we have time.

@vegaro vegaro marked this pull request as ready for review July 31, 2024 11:02
@vegaro

vegaro commented Jul 31, 2024

Copy link
Copy Markdown
Member Author

Is this ready for review?

Oh yes, I opened it as draft by mistake 😄

@vegaro

vegaro commented Jul 31, 2024

Copy link
Copy Markdown
Member Author

Added a test to ManageSubscriptionsViewModelTest since it wasn't too complex

@vegaro vegaro merged commit c255107 into integration/customer_support_workflow Jul 31, 2024
@vegaro vegaro deleted the fix-checking-eligibility branch July 31, 2024 15:14
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.

2 participants