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

[NT-438] Create backing with free shipping #901

Merged
merged 11 commits into from
Oct 21, 2019

Conversation

ifbarrera
Copy link
Contributor

@ifbarrera ifbarrera commented Oct 17, 2019

📲 What

Adds support for creating a pledge with a reward that has free shipping.

🤔 Why

This was missed during the create backing implementation – often rewards ship for free, so we shouldn't be checking on the cost of shipping being greater than 0.

🛠 How

Also includes the following:

  • refactoring shared logic between CreateApplePay, UpdateBacking and CreateBacking
  • we now show the free shipping summary item in the Apple Pay share sheet, something we didn't used to show

👀 See

Create Backing (Free shipping) Update Backing (Free Shipping)
zLmNUuAEp5 WdoGY9AdxU

Also see the new free shipping summary item on the Apple Pay share sheet.

Simulator Screen Shot - iPhone 8 - 2019-10-17 at 16 47 22

✅ Acceptance criteria

  • Navigate to a pledge screen for a reward that has free shipping. Create a pledge - the pledge should succeed.
  • Navigate to update backing for a backing that has free shipping. Change your amount or shipping location. Confirm the backing update. This should succeed.

Copy link
Contributor

@justinswart justinswart left a comment

Choose a reason for hiding this comment

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

Cool minor comments!


let rewardId = reward == Reward.noReward ? nil : reward.graphID
let pledgeParams
= formattedPledgeParameters(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we move this to the same line as let pledgeParams

let rewardId: String? = updateBackingData.reward.graphID
let backingId = updateBackingData.backing.graphID
let (pledgeTotal, rewardId, locationId) =
formattedPledgeParameters(
Copy link
Contributor

Choose a reason for hiding this comment

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

And here?

@@ -216,3 +216,27 @@ internal func classNameWithoutModule(_ class: AnyClass) -> String {
.dropFirst()
.joined(separator: ".")
}

typealias FormattedPledgeParameters = (pledgeTotal: String, rewardId: String?, locationId: String?)
Copy link
Contributor

Choose a reason for hiding this comment

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

When I read FormattedPledgeParameters I think about this going through a formatter, I'm wondering if SanitizedPledgeParameters or SanitizedPledgeParams describes what we're doing more accurately?

Copy link
Contributor

@justinswart justinswart left a comment

Choose a reason for hiding this comment

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

Nice, works!

@ifbarrera ifbarrera merged commit 20f7c74 into master Oct 21, 2019
@ifbarrera ifbarrera deleted the create-backing-free-shipping branch October 21, 2019 21:18
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.

None yet

2 participants