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

[MBL-1684] Remove Redundant No Shipping At Checkout Feature Flag Checks #2140

Merged
merged 4 commits into from
Sep 3, 2024

Conversation

scottkicks
Copy link
Contributor

@scottkicks scottkicks commented Aug 29, 2024

📲 What

Removes Redundant featureNoShippingAtCheckout Checks

Also does a quick fix for an issue found in QA for the Stripe Sheet not showing on NoShippingPledgeViewController.

🤔 Why

The ProjectPageVIewController determines whether to start the "No Shipping At Checkout" flow or the current flow in Production.

We have unnecessary if featureNoShippingAtCheckout() conditions in the "No Shipping At Checkout" View Controllers.

Since they'll never be shown when the flag is disabled, I'm removing them. They should never have been here tbh.
This all cleanup work.

🛠 How

Goes through each of these VCs and removes unused code and unnecessary feature flag checks:

  • WithShippingRewardsCollectionVC + RewardsCollectionVC
  • RewardAddOnSelectionNoShippingVC + RewardAddOnSelectionVC

✅ Acceptance criteria

  • Both Checkout flows function as expected when featureNoShippingAtCheckout is OFF
  • Both Checkout flows function as expected when featureNoShippingAtCheckout is ON
  • All tests pass

this VC is only instantiated when featureNoShippingAtCheckout is enabled so we can remove the extra conditionals and this method since we don't want to go to confirm details at all from here anymore
This class is only instantiated when featureNoShippingAtCheckout is disabled. The check happens on the project page so no need to do it here.
Comment on lines 160 to 164
assert(
topViewController is PledgeViewController ||
topViewController is NoShippingPledgeViewController ||
topViewController is PostCampaignCheckoutViewController,
"PledgePaymentMethodsViewController is only intended to be presented as part of a pledge flow."
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 need to add this type-check here so that the Stripe payment sheet is presented on the NoShippingPledgeViewController

}

self.viewModel.outputs.goToPledge
self.viewModel.outputs.goToCustomizeYourReward
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the name of this output because this is called anytime a nonshippable reward is selected taking the user to the screens that only have the pledge amount or bonus amount selectors shown.

@scottkicks scottkicks marked this pull request as ready for review August 30, 2024 00:13
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.

Nice!

@scottkicks scottkicks merged commit ca534c9 into main Sep 3, 2024
5 checks passed
@scottkicks scottkicks deleted the scott/redemption/remove-redunancy-checks branch September 3, 2024 15:52
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