Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[MBL-2062] Cleanup NoShippingAtCheckout Feature Flag #2269

Merged
merged 9 commits into from
Feb 3, 2025
Merged

[MBL-2062] Cleanup NoShippingAtCheckout Feature Flag #2269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e2da269
commit remove featureNoShippingAtCheckout() and cleanup code that use…
scottkicks Jan 30, 2025
8436cab
Merge branch 'main' into scott/mbl-2062
scottkicks Jan 30, 2025
d4fcec5
update PledgeViewControllerTests
scottkicks Jan 30, 2025
beee147
update RewardAddOnSelectionViewControllerTests
scottkicks Jan 30, 2025
7d528f5
update RewardCardContainerViewTests
scottkicks Jan 30, 2025
944794e
more snapshot updates
scottkicks Jan 30, 2025
c58d41c
fix view model tests
scottkicks Jan 30, 2025
0b0915d
remove updatePledge from manage pledge menu
scottkicks Jan 30, 2025
914ca91
fix commented out tests and add an inline comment
scottkicks Feb 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 18 additions & 69 deletions Kickstarter-iOS/Features/ManagePledge/Controllers/ManagePledgeViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,12 +289,6 @@ final class ManagePledgeViewController: UIViewController, MessageBannerViewContr
self?.goToRewards(project)
}

self.viewModel.outputs.goToUpdatePledge
.observeForControllerAction()
.observeValues { [weak self] data in
self?.goToUpdatePledge(data: data)
}

self.viewModel.outputs.goToChangePaymentMethod
.observeForControllerAction()
.observeValues { [weak self] data in
Expand Down Expand Up @@ -509,8 +503,6 @@ final class ManagePledgeViewController: UIViewController, MessageBannerViewContr
let title: String

switch option {
case .updatePledge:
title = Strings.Update_pledge()
case .changePaymentMethod:
title = Strings.Change_payment_method()
case .chooseAnotherReward:
Expand Down Expand Up @@ -546,42 +538,15 @@ final class ManagePledgeViewController: UIViewController, MessageBannerViewContr
// MARK: - Functions

private func goToRewards(_ project: Project) {
if featureNoShippingAtCheckout() {
/// Render rewards carousel that has the shipping location dropdown
let vc = WithShippingRewardsCollectionViewController.instantiate(
with: project,
refTag: nil,
context: .managePledge
)
vc.pledgeViewDelegate = self

self.navigationController?.pushViewController(vc, animated: true)
} else {
let vc = RewardsCollectionViewController.instantiate(
with: project,
refTag: nil,
context: .managePledge
)
vc.pledgeViewDelegate = self

self.navigationController?.pushViewController(vc, animated: true)
}
}

private func goToUpdatePledge(data: PledgeViewData) {
if featureNoShippingAtCheckout() {
let vc = NoShippingPledgeViewController.instantiate()
vc.configure(with: data)
vc.delegate = self

self.navigationController?.pushViewController(vc, animated: true)
} else {
let vc = PledgeViewController.instantiate()
vc.configure(with: data)
vc.delegate = self
/// Render rewards carousel that has the shipping location dropdown
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This comment doesn't seem relevant anymore

let vc = WithShippingRewardsCollectionViewController.instantiate(
with: project,
refTag: nil,
context: .managePledge
)
vc.pledgeViewDelegate = self

self.navigationController?.pushViewController(vc, animated: true)
}
self.navigationController?.pushViewController(vc, animated: true)
}

private func goToCancelPledge(with data: CancelPledgeViewData) {
Expand All @@ -593,35 +558,19 @@ final class ManagePledgeViewController: UIViewController, MessageBannerViewContr
}

private func goToChangePaymentMethod(data: PledgeViewData) {
if featureNoShippingAtCheckout() {
let vc = NoShippingPledgeViewController.instantiate()
vc.configure(with: data)
vc.delegate = self

self.navigationController?.pushViewController(vc, animated: true)
} else {
let vc = PledgeViewController.instantiate()
vc.configure(with: data)
vc.delegate = self

self.navigationController?.pushViewController(vc, animated: true)
}
let vc = NoShippingPledgeViewController.instantiate()
vc.configure(with: data)
vc.delegate = self

self.navigationController?.pushViewController(vc, animated: true)
}

private func goToFixPaymentMethod(data: PledgeViewData) {
if featureNoShippingAtCheckout() {
let vc = NoShippingPledgeViewController.instantiate()
vc.configure(with: data)
vc.delegate = self

self.navigationController?.pushViewController(vc, animated: true)
} else {
let vc = PledgeViewController.instantiate()
vc.configure(with: data)
vc.delegate = self

self.navigationController?.pushViewController(vc, animated: true)
}
let vc = NoShippingPledgeViewController.instantiate()
vc.configure(with: data)
vc.delegate = self

self.navigationController?.pushViewController(vc, animated: true)
}

private func goToContactCreator(
Expand Down
12 changes: 2 additions & 10 deletions ...Features/ManagePledge/Controllers/ManageViewPledgeRewardReceivedViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,8 @@ final class ManageViewPledgeRewardReceivedViewControllerTests: TestCase {
)

let devices = [Device.phone4_7inch, Device.phone5_8inch, Device.pad]
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]
orthogonalCombos(Language.allLanguages, devices).forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = ManageViewPledgeRewardReceivedViewController.instantiate()
controller.configureWith(data: data)

Expand All @@ -118,12 +114,8 @@ final class ManageViewPledgeRewardReceivedViewControllerTests: TestCase {
)

let devices = [Device.phone4_7inch, Device.phone5_8inch, Device.pad]
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]
orthogonalCombos(Language.allLanguages, devices).forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = ManageViewPledgeRewardReceivedViewController.instantiate()
controller.configureWith(data: data)

Expand Down
27 changes: 6 additions & 21 deletions ...atures/PledgePaymentMethods/Controller/PledgeRewardsSummaryTotalViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,17 +50,12 @@ final class PledgeRewardsSummaryTotalViewControllerTests: TestCase {
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, good to see these tests cleaned up. Did any other tests change or break now that the feature flag is permanently on?

}

func testViewWithoutReward_WithNoShippingAtCheckoutEnabled() {
func testViewWithoutReward() {
let project = Project.template

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos([Language.en], [Device.phone4_7inch, Device.pad])
.forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = PledgeRewardsSummaryTotalViewController.instantiate()

let data = PledgeSummaryViewData(
Expand All @@ -87,17 +82,12 @@ final class PledgeRewardsSummaryTotalViewControllerTests: TestCase {
}
}

func testViewWithPledgeInFull_WithNoShippingAtCheckoutEnabled() {
func testViewWithPledgeInFull() {
let project = Project.template

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos([Language.en], [Device.phone4_7inch, Device.pad])
.forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = PledgeRewardsSummaryTotalViewController.instantiate()

let data = PledgeSummaryViewData(
Expand Down Expand Up @@ -133,17 +123,12 @@ final class PledgeRewardsSummaryTotalViewControllerTests: TestCase {
}
}

func testViewWithPledgeOverTime_WithNoShippingAtCheckoutEnabled() {
func testViewWithPledgeOverTime() {
let project = Project.template

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos([Language.en], [Device.phone4_7inch, Device.pad])
.forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = PledgeRewardsSummaryTotalViewController.instantiate()

let data = PledgeSummaryViewData(
Expand Down
Binary file added ...SummaryTotalViewControllerTests/testViewWithPledgeInFull.lang_en_device_pad.png
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: These snapshots are only new because the test names changed. It would be nice to mark them as moved instead of added (I don't think that's worth doing now but definitely keep it in mind when you rename files!). Do please delete the old snapshots, though!

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...talViewControllerTests/testViewWithPledgeInFull.lang_en_device_phone4_7inch.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...mmaryTotalViewControllerTests/testViewWithPledgeOverTime.lang_en_device_pad.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...lViewControllerTests/testViewWithPledgeOverTime.lang_en_device_phone4_7inch.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...rdsSummaryTotalViewControllerTests/testViewWithoutReward.lang_en_device_pad.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added ...yTotalViewControllerTests/testViewWithoutReward.lang_en_device_phone4_7inch.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
41 changes: 6 additions & 35 deletions Kickstarter-iOS/Features/PledgeView/Controllers/NoShippingPledgeViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,17 +31,12 @@ final class NoShippingPledgeViewControllerTests: TestCase {
let mockService = MockService(fetchGraphUserResult: .success(response))
let project = Project.template
|> \.availableCardTypes .~ [CreditCardType.discover.rawValue]
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos(Language.allLanguages, Device.allCases).forEach { language, device in
withEnvironment(
apiService: mockService,
currentUser: User.template,
language: language,
remoteConfigClient: mockConfigClient
language: language
) {
let controller = NoShippingPledgeViewController.instantiate()

Expand Down Expand Up @@ -89,17 +84,12 @@ final class NoShippingPledgeViewControllerTests: TestCase {
|> Backing.lens.bonusAmount .~ 695.0
|> Backing.lens.shippingAmount .~ 0
)
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos([Language.en], [Device.phone4_7inch]).forEach { language, device in
withEnvironment(
apiService: mockService,
currentUser: User.template,
language: language,
remoteConfigClient: mockConfigClient
language: language
) {
let controller = NoShippingPledgeViewController.instantiate()
let reward = Reward.noReward
Expand Down Expand Up @@ -132,14 +122,10 @@ final class NoShippingPledgeViewControllerTests: TestCase {
func testView_ShowsShippingSummaryViewSection() {
let reward = Reward.template
|> (Reward.lens.shipping .. Reward.Shipping.lens.enabled) .~ true
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos(Language.allLanguages, [Device.phone4_7inch, Device.pad])
.forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = NoShippingPledgeViewController.instantiate()
let data = PledgeViewData(
project: .template,
Expand Down Expand Up @@ -174,14 +160,10 @@ final class NoShippingPledgeViewControllerTests: TestCase {
|> Reward.lens.shipping.enabled .~ true
|> Reward.lens.shippingRulesExpanded .~ [shippingRule]
|> Reward.lens.id .~ 99
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos(Language.allLanguages, [Device.phone4_7inch, Device.pad])
.forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = NoShippingPledgeViewController.instantiate()
let data = PledgeViewData(
project: .template,
Expand Down Expand Up @@ -233,14 +215,10 @@ final class NoShippingPledgeViewControllerTests: TestCase {
refTag: .projectPage,
context: .pledge
)
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos([Language.en], [Device.phone4_7inch, Device.pad])
.forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = NoShippingPledgeViewController.instantiate()
controller.configure(with: data)
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller)
Expand Down Expand Up @@ -274,14 +252,10 @@ final class NoShippingPledgeViewControllerTests: TestCase {
refTag: .projectPage,
context: .pledge
)
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos([Language.en], [Device.phone4_7inch, Device.pad])
.forEach { language, device in
withEnvironment(language: language, remoteConfigClient: mockConfigClient) {
withEnvironment(language: language) {
let controller = NoShippingPledgeViewController.instantiate()
controller.configure(with: data)
let (parent, _) = traitControllers(device: device, orientation: .portrait, child: controller)
Expand Down Expand Up @@ -314,7 +288,6 @@ final class NoShippingPledgeViewControllerTests: TestCase {

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true,
RemoteConfigFeature.pledgeOverTime.rawValue: true
]

Expand Down Expand Up @@ -369,7 +342,6 @@ final class NoShippingPledgeViewControllerTests: TestCase {

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true,
RemoteConfigFeature.pledgeOverTime.rawValue: true
]

Expand Down Expand Up @@ -424,7 +396,6 @@ final class NoShippingPledgeViewControllerTests: TestCase {

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true,
RemoteConfigFeature.pledgeOverTime.rawValue: true
]

Expand Down
21 changes: 3 additions & 18 deletions ...arter-iOS/Features/PledgeView/Controllers/NoShippingPostCampaignViewControllerTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,12 @@ final class NoShippingPostCampaignViewControllerTests: TestCase {
)
let project = Project.template
|> \.availableCardTypes .~ [CreditCardType.discover.rawValue]
let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]

orthogonalCombos(Language.allLanguages, Device.allCases).forEach { language, device in
withEnvironment(
apiService: mockService,
currentUser: User.template,
language: language,
remoteConfigClient: mockConfigClient
language: language
) {
let controller = NoShippingPostCampaignCheckoutViewController.instantiate()

Expand Down Expand Up @@ -113,10 +108,6 @@ final class NoShippingPostCampaignViewControllerTests: TestCase {
|> Reward.lens.shippingRulesExpanded .~ [shippingRule]
|> Reward.lens.id .~ 99

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]
let userResponse = UserEnvelope<GraphUser>(me: self.userWithoutCards)
let mockService = MockService(
createCheckoutResult: .success(self.checkoutResponse),
Expand All @@ -128,8 +119,7 @@ final class NoShippingPostCampaignViewControllerTests: TestCase {
withEnvironment(
apiService: mockService,
currentUser: User.template,
language: language,
remoteConfigClient: mockConfigClient
language: language
) {
let controller = NoShippingPostCampaignCheckoutViewController.instantiate()
let data = PledgeViewData(
Expand Down Expand Up @@ -180,18 +170,13 @@ final class NoShippingPostCampaignViewControllerTests: TestCase {
context: .latePledge
)

let mockConfigClient = MockRemoteConfigClient()
mockConfigClient.features = [
RemoteConfigFeature.noShippingAtCheckout.rawValue: true
]
let mockService = MockService(createCheckoutResult: .success(self.checkoutResponse))

orthogonalCombos(Language.allLanguages, [Device.phone4_7inch, Device.pad])
.forEach { language, device in
withEnvironment(
apiService: mockService,
language: language,
remoteConfigClient: mockConfigClient
language: language
) {
let controller = NoShippingPostCampaignCheckoutViewController.instantiate()
controller.configure(with: data)
Expand Down
Loading