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

fix: arithmetic operators are removed from Dashboard query builder formulas #6276

Merged
merged 2 commits into from
Oct 26, 2024

Conversation

vikrantgupta25
Copy link
Member

@vikrantgupta25 vikrantgupta25 commented Oct 26, 2024

Summary

  • the redirection from dashboards page to edit widget page was manually setting query params and not utilising the URLSearchParams due to which proper encodings were not in place.
  • the result of this was that decoding was not correct since we have moved to strict standards now.

Related Issues / PR's

fixes #6274

Screenshots

Screen.Recording.2024-10-26.at.1.09.13.PM.mov

Affected Areas and Manually Tested Areas

  • dashboards page to widget edit and back / reload the page

Important

Fixes URL encoding issue in WidgetHeader by using useUrlQuery for query parameter management in onEditHandler.

  • Behavior:
    • Fixes URL encoding issue in onEditHandler in WidgetHeader by using useUrlQuery for query parameter management.
    • Corrects redirection from dashboards to edit widget page, ensuring proper URL encoding.
  • Functions:
    • Updates onEditHandler to use useUrlQuery for setting widgetId, graphType, and compositeQuery parameters.
  • Misc:

This description was created by Ellipsis for 7537040. It will automatically update as commits are pushed.

@github-actions github-actions bot added the bug Something isn't working label Oct 26, 2024
@vikrantgupta25 vikrantgupta25 changed the title Fix dashboards formula expression fix: arithmetic operators are removed from Dashboard query builder formulas Oct 26, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 7537040 in 20 seconds

More details
  • Looked at 38 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. frontend/src/container/GridCardLayout/WidgetHeader/index.tsx:83
  • Draft comment:
    Redundant use of encodeURIComponent with useUrlQuery.set. useUrlQuery should handle encoding internally, so this can be removed to prevent double encoding issues.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable:
    The comment is about a change made in the diff, specifically the use of encodeURIComponent with useUrlQuery.set. The comment suggests a potential code improvement by removing redundant encoding. However, without documentation or evidence that useUrlQuery.set handles encoding, it's speculative.
    I might be missing documentation or implementation details of useUrlQuery.set that confirm it handles encoding. Without this, the comment could be speculative.
    The comment assumes useUrlQuery.set handles encoding, but without evidence, it remains speculative. The comment should be removed unless there's strong evidence supporting it.
    Remove the comment as it is speculative without evidence that useUrlQuery.set handles encoding internally.
2. frontend/src/container/GridCardLayout/WidgetHeader/index.tsx:73
  • Draft comment:
    Avoid using inline styles for the Spinner component. Instead, use external stylesheets, CSS classes, or styled components. This issue is also applicable to other components if inline styles are used.
  • Reason this comment was not posted:
    Comment was on unchanged code.

Workflow ID: wflow_WnA1GJ2I2UTtYp6g


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@vikrantgupta25 vikrantgupta25 merged commit 8137ec5 into develop Oct 26, 2024
14 of 15 checks passed
@vikrantgupta25 vikrantgupta25 deleted the fix-dashboards-formula-expression branch October 26, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arithmetic operators are removed from Dashboard query builder formulas
2 participants