mikebridge commented on code in PR #40128:
URL: https://github.com/apache/superset/pull/40128#discussion_r3508853205
##########
superset/daos/dashboard.py:
##########
@@ -245,13 +245,26 @@ def get_dashboard_and_datasets_changed_on( # pylint:
disable=invalid-name
@staticmethod
def validate_slug_uniqueness(slug: str) -> bool:
+ # The ``SoftDeleteMixin`` listener auto-appends ``deleted_at IS NULL``
+ # to this query, so soft-deleted rows are correctly ignored on
+ # PostgreSQL and MySQL 8.0+ where slug uniqueness is enforced by a
+ # partial index (``ix_dashboards_active_slug``). On SQLite, MariaDB,
+ # and MySQL <8.0 the database retains a full unique constraint on
+ # ``slug``; a soft-deleted row will still block insertion of a new
+ # row with the same slug at flush time, even though this check
+ # passes. The fallback constraint is documented in UPDATING.md and
+ # in the 9e1f3b8c4d2a migration.
if not slug:
return True
dashboard_query = db.session.query(Dashboard).filter(Dashboard.slug ==
slug)
return not db.session.query(dashboard_query.exists()).scalar()
@staticmethod
def validate_update_slug_uniqueness(dashboard_id: int, slug: str | None)
-> bool:
+ # See ``validate_slug_uniqueness`` for the same dialect-gap note:
+ # the partial-index dialects (Postgres / MySQL 8.0+) match this
+ # application-level check; the full-constraint dialects can still
+ # surface an IntegrityError when the colliding row is soft-deleted.
Review Comment:
Fixed — corrected both the model comment and the migration docstring to the
precise rule (**MySQL 8.0.13+, excluding MariaDB**; SQLite / MariaDB / MySQL
<8.0.13 keep the full unique constraint), matching
`_mysql_supports_functional_index`. Pushed in `b9d9a60`.
##########
superset/models/dashboard.py:
##########
@@ -141,7 +145,13 @@ class Dashboard(CoreDashboard, AuditMixinNullable,
ImportExportMixin):
certified_by = Column(Text)
certification_details = Column(Text)
json_metadata = Column(utils.MediumText())
- slug = Column(String(255), unique=True)
+ # Slug uniqueness is enforced via a partial unique index
+ # (``ix_dashboards_active_slug WHERE deleted_at IS NULL``) on
+ # PostgreSQL and MySQL 8.0+, so soft-deleted rows do not reserve
+ # their slug. SQLite and MySQL <8.0 keep the original full unique
+ # constraint via the migration; on those dialects slug reservation
+ # persists across soft-delete. See the 9e1f3b8c4d2a migration for details.
Review Comment:
Fixed — corrected both the model comment and the migration docstring to the
precise rule (**MySQL 8.0.13+, excluding MariaDB**; SQLite / MariaDB / MySQL
<8.0.13 keep the full unique constraint), matching
`_mysql_supports_functional_index`. Pushed in `b9d9a60`.
##########
superset/translations/fa/LC_MESSAGES/messages.po:
##########
@@ -4383,6 +4383,9 @@ msgstr "داشبورد قابل بهروزرسانی نیست."
msgid "Dashboard could not be deleted."
msgstr "داشبورد نمیتواند حذف شود."
+msgid "Dashboard could not be restored."
+msgstr ""
Review Comment:
An empty `msgstr` is expected for newly-extracted strings — the `.po`
catalogs carry the new message ids with empty translations until translators
fill them in; an untranslated string falls back to the source text and is not a
defect. Declining.
--
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]
