Skip to content

feat(profiling): Add banner to set budget for profile hours #91661

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Zylphrex
Copy link
Member

This shows a banner when the user is trying to use continuous/ui profiling without configuring a budget for profile hours.

This shows a banner when the user is trying to use continuous/ui profiling
without configuring a budget for profile hours.
@Zylphrex Zylphrex requested review from a team as code owners May 14, 2025 20:14
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label May 14, 2025
Copy link
Member

@isabellaenriquez isabellaenriquez left a comment

Choose a reason for hiding this comment

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

i think we should be able to condense the two tier-specific components into one but lmk if there are any other differences that can't be handled without splitting them

Comment on lines +401 to +402
!subscription.categories.profileDuration?.usageExceeded &&
!subscription.categories.profileDurationUI?.usageExceeded
Copy link
Member

Choose a reason for hiding this comment

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

i believe customers without PAYG that aren't using transaction profiles will fall under this condition too since having usage is not exceeded for profile hour categories unless they have PAYG set up and it's all used up

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure i understand what you're saying here.

Comment on lines +434 to +436
{t(
'Continuous Profiling and UI Profiling requires you to set your On-demand budget.'
)}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{t(
'Continuous Profiling and UI Profiling requires you to set your On-demand budget.'
)}
{tct(
'Continuous Profiling and UI Profiling requires you to set your [budgetTerm] budget.',
{budgetTerm: subscription.planDetails.budgetTerm}
)}

if you use this looks like you won't need to split into the different tier banners? not sure if there were any other changes

Copy link
Member Author

Choose a reason for hiding this comment

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

im tempted to say let's leave it as separate components as this feels like something that we're likely to iterate on a few times before getting it right and the differences between AM2 and AM3 plans makes it annoying to handle if we have to litter isAm2Plan and isAm3Plan checks everywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants