Skip to content

ui: Query validation for icicle chart visualization#5633

Merged
manojVivek merged 7 commits into
mainfrom
icicle-chart-validations
May 6, 2025
Merged

ui: Query validation for icicle chart visualization#5633
manojVivek merged 7 commits into
mainfrom
icicle-chart-validations

Conversation

@manojVivek
Copy link
Copy Markdown
Contributor

This PR adds some validations on the query before querying for flamecharts and renders appropriate error message in the UI.

Screenshots:
Valid Query:
Screenshot 2025-05-06 at 3 26 19 PM

Invalid Queries:

Screenshot 2025-05-06 at 3 23 18 PM Screenshot 2025-05-06 at 3 24 42 PM

@manojVivek manojVivek requested a review from a team as a code owner May 6, 2025 09:58
@alwaysmeticulous
Copy link
Copy Markdown

alwaysmeticulous Bot commented May 6, 2025

✅ Meticulous spotted visual differences in 2 of 304 screens tested, but all differences have already been approved: view differences detected.

Meticulous evaluated ~4 hours of user flows against your PR.

Last updated for commit 877e301. This comment will update as new commits are pushed.


export const validateIcicleChartQuery = (profileSource: MergedProfileSource) => {
const isNonDelta = profileSource.ProfileType().delta !== true;
const isDurationTooLong = profileSource.mergeTo - profileSource.mergeFrom > 10000;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Let's make it a high limit. I've successfully looked at 10+ seconds.
How about a minute for now?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, sounds good. Will update it.

@manojVivek manojVivek merged commit c9ed293 into main May 6, 2025
37 of 38 checks passed
@manojVivek manojVivek deleted the icicle-chart-validations branch May 6, 2025 11:42
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.

2 participants