-
Notifications
You must be signed in to change notification settings - Fork 1.2k
UI fix api in project view #11191
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
UI fix api in project view #11191
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #11191 +/- ##
============================================
+ Coverage 16.56% 16.58% +0.01%
- Complexity 14010 14038 +28
============================================
Files 5758 5758
Lines 511578 511733 +155
Branches 62192 62216 +24
============================================
+ Hits 84756 84869 +113
- Misses 417350 417390 +40
- Partials 9472 9474 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@vishesh92 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
DaanHoogland
left a comment
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.
code looks good and reviewed the deploy instance from image dropdown. To be clear @vishesh92 this is a regression on 4.21-HEAD only right? none of the functionality existed in 4.20?
|
@blueorangutan package |
|
@vishesh92 a [SL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress. |
|
Packaging result [SF]: ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 14168 |
|
@vishesh92 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
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.
Pull Request Overview
This PR corrects API usage in several project-related views and centralizes request parameter handling.
- Replaces generic
apicalls withgetAPIfor GET requests andpostAPIfor POST requests in multiple Vue components. - Introduces
handleGetRequestParamsandhandlePostRequestParamsin the request interceptor to automatically manage project andignoreprojectparameters. - Simplifies
postAPIto only excludeundefinedvalues when appending parameters.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/views/compute/KubernetesRemoveNodes.vue | Swapped api import/usage for postAPI in the remove-nodes API |
| ui/src/views/compute/KubernetesAddNodes.vue | Updated imports and replaced api calls with getAPI/postAPI |
| ui/src/utils/request.js | Added handleGetRequestParams and handlePostRequestParams |
| ui/src/components/view/ImageDeployInstanceButton.vue | Replaced api calls with getAPI |
| ui/src/api/index.js | Adjusted postAPI to filter out only undefined values |
Comments suppressed due to low confidence (1)
ui/src/utils/request.js:153
- Consider adding unit tests for
handleGetRequestParamsandhandlePostRequestParamsto verify that project IDs andignoreprojectflags are correctly added or removed under different scenarios.
handleGetRequestParams(config)
| removeNodesFromKubernetesCluster (params) { | ||
| return new Promise((resolve, reject) => { | ||
| api('removeNodesFromKubernetesCluster', params).then(json => { | ||
| postAPI('removeNodesFromKubernetesCluster', params).then(json => { |
Copilot
AI
Jul 15, 2025
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.
Avoid wrapping postAPI in a new Promise. Instead, return the promise directly and chain .then() to extract the job ID, which simplifies the code and ensures errors propagate correctly.
| addNodesToKubernetesCluster (params) { | ||
| return new Promise((resolve, reject) => { | ||
| api('addNodesToKubernetesCluster', params).then(json => { | ||
| postAPI('addNodesToKubernetesCluster', params).then(json => { |
Copilot
AI
Jul 15, 2025
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.
You can simplify this method by returning postAPI(...).then(json => json.addnodestokubernetesclusterresponse.jobid) directly, removing the explicit new Promise wrapper and reducing boilerplate.
| if (command === 'listTags') { | ||
| config.data.append('projectid', '-1') | ||
| } else if (command !== 'assignVirtualMachine') { |
Copilot
AI
Jul 15, 2025
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.
[nitpick] Hardcoding command names like 'listTags' and 'assignVirtualMachine' can be brittle. Consider centralizing these exceptions into a configuration constant or lookup table to improve maintainability.
| if (command === 'listTags') { | |
| config.data.append('projectid', '-1') | |
| } else if (command !== 'assignVirtualMachine') { | |
| if (command === COMMANDS.LIST_TAGS) { | |
| config.data.append('projectid', '-1') | |
| } else if (command !== COMMANDS.ASSIGN_VM) { |
|
@vishesh92 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
Co-authored-by: Suresh Kumar Anaparti <[email protected]>
e3abc5d to
3908815
Compare
|
@vishesh92 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
|
@vishesh92 a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
|
UI build: ✔️ |
Yes. This is in main branch only as of now and didn't exist in 4.20. Some of the bugs @rosi-shapeblue shared with me were:
|
|
verified the ui build, and some calls from ui in simulator env. |
* Add project id for post requests as well in the params * Replace leftover api calls to getAPI calls * ui: don't remove values from request if the value is null or empty string * Address comments * Apply suggestions from code review * Apply suggestions from code review Co-authored-by: Suresh Kumar Anaparti <[email protected]> * fixup * Return null if guiTheme requests fails --------- Co-authored-by: Suresh Kumar Anaparti <[email protected]>
Description
This PR fixes the project views for different resources. This bug was introduced in #10899
Generated Summary
This pull request introduces several changes aimed at improving API request handling and refactoring the codebase for better readability and maintainability. The most significant updates include adjustments to how parameters are handled in API requests, the introduction of helper functions for GET and POST request parameters, and the replacement of the
apifunction withgetAPIacross multiple components.Improvements to API request handling:
ui/src/api/index.js: Simplified the parameter-checking logic in thepostAPIfunction by removing checks fornulland empty strings, ensuring onlyundefinedvalues are excluded.ui/src/utils/request.js: Added helper functionshandleGetRequestParamsandhandlePostRequestParamsto streamline the processing of GET and POST request parameters. These functions ensure proper handling of project-related parameters and clean up unnecessary fields likeignoreproject. [1] [2]Codebase refactoring:
apifunction withgetAPIacross multiple components, includingImageDeployInstanceButton.vue,KubernetesAddNodes.vue, andKubernetesRemoveNodes.vue, to improve consistency and readability:ui/src/components/view/ImageDeployInstanceButton.vue: Updated API calls for fetching image zones and zone details. [1] [2] [3]ui/src/views/compute/KubernetesAddNodes.vue: Updated API calls for listing virtual machines and adding nodes to Kubernetes clusters. [1] [2] [3]ui/src/views/compute/KubernetesRemoveNodes.vue: Updated API calls for removing nodes from Kubernetes clusters. [1] [2]Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?