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-1958] Use New PLOT Disclaimer Copy On Project Fragment #2237

Merged
merged 11 commits into from
Jan 13, 2025

Conversation

scottkicks
Copy link
Contributor

@scottkicks scottkicks commented Jan 9, 2025

📲 What

Exposes a new pledgeOverTimeMinimumExplanation property on the Project fragment and implements it.

The threshold should be $125

🤔 Why

To keep this string dynamic in case the plot eligibility threshold changes in the future, payments added it for us on the Project fragment.

🛠 How

  • exposes the new string
  • uses it in the PLOT UI checkout component

👀 See

plot component

✅ Acceptance criteria

  • New string is being used when PLOT is ineligible for the current pledge

@scottkicks scottkicks changed the title [MBL-1958] [MBL-1958] Use "Available for pledges over {amount}" Copy On Project Fragment Jan 9, 2025
@scottkicks scottkicks changed the title [MBL-1958] Use "Available for pledges over {amount}" Copy On Project Fragment [MBL-1958] Use New PLOT Disclaimer Copy On Project Fragment Jan 9, 2025
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.

👍 Nice! Great to have this moved to the server.

@scottkicks scottkicks marked this pull request as ready for review January 9, 2025 21:03
@scottkicks scottkicks requested a review from jovaniks January 9, 2025 21:03
Copy link
Contributor

@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.

LGTM overall! Just one small concern: could we clean up the unused thresholdAmount fields?

@@ -123,7 +123,7 @@ public final class PledgePaymentPlansOptionViewModel:

self.ineligibleBadgeText = configData
.filterWhenLatestFrom(ineligible, satisfies: { $0 == true })
.map { getIneligibleBadgeText(with: $0.project, thresholdAmount: $0.thresholdAmount) }
Copy link
Contributor

Choose a reason for hiding this comment

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

Since thresholdAmount will no longer be used, should we remove all references to it? Also, what about PledgePaymentPlansAndSelectionData.thresholdAmount? It might be a good idea to clean those up if they're not needed anymore.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good call!

@scottkicks scottkicks requested a review from jovaniks January 13, 2025 15:47
Copy link
Contributor

@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.

Nice!!!

@scottkicks scottkicks merged commit 70c3309 into main Jan 13, 2025
5 checks passed
@scottkicks scottkicks deleted the scott/mbl-1958 branch January 13, 2025 20:49
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.

3 participants