Copilot commented on code in PR #11191:
URL: https://github.com/apache/cloudstack/pull/11191#discussion_r2206836154
##########
ui/src/views/compute/KubernetesRemoveNodes.vue:
##########
@@ -137,7 +137,7 @@ export default {
},
removeNodesFromKubernetesCluster (params) {
return new Promise((resolve, reject) => {
- api('removeNodesFromKubernetesCluster', params).then(json => {
+ postAPI('removeNodesFromKubernetesCluster', params).then(json => {
Review Comment:
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.
##########
ui/src/utils/request.js:
##########
@@ -160,11 +169,30 @@ service.interceptors.request.use(config => {
}
}
if (config.params.ignoreproject !== undefined) {
- config.params.ignoreproject = null
+ delete config.params.ignoreproject
}
}
- return config
-}, err)
+}
+
+function handlePostRequestParams (config) {
+ if (config && config.data && config.data instanceof URLSearchParams) {
+ const project = vueProps.$localStorage.get(CURRENT_PROJECT)
+ const command = config.data.get('command')
+ const hasProjectId = config.data.has('projectid')
+ const ignoreProject = config.data.has('ignoreproject')
+
+ if (!hasProjectId && !ignoreProject && project && project.id) {
+ if (command === 'listTags') {
+ config.data.append('projectid', '-1')
+ } else if (command !== 'assignVirtualMachine') {
Review Comment:
[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.
```suggestion
if (command === COMMANDS.LIST_TAGS) {
config.data.append('projectid', '-1')
} else if (command !== COMMANDS.ASSIGN_VM) {
```
##########
ui/src/views/compute/KubernetesAddNodes.vue:
##########
@@ -172,7 +172,7 @@ export default {
},
addNodesToKubernetesCluster (params) {
return new Promise((resolve, reject) => {
- api('addNodesToKubernetesCluster', params).then(json => {
+ postAPI('addNodesToKubernetesCluster', params).then(json => {
Review Comment:
You can simplify this method by returning `postAPI(...).then(json =>
json.addnodestokubernetesclusterresponse.jobid)` directly, removing the
explicit `new Promise` wrapper and reducing boilerplate.
##########
ui/src/api/index.js:
##########
@@ -47,7 +47,7 @@ export function postAPI (command, data = {}) {
params.append('response', 'json')
if (data) {
Object.entries(data).forEach(([key, value]) => {
- if (value !== undefined && value !== null && value !== '') {
+ if (value !== undefined) {
Review Comment:
The updated filter now appends `null` and empty string values to
`URLSearchParams`, which may inadvertently pass parameters with value `'null'`
or `''`. If those should be excluded, reintroduce checks for `null` or empty
strings.
```suggestion
if (value !== undefined && value !== null && value !== '') {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
