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-1720] Handle estimated shipping on the manage pledge screen #2154

Merged
merged 5 commits into from
Sep 11, 2024

Conversation

ifosli
Copy link
Contributor

@ifosli ifosli commented Sep 10, 2024

📲 What

Show estimated shipping on the manage pledge screen and hide "shipping" if it's $0.

🛠 How

In order to work with our existing helpers, this fetches all the shipping rules for a reward using an extra api call. I also update tests, including deleting a couple tests that are checking specific view parameters (tested in snapshots anyways) and moving some snapshots to use orthogonal combos instead.

👀 See

Jira

Before 🐛 After 🦋
image image

✅ Acceptance criteria

  • Estimated shipping shows on manage pledge screen if flag is on and the project has estimated shipping
  • If shipping cost is zero, shipping doesn't show on pledge screen. (I didn't flag guard this change because I think it makes sense outside of the pledge redemption flow, too - if it ever happens.)

⏰ TODO

  • Check if I can move helper function to take in locationId instead of entire shipping rule. If so, I'll put up a separate pr that does that and simplifies everything
  • We should test accessibility before launch, though I don't think that needs to block this pr.

@ifosli ifosli self-assigned this Sep 10, 2024
@ifosli ifosli marked this pull request as ready for review September 10, 2024 21:22
@ifosli ifosli requested a review from scottkicks September 10, 2024 21:22
@ifosli ifosli changed the title Handle estimated shipping on the manage pledge screen [MBL-1720] Handle estimated shipping on the manage pledge screen Sep 10, 2024
Copy link
Contributor

@scottkicks scottkicks left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ifosli ifosli merged commit 579dc69 into main Sep 11, 2024
5 checks passed
@ifosli ifosli deleted the managePledgeEstimatedShipping branch September 11, 2024 15:54
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.

2 participants