Feature/global redash subdashboards by imenattatra · Pull Request #79 · metr-systems/redash

imenattatra · 2026-07-27T16:31:13Z

Closes https://github.com/metr-systems/backlog/issues/4783

AI Disclaimer: Yes claude to plan and assist implementing and helped me shape up this PR description

Summary

This PR adds the ability to manage sub-dashboards in the Redash Global admin:
- browse the dashboards that live in the template org (se_template in production and se_staging_template in staging
- assign or unassign them to individual organizations.

A lot of decisions in this PR will make it easy for the next 2 PRs to be done and this is why it is quite big. Let me know if you need to discuss anything.

Code Strategy

Patterns reused from main Redash

List UI: The sub-dashboard list is built on Redash's shared list components (ItemsList, ItemsTable, Paginator).
API layer: The global axios instance unwraps responses to the body, mirroring the main app.
CSRF: The token cookie is set the same way as redash/security.py—refreshed on every response so it doesn't expire while the console stays open.
App layout: We reuse layout and routing machinery (ApplicationLayout + Router), but defines its own routes. The navbar is the one piece that's replaced, because Redash's original relies on an org-specific logged-in user that the global console doesn't have.

Structure choices

One file per resource: Endpoints are split into views/subdashboards.py, organizations.py, and assignments.py, all registered in routes.py.
Kept thin: The assignment endpoints don't re-check that the organization exists—the picker only submits values the API itself served.
Catch-all last: Refreshing a client-side URL like /sub-dashboards makes the browser ask the server for a path it has no route for, which would 404. A catch-all route serves the app's HTML for those paths so React can render them. It's the least specific rule and registered last, so real routes (/static, /login, /global-api/*) always win.

Endpoint formats

Sub-dashboards → paginated, so it needs a total count and a page slice.
Organizations → plain list: It just feeds a picker that loads everything at once, so pagination would be unnecessary.
Dashboard links use the template org's own slug and both id and slug.
Empty over error: A missing template org returns an empty page instead of failing.

Edge cases

Duplicate assignment returns 409, not a server error.
Assignment endpoints return 404 when the dashboard doesn't exist.
Each row's Remove button has its own spinner and disables the others, so removals can't overlap.
The assign modal can't be closed mid-save.

QA

How did you test it? Keep your future self in mind to help debug things later.

Unit tests (pytest)
Manually ( on staging ) , can be seen here https://global-dashboard.staging.metr.systems/sub-dashboards?order=-created_at&page=1&page_size=20

@helenalebreton this is minor, but can you please also check what should be the default (home) page we access on https://global-dashboard.staging.metr.systems/ ? we can also just redirect it to the subdashbaords or composed dashboard page directly.
Also, please let me know whenever you have time so that we can talk about details, i know this version shows minimum information, either for the list of dashboards or for the assignments but we can easily add later on any information you see that it needs to be there.

- Add GET /global-api/sub-dashboards, login-gated returning published, non-archived sub-dashboards newest-first with simple page/page_size pagination (no search or sort params yet — listing is all the UI needs for now). - Source the template org slug from TEMPLATE_ORG_SLUG (default se_template) so the deployment controls it rather than hardcoding a convention. - Return an empty page instead of erroring when the template org doesn't exist.

- Add GET /global-api/organizations, login-gated like the rest of the console, returning every org as {id, name, slug} ordered by name. - Return a bare list rather than the paginated since it will be used with the picker.

GET/POST/DELETE endpoints for sub-dashboard assignments. Kept thin for now, the endpoints trust the frontend, which only submits values the same API served.

CSRFProtect is already active on the global app, so it enforces the X-CSRFToken header on the write endpoints (assignment create/delete) — but nothing handed the SPA a token to send back, so those writes would have been rejected the moment the frontend called them. This closes that gap without weakening the protection: a cross-site attacker still can't read the cookie, so it still can't forge the token. - Inject a csrf_token cookie on every response in security.init_app, mirroring redash/security.py, so the delivery mechanism lives beside the CSRFProtect config it complements rather than in the SPA view. - Refresh it on every response (not once at page load) because generate_csrf re-stamps the token's timestamp, keeping it from lapsing past CSRF_TIME_LIMIT (6h) while the SPA stays open. - Keep the cookie non-HttpOnly by design so the frontend's JS can read it and echo it in X-CSRFToken; add a test asserting both that it is set and that it stays JS-readable.

…izations. - Add a dedicated global axios instance pointed at /global-api. - Unwrap responses to the body in an interceptor, mirroring the main app so services resolve to data directly instead of the full axios response. - Add the organization and sub-dashboard services covering the list, assignment read, assign, and remove-assignment endpoints.

- Add SubDashboardList.jsx on top of the shared items-list machinery (ResourceItemsSource + UrlStateStorage) so pagination, sorting, and URL-state come for free; default to newest-first. - Link each row to its per-dashboard assignments route. - Register the /sub-dashboards route in GlobalApplicationArea. - Add a Sub-Dashboards item to GlobalDesktopNavbar with a DesktopOutlined icon for discoverability. - Widen the global navbar to 96px and let the label wrap instead of being truncated by overflow:hidden, since "Sub-Dashboards" is too long for the default column on one line.

…he template org

Client-side routes like /sub-dashboards have no server-side rule, so a hard navigation or refresh returned a 404 instead of loading the app. Adding a catch-all lets the React router resolve these paths.

- Add SubDashboardAssignments page - Track removal per-row via removingId (not a bool) so each Remove button shows its own spinner while the rest are disabled, preventing concurrent mutations without freezing the whole table. - Disable closing the modal while an assign is saving, so the user can't dismiss it mid-request and have it try to update a modal that's gone. - Register the /sub-dashboards/:dashboardId/assignments route so the page is reachable from the sub-dashboard list.

Can happen in case we try to assign at same time even if this is very rare to happen.

helenalebreton · 2026-07-28T07:16:03Z

@imenattatra thank you for the PR - how do I log in https://global-dashboard.staging.metr.systems/sub-dashboards?order=-created_at&page=1&page_size=20
I tried the normal staging engineering user with from lastpass but i dont get in

thiagogds

Besides my comments, there's a big thing I missed in the last PR, the tests.

You added new tests now in the global_redash and you have some others in the tests directory outside. I think it's better to keep all of them together in global_redash.

Also, you're using unittest and we don't use that anymore anywhere, because global_redash is a metr product, you should keep to the conventions of our team. Other developers will also work on this later and it's easier if we follow the same practices here.

So please use pytest and move the tests to the same place. Also, when using pytest, please use fixtures and don't use base classes similar to unittest.

thiagogds · 2026-07-28T07:33:32Z

+def _serialize_dashboard(dashboard):
+    metr_dashboard = dashboard.metr_dashboard
+    return {
+        "id": dashboard.id,
+        "name": dashboard.name,
+        "slug": dashboard.slug,
+        "url_identifier": metr_dashboard.url_identifier if metr_dashboard else None,
+    }


This is basically the same serializer as the one in subdashboards. Move them to a serializer.py

I think that we are enforcing Django patterns in a flask application by doing this. I'm personally fine with the serializers in the same file considering the design decision from @imenattatra (that I also agree with) to have one file per endpoint. Considering this pattern, no Django's one, it makes sense to me to have these serialization methods here as it's unlikely that they are gonna be used elsewhere.

But, if at some point we import any one of these methods to re-use them at other points of the code, I'm 100% with moving them to a serializers.py module.

A solution used by Redash itself is to move them to the models. For me the most important part is that they are exactly the same thing, a Dashboard serializer, so should have the same reason to change

Thank you ! I had other plans for the serializers actually. We already talked during the POC about using pydantic and i believe that, after we do that, the way we are using serializers in global redash will change.

I am also not sure about adopting the same thing as redash putting them in models because they already regret doing so, they say it here

redash/redash/serializers/__init__.py

Line 2 in e02c02a

This will eventually replace all the `to_dict` methods of the different model

.

I would like to work on this in a separate PR that resolved this newly created issue about adopting pydantic https://github.com/metr-systems/backlog/issues/4981 . What do you think? @berinhard @thiagogds

Just don't add this task to this milestone. It's a improvement and we need to have the whole global redash done soon

I moved it to a separate project about "Improvements for Global Redash" since i am only managing the project and not the milestone "Redash Scalability" , I added a comment about that so that @helenalebreton knows

thiagogds · 2026-07-28T07:34:45Z

+        db.session.commit()
+    except IntegrityError:
+        db.session.rollback()
+        return jsonify({"message": "This dashboard is already assigned to that organization."}), 409


This should be a serializer validation

As we don't have serializers classes (at least not that I'm aware of), I'm also fine with leaving it as it's. Although I agree that this is a serializer validation, in Django realms, we don't have easy or no-brainer ways of doing that within Flask without installing third party apps like, for example, flask-serialize. I'm not advising us to do that hehehe

I would add it to pydantic as i mentioned it in the previous command, please check issue https://github.com/metr-systems/backlog/issues/4981

imenattatra · 2026-07-28T08:33:06Z

@imenattatra thank you for the PR - how do I log in https://global-dashboard.staging.metr.systems/sub-dashboards?order=-created_at&page=1&page_size=20 I tried the normal staging engineering user with from lastpass but i dont get in

@helenalebreton Sorry forgot that, I added the access created to lastpass under the name of global_redash staging under the folder shared-operations/dashboards

imenattatra · 2026-07-28T08:37:17Z

Besides my comments, there's a big thing I missed in the last PR, the tests.

You added new tests now in the global_redash and you have some others in the tests directory outside. I think it's better to keep all of them together in global_redash.

@thiagogds Thank you for your review , the tests are not under global_redash they are all under the tests directory outside, i just prefixed them with test_global_redash.., but yes since they are many files now it makes sense to create a separate folder for them and also to keep it inside global_redash.

berinhard

The only blocker I have is the same pointed by @thiagogds about the template organization. I really liked the organization of the views as it was easy to compare with the tests with the 1 to 1 file mapping.

berinhard · 2026-07-28T08:56:38Z

+def _serialize_dashboard(dashboard):
+    metr_dashboard = dashboard.metr_dashboard
+    return {
+        "id": dashboard.id,
+        "name": dashboard.name,
+        "slug": dashboard.slug,
+        "url_identifier": metr_dashboard.url_identifier if metr_dashboard else None,
+    }


I think that we are enforcing Django patterns in a flask application by doing this. I'm personally fine with the serializers in the same file considering the design decision from @imenattatra (that I also agree with) to have one file per endpoint. Considering this pattern, no Django's one, it makes sense to me to have these serialization methods here as it's unlikely that they are gonna be used elsewhere.

But, if at some point we import any one of these methods to re-use them at other points of the code, I'm 100% with moving them to a serializers.py module.

berinhard · 2026-07-28T08:58:36Z

+        db.session.commit()
+    except IntegrityError:
+        db.session.rollback()
+        return jsonify({"message": "This dashboard is already assigned to that organization."}), 409


As we don't have serializers classes (at least not that I'm aware of), I'm also fine with leaving it as it's. Although I agree that this is a serializer validation, in Django realms, we don't have easy or no-brainer ways of doing that within Flask without installing third party apps like, for example, flask-serialize. I'm not advising us to do that hehehe

helenalebreton · 2026-07-28T10:18:56Z

@imenattatra thank you . I took a look. I dont think there is much more information to display at the moment to be honest. I guess at some point in the future we will want to see from the orga side i.e. which orga has whcih sub dashboards. But at the moment we dont have bespoke client template dashboard so it could be added later.
What is the next step ?

The slug names the org holding sub-dashboard templates, which only Redash Global reads. Keeping it in redash/settings gave the main app a constant it never uses and blurred the boundary between the two apps. It's now also required rather than silently defaulted.

The Redash Global tests lived under tests/ and inherited from BaseTestCase, so they were coupled to the main suite's unittest base class and scattered across files named after the app rather than owned by it. Rewriting them as pytest fixtures inside redash_global/ makes the app's tests self-contained: the global app's tests now sit next to the code they exercise and share nothing with the main suite but the model factories and the test database. - Add redash_global/tests/ as the app's own test package - Replace BaseTestCase/GlobalBaseTestCase with fixtures - Set GLOBAL_SECRET_KEY and TEMPLATE_ORG_SLUG in the new package's __init__, and import tests/__init__ from it, because both settings modules read the environment at import time and Redash Global runs against the main Redash database, so it needs the same test environment - Stub flask_babel._ in the new conftest with pytest.MonkeyPatch, not unittest.mock - Point CI, bin/docker-entrypoint and .coveragerc at the new directory, as both pass explicit test paths and would have silently skipped it - Drop TEMPLATE_ORG_SLUG from tests/__init__, now that nothing under tests/ starts the global app - Disable the global rate limiter in the app fixture

imenattatra · 2026-07-29T17:26:56Z

@imenattatra thank you . I took a look. I dont think there is much more information to display at the moment to be honest. I guess at some point in the future we will want to see from the orga side i.e. which orga has whcih sub dashboards. But at the moment we dont have bespoke client template dashboard so it could be added later. What is the next step ?

@helenalebreton As agreed with the team, the next step is the backend for the deploy/redeploy as explained in detail here in this comment https://github.com/metr-systems/backlog/issues/4769#issuecomment-5121270418

thiagogds

Since you're using Claude, please also update the claude.md file with the reviews you got in in this PR, like always using pytest, and create factories for models for example. This will save us time next time

thiagogds · 2026-07-30T07:14:21Z

+
+@pytest.fixture
+def url(dashboard):
+    return "/global-api/sub-dashboards/{}/assignments".format(dashboard.id)


Please use f-strings

thiagogds · 2026-07-30T07:16:12Z

+    def create(org):
+        assignment = SubDashboardAssignment(dashboard_id=dashboard.id, organization_id=org.id)
+        db.session.add(assignment)
+        db.session.commit()
+        return assignment


Redash already have a Factory pattern, you should use that too

thiagogds · 2026-07-30T07:18:04Z

+    def create(name="Template A", **kwargs):
+        dashboard = factory.create_dashboard(org=template_org, name=name, **kwargs)
+        db.session.commit()
+        return dashboard


Same thing about the Factory pattern

berinhard

I'm adding my review as a simple comment one, as I'm only agreeing with @thiagogds points

imenattatra added 15 commits July 22, 2026 20:15

Add SubDashboardAssignment model and migration

c56e099

Add organizations list API to Redash Global

40a31f7

- Add GET /global-api/organizations, login-gated like the rest of the console, returning every org as {id, name, slug} ordered by name. - Return a bare list rather than the paginated since it will be used with the picker.

Add sub-dashboard assignment API to Redash Global

6cfc3bc

GET/POST/DELETE endpoints for sub-dashboard assignments. Kept thin for now, the endpoints trust the frontend, which only submits values the same API served.

Use url with both id and slug for template dashboards

9b4d11e

Fix Organization Service name

b8b92e5

Fix sub-dashboard URLs to use the dashboard's own org slug which is t…

be539e1

…he template org

Fix 404 on refreshing or hard-navigating client-side routes

3f59585

Client-side routes like /sub-dashboards have no server-side rule, so a hard navigation or refresh returned a 404 instead of loading the app. Adding a catch-all lets the React router resolve these paths.

Stop tracking compiled CSS in redash_global

5ef4d1a

Return 409 instead of 500 on duplicate sub-dashboard assignment

bee401e

Can happen in case we try to assign at same time even if this is very rare to happen.

Return 404 for assignment endpoints on a missing dashboard

4d9d250

imenattatra requested review from a team and helenalebreton July 27, 2026 16:31

imenattatra self-assigned this Jul 27, 2026

thiagogds requested changes Jul 28, 2026

View reviewed changes

berinhard requested changes Jul 28, 2026

View reviewed changes

imenattatra added 4 commits July 29, 2026 12:34

move TEMPLATE_ORG_SLUG to settings

879195d

Test that Redash Global refuses to start without its required config

24c1570

imenattatra requested review from berinhard and thiagogds July 29, 2026 17:26

thiagogds requested changes Jul 30, 2026

View reviewed changes

berinhard reviewed Jul 30, 2026

View reviewed changes

Uh oh!

Conversation

imenattatra commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Code Strategy

Patterns reused from main Redash

Structure choices

Endpoint formats

Edge cases

QA

Uh oh!

helenalebreton commented Jul 28, 2026

Uh oh!

thiagogds left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imenattatra commented Jul 28, 2026

Uh oh!

imenattatra commented Jul 28, 2026

Uh oh!

berinhard left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

helenalebreton commented Jul 28, 2026

Uh oh!

imenattatra commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thiagogds left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

berinhard left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

imenattatra commented Jul 27, 2026 •

edited

Loading

imenattatra commented Jul 29, 2026 •

edited

Loading