-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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-1852] Update CreateBackingInput #2203
base: main
Are you sure you want to change the base?
Conversation
125e125
to
81ea7ba
Compare
Generated by 🚫 Danger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! Just one typo. We will want tests at some point where pledge over time is enabled, but I trust your judgement for whether that's something that should be tested now or if it should wait.
@@ -15,6 +16,7 @@ public struct CreateBackingInput: GraphMutationInput, Encodable { | |||
|
|||
- parameter amount: The amount. | |||
- parameter applePay: The optional ApplePayParams. | |||
- parameter incremental: The optional Bool indicating whether pledge over time has been selected.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: extra .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a test where incremental
is true
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Definitely. My plan is to add tests when I get going on the implementation piece.
@@ -521,7 +521,8 @@ public class NoShippingPledgeViewModel: NoShippingPledgeViewModelType, NoShippin | |||
paymentSourceId: paymentSourceId, | |||
setupIntentClientSecret: nil, | |||
applePayParams: applePayParams, | |||
refTag: refTag | |||
refTag: refTag, | |||
incremental: false // TODO: implementation in [mbl-1853](https://kickstarter.atlassian.net/browse/MBL-1853) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like incremental
is always false in this PR, is the idea to make it true sometimes in that ticket?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah! Just exposing the Boolean here. Wiring it up to the PLOT component will be done in that ticket in the inline comment.
📲 What
Updates the
CreateBackingInput
GraphQL object to support its newincremental: Bool?
property.Actual implementation will be done separately in mbl-1853.
🤔 Why
This flag will let the backend know if backers have selected the Pledge Over Time option at checkout.
🛠 How
false
to call points for now since implementation will be done in mbl-1853👀 See
Tested that the new property is being sent as false in when attempting a crowdfund pledging:
✅ Acceptance criteria
incremental
is being added to theCreateBackingInput
and is false for now.