Skip to content

[Batch Delete] Use the batch delete-info endpoint in the grid delete confirmation - #3961

Merged
xIrusux merged 13 commits into
2026.2from
3666-use-batch-delete-info-endpoint
Aug 6, 2026
Merged

[Batch Delete] Use the batch delete-info endpoint in the grid delete confirmation#3961
xIrusux merged 13 commits into
2026.2from
3666-use-batch-delete-info-endpoint

Conversation

@xIrusux

@xIrusux xIrusux commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Changes in this pull request

Resolves #3666

The grid batch-delete confirm dialog fired one delete-info request per selected row (N parallel requests) to decide between "Delete" and "Delete permanently", and only opened once all of them resolved — large selections hit browser connection limits and froze the UI without any loading indicator.

It now uses the aggregated endpoint from pimcore/studio-backend-bundle#1954 (released in 2026.2.1, already covered by the composer constraint ^2026.2.1):

  • A single POST /elements/{elementType}/batch-delete-info request runs first; the confirm dialog then opens once, fully formed — the same fetch-first shape as the folder delete flow, no post-open update() patching.
  • canUseRecycleBin decides the OK label as before.
  • The aggregated hasDependencies now drives the dependency warning: a concrete "These items have child objects or other dependencies" line shows only when dependencies actually exist, and no line when there are none. If the pre-check request fails, the API error is raised via the central error handler and the dialog closes (no under-warning "Delete" state); the now-unused generic element.delete.batch.dependencies-warning key was removed from all languages.
  • The batch-info call is a read-only pre-check, so the enhanced slice drops its Elements cache invalidation (same pattern as elementLock/elementUnlock); the request handle is reset() afterwards, which also fixes the previously never-unsubscribed per-id cache entries.
  • OpenAPI snapshot: only the new batch-delete-info path was spliced in (taken from a freshly generated spec); SDK regenerated via build-api-client.
  • New translation key element.delete.batch.dependencies-warning.confirmed, translated for en/de/fr/it/es/sv/no.

Additional info

  • The wording of the new dependency warning line could use a quick look from @idaiv — the previously agreed static text ("There may be dependencies, delete anyway?") intentionally remains as the error fallback, where the uncertainty is real.
  • Verified locally against a demo instance: the endpoint responds and the rebuilt frontend is served; eslint, the element-module jest suites, and a full build-app are green.

🤖 Generated with Claude Code

…confirmation

The grid batch-delete confirm dialog fired one delete-info request per
selected row (N parallel requests) to decide between "Delete" and
"Delete permanently", and only opened once all of them resolved - large
selections hit browser connection limits and froze the UI without any
loading indicator.

The dialog now opens immediately with the OK button in loading state and
fetches the aggregated info with a single
POST /elements/{elementType}/batch-delete-info request
(pimcore/studio-backend-bundle#1954, released in 2026.2.1):

- canUseRecycleBin decides the OK label as before
- the aggregated hasDependencies drives the dependency warning: the
  concrete "have child objects or other dependencies" line only shows
  when dependencies actually exist; if the request fails, the previous
  generic warning and the "Delete" label are kept as a safe fallback
- the batch-info call is a read-only pre-check, so the enhanced slice
  drops its "Elements" cache invalidation to avoid spurious refetches
- OpenAPI snapshot: only the new batch-delete-info path was spliced in
  (taken from a freshly generated spec); SDK regenerated via
  build-api-client

Refs #3666

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings August 4, 2026 13:47

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates batch-delete confirmation to use the backend’s aggregated delete-info endpoint, avoiding per-row requests.

Changes:

  • Opens the dialog immediately while one batch pre-check runs.
  • Updates permanence and dependency warnings from the response.
  • Adds generated API support and localized warning text.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
translations/studio.sv.yaml Adds Swedish warning text.
translations/studio.no.yaml Adds Norwegian warning text.
translations/studio.it.yaml Adds Italian warning text.
translations/studio.fr.yaml Adds French warning text.
translations/studio.es.yaml Adds Spanish warning text.
translations/studio.en.yaml Adds English warning text.
translations/studio.de.yaml Adds German warning text.
assets/js/src/core/modules/element/element-api-slice.gen.ts Generates the batch delete-info endpoint.
assets/js/src/core/modules/element/element-api-slice-enhanced.ts Prevents read-only pre-check cache invalidation.
assets/js/src/core/modules/element/actions/delete/use-batch-delete-confirm.tsx Implements asynchronous modal loading and aggregated warnings.

Comment on lines +57 to +58
} catch {
return { isPermanent: false, hasDependencies: null }
@xIrusux
xIrusux marked this pull request as draft August 4, 2026 13:54
@xIrusux
xIrusux force-pushed the 3666-use-batch-delete-info-endpoint branch from 65685ea to df28622 Compare August 5, 2026 07:44
@xIrusux
xIrusux changed the base branch from 2026.2 to 2026.x August 5, 2026 07:45
@xIrusux
xIrusux marked this pull request as ready for review August 5, 2026 08:09
xIrusux added a commit that referenced this pull request Aug 5, 2026
…handler

The batch pre-check swallowed request failures silently (inherited from
the old per-id code). API-shaped failures now go through
trackError(new ApiError()) like the other element hooks; the safe
fallback (Delete label + generic dependency warning) is unchanged.

Addresses review feedback on #3961.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@pimcore-deployments

pimcore-deployments commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

🌐 Translations synced automatically

The key changes below were generated for all target languages, passed the
mechanical validation gate (key parity, order, placeholders, types, plurals),
and are being committed to this branch by the workflow (check the run status
if the commit is not visible).
Please review the generated translations.

Added (1):

  • element.delete.batch.dependencies-warning.confirmed

Removed (1):

  • element.delete.batch.dependencies-warning

🧹 Pre-existing translation errors (backlog partly cleared)

18 validation error(s) predating this PR were present at the merge base. This run also fixed 6 of them; 12 still remain. The remainder is not blocking — later runs keep chipping at it.

xIrusux added a commit that referenced this pull request Aug 5, 2026
antd's hook modal (App.useApp().modal) ignores the function form of
ConfigUpdate - update() merges the argument with Object.assign, so a
function updater copies no properties and the modal silently never
leaves its loading state. Use the plain-object form, which antd
shallow-merges into the current config.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
xIrusux added a commit that referenced this pull request Aug 5, 2026
…k fails

Replace unwrap() with the plain initiate-result pattern used elsewhere
(await the request, check the error member). On a failed pre-check the
dialog no longer settles into a possibly under-warning "Delete" state:
the API error is raised via the central error handler and the dialog
closes. This removes the unknown-state fallback, so the now-unused
generic warning key element.delete.batch.dependencies-warning is
dropped from all languages.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@xIrusux xIrusux added this to the 2026.3.0 milestone Aug 5, 2026
@xIrusux xIrusux self-assigned this Aug 5, 2026
xIrusux added a commit that referenced this pull request Aug 5, 2026
… preflight

The batch pre-check passed a possibly-undefined RTK error into ApiError
(TS2345 in check-types); guard it like use-open-saved-search does.

The single-folder delete flow had the same preflight bug class the batch
flow just fixed, but worse: elementGetDeleteInfo failures were entirely
unhandled, silently falling back to the harmless recoverable-delete
wording even when deletion would be permanent, and the query
subscription was never released. Failures now raise the API error and
abort instead of opening a possibly under-warning dialog; the two
almost-identical modal configs are merged.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
xIrusux added a commit that referenced this pull request Aug 5, 2026
Replace the open-then-update() flow with fetch-first: the batch
delete-info request runs while the triggering dropdown item shows a
loading state (same pattern as locate-in-tree), and the confirm dialog
opens once, fully formed. This drops the placeholder content state, the
destroy-on-error modal flash and the antd ConfigUpdate workaround, and
matches the folder-delete flow.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
xIrusux added a commit that referenced this pull request Aug 5, 2026
…item

MenuItemType.isLoading is declared but never rendered by any menu/dropdown
component (the existing useDelete and locate-in-tree usages are equally
inert), and the menu closes on click anyway. Remove the no-op plumbing -
the flow is plain fetch-first like the folder delete.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
xIrusux and others added 6 commits August 5, 2026 15:17
…handler

The batch pre-check swallowed request failures silently (inherited from
the old per-id code). API-shaped failures now go through
trackError(new ApiError()) like the other element hooks; the safe
fallback (Delete label + generic dependency warning) is unchanged.

Addresses review feedback on #3961.

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
antd's hook modal (App.useApp().modal) ignores the function form of
ConfigUpdate - update() merges the argument with Object.assign, so a
function updater copies no properties and the modal silently never
leaves its loading state. Use the plain-object form, which antd
shallow-merges into the current config.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…k fails

Replace unwrap() with the plain initiate-result pattern used elsewhere
(await the request, check the error member). On a failed pre-check the
dialog no longer settles into a possibly under-warning "Delete" state:
the API error is raised via the central error handler and the dialog
closes. This removes the unknown-state fallback, so the now-unused
generic warning key element.delete.batch.dependencies-warning is
dropped from all languages.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… preflight

The batch pre-check passed a possibly-undefined RTK error into ApiError
(TS2345 in check-types); guard it like use-open-saved-search does.

The single-folder delete flow had the same preflight bug class the batch
flow just fixed, but worse: elementGetDeleteInfo failures were entirely
unhandled, silently falling back to the harmless recoverable-delete
wording even when deletion would be permanent, and the query
subscription was never released. Failures now raise the API error and
abort instead of opening a possibly under-warning dialog; the two
almost-identical modal configs are merged.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the open-then-update() flow with fetch-first: the batch
delete-info request runs while the triggering dropdown item shows a
loading state (same pattern as locate-in-tree), and the confirm dialog
opens once, fully formed. This drops the placeholder content state, the
destroy-on-error modal flash and the antd ConfigUpdate workaround, and
matches the folder-delete flow.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…item

MenuItemType.isLoading is declared but never rendered by any menu/dropdown
component (the existing useDelete and locate-in-tree usages are equally
inert), and the menu closes on click anyway. Remove the no-op plumbing -
the flow is plain fetch-first like the folder delete.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@xIrusux
xIrusux changed the base branch from 2026.x to 2026.2 August 5, 2026 13:21
@xIrusux
xIrusux force-pushed the 3666-use-batch-delete-info-endpoint branch from 888ebaf to 959249b Compare August 5, 2026 13:21
xIrusux and others added 3 commits August 5, 2026 13:26
"These items have dependencies, delete anyway?" replaces the longer
child-objects wording, in English and all six languages.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Joining the permanence note and the dependencies question produced
clunky text ("These items ... These items ...") and fragment
composition does not translate cleanly. The warning is now one full
sentence per state: permanent + dependencies, permanent only (which
also gains the previously missing "Delete anyway?"), and dependencies
only. The element.delete.batch.note key is replaced by
note.permanent and note.permanent-dependencies in all languages.

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
xIrusux and others added 2 commits August 5, 2026 17:14
- Every warning state starts with "Please note:" and is a plain
  statement; the trailing "Delete anyway?" is gone - the modal's own
  question line already asks for confirmation.
- The permanent-only state reuses the element.delete.batch.note key
  ("... will be deleted permanently and can not be restored.").
- All three warnings are pluralized via i18next count suffixes
  (_one/_other), like the existing question line - single selections
  read "This item ...".
- In the permanent-with-dependencies state the restore sentence sits
  on its own line (\n in the translation, white-space: pre-line on
  the warning style).

Refs #3961

Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@xIrusux xIrusux modified the milestones: 2026.3.0, 2026.2.4 Aug 6, 2026
@xIrusux
xIrusux merged commit 3b567fb into 2026.2 Aug 6, 2026
3 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Batch Delete Modal] Accordion for listing the items

3 participants