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

[MBL-1897] Hide "Edit Reward" when pledge over time #2221

Merged
merged 18 commits into from
Jan 6, 2025
Merged

[MBL-1897] Hide "Edit Reward" when pledge over time #2221

merged 18 commits into from
Jan 6, 2025
+61 −4

Conversation

jovaniks
Copy link
Contributor

@jovaniks jovaniks commented Dec 16, 2024 •
edited by jira bot
Loading

📲 What

This PR hides the "Edit Reward" option for PLOT-enabled pledges in the "Manage Pledge" screen.

🤔 Why

Backers with PLOT (Pledge Over Time) enabled cannot edit their pledge after the initial checkout. The "Edit Reward" option must be removed to avoid confusion and align with the PLOT functionality restrictions.

🛠 How

  • Since the API for validating backing.PaymentIncrements is not yet available (MBL-1851), a temporary validation has been added for development and testing purposes.
  • The function featurePledgeOverTimeEnabled() is used to simulate that the pledge is Pledge Over Time.
  • Updated the action sheet logic to hide the "Edit Reward" option based on this temporary validation:
private func isPledgeOverTime(with _: Backing) -> Bool {
  return featurePledgeOverTimeEnabled()
}
  • Once the API is available, this logic will be replaced to validate backing.PaymentIncrements properly.

👀 See

No PLOT Pledge PLOT Pledge
Simulator Screenshot - iPhone SE (3rd generation) - 2024-12-16 at 08 42 09 Simulator Screenshot - iPhone SE (3rd generation) - 2024-12-16 at 08 38 29

✅ Acceptance criteria

  • The "Edit Reward" option is hidden when the Remote Config feature flag Pledge Over Time is enabled.
  • The "Edit Reward" option is visible when the Remote Config feature flag Pledge Over Time is disabled.

⏰ TODO

  • Replace featurePledgeOverTimeEnabled() with backing.PaymentIncrements once MBL-1851 is implemented.

@jovaniks jovaniks changed the title Hide "Edit Reward" when pledge over time [MBL-1897] Hide "Edit Reward" when pledge over time Dec 16, 2024
@jovaniks jovaniks self-assigned this Dec 16, 2024
amy-at-kickstarter
amy-at-kickstarter requested changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter left a comment

Choose a reason for hiding this comment

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

Small change to the test may be required, correct me if I'm wrong though.

Library/ViewModels/ManagePledgeViewModelTests.swift Outdated

self.showActionSheetMenuWithOptions.assertValues([
[
ManagePledgeAlertAction.updatePledge,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should it be including updatePledge? If this is only implemented when featureNoShippingAtCheckout() is true, then it looks like updatePledge should also be filtered out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're correct – since pledgeOverTime coexists with noShippingAtCheckout, I should include that feature flag as well and remove updatePledge from this test. Thanks for pointing it out!

ifosli
ifosli requested changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@ifosli ifosli left a comment

Choose a reason for hiding this comment

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

Looks good apart from the test thing that Amy already pointed out!

Library/ViewModels/ManagePledgeViewModelTests.swift Outdated

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.pledgeOverTime.rawValue: true
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with Amy - just add the featureNoShippingAtCheckout flag here, too, so that the test makes sense!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also agree – the test has been updated to include the featureNoShippingAtCheckout flag. Thanks for the input!

Base automatically changed from jluna/MBL-1815/plot-plan-selector-selected-state to main December 17, 2024 20:08
@jovaniks
Merge branch 'jluna/MBL-1815/plot-plan-selector-selected-state-2' int…
5460288 
…o jluna/MBL-1897/plot-manage-pledge-summary-hide-edit-reward
@jovaniks
Merge branch 'main' into jluna/MBL-1897/plot-manage-pledge-summary-hi…
148865f 
…de-edit-reward

# Conflicts:
#	Kickstarter-iOS/Features/PledgeOverTime/Controller/PledgePaymentPlansViewController.swift
#	Kickstarter-iOS/Features/PledgeOverTime/Controller/PledgePaymentPlansViewControllerTest.swift
#	Kickstarter-iOS/Features/PledgeOverTime/Views/PledgePaymentPlanOptionView.swift
#	Library/ViewModels/NoShippingPledgeViewModel.swift
#	Library/ViewModels/PledgePaymentPlansOptionViewModel.swift
#	Library/ViewModels/PledgePaymentPlansOptionViewModelTest.swift
#	Library/ViewModels/PledgePaymentPlansViewModel.swift
#	Library/ViewModels/PledgePaymentPlansViewModelTest.swift
@jovaniks
Add noShippingAtCheckout to remove updatePledge option since `ple…
d5477ba 
…dgeOverTime` coexists with `noShippingAtCheckout`
jovaniks
jovaniks commented Dec 18, 2024
View reviewed changes
Copy link
Contributor Author

@jovaniks jovaniks left a comment

Choose a reason for hiding this comment

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

Agree with you both, test updated

Library/ViewModels/ManagePledgeViewModelTests.swift Outdated

self.showActionSheetMenuWithOptions.assertValues([
[
ManagePledgeAlertAction.updatePledge,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're correct – since pledgeOverTime coexists with noShippingAtCheckout, I should include that feature flag as well and remove updatePledge from this test. Thanks for pointing it out!

Library/ViewModels/ManagePledgeViewModelTests.swift Outdated

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.pledgeOverTime.rawValue: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also agree – the test has been updated to include the featureNoShippingAtCheckout flag. Thanks for the input!

@jovaniks jovaniks merged commit 1317885 into main Jan 6, 2025
5 checks passed
@jovaniks jovaniks deleted the jluna/MBL-1897/plot-manage-pledge-summary-hide-edit-reward branch January 6, 2025 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ifosli ifosli ifosli approved these changes

@amy-at-kickstarter amy-at-kickstarter amy-at-kickstarter approved these changes

Assignees

@jovaniks jovaniks

Labels
Pledge Over Time
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jovaniks @ifosli @amy-at-kickstarter