codeant-ai-for-open-source[bot] commented on code in PR #40128:
URL: https://github.com/apache/superset/pull/40128#discussion_r3492993057
##########
superset/models/dashboard.py:
##########
@@ -192,6 +202,27 @@ class Dashboard(CoreDashboard, AuditMixinNullable,
ImportExportMixin):
]
extra_import_fields = ["is_managed_externally", "external_url", "theme_id"]
+ @classmethod
+ def _unique_constraints(cls) -> list[set[str]]:
+ """Import identity keys for ``import_from_dict``.
+
+ ``slug`` lost its column-level ``unique=True`` when the full unique
+ constraint was replaced by a partial (active-rows-only) index for
+ soft-delete. ``ImportExportMixin._unique_constraints`` derives import
+ lookup keys from unique columns/constraints, so without this override a
+ re-import whose UUID differs but whose ``slug`` matches an existing
+ active dashboard would no longer be matched-and-updated by slug — it
+ would fall through to an insert and collide on the partial active-slug
+ index at flush. Re-add ``{"slug"}`` here so import keeps matching by
+ slug (the pre-soft-delete behaviour) while DB-level uniqueness stays
+ partial. A ``NULL`` slug is skipped by the importer's filter builder,
+ so this only adds a real lookup when the imported config carries a
slug.
+ """
+ constraints = super()._unique_constraints()
+ if {"slug"} not in constraints:
+ constraints.append({"slug"})
+ return constraints
Review Comment:
**Suggestion:** Adding `{"slug"}` unconditionally to import unique
constraints breaks imports where slug is null or omitted: `import_from_dict`
builds an empty `and_()` for that constraint, which evaluates true and broadens
the lookup to unrelated rows (often causing `MultipleResultsFound`). Only apply
slug matching when the incoming payload has a non-null slug, instead of
globally declaring slug as an always-on uniqueness key. [incorrect condition
logic]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Slug-less dashboard imports can crash with MultipleResultsFound.
- ⚠️ Bulk dashboard migration via YAML bundles becomes unreliable.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Ensure there are at least two active dashboards in the database (multiple
`Dashboard`
rows with `deleted_at IS NULL`), and create or select a dashboard whose
`slug` is `NULL`
so it will export without a slug (export uses
`ImportExportMixin.export_to_dict` in
`superset/models/helpers.py:2-37`, which omits fields where `getattr(self,
c.name) is
None`).
2. Export this slug-less dashboard via the dashboards export API (in
`superset/dashboards/api.py`, which calls the export command using
`Dashboard.export_to_dict`); inspect the generated `dashboards/*.yaml` to
confirm it
contains `uuid` but no `slug` field.
3. Import the exported bundle through the dashboards import endpoint in
`superset/dashboards/api.py:2050-2075`, which instantiates
`ImportDashboardsCommand` and,
in `superset/commands/dashboard/importers/v1/__init__.py:13-25`, calls
`import_dashboard(config, overwrite=overwrite)` for each `dashboards/*.yaml`
config.
4. Inside `import_dashboard` (utils.py:159-181), the code calls
`Dashboard.import_from_dict(config, recursive=False)` at utils.py:181-182;
`ImportExportMixin.import_from_dict` (`superset/models/helpers.py:94-150`)
computes
`unique_constraints = Dashboard._unique_constraints()` where the override at
`superset/models/dashboard.py:205-224` appends `{"slug"}` unconditionally, so
`unique_constraints` becomes `[{"uuid"}, {"slug"}]`; for this config,
`dict_rep` has no
`slug`, so the slug constraint contributes an empty `and_()` (logical
`TRUE`) at
helpers.py:140-147, and `filters` becomes `[or_(uuid == <uuid>, TRUE)]` at
helpers.py:148-150, making the overall query
`db.session.query(Dashboard).filter(and_(or_(...)))` at helpers.py:153-155
match all
active dashboards; with more than one row, `.one_or_none()` raises
`MultipleResultsFound`
(helpers.py:156-164), breaking the import instead of updating or creating
the intended
dashboard.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=5689866640ab4f819afcfcc3c4dfc9a4&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=5689866640ab4f819afcfcc3c4dfc9a4&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/models/dashboard.py
**Line:** 221:224
**Comment:**
*Incorrect Condition Logic: Adding `{"slug"}` unconditionally to import
unique constraints breaks imports where slug is null or omitted:
`import_from_dict` builds an empty `and_()` for that constraint, which
evaluates true and broadens the lookup to unrelated rows (often causing
`MultipleResultsFound`). Only apply slug matching when the incoming payload has
a non-null slug, instead of globally declaring slug as an always-on uniqueness
key.
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%2F40128&comment_hash=bff74826126fefb2095c8e3b05bb6631052a9a5bab53c290f2fee18f116c3e1c&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40128&comment_hash=bff74826126fefb2095c8e3b05bb6631052a9a5bab53c290f2fee18f116c3e1c&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]
