Copilot commented on code in PR #12681:
URL: https://github.com/apache/cloudstack/pull/12681#discussion_r2839015050
##########
ui/src/views/image/TemplateZones.vue:
##########
@@ -139,7 +139,13 @@
:pageSize="pageSize"
:total="itemCount"
:showTotal="total => `${$t('label.total')} ${total}
${$t('label.items')}`"
- :pageSizeOptions="['10', '20', '40', '80', '100']"
+ :pageSizeOptions="[
+ '10',
+ '20',
+ '40',
+ '80',
+ '100'
+ ]"
Review Comment:
This multi-line formatting of the pageSizeOptions array is inconsistent with
the codebase convention. Throughout the codebase (e.g., IsoZones.vue:133,
FirewallRules.vue:136, MigrateWizard.vue:90), pageSizeOptions arrays are
consistently formatted as inline arrays. This should be kept as
`:pageSizeOptions="['10', '20', '40', '80', '100']"` to maintain consistency.
```suggestion
:pageSizeOptions="['10', '20', '40', '80', '100']"
```
##########
ui/src/views/image/TemplateZones.vue:
##########
@@ -391,10 +399,10 @@ export default {
},
computed: {
isActionsOnTemplatePermitted () {
- return (['Admin'].includes(this.$store.getters.userInfo.roletype) || //
If admin or owner or belongs to current project
+ return (['Admin'].includes(this.$store.getters.userInfo.roletype) ||
(this.resource.domainid === this.$store.getters.userInfo.domainid &&
this.resource.account === this.$store.getters.userInfo.account) ||
(this.resource.domainid === this.$store.getters.userInfo.domainid &&
this.resource.projectid && this.$store.getters.project &&
this.$store.getters.project.id && this.resource.projectid ===
this.$store.getters.project.id)) &&
- (this.resource.isready || !this.resource.status ||
this.resource.status.indexOf('Downloaded') === -1) && // Template is ready or
downloaded
+ (this.resource.isready || !this.resource.status ||
this.resource.status.indexOf('Downloaded') === -1) &&
this.resource.templatetype !== 'SYSTEM'
}
},
Review Comment:
This PR description claims to fix a 404 issue after bulk deleting template
zones by redirecting to the Templates list view when all zones are deleted.
However, the diff only contains formatting changes with no actual logic
modifications to address this issue. The actual bug fix for the 404 navigation
issue is missing from this PR. The `handleCancel` method (around line 486)
still uses `this.$router.go(-1)` which goes back in browser history and could
lead to a 404 page. It should be changed to `this.$router.push({ path:
'/template' })` to explicitly redirect to the Templates list.
##########
ui/src/views/image/TemplateZones.vue:
##########
@@ -217,7 +223,9 @@
<a-alert type="error">
<template #message>
<exclamation-circle-outlined style="color: red; fontSize: 30px;
display: inline-flex" />
- <span style="padding-left: 5px"
v-html="`<b>${selectedRowKeys.length} ` + $t('label.items.selected') + `.
</b>`" />
+ <span
+ style="padding-left: 5px"
+ v-html="'<b>' + selectedRowKeys.length + ' ' +
$t('label.items.selected') + '. </b>'" />
Review Comment:
This string concatenation change is inconsistent with the codebase
convention and reduces readability. The codebase consistently uses template
literals for v-html attributes with interpolated values (see
AutogenView.vue:229, 241 and SnapshotZones.vue:202). This should use template
literals like: `v-html="\`<b>${selectedRowKeys.length} \` +
$t('label.items.selected') + \`. </b>\`"`
```suggestion
v-html="`<b>${selectedRowKeys.length} ` +
$t('label.items.selected') + `. </b>`" />
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
