mikebridge commented on code in PR #41550:
URL: https://github.com/apache/superset/pull/41550#discussion_r3692117596
##########
superset-frontend/packages/superset-ui-core/src/components/DeleteModal/index.tsx:
##########
@@ -74,32 +81,34 @@ export function DeleteModal({
return (
<Modal
- disablePrimaryButton={disableChange}
+ disablePrimaryButton={showConfirmationInput ? disableChange : false}
Review Comment:
Real, and pre-existing in the published component — fixed in `1816052493`:
`hide()` and `confirm()` now re-arm `disableChange` alongside clearing the
text, so cancel-and-reopen faces a disabled button again. Worth taking here
because this PR re-enabled the type-to-confirm gate for permanent purge, which
is exactly the flow the gap undermined. Regression test fails against the
pre-fix component (typed DELETE → cancel → button was still enabled).
##########
superset/models/helpers.py:
##########
@@ -692,6 +692,31 @@ def _user(user: User) -> str:
return escape(user)
+def format_time_humanized(timestamp: datetime) -> str:
+ """Humanize *timestamp* against the server's naive-local clock.
+
+ Module-level rather than a mixin method so values projected outside an
+ entity (the archive list reads ``deleted_at`` in a bare column query) can
+ be humanized identically. The subtraction uses ``datetime.now()`` because
+ the audit columns and ``deleted_at`` are stamped with it; humanizing here,
+ on the clock that did the stamping, is what spares every client from
+ guessing the server's timezone.
+ """
+ locale = str(get_locale())
+ time_diff = datetime.now() - timestamp
+ # Skip activation for 'en' locale as it's humanize's default locale
+ if locale == "en":
+ return humanize.naturaltime(time_diff)
+ try:
+ humanize.i18n.activate(locale)
+ result = humanize.naturaltime(time_diff)
+ humanize.i18n.deactivate()
+ return result
Review Comment:
Half right — the fix for the real half is in `1816052493`. `humanize.i18n`'s
activation is **thread-local** (`_CURRENT = threading.local()` in humanize's
source), so concurrent requests on different workers/threads cannot
cross-contaminate; the stronger claim does not hold. What was real: without a
`finally`, a `naturaltime` failure after `activate` left the locale active for
whatever request that same thread served next. `deactivate` now runs in a
`finally`. (The pattern itself is inherited verbatim from the long-standing
`changed_on_delta_humanized` implementation on master; the hoist did not change
its semantics.)
##########
superset-frontend/playwright/tests/recently-archived/recently-archived.spec.ts:
##########
@@ -0,0 +1,223 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * End-to-end coverage for the Archive (Recently-Archived) view.
+ *
+ * Requires the running instance to have the SOFT_DELETE feature flag enabled.
+ * The flag is off by default everywhere — including the docker dev stack — so
+ * these specs skip unless the instance opts in; in CI that is the dedicated
+ * "Soft-delete Tests" step in superset-e2e.yml, which boots its server with
+ * SUPERSET_FEATURE_SOFT_DELETE=true. Each test creates a disposable object via
+ * the authenticated REST API, soft-deletes it, then drives the real UI to
+ * restore it and asserts — via the API — that it is live again.
+ */
+import { test, expect, Page } from '@playwright/test';
+import { apiGet, apiPost } from '../../helpers/api/requests';
+import { extractIdFromResponse } from '../../helpers/api/assertions';
+import {
+ apiPostChart,
+ apiGetChart,
+ apiDeleteChart,
+} from '../../helpers/api/chart';
+import {
+ apiPostDashboard,
+ apiGetDashboard,
+ apiDeleteDashboard,
+} from '../../helpers/api/dashboard';
+import {
+ createTestVirtualDataset,
+ apiGetDataset,
+ apiDeleteDataset,
+} from '../../helpers/api/dataset';
+import { skipUnlessFeatureEnabled } from '../../helpers/featureFlags';
+
+test.beforeEach(async ({ page }) => {
+ await skipUnlessFeatureEnabled(page, 'SOFT_DELETE');
+});
+
+interface TypeConfig {
+ key: string;
+ label: string;
+ create: (page: Page, name: string) => Promise<number>;
+ softDelete: (page: Page, id: number) => Promise<{ ok: () => boolean }>;
+ status: (page: Page, id: number) => Promise<number>;
+}
+
+async function anyDatasetId(page: Page): Promise<number> {
+ const res = await apiGet(page, 'api/v1/dataset/?q=(page_size:1)');
+ const body = await res.json();
+ return body.result[0].id;
+}
+
+const TYPES: TypeConfig[] = [
+ {
+ key: 'dashboard',
+ label: 'Dashboard',
+ create: async (page, name) =>
+ extractIdFromResponse(
+ await apiPostDashboard(page, { dashboard_title: name }),
+ ),
+ softDelete: (page, id) => apiDeleteDashboard(page, id),
+ status: async (page, id) => (await apiGetDashboard(page, id)).status(),
+ },
+ {
+ key: 'chart',
+ label: 'Chart',
+ create: async (page, name) => {
+ const datasourceId = await anyDatasetId(page);
+ const res = await apiPostChart(page, {
+ slice_name: name,
+ datasource_id: datasourceId,
+ datasource_type: 'table',
+ viz_type: 'table',
+ });
+ return extractIdFromResponse(res);
+ },
+ softDelete: (page, id) => apiDeleteChart(page, id),
+ status: async (page, id) => (await apiGetChart(page, id)).status(),
+ },
+ {
+ key: 'dataset',
+ label: 'Dataset',
+ create: async (page, name) => {
+ const id = await createTestVirtualDataset(page, name);
+ if (!id) throw new Error('failed to create virtual dataset');
+ return id;
+ },
+ softDelete: (page, id) => apiDeleteDataset(page, id),
+ status: async (page, id) => (await apiGetDataset(page, id)).status(),
+ },
+];
+
+async function openArchive(page: Page, typeLabel: string, name: string) {
+ await page.goto('archived/');
+ await expect(page.getByTestId('archived-list-view')).toBeVisible();
+ // Select the object type, then narrow to the unique name. The antd Select's
+ // value chip overlays the combobox input, so force the click to open it,
then
+ // pick the option from the portal listbox.
+ await page.getByRole('combobox', { name: 'Type' }).click({ force: true });
+ await page.getByRole('option', { name: typeLabel, exact: true }).click();
+ const search = page.getByPlaceholder(/type a value/i);
+ await search.click();
+ await search.fill(name);
+ await search.press('Enter');
+}
+
+for (const cfg of TYPES) {
+ test(`restores a soft-deleted ${cfg.key} from the archive`, async ({
+ page,
+ }) => {
+ const name = `e2e_archive_${cfg.key}_${Date.now()}`;
+ const id = await cfg.create(page, name);
+ expect(id, 'created id').toBeTruthy();
+
+ const del = await cfg.softDelete(page, id);
+ expect(del.ok(), 'soft-delete should succeed').toBeTruthy();
+
+ await openArchive(page, cfg.label, name);
+
+ // The archived row is listed; restore it (scope to the named row so any
+ // unrelated archived residue on the instance can't make the action
ambiguous).
+ const row = page.getByRole('row').filter({ hasText: name });
+ await expect(row).toBeVisible();
+ await row.getByTestId('archived-row-restore').click();
+
+ // Success toast, and the object is live again per the API.
+ await expect(
+ page.getByText(`${name} restored successfully`, { exact: false }),
+ ).toBeVisible({ timeout: 15000 });
+ await expect.poll(() => cfg.status(page, id)).toBe(200);
+
+ // Cleanup: re-archive so it leaves the normal lists.
+ await cfg.softDelete(page, id);
+ });
Review Comment:
Fixed in `1816052493` — cleanup for the restore round-trip tests and the
stale-restore test now runs in `finally` with a swallowed best-effort
re-archive, so a mid-test failure cannot leave a live object polluting the
normal lists. (In CI the instance is per-job and discarded, so this mattered
mainly for local runs against a persistent stack.)
##########
superset-frontend/playwright/tests/recently-archived/recently-archived.spec.ts:
##########
@@ -0,0 +1,223 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/**
+ * End-to-end coverage for the Archive (Recently-Archived) view.
+ *
+ * Requires the running instance to have the SOFT_DELETE feature flag enabled.
+ * The flag is off by default everywhere — including the docker dev stack — so
+ * these specs skip unless the instance opts in; in CI that is the dedicated
+ * "Soft-delete Tests" step in superset-e2e.yml, which boots its server with
+ * SUPERSET_FEATURE_SOFT_DELETE=true. Each test creates a disposable object via
+ * the authenticated REST API, soft-deletes it, then drives the real UI to
+ * restore it and asserts — via the API — that it is live again.
+ */
+import { test, expect, Page } from '@playwright/test';
+import { apiGet, apiPost } from '../../helpers/api/requests';
+import { extractIdFromResponse } from '../../helpers/api/assertions';
+import {
+ apiPostChart,
+ apiGetChart,
+ apiDeleteChart,
+} from '../../helpers/api/chart';
+import {
+ apiPostDashboard,
+ apiGetDashboard,
+ apiDeleteDashboard,
+} from '../../helpers/api/dashboard';
+import {
+ createTestVirtualDataset,
+ apiGetDataset,
+ apiDeleteDataset,
+} from '../../helpers/api/dataset';
+import { skipUnlessFeatureEnabled } from '../../helpers/featureFlags';
+
+test.beforeEach(async ({ page }) => {
+ await skipUnlessFeatureEnabled(page, 'SOFT_DELETE');
+});
+
+interface TypeConfig {
+ key: string;
+ label: string;
+ create: (page: Page, name: string) => Promise<number>;
+ softDelete: (page: Page, id: number) => Promise<{ ok: () => boolean }>;
+ status: (page: Page, id: number) => Promise<number>;
+}
+
+async function anyDatasetId(page: Page): Promise<number> {
+ const res = await apiGet(page, 'api/v1/dataset/?q=(page_size:1)');
+ const body = await res.json();
+ return body.result[0].id;
+}
+
+const TYPES: TypeConfig[] = [
+ {
+ key: 'dashboard',
+ label: 'Dashboard',
+ create: async (page, name) =>
+ extractIdFromResponse(
+ await apiPostDashboard(page, { dashboard_title: name }),
+ ),
+ softDelete: (page, id) => apiDeleteDashboard(page, id),
+ status: async (page, id) => (await apiGetDashboard(page, id)).status(),
+ },
+ {
+ key: 'chart',
+ label: 'Chart',
+ create: async (page, name) => {
+ const datasourceId = await anyDatasetId(page);
+ const res = await apiPostChart(page, {
+ slice_name: name,
+ datasource_id: datasourceId,
+ datasource_type: 'table',
+ viz_type: 'table',
+ });
+ return extractIdFromResponse(res);
+ },
+ softDelete: (page, id) => apiDeleteChart(page, id),
+ status: async (page, id) => (await apiGetChart(page, id)).status(),
+ },
+ {
+ key: 'dataset',
+ label: 'Dataset',
+ create: async (page, name) => {
+ const id = await createTestVirtualDataset(page, name);
+ if (!id) throw new Error('failed to create virtual dataset');
+ return id;
+ },
+ softDelete: (page, id) => apiDeleteDataset(page, id),
+ status: async (page, id) => (await apiGetDataset(page, id)).status(),
+ },
+];
+
+async function openArchive(page: Page, typeLabel: string, name: string) {
+ await page.goto('archived/');
+ await expect(page.getByTestId('archived-list-view')).toBeVisible();
+ // Select the object type, then narrow to the unique name. The antd Select's
+ // value chip overlays the combobox input, so force the click to open it,
then
+ // pick the option from the portal listbox.
+ await page.getByRole('combobox', { name: 'Type' }).click({ force: true });
+ await page.getByRole('option', { name: typeLabel, exact: true }).click();
+ const search = page.getByPlaceholder(/type a value/i);
+ await search.click();
+ await search.fill(name);
+ await search.press('Enter');
+}
+
+for (const cfg of TYPES) {
+ test(`restores a soft-deleted ${cfg.key} from the archive`, async ({
+ page,
+ }) => {
+ const name = `e2e_archive_${cfg.key}_${Date.now()}`;
+ const id = await cfg.create(page, name);
+ expect(id, 'created id').toBeTruthy();
+
+ const del = await cfg.softDelete(page, id);
+ expect(del.ok(), 'soft-delete should succeed').toBeTruthy();
+
+ await openArchive(page, cfg.label, name);
+
+ // The archived row is listed; restore it (scope to the named row so any
+ // unrelated archived residue on the instance can't make the action
ambiguous).
+ const row = page.getByRole('row').filter({ hasText: name });
+ await expect(row).toBeVisible();
+ await row.getByTestId('archived-row-restore').click();
+
+ // Success toast, and the object is live again per the API.
+ await expect(
+ page.getByText(`${name} restored successfully`, { exact: false }),
+ ).toBeVisible({ timeout: 15000 });
+ await expect.poll(() => cfg.status(page, id)).toBe(200);
+
+ // Cleanup: re-archive so it leaves the normal lists.
+ await cfg.softDelete(page, id);
Review Comment:
Same as the sibling thread on the loop tests — fixed in `1816052493` with
`try/finally` + best-effort re-archive in both places.
##########
superset/commands/purge.py:
##########
@@ -0,0 +1,177 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Owner/admin-gated permanent delete (force-purge) of a soft-deleted entity.
+
+This is the RBAC-enforced REST surface anticipated by the deletion-retention
+force-purge contract: it verifies ownership (owners/admins, mirroring restore)
+on the soft-deleted row, then delegates the irreversible cascade removal to
+``ForcePurgeCommand`` (which handles dependents, M:N rows, version history,
+audit, and the commit). Restricted to *soft-deleted* rows so it only operates
on
+items the user already sees in the archive.
+"""
+
+import logging
+from dataclasses import dataclass
+from typing import Any
+
+from sqlalchemy.exc import SQLAlchemyError
+
+from superset import is_feature_enabled, security_manager
+from superset.commands.base import BaseCommand
+from superset.commands.deletion_retention.force_purge import ForcePurgeCommand
+from superset.daos.base import BaseDAO
+from superset.daos.exceptions import DAODeleteFailedError
+from superset.exceptions import SupersetSecurityException
+from superset.models.helpers import SoftDeleteMixin
+from superset.tasks.utils import get_current_user
+
+#: Recorded when the audit trail cannot name the acting user. The purge routes
+#: are ``@protect()``-ed, so this should be unreachable; it exists so an
+#: anomaly is visible as one rather than disguised as a plausible username.
+logger = logging.getLogger(__name__)
+
+UNKNOWN_ACTOR = "unknown"
+
+
+@dataclass(frozen=True)
+class SoftDeleteBinding:
+ """Entity-specific bindings for the soft-delete purge command.
+
+ Lets one command serve every soft-delete type without a subclass per
+ entity. The REST route supplies the binding for its entity (see each
+ ``*RestApi``).
+ """
+
+ dao: type[BaseDAO[Any]]
+ not_found: type[Exception]
+ forbidden: type[Exception]
+ delete_failed: type[Exception]
+
+
+class PurgeArchivedCommand(BaseCommand):
+ """Permanently delete a single soft-deleted entity, by UUID."""
+
+ def __init__(self, model_uuid: str, binding: SoftDeleteBinding) -> None:
+ self._model_uuid = model_uuid
+ self._binding = binding
+ #: The authorized entity, resolved by ``validate()``. ``BaseCommand``
+ #: fixes ``validate()``'s return type as ``None``, so the model is
+ #: handed to ``run()`` here rather than returned.
+ self._model: SoftDeleteMixin | None = None
+
+ def run(self) -> None:
+ self.validate()
+ model = self._model
+ if model is None: # pragma: no cover — validate() raises or sets it
+ raise self._binding.not_found(f"No row with
uuid={self._model_uuid!r}")
+ try:
+ # ForcePurgeCommand owns the cascade + commit + audit.
+ #
+ # model_cls pins resolution to the type this route authorized:
+ # UUIDs are unique per table but not across them, so an
+ # unconstrained search could purge a different entity than the one
+ # validate() checked the caller against.
+ #
+ # require_archived re-asserts soft-deleted state at resolution
+ # time, closing the window between authorization and purge in which
+ # a concurrent restore would otherwise expose a live row.
+ result = ForcePurgeCommand(
+ self._model_uuid,
+ actor=get_current_user() or UNKNOWN_ACTOR,
+ model_cls=type(model),
+ require_archived=True,
+ # An end user's irreversible purge must never run unaudited;
+ # the CLI's fail-open default is an operator-trust decision
+ # that does not extend to REST principals.
+ require_audit=True,
Review Comment:
Declining as within accepted command semantics. The re-resolution cannot
swap the authorization subject: `_identity_predicates` pins the second lookup
to the same `(id, uuid)` the editorship check ran against, so the row either is
the identical entity or the purge no-ops — the id-reuse race this pattern used
to have was closed on the locked claim in `5c99638174`. What remains is
"editorship revoked mid-request", which is the same validate-then-execute
window every command in the codebase has (UpdateChartCommand,
DeleteDashboardCommand, …); re-checking authorization inside the transaction
would be a codebase-wide policy change, not a purge-specific fix.
##########
superset-frontend/src/pages/ArchivedList/index.tsx:
##########
@@ -0,0 +1,558 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { useCallback, useMemo, useRef, useState } from 'react';
+import { useHistory } from 'react-router-dom';
+import { useAppSelector } from 'src/views/store';
+import { getClientErrorObject, SupersetClient } from '@superset-ui/core';
+import { t } from '@apache-superset/core/translation';
+import { styled } from '@apache-superset/core/theme';
+import {
+ ActionButton,
+ ConfirmStatusChange,
+ Select,
+ Tooltip,
+} from '@superset-ui/core/components';
+import { Icons } from '@superset-ui/core/components/Icons';
+import { useListViewResource } from 'src/views/CRUD/hooks';
+import {
+ ListView,
+ ListViewFilterOperator as FilterOperator,
+ type ListViewProps,
+ type ListViewFilters,
+} from 'src/components';
+import SubMenu from 'src/features/home/SubMenu';
+import withToasts from 'src/components/MessageToasts/withToasts';
+import { recoveredToast } from 'src/utils/softDeleteCopy';
+import { findPermission } from 'src/utils/findPermission';
+import { UserWithPermissionsAndRoles } from 'src/types/bootstrapTypes';
+import {
+ ARCHIVED_TYPES,
+ ARCHIVED_TYPE_CONFIG,
+ type ArchivedItem,
+ type ArchivedType,
+} from './types';
+
+const PAGE_SIZE = 25;
+
+const TypeSelectRow = styled.div`
+ ${({ theme }) => `
+ padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
+ width: 240px;
+ `}
+`;
+
+const StyledActions = styled.div`
+ ${({ theme }) => `
+ color: ${theme.colorIcon};
+
+ /* TableCollection hides .actions with opacity and reveals them on row
+ hover. Without a focus companion, tabbing lands on fully transparent
+ controls — and on this page recovering and permanently deleting are
+ the only actions there are. Scoped here rather than in the shared
+ component, which has the same gap on every list view. */
+ &:focus-within {
+ opacity: 1;
+ }
+ `}
+`;
+
+const EmptyStateRow = styled.div`
+ ${({ theme }) => `
+ padding: ${theme.sizeUnit * 6}px;
+ color: ${theme.colorTextSecondary};
+ `}
+`;
+
+const TYPE_LABELS: Record<ArchivedType, string> = {
+ chart: t('Chart'),
+ dashboard: t('Dashboard'),
+ dataset: t('Dataset'),
+};
+
+interface ToastProps {
+ addDangerToast: (msg: string) => void;
+ addSuccessToast: (msg: string, options?: { allowHtml?: boolean }) => void;
+}
+
+/** The per-row Recover + Delete-permanently actions. */
+function ArchivedRowActions({
+ item,
+ name,
+ onRestore,
+ onPurge,
+ busy = false,
+}: {
+ item: ArchivedItem;
+ name: string;
+ onRestore: (item: ArchivedItem) => void;
+ onPurge: (item: ArchivedItem) => void;
+ /** A request for this row is in flight; both actions stand down. */
+ busy?: boolean;
+}) {
+ return (
+ <StyledActions className="actions">
+ <ActionButton
+ label={t('Recover')}
+ tooltip={t('Recover this item')}
+ placement="bottom"
+ icon={<Icons.RollbackOutlined iconSize="l" />}
+ dataTest="archived-row-restore"
+ disabled={busy}
+ onClick={() => onRestore(item)}
+ />
+ <ConfirmStatusChange
+ title={t('Delete permanently %(name)s?', { name })}
+ description={t(
+ "If you delete this item, you won't be able to recover it.",
+ )}
+ onConfirm={() => onPurge(item)}
+ >
+ {confirmDelete => (
+ <ActionButton
+ label={t('Delete permanently')}
+ tooltip={t('Delete permanently')}
+ placement="bottom"
+ icon={<Icons.DeleteOutlined iconSize="l" />}
+ dataTest="archived-row-purge"
+ disabled={busy}
+ onClick={confirmDelete}
+ />
+ )}
+ </ConfirmStatusChange>
+ </StyledActions>
+ );
+}
+
+/**
+ * The per-type table body. Mounted with `key={type}` by the parent so the
+ * `useListViewResource` state and derived columns reset cleanly on a type
+ * switch. Sourced from the selected type's existing list endpoint with the
+ * soft-delete `<type>_deleted_state:only` baseline filter.
+ */
+function ArchivedListBody({
+ type,
+ addDangerToast,
+ addSuccessToast,
+}: ToastProps & { type: ArchivedType }) {
+ const config = ARCHIVED_TYPE_CONFIG[type];
+
+ const baseFilters = useMemo(
+ () => [{ id: 'id', operator: config.deletedStateOperator, value: 'only' }],
+ [config.deletedStateOperator],
+ );
+
+ const {
+ state: { loading, resourceCount, resourceCollection },
+ fetchData,
+ refreshData,
+ } = useListViewResource<ArchivedItem>(
+ config.resource,
+ TYPE_LABELS[type],
+ addDangerToast,
+ true,
+ [],
+ baseFilters,
+ );
+
+ // Restore is immediate (no confirm dialog). On success, refetch the full
page
+ // so the server-side count/pagination stays consistent and the row drops
out;
+ // on any error surface a danger toast and leave the row in place. The list
+ // read is already owner-scoped, so every visible row is restorable.
+ // A second activation while a request is in flight races the first: by the
+ // time the retry lands the row is already restored (or purged), so the
+ // server answers 404 and the user is shown a failure after a success. The
+ // ref is the guard rather than the state, because state updates are async
+ // and two quick clicks could both pass a state check; the state mirrors it
+ // so the buttons can render disabled meanwhile.
+ const inFlightRef = useRef<Set<string>>(new Set());
+ const [inFlight, setInFlight] = useState<readonly string[]>([]);
+
+ const beginAction = useCallback((uuid: string): boolean => {
+ if (inFlightRef.current.has(uuid)) {
+ return false;
+ }
+ inFlightRef.current.add(uuid);
+ setInFlight([...inFlightRef.current]);
+ return true;
+ }, []);
+
+ const endAction = useCallback((uuid: string) => {
+ inFlightRef.current.delete(uuid);
+ setInFlight([...inFlightRef.current]);
+ }, []);
+
+ const handleRestore = useCallback(
+ async (item: ArchivedItem) => {
+ const name = String(item[config.nameField] ?? '');
+ if (!beginAction(item.uuid)) {
+ return;
+ }
+ try {
+ await SupersetClient.post({
+ endpoint: `/api/v1/${config.resource}/${item.uuid}/restore`,
+ });
+ const { text, options } = recoveredToast(
+ name,
+ TYPE_LABELS[type],
+ item.url ?? item.explore_url,
+ );
+ addSuccessToast(text, options);
+ // Awaited so the finally's endAction does not re-enable this row's
+ // buttons while the stale, already-restored row is still rendered --
+ // a keyboard user could re-activate it and get a 404 after success.
+ await refreshData();
Review Comment:
Working as intended. `useListViewResource` surfaces its own fetch failures
via `addDangerToast` internally and resolves — so the refresh has its own error
channel, and the success toast refers to the restore/purge mutation, which
*did* succeed by that point. The `await` exists for ordering (the refetch lands
before the busy-state clears), not error propagation; bubbling a refresh
failure into the mutation's error path would mislabel a completed restore as
failed.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
