Copilot commented on code in PR #13104:
URL: https://github.com/apache/cloudstack/pull/13104#discussion_r3186726132
##########
ui/src/views/AutogenView.vue:
##########
@@ -1642,6 +1666,12 @@ export default {
},
handleSubmit (e) {
if (this.actionLoading) return
+
+ if (this.currentAction?.requireNameConfirmation &&
!(this.currentAction.groupAction && this.selectedRowKeys.length > 0)) {
+ if (this.actionConfirmText.trim() !== this.resource?.name?.trim()) {
+ return
+ }
+ }
Review Comment:
The name-confirmation guard is explicitly skipped for group actions
(`currentAction.groupAction && selectedRowKeys.length > 0`), so bulk deletes
still proceed without any extra confirmation. This seems to conflict with the
PR description of enforcing name confirmation for deletes. Consider either
disabling bulk delete when `requireNameConfirmation` is enabled, or introducing
a bulk-friendly confirmation (e.g., typing a fixed keyword like "DELETE" or the
number of selected items).
##########
ui/src/views/AutogenView.vue:
##########
@@ -270,8 +270,18 @@
</a-table>
</div>
<br v-if="currentAction.paramFields.length > 0" />
- </span>
- <a-form
+ </span>
+ <div v-if="currentAction.requireNameConfirmation &&
!(currentAction.groupAction && selectedRowKeys.length > 0)"
style="margin-bottom: 5px">
+ <a-form-item>
+ <a-input v-model:value="actionConfirmText"
:placeholder="resource.name" />
+ </a-form-item>
Review Comment:
`requireNameConfirmation` renders a standalone `<a-form-item>` outside of
any `<a-form>`. In ant-design-vue this FormItem relies on injected form
context; outside a form it can lose layout/validation behavior and may emit
warnings. Consider either moving this confirmation input inside the existing
`<a-form>` (as a simple `<a-form-item>` without validation rules) or replace it
with plain layout components (e.g., `<div>` + `<a-input>`).
##########
ui/src/config/section/project.js:
##########
@@ -162,6 +162,7 @@ export default {
},
groupAction: true,
popup: true,
+ requireNameConfirmation: true,
groupMap: (selection, values) => { return selection.map(x => { return {
id: x, cleanup: values.cleanup || null } }) },
Review Comment:
`requireNameConfirmation: true` is enabled for `deleteProject` while
`groupAction: true` remains. In bulk-delete mode the UI currently bypasses the
name-confirmation flow, so this config doesn't fully enforce the intended
safety check. Consider either disabling `groupAction` for deletes that require
name confirmation, or pairing it with a bulk confirmation mechanism.
##########
ui/src/views/AutogenView.vue:
##########
@@ -195,7 +195,7 @@
:footer="null"
style="top: 20px;"
:width="modalWidth"
- :ok-button-props="getOkProps()"
+ :ok-button-props="okButtonProps"
:cancel-button-props="getCancelProps()"
Review Comment:
`okButtonProps` is passed into `<a-modal>` but this modal sets
`:footer="null"` and uses custom footer buttons, so `ok-button-props` will not
affect the actual submit button state. Consider removing `okButtonProps`
entirely (and rely only on `:disabled="isSubmitDisabled"`), or switching back
to the built-in modal footer and wiring `@ok` so the disabled state is enforced
in one place.
##########
ui/src/views/AutogenView.vue:
##########
@@ -1642,6 +1666,12 @@ export default {
},
handleSubmit (e) {
if (this.actionLoading) return
+
+ if (this.currentAction?.requireNameConfirmation &&
!(this.currentAction.groupAction && this.selectedRowKeys.length > 0)) {
+ if (this.actionConfirmText.trim() !== this.resource?.name?.trim()) {
+ return
+ }
+ }
Review Comment:
There are existing unit tests for `handleSubmit`, but none cover the new
`requireNameConfirmation` path (blocking submit when the text doesn't match,
allowing when it does, and ensuring bulk actions behave as intended). Add tests
to `ui/tests/unit/views/AutogenView.spec.js` to prevent regressions in this
deletion-safety feature.
--
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]
