Skip to content

[MBL-3164] Update the ppo card id to only be based on the project id#2820

Open
ifosli wants to merge 1 commit intomainfrom
ifosli/ppoId
Open

[MBL-3164] Update the ppo card id to only be based on the project id#2820
ifosli wants to merge 1 commit intomainfrom
ifosli/ppoId

Conversation

@ifosli
Copy link
Copy Markdown
Contributor

@ifosli ifosli commented Mar 31, 2026

📲 What

Update the PPO card identifier to be equal to the project id, instead of also being based on the tier type and actions.

🤔 Why

I did some testing and this doesn't make a difference in most cases. But I think it'll make PPO actions a little better in the edge case where a card tier type changes, but the cards position in the list doesn't change. I wasn't able to find this edge case in any of my test accounts, but it could theoretically make the auto refresh better for some users.

Also, now that we've decided we'll have at most one card per project id, this change makes the identifier more intuitive for any future work in this space.

👀 See

Jira

✅ Acceptance criteria

  • PPO behavior isn't worse (in almost all cases, the list will still behave the same)

⏰ TODO

  • Wait for https://kickstarter.atlassian.net/browse/MBL-3139 to be fixed and merged before merging this pr. (Currently, there could be separate PPO cards for a canceled and successful pledge to the same project, and I want those cards to have separate ids while they can coexist)

@ifosli ifosli self-assigned this Mar 31, 2026
@ifosli ifosli marked this pull request as ready for review March 31, 2026 19:17
@ifosli ifosli requested review from a team and amy-at-kickstarter and removed request for a team March 31, 2026 19:18
Copy link
Copy Markdown
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.

LGTM!

My one thought is we might want to add an assertion somewhere that logs if we ever get duplicate projects in this list. It feels impossible, but better safe than sorry.

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