codeant-ai-for-open-source[bot] commented on code in PR #41550:
URL: https://github.com/apache/superset/pull/41550#discussion_r3690816614
##########
superset/commands/deletion_retention/audit.py:
##########
@@ -219,9 +228,8 @@ def reconcile_pending(stale_before: datetime | None = None)
-> dict[str, int]:
)
for record in records:
if _entity_exists(session, record) is False:
- record.status = STATUS_CONFIRMED
- record.confirmed_on = _utc_now()
- confirmed += 1
+ record.status = STATUS_RECONCILED_ABSENT
Review Comment:
**Suggestion:** The audit model stores `status` in a `String(16)` column,
but `STATUS_RECONCILED_ABSENT` is 17 characters long. On databases that enforce
varchar length, reconciling a missing entity raises during commit and rolls
back the reconciliation, leaving the stale pending row unresolved. Increase the
column size or shorten the status value and update the migration accordingly.
[type error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Stale missing-entity audit rows cannot be reconciled.
- ⚠️ Retention runs repeatedly encounter the same pending rows.
- ⚠️ Compliance audit status remains incomplete after committed purges.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a85d9ab68f4b49f1ae2cbbb07197f186&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a85d9ab68f4b49f1ae2cbbb07197f186&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/deletion_retention/audit.py
**Line:** 231:231
**Comment:**
*Type Error: The audit model stores `status` in a `String(16)` column,
but `STATUS_RECONCILED_ABSENT` is 17 characters long. On databases that enforce
varchar length, reconciling a missing entity raises during commit and rolls
back the reconciliation, leaving the stale pending row unresolved. Increase the
column size or shorten the status value and update the migration accordingly.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=6f9858f2f7fa6d394ffef56de02cf8468b53249f3eebb96f3f4e3bb4b9764029&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=6f9858f2f7fa6d394ffef56de02cf8468b53249f3eebb96f3f4e3bb4b9764029&reaction=dislike'>👎</a>
##########
superset/views/base.py:
##########
@@ -510,6 +544,8 @@ def cached_common_bootstrap_data( # pylint:
disable=unused-argument
# should not expose API TOKEN to frontend
frontend_config = {k: _get_frontend_config_value(k) for k in
FRONTEND_CONF_KEYS}
+ frontend_config.update(_soft_delete_conf())
Review Comment:
**Suggestion:** The retention value is resolved inside
`cached_common_bootstrap_data`, which is memoized for 60 seconds. Consequently,
changing the retention window or enabling `SOFT_DELETE` at runtime can leave
users with a stale or missing `SOFT_DELETE_RETENTION_DAYS` value until the
cache expires, causing the UI to advertise an incorrect recovery period.
Invalidate this cache when the shared retention setting or feature flag
changes, or resolve this value outside the memoized payload. [cache]
<details>
<summary><b>Severity Level:</b> Minor 🧹</summary>
```mdx
- ⚠️ Delete confirmations can show an outdated recovery window.
- ⚠️ Runtime SOFT_DELETE changes may be reflected late in bootstrap data.
- ⚠️ Users may receive incorrect retention guidance temporarily.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=222d95bdc8ea4bfab32d9752044c6f88&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=222d95bdc8ea4bfab32d9752044c6f88&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/views/base.py
**Line:** 547:547
**Comment:**
*Cache: The retention value is resolved inside
`cached_common_bootstrap_data`, which is memoized for 60 seconds. Consequently,
changing the retention window or enabling `SOFT_DELETE` at runtime can leave
users with a stale or missing `SOFT_DELETE_RETENTION_DAYS` value until the
cache expires, causing the UI to advertise an incorrect recovery period.
Invalidate this cache when the shared retention setting or feature flag
changes, or resolve this value outside the memoized payload.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=4cc17778e6b79ee195cca4288a5876afd17e386d48d0dbf17932b7b8adb7e5c3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=4cc17778e6b79ee195cca4288a5876afd17e386d48d0dbf17932b7b8adb7e5c3&reaction=dislike'>👎</a>
##########
superset-frontend/playwright/tests/recently-archived/recently-archived.spec.ts:
##########
@@ -0,0 +1,216 @@
+/**
+ * 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 docker dev stack does). 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 {
+ 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) =>
+ (await (await apiPostDashboard(page, { dashboard_title: name })).json())
+ .id,
Review Comment:
**Suggestion:** The dashboard creation API can return the identifier under
`result.id`, as the existing Playwright helpers accommodate, but this test
reads only the top-level `id`. On the normal response shape `id` is undefined,
so the test proceeds with an invalid dashboard ID and fails during deletion or
status verification. Read `result?.id` with the supported fallback and assert
the creation response succeeded before continuing. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dashboard archive restoration test cannot create valid test state.
- ❌ Delete and status requests use an undefined dashboard ID.
- ⚠️ The dashboard case fails before exercising Recently Archived UI
behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=6086948182bd40abb8cf5d7693cc94b6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=6086948182bd40abb8cf5d7693cc94b6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/playwright/tests/recently-archived/recently-archived.spec.ts
**Line:** 69:71
**Comment:**
*Api Mismatch: The dashboard creation API can return the identifier
under `result.id`, as the existing Playwright helpers accommodate, but this
test reads only the top-level `id`. On the normal response shape `id` is
undefined, so the test proceeds with an invalid dashboard ID and fails during
deletion or status verification. Read `result?.id` with the supported fallback
and assert the creation response succeeded before continuing.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=507b488514e3e3118f4d24a038e4c82bbfca200bd0ea12015e6f3b3cf9d817e1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=507b488514e3e3118f4d24a038e4c82bbfca200bd0ea12015e6f3b3cf9d817e1&reaction=dislike'>👎</a>
##########
superset-frontend/playwright/tests/recently-archived/recently-archived.spec.ts:
##########
@@ -0,0 +1,216 @@
+/**
+ * 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 docker dev stack does). 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 {
+ 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) =>
+ (await (await apiPostDashboard(page, { dashboard_title: name })).json())
+ .id,
+ 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 (await res.json()).id;
Review Comment:
**Suggestion:** The chart creation API may return the created identifier as
`result.id`, but this code reads only the top-level `id`. Consequently the
chart archive test receives `undefined` as its ID and cannot reliably
soft-delete, restore, or verify the chart. Use the same response-shape handling
as the existing chart test helpers and validate the response before returning
the ID. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Chart archive restoration test cannot create valid test state.
- ❌ Soft-delete and status checks target an invalid chart ID.
- ⚠️ The chart Recently Archived path is not exercised reliably.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=17873d43cc78487da1dba241ddb2cfb9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=17873d43cc78487da1dba241ddb2cfb9&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/playwright/tests/recently-archived/recently-archived.spec.ts
**Line:** 79:86
**Comment:**
*Api Mismatch: The chart creation API may return the created identifier
as `result.id`, but this code reads only the top-level `id`. Consequently the
chart archive test receives `undefined` as its ID and cannot reliably
soft-delete, restore, or verify the chart. Use the same response-shape handling
as the existing chart test helpers and validate the response before returning
the ID.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=9f9b0f0b1378194619122ff6fe2fa2bbefb7b4e3782e89c63c7d2c343689279b&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41550&comment_hash=9f9b0f0b1378194619122ff6fe2fa2bbefb7b4e3782e89c63c7d2c343689279b&reaction=dislike'>👎</a>
--
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]
