(superset) 03/03: fix(asset-approval): make the seed produce the state it describes

[am...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

amaannawab923 pushed a commit to branch asset-demo
in repository https://gitbox.apache.org/repos/asf/superset.git

commit 5b8532c2845e017d335959048847bfa358ba23ab
Author: amaannawab923 <am...@gmail.com>
AuthorDate: Thu Oct 22 16:55:17 2026 +0530

    fix(asset-approval): make the seed produce the state it describes
    
    Two helpers had grown apart: one added editors, the other set them. The
    adding one could not produce a known state -- an editor left from an
    earlier run, or from someone else's session, stays and quietly hands the
    author an approver they should not have. That is how a one-editor asset
    stops being able to show "nobody can approve this yet", and how a run on
    a used stack stops matching the walkthrough that describes it. On this
    stack USA Births Names had four editors, two of them from a previous
    cast; it has the two the seed names now.
    
    One helper, which sets the list. Also fixes the import order ruff was
    reporting on the file.
---
 local_extensions/asset-approval/qa/seed.py | 46 ++++++++++++------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/local_extensions/asset-approval/qa/seed.py b/local_extensions/asset-approval/qa/seed.py
index 0d3a6c88238..d6732c97670 100644
--- a/local_extensions/asset-approval/qa/seed.py
+++ b/local_extensions/asset-approval/qa/seed.py
@@ -130,40 +130,31 @@ def ensure_user(username: str, role_name: str = ROLE):
     return user
 
 
-def grant_editorship(model, user_ids: list[int]) -> None:
-    """Make these users editors of an asset, which is what lets them review it.
+def set_editors(model, user_ids: list[int]) -> None:
+    """Make these users the editors of an asset -- exactly these, no others.
 
     Editorship is a subject on the asset, not a role: a reviewer has to be an
     editor of *this* object. Admin's blanket access deliberately does not
     count -- see `eligibility.is_strict_editor`.
-    """
-    from superset import db
-    from superset.subjects.utils import get_or_create_user_subject
-
-    existing = {s.user_id for s in model.editors}
-    for user_id in user_ids:
-        if user_id in existing:
-            continue
-        if subject := get_or_create_user_subject(user_id):
-            model.editors.append(subject)
-    db.session.commit()
 
-
-def set_sole_editor(model, user_id: int) -> None:
-    """Exactly one editor, whoever else was there before.
-
-    Adding an editor is not enough: anyone else already holding this asset
-    gives the author an approver, and then the state this is here to produce
-    -- an author with nobody to send to -- cannot be reached. The seed claims
-    this asset has one editor, so it has to make that true rather than hope.
+    It sets the list rather than adding to it, because the seed's job is to
+    produce a known state and appending cannot. An editor left over from an
+    earlier run, or from someone else's session, silently hands the author an
+    approver they should not have -- which is how a one-editor asset stops
+    being able to show "nobody can approve this yet", and how a run on a
+    used stack stops matching the walkthrough that describes it.
     """
     from superset import db
     from superset.subjects.utils import get_or_create_user_subject
 
-    subject = get_or_create_user_subject(user_id)
-    if subject is None:
+    subjects = [
+        subject
+        for subject in (get_or_create_user_subject(user_id) for user_id in user_ids)
+        if subject is not None
+    ]
+    if not subjects:
         return
-    model.editors = [subject]
+    model.editors = subjects
     db.session.commit()
 
 
@@ -226,7 +217,7 @@ def main() -> None:
             db.session.query(SqlaTable).order_by(SqlaTable.id).first(),
         ):
             if model is not None:
-                grant_editorship(model, both)
+                set_editors(model, both)
 
         # One asset with a single editor, so an author can reach the state
         # where there is no one to send the change to and the dialog has to
@@ -237,7 +228,7 @@ def main() -> None:
             .first()
         )
         if solo is not None:
-            set_sole_editor(solo, maker.id)
+            set_editors(solo, [maker.id])
 
         # Start from nothing pending, and from the title the examples ship.
         from asset_approval.models import ChangeRequest
@@ -263,9 +254,10 @@ def main() -> None:
         )
 
     with app.app_context():
-        from superset import db
         from asset_approval.models import ChangeRequest
 
+        from superset import db
+
         change = (
             db.session.query(ChangeRequest).order_by(ChangeRequest.id.desc()).first()
         )