You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by am...@apache.org on 2026/10/22 09:56:28 UTC
(superset) 02/04: docs(asset-approval): set the QA environment up from the repository
This is an automated email from the ASF dual-hosted git repository.
amaannawab923 pushed a commit to branch sandbox/qa-env-7f3a1c
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 7ff17c38627dec8f9ac6e37c97b64be8cd549408
Author: amaannawab923 <am...@gmail.com>
AuthorDate: Wed Oct 21 21:33:12 2026 +0530
docs(asset-approval): set the QA environment up from the repository
Three things QA needed that only existed on someone's laptop:
- the extension and the review flag are turned on in the light stack's
config, so a checkout is enough to get a working environment;
- `qa/seed.py` creates the two accounts, grants each of them editorship of
an example dashboard, chart and dataset, and leaves one change waiting
for a reviewer. It drives the app's own API rather than writing rows, so
what it seeds is a shape the application actually produces, and it is
safe to re-run whenever a session has left the queue somewhere else;
- the README opens with the walkthrough: what to build, what to start, who
to log in as, and the six steps of the flow.
The README says to test as `maker` and `checker` rather than `admin`,
because admin's blanket access deliberately does not count as editorship
for approval -- so admin cannot see what a real reviewer sees, and the
permission bugs that matter only appear on the other two accounts.
Superset's own README is kept below the guide.
---
README.md | 154 +++++++++++++++++++++++
local_extensions/asset-approval/qa/seed.py | 196 +++++++++++++++++++++++++++++
2 files changed, 350 insertions(+)
diff --git a/README.md b/README.md
index baf97be8097..0a25ebb06d0 100644
--- a/README.md
+++ b/README.md
@@ -17,6 +17,160 @@ specific language governing permissions and limitations
under the License.
-->
+# Asset approval — QA build
+
+Apache Superset with **maker-checker review** turned on: an edit to a
+dashboard, chart or dataset does not reach viewers until a second eligible
+editor approves it.
+
+This branch exists for testing that feature and nothing else. It is not
+proposed for merge, and the parts of it that belong in Superset go upstream
+as their own pull requests. Superset's own README follows this guide.
+
+---
+
+## Setting it up
+
+You need Docker with roughly 8 GB available to it. Everything below runs
+from the root of this checkout. The first run takes 15–25 minutes, nearly
+all of it image building and example loading.
+
+### 1. Build the extension
+
+The review feature ships as a Superset extension. Its source is in the
+repository; its build output is not, so you build it once:
+
+```bash
+docker run --rm -v "$PWD:/app" -w /app/local_extensions/asset-approval/frontend node:20 bash -lc "npm install --legacy-peer-deps && npm run build"
+```
+
+`--legacy-peer-deps` is required, not optional: the published
+`@apache-superset/core` still declares a peer dependency on React 17 while
+Superset itself is on React 18, so a plain `npm install` refuses to resolve.
+
+Then assemble the loadable bundle:
+
+```bash
+docker run --rm -v "$PWD:/app" -w /app/local_extensions/asset-approval node:20 bash -lc "bash build.sh"
+```
+
+That writes `local_extensions/asset-approval/dist/`. It worked if it prints
+`built: 19 files` and the manifest lists two contributed pages.
+
+### 2. Start Superset
+
+```bash
+NODE_PORT=9002 docker compose -p asset-approval-qa -f docker-compose-light.yml up -d
+```
+
+The first run builds images and loads the example dashboards, which is the
+slow part. Follow it with:
+
+```bash
+docker compose -p asset-approval-qa logs -f superset-init-light
+```
+
+When that container exits, open **http://localhost:9002** and sign in as
+`admin` / `admin`.
+
+### 3. Seed the review flow
+
+```bash
+docker compose -p asset-approval-qa exec superset-light python /app/local_extensions/asset-approval/qa/seed.py
+```
+
+This creates the two test accounts, makes them both editors of an example
+dashboard, chart and dataset, and leaves exactly one change waiting for a
+reviewer. It prints what it set up, and it is safe to re-run at any time.
+
+---
+
+## Logins
+
+| user | password | role | use it for |
+|---|---|---|---|
+| `maker` | `general` | Alpha | making changes |
+| `checker` | `general` | Alpha | reviewing them |
+| `admin` | `admin` | Admin | setup only |
+
+**Test as `maker` and `checker`, not as `admin`.** Admin's blanket access
+deliberately does not count as editorship for approval, so admin sees a
+more permissive picture than a real user does. Permission and UI bugs only
+show up on the non-admin accounts.
+
+Two accounts exist because an author may never approve their own work: the
+gate looks for a *second* editor of the asset, so one account cannot
+demonstrate the flow at all.
+
+---
+
+## The flow to walk
+
+Seeding leaves one change already waiting, so you can start at step 3 to see
+the reviewer's side straight away, or at step 1 to do a full round yourself.
+
+1. **As `maker`** — open a dashboard, change something, Save. The toast says
+ viewers still see the live version. Nothing has gone live.
+2. **As `maker`** — Request review.
+3. **As `checker`** — the notification bell carries a count, and its popover
+ lists what is waiting. **Settings → Manage → Approvals** shows the same
+ as a list.
+4. **As `checker`** — open the review. It takes you to the dashboard itself:
+ a banner across the top, and a panel on the right with the field-by-field
+ diff, "Show changes", and a link to the live version.
+5. **As `checker`** — Approve, which asks for confirmation first. Or Request
+ changes, to send it back with a note.
+6. **As `maker`** — a returned change appears in the bell as "returned".
+
+Datasets are reviewed from the Approvals list rather than from a page of
+their own, because a dataset has no page to put a banner on.
+
+---
+
+## Known gaps
+
+Understood already — no need to file these:
+
+- **No count on the Settings → Manage entry.** The design asks for the
+ number to appear there as well as on the bell. Superset's menu entries
+ carry no badge, so that needs a new capability rather than a placement
+ change. The bell itself counts correctly.
+- **No page per change request.** Review happens on the asset, or from the
+ list for a dataset. Links that used to point at a change request's own
+ page open the list instead.
+- **Deleting a chart customization under review** clears the filter from
+ your own view even though nothing was applied for anyone else. Superset
+ clears those locally before the request goes out, and this branch does not
+ reorder that.
+
+## Reporting what you find
+
+Say which account you were using, which of the six steps you were on, and
+whether the change was new or one the seed created. Where the UI and the
+data disagree, the API is the tie-breaker:
+
+ /api/v1/change_request/count what the bell should show
+ /api/v1/change_request/?show=for_me what is waiting on you
+
+## Starting over
+
+Back to one change waiting for review, keeping everything else:
+
+```bash
+docker compose -p asset-approval-qa exec superset-light python /app/local_extensions/asset-approval/qa/seed.py
+```
+
+Back to nothing at all, database included:
+
+```bash
+docker compose -p asset-approval-qa -f docker-compose-light.yml down -v
+```
+
+---
+---
+
+*Everything below is Apache Superset's own README, unchanged.*
+
# Superset
[](https://opensource.org/license/apache-2-0)
diff --git a/local_extensions/asset-approval/qa/seed.py b/local_extensions/asset-approval/qa/seed.py
new file mode 100644
index 00000000000..7dd9c39d97c
--- /dev/null
+++ b/local_extensions/asset-approval/qa/seed.py
@@ -0,0 +1,196 @@
+# 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.
+"""Put the review flow into a state someone can walk through.
+
+Run it to set the environment up the first time, and again whenever a run
+has left the queue in a state you no longer want. It is idempotent: it
+creates what is missing, leaves what is already right alone, and finishes
+with exactly one change waiting for a reviewer.
+
+ docker compose -p asset-approval-qa exec superset-light \
+ python /app/local_extensions/asset-approval/qa/seed.py
+
+Two users, because the point of maker-checker is that an author cannot
+approve their own work: the gate looks for a *second* editor of the asset,
+so one account can never demonstrate the flow.
+"""
+
+from __future__ import annotations
+
+import sys
+
+from superset.app import create_app
+
+MAKER = "maker"
+CHECKER = "checker"
+PASSWORD = "general" # noqa: S105 -- a local QA fixture, never a deployment
+ROLE = "Alpha"
+
+# The example dashboard the walkthrough edits. Shipped by `superset load-examples`,
+# so it is the same object in every environment built from the README.
+DASHBOARD_TITLE = "USA Births Names"
+PROPOSED_TITLE = "USA Births Names (Q4)"
+
+app = create_app()
+# The seed drives the app through its own HTTP API rather than writing rows:
+# a change request has to be *made* by a write passing through the gate, and
+# going around that would seed a shape the application never produces.
+app.config["WTF_CSRF_ENABLED"] = False
+
+
+def ensure_user(username: str):
+ """The QA account, created once and reused after that."""
+ from superset.extensions import security_manager
+
+ if user := security_manager.find_user(username=username):
+ return user
+ role = security_manager.find_role(ROLE)
+ user = security_manager.add_user(
+ username=username,
+ first_name=username.capitalize(),
+ last_name="(QA)",
+ email=f"{username}@example.invalid",
+ role=role,
+ password=PASSWORD,
+ )
+ if not user:
+ sys.exit(f"could not create {username}")
+ print(f"created {username} ({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.
+
+ 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 login(username: str):
+ client = app.test_client()
+ response = client.post(
+ "/login/",
+ data={"username": username, "password": PASSWORD},
+ follow_redirects=True,
+ )
+ if response.status_code != 200:
+ sys.exit(f"{username} could not log in ({response.status_code})")
+ return client
+
+
+def main() -> None:
+ with app.app_context():
+ from superset import db
+ from superset.connectors.sqla.models import SqlaTable
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+
+ maker = ensure_user(MAKER)
+ checker = ensure_user(CHECKER)
+ both = [maker.id, checker.id]
+
+ dashboard = (
+ db.session.query(Dashboard)
+ .filter_by(dashboard_title=DASHBOARD_TITLE)
+ .first()
+ )
+ if not dashboard:
+ sys.exit(
+ f'no dashboard called "{DASHBOARD_TITLE}" -- run '
+ "`superset load-examples` first (see the README)"
+ )
+ dashboard_id = dashboard.id
+
+ # A chart and a dataset too, so a reviewer can be found for all three
+ # asset types the gate covers and not just the dashboard.
+ for model in (
+ dashboard,
+ db.session.query(Slice).order_by(Slice.id).first(),
+ db.session.query(SqlaTable).order_by(SqlaTable.id).first(),
+ ):
+ if model is not None:
+ grant_editorship(model, both)
+
+ # Start from nothing pending, and from the title the examples ship.
+ from asset_approval.models import ChangeRequest
+
+ db.session.query(ChangeRequest).delete()
+ dashboard.dashboard_title = DASHBOARD_TITLE
+ db.session.commit()
+
+ headers = {"Referer": "http://localhost/"}
+
+ # One change, made and submitted as the author, so the bell, the queue and
+ # the dashboard's banner all have something to show on first login.
+ maker_client = login(MAKER)
+ saved = maker_client.put(
+ f"/api/v1/dashboard/{dashboard_id}",
+ json={"dashboard_title": PROPOSED_TITLE},
+ headers=headers,
+ )
+ if saved.status_code != 202:
+ sys.exit(
+ f"the gate did not capture the edit (got {saved.status_code}, "
+ "expected 202) -- is CHANGE_MANAGEMENT on?"
+ )
+
+ with app.app_context():
+ from superset import db
+ from asset_approval.models import ChangeRequest
+
+ change = (
+ db.session.query(ChangeRequest).order_by(ChangeRequest.id.desc()).first()
+ )
+ if not change:
+ sys.exit("the edit was captured but no change request was written")
+ change_id = change.id
+
+ submitted = maker_client.post(
+ f"/api/v1/change_request/{change_id}/submit",
+ json={"note": "Renamed for the Q4 pack"},
+ headers=headers,
+ )
+ if submitted.status_code >= 400:
+ sys.exit(f"could not submit for review ({submitted.status_code})")
+
+ checker_client = login(CHECKER)
+ queue = checker_client.get("/api/v1/change_request/?show=for_me").get_json()
+ bell = checker_client.get("/api/v1/change_request/count").get_json()["result"]
+
+ print()
+ print("Ready.")
+ print(f" logins {MAKER} / {CHECKER} (password: {PASSWORD})")
+ print(f" waiting {queue['count']} change(s) for {CHECKER}")
+ if queue["count"]:
+ print(f" on {queue['result'][0]['asset_title']}")
+ print(f" bell needs_me={bell['needs_me']} returned={bell['returned']}")
+
+
+if __name__ == "__main__":
+ main()