feat: add jest test cases for new logic's utils, functions and processors - dashboardVariables #6621
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> |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to f098518 in 36 seconds
More details
- Looked at
174lines of code in3files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:35
- Draft comment:
Remove console.log statements to clean up the code. This applies to lines 35, 50, 83, 130, and 148. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_JBF3hVpIiP1cYa44
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -32,6 +32,7 @@ export const convertVariablesToDbFormat = ( | |||
| }, {}); | |||
|
|
|||
| const getDependentVariables = (queryValue: string): string[] => { | |||
| console.log('getDependentVariables', queryValue); | |||
There was a problem hiding this comment.
Remove console.log statements used for debugging to maintain clean code. This applies to other console.log statements in this file as well.
|
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> |
There was a problem hiding this comment.
❌ Changes requested. Incremental review on c991ee6 in 35 seconds
More details
- Looked at
155lines of code in3files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:168
- Draft comment:
Remove console.log statements used for debugging to maintain clean code. - Reason this comment was not posted:
Marked as duplicate.
2. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:127
- Draft comment:
Remove console.log statements used for debugging to maintain clean code. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_G848MtWm7F87VKNq
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| @@ -164,6 +168,12 @@ export const checkAPIInvocation = ( | |||
| variableData: IDashboardVariable, | |||
| parentDependencyGraph?: VariableGraph, | |||
| ): boolean => { | |||
| console.log( | |||
There was a problem hiding this comment.
Avoid using console.log for debugging in production code. Consider using a proper logging mechanism or remove it if not needed. This applies to other instances in this file as well.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
1 similar comment
|
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.
👍 Looks good to me! Incremental review on 23d808a in 10 minutes and 32 seconds
More details
- Looked at
256lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. frontend/src/container/NewDashboard/DashboardVariablesSelection/__test__/dashboardVariables.test.tsx:34
- Draft comment:
The test case for 'node not in topological order' is redundant as it uses the same node 'unknown' as 'node not in graph'. Consider removing or differentiating it. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_7OrMQI2IT8hbrPgX
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
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> |
1 similar comment
|
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.
👍 Looks good to me! Incremental review on ae98aaa in 35 seconds
More details
- Looked at
355lines of code in3files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:79
- Draft comment:
Remove console.log statements from production code to avoid performance issues and console clutter. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. frontend/src/container/NewDashboard/DashboardVariablesSelection/util.ts:79
- Draft comment:
Remove theconsole.logstatement to clean up the code.
- **Reason this comment was not posted:**
Marked as duplicate.
</details>
Workflow ID: <workflowid>`wflow_NOiVdaMISMS8ruOq`</workflowid>
</details>
----
You can customize Ellipsis with :+1: / :-1: [feedback](https://docs.ellipsis.dev/review), review rules, user-specific overrides, `quiet` mode, and [more](https://docs.ellipsis.dev/config).
SagarRajput-7
changed the base branch from
limiting-api-via-keys
to
variable-update-queue
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
…sors - dashboardVariables (#6621) * 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 * feat: add jest test cases for new logic's utils, functions and processors - dashboardVariables * feat: added test for checkAPIInvocation * feat: refactor code * feat: added more test on graph utilities
Summary
Related Issues / PR's
Screenshots
Tests:
Affected Areas and Manually Tested Areas
Important
Add Jest test cases for
onUpdateVariableNodeandcheckAPIInvocationindashboardVariables, with mock data and debugging logs.dashboardVariables.test.tsxto testonUpdateVariableNodewith scenarios: root, middle, leaf elements, and nodes not in graph or order.dashboardVariables.test.tsx.checkAPIInvocationcovering edge cases and variable sequences.mock.tsfor testing.util.tsforgetDependentVariables,buildDependencies,buildDependencyGraph,onUpdateVariableNode, andbuildParentDependencyGraph.This description was created by
for 23d808a. It will automatically update as commits are pushed.