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-2052 PLOT projects not working after scheme change on BE #2215

Merged
merged 4 commits into from
Jan 30, 2025

Conversation

jlplks
Copy link
Contributor

@jlplks jlplks commented Jan 29, 2025

📲 What

After a change on BE they remove the prop projectIsPledgeOverTimeAllowed from PaymentPlan.
We have to remove it from the project too.

🤔 Why

Can't pledge PLOT projects with PLOT becuase the component doesn't render do a validation with that prop.

🛠 How

Remove all the uses for that prop and change the only validation where it's used.

👀 See

| Before 🐛 |
error_message

| After 🦋 |
plot_fixed.webm

📋 QA

Enter with a valid user to staging an look for Crossfit Watch Hot Sauce

the project is a PLOT project so if you follow the PLOT flow you should see the PLOT component rendering correctly and have no problem to pledge it

Story 📖

MBL-2052 PLOT projects not working after scheme change on BE

@@ -277,7 +277,7 @@ class CrowdfundCheckoutViewModel(val environment: Environment, bundle: Bundle? =
emitCurrentState(isLoading = false)
}
.collectLatest {
showPlotWidget = it.projectIsPledgeOverTimeAllowed
showPlotWidget = !it.paymentIncrements.isNullOrEmpty()
Copy link
Contributor

@leighdouglas leighdouglas Jan 29, 2025

Choose a reason for hiding this comment

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

Won't the payment increments be null or empty if the pledge doesn't meet the PLOT minimum? Fundamentally, the widget visibility should be toggled based on whether the project has been opted into PLOT, not on whether the payment increments exist.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, Fixed that on the next commit. Thanks for noticing it

@leighdouglas
Copy link
Contributor

leighdouglas commented Jan 29, 2025

QAed this and the PLOT widget does not appear when the pledge does not meet the PLOT minimum. See my comment above to fix this issue. Thanks for catching this!

@jlplks jlplks requested a review from leighdouglas January 29, 2025 18:11
@codecov-commenter
Copy link

codecov-commenter commented Jan 29, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 68.35%. Comparing base (d6261ed) to head (6ee2709).

Files with missing lines Patch % Lines
...rc/main/java/com/kickstarter/models/PaymentPlan.kt 0.00% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #2215   +/-   ##
=========================================
  Coverage     68.35%   68.35%           
+ Complexity     2200     2199    -1     
=========================================
  Files           352      352           
  Lines         23734    23726    -8     
  Branches       3483     3482    -1     
=========================================
- Hits          16223    16219    -4     
+ Misses         5676     5672    -4     
  Partials       1835     1835           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@leighdouglas leighdouglas left a comment

Choose a reason for hiding this comment

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

lgtm!

@jlplks jlplks merged commit b6ab5da into master Jan 30, 2025
3 checks passed
@jlplks jlplks deleted the jpulido/MBL-2052 branch January 30, 2025 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants