-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
feat: added API limiting to reduce unnecessary api call for dashboard variables #6609
Conversation
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
8adaf46
to
f6988e1
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
f6988e1
to
eb75e63
Compare
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this 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! Incremental review on 421d355 in 35 seconds
More details
- Looked at
174
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
4
drafted comments based on config settings.
1. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:4
- Draft comment:
Consider enhancingareArraysEqual
to handle nested arrays or objects, if applicable, to ensure accurate comparisons. - Reason this comment was not posted:
Confidence changes required:50%
The functionareArraysEqual
is used multiple times across the codebase to compare arrays. However, it does not account for cases where the arrays might contain objects or nested arrays. This could lead to incorrect comparisons if such cases arise.
2. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:171
- Draft comment:
The check forisEmpty(parentDependencyGraph)
is redundant as it is already performed at the start of the function. Consider removing it to simplify the code. - Reason this comment was not posted:
Confidence changes required:50%
ThecheckAPIInvocation
function has a redundant check forparentDependencyGraph
being empty. This check is already performed at the beginning of the function, making the second check unnecessary.
3. frontend/src/container/NewDashboard/DashboardVariablesSelection/VariableItem.tsx:224
- Draft comment:
Ensure thatcheckAPIInvocation
is correctly determining when API calls should be made, as it is crucial for performance optimization. - Reason this comment was not posted:
Confidence changes required:50%
ThecheckAPIInvocation
function is used in multiple places, and its logic is crucial for determining when API calls should be made. Ensuring its correctness and efficiency is important for performance.
4. frontend/src/container/NewDashboard/DashboardVariablesSelection/VariableItem.tsx:218
- Draft comment:
Use design tokens or predefined color constants instead of hardcoding color values. This is also applicable in other parts of the code where colors are hardcoded. - Reason this comment was not posted:
Comment was not on a valid diff hunk.
Workflow ID: wflow_FSbDXRI0JFo5mjpl
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet
mode, and more.
… variables (#6609) * feat: added API limiting to reduce unneccesary api call for dashboard variables * feat: fixed dropdown open triggering the api calls for single-select and misc
Summary
Related Issues / PR's
Screenshots
NA
Affected Areas and Manually Tested Areas
Important
Adds dependency management and API call optimization for dashboard variables in
DashboardVariableSelection
andVariableItem
.buildDependencies
,buildDependencyGraph
,buildParentDependencyGraph
, andonUpdateVariableNode
inutil.ts
to manage variable dependencies.DashboardVariableSelection.tsx
initializes and updatesdependencyData
using these functions.onValueUpdate
inDashboardVariableSelection.tsx
to handle variable dependencies when updating values.VariableItem.tsx
usesvalidVariableUpdate
to determine if a variable update is valid based on dependencies.checkAPIInvocation
inutil.ts
to optimize when API calls are made based on variable dependencies.VariableItem.tsx
uses this function to conditionally enable API queries.This description was created by for 421d355. It will automatically update as commits are pushed.