Copilot commented on code in PR #11991:
URL: https://github.com/apache/cloudstack/pull/11991#discussion_r2497642823
##########
ui/src/views/storage/CreateTemplate.vue:
##########
@@ -73,42 +73,30 @@
<template #label>
<tooltip-label :title="$t('label.domainid')"
:tooltip="apiParams.domainid.description"/>
</template>
- <a-select
+ <infinite-scroll-select
v-model:value="form.domainid"
- showSearch
- optionFilterProp="label"
- :filterOption="(input, option) => {
- return option.label.toLowerCase().indexOf(input.toLowerCase())
>= 0
- }"
- :loading="domainLoading"
+ api="listDomains"
+ :apiParams="domainsApiParams"
+ resourceType="domain"
+ optionValueKey="id"
+ optionLabelKey="path"
+ defaultIcon="block-outlined"
:placeholder="apiParams.domainid.description"
- @change="val => { handleDomainChange(val) }">
- <a-select-option v-for="(opt, optIndex) in this.domains"
:key="optIndex" :label="opt.path || opt.name || opt.description"
:value="opt.id">
- <span>
- <resource-icon v-if="opt && opt.icon"
:image="opt.icon.base64image" size="1x" style="margin-right: 5px"/>
- <block-outlined v-else style="margin-right: 5px" />
- {{ opt.path || opt.name || opt.description }}
- </span>
- </a-select-option>
- </a-select>
+ @change-option-value="handleDomainChange" />
</a-form-item>
<a-form-item name="account" ref="account" v-if="domainid">
<template #label>
<tooltip-label :title="$t('label.account')"
:tooltip="apiParams.account.description"/>
</template>
- <a-select
+ <infinite-scroll-select
v-model:value="form.account"
- showSearch
- optionFilterProp="label"
- :filterOption="(input, option) => {
- return option.value.toLowerCase().indexOf(input.toLowerCase())
>= 0
- }"
- :placeholder="apiParams.account.description"
- @change="val => { handleAccountChange(val) }">
- <a-select-option v-for="(acc, index) in accounts"
:value="acc.name" :key="index">
- {{ acc.name }}
- </a-select-option>
- </a-select>
+ api="listAccounts"
+ :apiParams="accountsApiParams"
+ resourceType="account"
+ optionValueKey="name"
+ optionLabelKey="name"
+ defaultIcon="team-outlined"
+ :placeholder="apiParams.account.description" />
Review Comment:
Missing `@change-option-value` event handler for account changes. In the
original code, `handleAccountChange` was called to update the `account`
variable. Without this handler, the `account` variable will not be updated when
the user selects a different account, potentially breaking functionality that
depends on this value.
##########
ui/src/views/compute/wizard/OwnershipSelection.vue:
##########
@@ -36,218 +36,161 @@
</a-select>
</a-form-item>
<a-form-item :label="$t('label.domain')" required>
- <a-select
- @change="changeDomain"
+ <infinite-scroll-select
v-model:value="selectedDomain"
- showSearch
- optionFilterProp="label"
- :filterOption="
- (input, option) => {
- return option.label.toLowerCase().indexOf(input.toLowerCase()) >= 0
- }
- "
- >
- <a-select-option
- v-for="domain in domains"
- :key="domain.name"
- :value="domain.id"
- :label="domain.path || domain.name || domain.description"
- >
- <span>
- <resource-icon
- v-if="domain && domain.icon"
- :image="domain.icon.base64image"
- size="1x"
- style="margin-right: 5px"
- />
- <block-outlined v-else />
- {{ domain.path || domain.name || domain.description }}
- </span>
- </a-select-option>
- </a-select>
+ api="listDomains"
+ :apiParams="domainsApiParams"
+ resourceType="domain"
+ optionValueKey="id"
+ optionLabelKey="path"
+ defaultIcon="block-outlined"
+ @change-option-value="handleDomainChange" />
</a-form-item>
<template v-if="selectedAccountType === 'Account'">
<a-form-item :label="$t('label.account')" required>
- <a-select
- @change="emitChangeEvent"
+ <infinite-scroll-select
v-model:value="selectedAccount"
- showSearch
- optionFilterProp="label"
- :filterOption="
- (input, option) => {
- return option.label.toLowerCase().indexOf(input.toLowerCase())
>= 0
- }
- "
- >
- <a-select-option v-for="account in accounts" :key="account.name"
:value="account.name">
- <span>
- <resource-icon
- v-if="account && account.icon"
- :image="account.icon.base64image"
- size="1x"
- style="margin-right: 5px"
- />
- <team-outlined v-else />
- {{ account.name }}
- </span>
- </a-select-option>
- </a-select>
+ api="listAccounts"
+ :apiParams="accountsApiParams"
+ resourceType="account"
+ optionValueKey="name"
+ optionLabelKey="name"
+ defaultIcon="team-outlined"
+ @change-option-value="handleAccountChange" />
</a-form-item>
</template>
<template v-else>
<a-form-item :label="$t('label.project')" required>
- <a-select
- @change="emitChangeEvent"
+ <infinite-scroll-select
v-model:value="selectedProject"
- showSearch
- optionFilterProp="label"
- :filterOption="
- (input, option) => {
- return option.label.toLowerCase().indexOf(input.toLowerCase())
>= 0
- }
- "
- >
- <a-select-option v-for="project in projects" :key="project.id"
:value="project.id" :label="project.name">
- <span>
- <resource-icon
- v-if="project && project.icon"
- :image="project.icon.base64image"
- size="1x"
- style="margin-right: 5px"
- />
- <project-outlined v-else />
- {{ project.name }}
- </span>
- </a-select-option>
- </a-select>
+ api="listProjects"
+ :apiParams="projectsApiParams"
+ resourceType="project"
+ optionValueKey="id"
+ optionLabelKey="name"
+ defaultIcon="project-outlined"
+ @change-option-value="handleProjectChange" />
</a-form-item>
</template>
</a-form>
</template>
<script>
-import { api } from '@/api'
-import ResourceIcon from '@/components/view/ResourceIcon.vue'
+import InfiniteScrollSelect from
'@/components/widgets/InfiniteScrollSelect.vue'
export default {
name: 'OwnershipSelection',
- components: { ResourceIcon },
+ components: { InfiniteScrollSelect },
data () {
return {
- domains: [],
- accounts: [],
- projects: [],
selectedAccountType: this.$store.getters.project?.id ? 'Project' :
'Account',
selectedDomain: null,
selectedAccount: null,
selectedProject: null,
- loading: false
+ selectedDomainOption: null,
+ selectedAccountOption: null,
+ selectedProjectOption: null
}
},
props: {
override: {
type: Object
}
},
- created () {
- this.fetchData()
- },
- methods: {
- fetchData () {
- this.loading = true
- api('listDomains', {
- response: 'json',
+ computed: {
+ domainsApiParams () {
+ return {
listAll: true,
showicon: true,
details: 'min'
- })
- .then((response) => {
- this.domains = response.listdomainsresponse.domain
- if (this.override) {
- this.domains = this.domains.filter(item =>
this.override.domains.has(item.id))
- }
- if (this.domains.length === 0) {
- this.selectedDomain = null
- this.selectedProject = null
- this.selectedAccount = null
- return
- }
- const domainIds = this.domains?.map(domain => domain.id)
- const ownerDomainId = this.$store.getters.project?.domainid ||
this.$store.getters.userInfo.domainid
- this.selectedDomain = domainIds?.includes(ownerDomainId) ?
ownerDomainId : this.domains?.[0]?.id
- this.changeDomain()
- })
- .catch((error) => {
- this.$notifyError(error)
- })
- .finally(() => {
- this.loading = false
- })
+ }
},
- fetchAccounts () {
- this.loading = true
- api('listAccounts', {
- response: 'json',
- domainId: this.selectedDomain,
+ accountsApiParams () {
+ if (!this.selectedDomain) {
+ return null
+ }
+ return {
+ domainid: this.selectedDomain,
showicon: true,
state: 'Enabled',
isrecursive: false
- })
- .then((response) => {
- this.accounts = response.listaccountsresponse.account || []
- if (this.override?.accounts && this.accounts) {
- this.accounts = this.accounts.filter(item =>
this.override.accounts.has(item.name))
- }
- const accountNames = this.accounts.map(account => account.name)
- if (this.selectedDomain === this.$store.getters.userInfo.domainid &&
accountNames.includes(this.$store.getters.userInfo.account)) {
- this.selectedAccount = this.$store.getters.userInfo.account
- } else {
- this.selectedAccount = this.accounts?.[0]?.name
- }
- this.selectedProject = null
- this.emitChangeEvent()
- })
- .catch((error) => {
- this.$notifyError(error)
- })
- .finally(() => {
- this.loading = false
- })
+ }
},
- fetchProjects () {
- this.loading = true
- api('listProjects', {
- response: 'json',
+ projectsApiParams () {
+ if (!this.selectedDomain) {
+ return null
+ }
+ return {
domainId: this.selectedDomain,
state: 'Active',
showicon: true,
details: 'min',
isrecursive: false
- })
- .then((response) => {
- this.projects = response.listprojectsresponse.project
- if (this.override?.projects && this.projects) {
- this.projects = this.projects.filter(item =>
this.override.projects.has(item.id))
- }
- this.selectedProject = this.projects?.map(project =>
project.id)?.includes(this.$store.getters.project?.id) ?
this.$store.getters.project?.id : this.projects?.[0]?.id
- this.selectedAccount = null
- this.emitChangeEvent()
- })
- .catch((error) => {
- this.$notifyError(error)
- })
- .finally(() => {
- this.loading = false
- })
+ }
+ }
+ },
+ created () {
+ // Set initial domain selection
+ const ownerDomainId = this.$store.getters.project?.domainid ||
this.$store.getters.userInfo.domainid
+ if (ownerDomainId) {
+ this.selectedDomain = ownerDomainId
+ }
+ },
+ methods: {
+ changeAccountType () {
+ // Reset account/project selection when switching types
+ this.selectedAccount = null
+ this.selectedProject = null
+
+ // Trigger initial selection based on type
+ this.handleDomainChange(this.selectedDomain)
},
- changeDomain () {
- if (this.selectedAccountType === 'Account') {
- this.fetchAccounts()
- } else {
- this.fetchProjects()
+ handleDomainChange (domainId) {
+ this.selectedDomain = domainId
+ // Reset child selections
+ this.selectedAccount = null
+ this.selectedProject = null
+
+ // Pre-select account if it's the user's domain
+ if (this.selectedAccountType === 'Account' &&
+ this.selectedDomain === this.$store.getters.userInfo.domainid) {
+ this.selectedAccount = this.$store.getters.userInfo.account
+ } else if (this.selectedAccountType === 'Project' &&
+ this.$store.getters.project?.id &&
+ this.selectedDomain === this.$store.getters.project?.domainid) {
+ // Pre-select project if applicable
+ this.selectedProject = this.$store.getters.project?.id
}
+
+ this.emitChangeEvent()
+ },
+ handleDomainOptionChange (option) {
+ this.selectedDomainOption = option
+ // Note: Override filtering is not implemented in InfiniteScrollSelect
migration
+ // If override.domains filtering is needed, it should be handled at a
higher level
+ // or by using a custom filtering mechanism
+ },
+ handleAccountChange (accountName) {
+ this.selectedAccount = accountName
+ this.selectedProject = null
+ this.emitChangeEvent()
+ },
+ handleAccountOptionChange (option) {
+ this.selectedAccountOption = option
+ // Note: Override filtering is not implemented in InfiniteScrollSelect
migration
+ // If override.accounts filtering is needed, it should be handled at a
higher level
+ },
+ handleProjectChange (projectId) {
+ this.selectedProject = projectId
+ this.selectedAccount = null
+ this.emitChangeEvent()
+ },
+ handleProjectOptionChange (option) {
+ this.selectedProjectOption = option
+ // Note: Override filtering is not implemented in InfiniteScrollSelect
migration
+ // If override.projects filtering is needed, it should be handled at a
higher level
},
Review Comment:
The handlers `handleDomainOptionChange`, `handleAccountOptionChange`, and
`handleProjectOptionChange` store the option but don't emit or use it anywhere
in the component. These handlers appear to be unused or incomplete
implementations. If the `selectedDomainOption`, `selectedAccountOption`, and
`selectedProjectOption` data properties are not used elsewhere, consider
removing these handlers entirely or documenting their intended purpose.
```suggestion
handleAccountChange (accountName) {
this.selectedAccount = accountName
this.selectedProject = null
this.emitChangeEvent()
},
handleProjectChange (projectId) {
this.selectedProject = projectId
this.selectedAccount = null
this.emitChangeEvent()
},
```
##########
ui/src/views/storage/UploadVolume.vue:
##########
@@ -162,37 +141,69 @@ import { api } from '@/api'
import { mixinForm } from '@/utils/mixin'
import ResourceIcon from '@/components/view/ResourceIcon'
import TooltipLabel from '@/components/widgets/TooltipLabel'
+import InfiniteScrollSelect from
'@/components/widgets/InfiniteScrollSelect.vue'
export default {
name: 'UploadVolume',
mixins: [mixinForm],
components: {
ResourceIcon,
- TooltipLabel
+ TooltipLabel,
+ InfiniteScrollSelect
},
data () {
return {
- zones: [],
- domainList: [],
- accountList: [],
formats: ['RAW', 'VHD', 'VHDX', 'OVA', 'QCOW2'],
- offerings: [],
zoneSelected: '',
selectedDiskOfferingId: null,
domainId: null,
account: null,
uploadParams: null,
- domainLoading: false,
+ customDiskOffering: false,
+ isCustomizedDiskIOps: false,
Review Comment:
New data properties `customDiskOffering` and `isCustomizedDiskIOps` were
added but these variables don't appear to be used elsewhere in the visible
diff. While they are being set in `onChangeDiskOffering`, ensure these
properties are actually consumed by other parts of the component (likely in the
template or other methods not shown in the diff).
##########
ui/src/views/storage/UploadLocalVolume.vue:
##########
@@ -173,37 +153,64 @@ import { axios } from '../../utils/request'
import { mixinForm } from '@/utils/mixin'
import ResourceIcon from '@/components/view/ResourceIcon'
import TooltipLabel from '@/components/widgets/TooltipLabel'
+import InfiniteScrollSelect from
'@/components/widgets/InfiniteScrollSelect.vue'
export default {
name: 'UploadLocalVolume',
mixins: [mixinForm],
components: {
ResourceIcon,
- TooltipLabel
+ TooltipLabel,
+ InfiniteScrollSelect
},
data () {
return {
fileList: [],
- zones: [],
- domainList: [],
- accountList: [],
- offerings: [],
- offeringLoading: false,
formats: ['RAW', 'VHD', 'VHDX', 'OVA', 'QCOW2'],
domainId: null,
account: null,
uploadParams: null,
- domainLoading: false,
+ customDiskOffering: false,
+ isCustomizedDiskIOps: false,
Review Comment:
New data properties `customDiskOffering` and `isCustomizedDiskIOps` were
added but these variables don't appear to be used elsewhere in the visible
diff. While they are being set in `onChangeDiskOffering`, ensure these
properties are actually consumed by other parts of the component (likely in the
template or other methods not shown in the diff).
```suggestion
```
##########
ui/src/views/iam/AddUser.vue:
##########
@@ -241,53 +242,19 @@ export default {
fetchData () {
this.account = this.$route.query && this.$route.query.account ?
this.$route.query.account : null
this.domainid = this.$route.query && this.$route.query.domainid ?
this.$route.query.domainid : null
- if (!this.domianid) {
- this.fetchDomains()
+ // Set initial domain if provided from route
+ if (this.domainid) {
+ this.form.domainid = this.domainid
Review Comment:
The original code had a typo check `if (!this.domianid)` (note the
misspelling) which would always evaluate to true since `domianid` is undefined.
The fix correctly uses `this.domainid`, but now the logic is inverted - it only
sets the form value if domainid exists. However, the original code called
`fetchDomains()` when domainid was falsy (due to the typo), so this changes the
behavior. The new code doesn't fetch any domains initially, which might be
intentional with InfiniteScrollSelect, but should be verified.
```suggestion
// Set initial domain if provided from route, otherwise set to null
if (this.domainid) {
this.form.domainid = this.domainid
} else {
this.form.domainid = null
```
##########
ui/src/views/storage/UploadVolume.vue:
##########
@@ -83,23 +77,15 @@
<template #label>
<tooltip-label :title="$t('label.diskofferingid')"
:tooltip="apiParams.diskofferingid.description || $t('label.diskoffering')"/>
</template>
- <a-select
+ <infinite-scroll-select
v-model:value="form.diskofferingid"
- :loading="loading"
- @change="id => onChangeDiskOffering(id)"
- showSearch
- optionFilterProp="label"
- :filterOption="(input, option) => {
- return option.label.toLowerCase().indexOf(input.toLowerCase())
>= 0
- }" >
- <a-select-option
- v-for="(offering, index) in offerings"
- :value="offering.id"
- :key="index"
- :label="offering.displaytext || offering.name">
- {{ offering.displaytext || offering.name }}
- </a-select-option>
- </a-select>
+ api="listDiskOfferings"
+ :apiParams="diskOfferingsApiParams"
+ resourceType="diskoffering"
+ optionValueKey="id"
+ optionLabelKey="displaytext"
+ defaultIcon="hdd-outlined"
+ @change-option="onChangeDiskOffering" />
Review Comment:
The event handler changed from `@change` with an ID parameter to
`@change-option` with an offering object parameter. This is correct, but note
that `onChangeDiskOffering` now receives the full offering object instead of
just the ID. Verify that the handler correctly extracts properties from the
object rather than treating it as an ID.
--
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]
