Kimirock/metabase
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Data apps: move resource collections into a data-apps namespace and serialize them with the app (#83766)

Browse Source
This commit is contained in:
Timofey Kachalov
2026-10-07 00:38:11 +04:00
committed by GitHub
parent b9b5c25e7a
commit ed1d3d1004
24 changed files with 362 additions and 187 deletions
Download Patch File Download Diff File Expand all files Collapse all files

49
e2e/support/helpers/e2e-data-app-helpers.ts
View File

@@ -6,6 +6,7 @@ import type {
CollectionPermission,
CollectionPermissionsGraph,
DataApp,
Group,
WritebackAction,
} from "metabase-types/api";
@@ -33,7 +34,7 @@ export const fakeDataApp = (overrides: Partial<DataApp> = {}): DataApp => ({
bundle_path: "dist/index.js",
enabled: true,
draft: false,
resource_collection_id: null,
resource_collection_id: 1,
permission_group_id: null,
table_ids: [],
allowed_hosts: [],
@@ -178,23 +179,47 @@ export function dataAppIframe(displayName: string) {
return getIframeBody(`iframe[title="${displayName}"]`);
}
/**
* Sets every non-admin group's access to a collection through the permission graph of
* the collection's namespace: an app's own collection lives in `data-apps`, whose graph
* is separate from the default one. A namespace graph lists only the groups holding a
* grant in it, so the groups come from the groups API, not from the graph.
*/
export function setDataAppCollectionAccess(
collectionId: CollectionId,
access: CollectionPermission,
) {
return cy
.request<CollectionPermissionsGraph>("GET", "/api/collection/graph")
.then(({ body: graph }) => {
const groups = Object.fromEntries(
Object.entries(graph.groups).map(([groupId, collections]) => [
groupId,
Number(groupId) === USER_GROUPS.ADMIN_GROUP
? collections
: { ...collections, [collectionId]: access },
]),
);
.request<Collection>("GET", `/api/collection/${collectionId}`)
.then(({ body: collection }) => {
const namespace = collection.namespace ?? undefined;
const graphUrl =
namespace === undefined
? "/api/collection/graph"
: `/api/collection/graph?namespace=${namespace}`;
cy.request("PUT", "/api/collection/graph", { ...graph, groups });
cy.request<Group[]>("GET", "/api/permissions/group").then(
({ body: allGroups }) => {
cy.request<CollectionPermissionsGraph>("GET", graphUrl).then(
({ body: graph }) => {
const groups = Object.fromEntries(
allGroups
.filter((group) => group.id !== USER_GROUPS.ADMIN_GROUP)
.map((group) => [
group.id,
{ ...graph.groups[group.id], [collectionId]: access },
]),
);
cy.request("PUT", "/api/collection/graph", {
...graph,
...(namespace === undefined ? {} : { namespace }),
groups,
});
},
);
},
);
});
}

40
e2e/test/scenarios/data-apps/viewing.cy.spec.ts
View File

@@ -3,7 +3,6 @@ import {
DATA_APP_NAME as APP_NAME,
visitDataAppRoute as visitAppRoute,
} from "e2e/support/helpers";
import type { DataApp } from "metabase-types/api";
import { DATA_APP_TEST_ENV as TEST_ENV } from "./helpers";
@@ -60,45 +59,6 @@ describe("scenarios > data apps > viewing & routing", () => {
});
});
it("shows unpublished app error when opening an app without a resource collection", () => {
cy.request<DataApp>("POST", `/api/apps/${APP_NAME}/draft`)
.its("body.resource_collection_id")
.as("resourceCollectionId");
cy.log("archive and delete the data app collection");
cy.get<number>("@resourceCollectionId").then((collectionId) => {
cy.request("PUT", `/api/collection/${collectionId}`, {
archived: true,
});
cy.request("DELETE", `/api/collection/${collectionId}`);
});
cy.log("fetching bundle should return 409 error");
cy.request({
url: `/api/apps/${APP_NAME}/bundle`,
failOnStatusCode: false,
})
.its("status")
.should("eq", 409);
cy.intercept("GET", `/api/apps/${APP_NAME}`).as("getUnpublishedApp");
H.openDataApp(APP_NAME);
cy.log("fetching data app metadata should return 409 error");
cy.wait("@getUnpublishedApp")
.its("response.statusCode")
.should("eq", 409);
H.main().within(() => {
cy.findByText("This data app isn’t published yet").should("be.visible");
cy.findByText(
"An administrator needs to publish this data app before it can be opened.",
).should("be.visible");
});
});
it("shows a not-found state for a disabled or missing app", () => {
// No app with this slug exists, so the metadata endpoint really 404s and the
// host renders a not-found state rather than a broken iframe — no mock needed.

52
enterprise/backend/src/metabase_enterprise/data_apps/README.md
View File

@@ -24,32 +24,42 @@ from everyone else, and a 409 to open.
## Serialization
A serialized app is a `data_app.yaml` in its own directory under `data_apps/`, with its bundle as a
plain file next to it at its `path`:
plain file next to it at its `path`. Its resource collection is a collection of the `data-apps`
namespace, serialized like any collection under `collections/`:
```
data_apps/
sales/
data_app.yaml # serdes/meta, entity_id, slug, name, description, version, path, allowed_hosts
data_app.yaml # serdes/meta, entity_id, slug, name, description, version, path, allowed_hosts, collection
dist/index.js # the bundle
collections/
main/
data_app__sales.yaml # the app's resource collection (namespace: data-apps)
data_app__sales/
*.yaml # the saved questions, metric copies, and action copies the app runs
```
The YAML keeps the keys a hand-written manifest uses: `slug` is the `name` column, `name` the
`display_name`, and `path` the `bundle_path`. The bundle travels as a serdes *resource file*: the
entity carries it in `:serdes/resources` on export, the storage writers put it next to the YAML,
and ingestion reads the paths `serdes/resource-paths` returns back in. A resource path must stay
inside the entity's directory.
`display_name`, `path` the `bundle_path`, and `collection` the entity ID of the app's resource
collection, which the app depends on and so loads after. The bundle travels as a serdes *resource
file*: the entity carries it in `:serdes/resources` on export, the storage writers put it next to
the YAML, and ingestion reads the paths `serdes/resource-paths` returns back in. A resource path
must stay inside the entity's directory.
Only the manifest is serialized. `enabled` is admin-owned and never leaves the instance; the
collection, permission group, and `table_ids` are server-managed; `bundle_hash` is recomputed from
the bundle on import. Drafts are not exported.
`enabled` is admin-owned and never leaves the instance; the permission group and `table_ids` are
server-managed; `bundle_hash` is recomputed from the bundle on import. Drafts are not exported.
A targeted export of an app brings its collection and what it holds along (`serdes/descendants`).
An import matches an app by `entity_id`, falling back to its slug so it takes over a draft, and
reasserts the app's resources. It is a no-op without the `:data-apps` feature.
reasserts the app's resources. A manifest that names a collection the repository lacks, or one
other than the collection the app already owns, fails to load. It is a no-op without the
`:data-apps` feature.
Remote sync treats data apps like any other entity, globally rather than per collection. Because
the bundle is a separate file, a pull that changes only a bundle, or an export that touches an app,
takes the full rather than the incremental path. An app's directory also holds its source, which
serialization doesn't own, so exports replace only the YAML and resource files in `data_apps/`.
Remote sync treats data apps like any other entity, globally rather than per collection; the app's
collection and what it holds travel with it as its serdes descendants. Because the bundle is a separate file, a pull that changes
only a bundle, or an export that touches an app, takes the full rather than the incremental path.
An app's directory also holds its source, which serialization doesn't own, so exports replace only
the YAML and resource files in `data_apps/`.
## Drafts
@@ -87,9 +97,13 @@ middleware's lookup doesn't pull in route code.
## Permissions
Each app owns two server-managed resources (`resources.clj`), created with the app (or its draft)
and reasserted on every import: a **collection** holding the copies the app is served from (saved
questions, actions, table-sourced metrics) and a **permissions group** its users belong to.
Each app owns two server-managed resources, created with the app (or its draft) and reasserted on
every import: a **collection** holding the copies the app is served from (saved questions, actions,
table-sourced metrics) and a **permissions group** its users belong to. The collection is a root
collection of the `data-apps` namespace, created as the app's row is inserted unless an import names
one (`models/data_app.clj`), and can never be swapped for another; `resources.clj` keeps its name and
permissions in step and brings it out of the trash. Deleting the app deletes both, and the
collection's own hooks delete what it holds.
The group is set database-level `view-data :blocked` on every database, so it grants **no data
access of its own** (which cascades `create-queries`/`download-results` to `:no`); every group but
@@ -97,9 +111,7 @@ admins is revoked from the collection before the app group gets read access. Del
both resources and everything in the collection.
**Viewing an app** requires read access to its resource collection. You have to be a member in
the app's group or be an admin. An app without a linked resource collection is considered _unpublished_.
The app's metadata and bundle endpoint returns HTTP 409 for all signed-in users. The frontend
shows the error "This data app isn’t published yet".
the app's group or be an admin.
**A viewer sees an app's data only through access they already hold.** The app group grants no
view-data of its own, so a viewer without access to an app's tables (e.g. a sandboxed user) sees no

12
enterprise/backend/src/metabase_enterprise/data_apps/api.clj
View File

@@ -96,7 +96,7 @@
[:bundle_path ms/NonBlankString]
[:enabled :boolean]
[:allowed_hosts [:sequential :string]]
[:resource_collection_id [:maybe ms/PositiveInt]]
[:resource_collection_id ms/PositiveInt]
[:permission_group_id [:maybe ms/PositiveInt]]
[:table_ids [:sequential ms/PositiveInt]]
[:has_user_permission_warnings {:optional true} :boolean]
@@ -246,16 +246,10 @@
(defn- read-check-data-app
"Check whether the current user can access a data app. Viewing requires read access to the app's
resource collection. An app with no linked resource collection has not been published yet: it is
not viewable by anyone through this endpoint (an admin must publish it first), signalled with a
409 so the client can show a dedicated \"not published\" screen rather than leaking metadata or
the bundle to every signed-in user."
resource collection, which every app has."
[app]
(api/read-check app)
(if-let [collection-id (:resource_collection_id app)]
(api/read-check :model/Collection collection-id)
(throw (ex-info (tru "This data app has not been published yet.")
{:status-code 409})))
(api/read-check :model/Collection (:resource_collection_id app))
app)
(defn- check-not-outdated

12
enterprise/backend/src/metabase_enterprise/data_apps/db.clj
View File

@@ -143,7 +143,12 @@
(defn resource-collection-ids
"The IDs of the resource collections owned by data apps."
[]
(t2/select-fn-set :resource_collection_id :model/DataApp :resource_collection_id [:not= nil]))
(t2/select-fn-set :resource_collection_id :model/DataApp))
(mu/defn resource-collection-id
"The ID of the resource collection owned by the DataApp with `data-app-id`."
[data-app-id :- ms/PositiveInt]
(t2/select-one-fn :resource_collection_id :model/DataApp :id data-app-id))
(defn databases-with-legacy-permissions
"Database IDs with legacy View Data permissions from groups not owned by apps."
@@ -165,11 +170,6 @@
[collection-id]
(t2/select-one :model/Collection :id collection-id))
(defn insert-resource-collection!
"Insert a resource collection and return it."
[row]
(t2/insert-returning-instance! :model/Collection row))
(defn update-resource-collection!
"Apply `changes` to the resource collection with `collection-id`."
[collection-id changes]

32
enterprise/backend/src/metabase_enterprise/data_apps/models/data_app.clj
View File

@@ -74,11 +74,23 @@
(t2/define-before-insert :model/DataApp
[data-app]
(prepare ::data-apps.schema/data-app.insert data-app))
;; Every app owns a resource collection from its first moment: one it is given (an import names the collection
;; its manifest references) or one created here, so the column is never null.
(let [row (prepare ::data-apps.schema/data-app.insert data-app)]
(cond-> row
(nil? (:resource_collection_id row))
(assoc :resource_collection_id (:id (data-app.resources/create-resource-collection! row))))))
(t2/define-before-update :model/DataApp
[data-app]
(merge data-app (some->> (t2/changes data-app) (prepare ::data-apps.schema/data-app.update))))
(let [changes (t2/changes data-app)]
;; A missing collection (the column is nullable, so the row survives its collection's deletion) may be set
;; again; one the app has is never replaced or cleared.
(when (and (contains? changes :resource_collection_id)
(some? (:resource_collection_id (t2/original data-app))))
(throw (ex-info (tru "A data app''s resource collection cannot be changed.")
{:status-code 400, :data-app-id (:id data-app)})))
(merge data-app (some->> changes (prepare ::data-apps.schema/data-app.update)))))
;; Reads always see `allowed_hosts` as a vector, never nil — a row synced before
;; the column existed has NULL until it's re-synced. Guard on `contains?` so
@@ -109,6 +121,8 @@
[app]
(merge app (data-app.resources/ensure-resources! app)))
;; The collection goes first, while the row still references it: the reference is nullable so the database can clear
;; it, and the collection's own hooks delete what it holds and the grants on it.
(t2/define-before-delete :model/DataApp
[app]
(data-app.resources/delete-resources! app))
@@ -141,8 +155,10 @@
;; set by the import itself
:draft :bundle_hash
;; server-managed resources, recreated on import
:resource_collection_id :permission_group_id :table_ids]
:permission_group_id :table_ids]
:transform {:created_at (serdes/date)
;; the app's resource collection, a collection in the `data-apps` namespace that loads before the app
:resource_collection_id (assoc (serdes/fk :model/Collection) :as :collection)
:name {:as :slug :export identity :import identity}
:display_name {:as :name :export identity :import identity}
:bundle_path {:as :path :export identity :import identity}
@@ -165,6 +181,16 @@
(data-apps.db/reducible-data-apps-with-bundles filter-column filter-ids
(serdes/extract-order-columns model-name opts))))
(defmethod serdes/deserialization-dependencies "DataApp" [{:keys [collection resource_collection_id]}]
;; A manifest names the collection as `collection`; serialization's own checks ask by the column.
(when-let [collection-entity-id (or collection resource_collection_id)]
[[{:model "Collection" :id collection-entity-id}]]))
(defmethod serdes/descendants "DataApp" [_model-name id _opts]
;; An app's resource collection, and through it what it holds, travel with the app.
(when-let [collection-id (data-apps.db/resource-collection-id id)]
{["Collection" collection-id] {"DataApp" id}}))
(defmethod serdes/storage-path "DataApp" [app _ctx]
[{:label data-app.config/apps-dir}
{:label (:slug app) :key (:entity_id app) :style :slug}

29
enterprise/backend/src/metabase_enterprise/data_apps/resources.clj
View File

@@ -1,5 +1,9 @@
(ns metabase-enterprise.data-apps.resources
"Lifecycle for the permission group and resource collection owned by a data app."
"Lifecycle for the permission group and resource collection owned by a data app.
The collection is created with the app, in the `data-apps` collection namespace, and is never replaced: the
model's hooks hold that invariant. What this namespace keeps in step afterwards is its name, its place out of
the trash, the permissions on it and on the group, and its existence, should it have been deleted on its own."
(:require
[metabase-enterprise.data-apps.db :as data-apps.db]
[metabase-enterprise.data-apps.permissions :as data-app.permissions]
@@ -12,6 +16,14 @@
(defn- resource-name [app]
(format "Data App: %s" (:name app)))
(defn create-resource-collection!
"Create the resource collection of `app`, a root collection in the `data-apps` namespace, as the app is inserted.
Created like any collection, so its events are published; as admin, since an import has no user bound."
[app]
(request/as-admin
(collection/create-collection! {:name (resource-name app)
:namespace (name collection/data-apps-ns)})))
(defn- create-permission-group! [app]
(let [group (data-apps.db/insert-permission-group! {:name (resource-name app)
:is_data_app_group true})]
@@ -61,16 +73,15 @@
(data-app.permissions/reconcile-app-group-permissions! (:id group) (data-apps.db/non-router-database-ids))
(apply-collection-permissions! group collection))
(defn- create-resource-collection! [app]
(let [collection (data-apps.db/insert-resource-collection! {:name (resource-name app)
:location "/"})]
(data-apps.db/update-data-app! (:id app) {:resource_collection_id (:id collection)})
collection))
(defn- resource-collection! [app]
(defn- resource-collection!
"The resource collection of `app`, created again if the app's was deleted out from under it: the reference is
nullable so that deleting the app can delete the collection first, which also lets the collection go alone."
[app]
(or (some->> (:resource_collection_id app)
(data-apps.db/resource-collection))
(create-resource-collection! app)))
(let [collection (create-resource-collection! app)]
(data-apps.db/update-data-app! (:id app) {:resource_collection_id (:id collection)})
collection)))
(defn ensure-resources!
"Create or restore the server-owned permission resources for `app` and return their IDs."

14
enterprise/backend/test/metabase_enterprise/data_apps/api_test.clj
View File

@@ -66,20 +66,6 @@
(is (= "You don't have permissions to do that."
(mt/user-http-request :rasta :put 403 "apps/demo" {:enabled false}))))))))
(deftest data-app-without-a-resource-collection-is-not-published-test
(mt/test-helpers-set-global-values!
(mt/with-premium-features #{:data-apps}
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(create-app!)
(t2/update! :model/DataApp :name "demo" {:resource_collection_id nil})
(testing "an app with no resource collection is not published: neither its metadata nor its
bundle is served — to anyone — and a 409 lets the client show a \"not published\" screen"
(doseq [user [:rasta :crowberto]]
(is (= "This data app has not been published yet."
(mt/user-http-request user :get 409 "apps/demo")))
(is (= "This data app has not been published yet."
(mt/user-http-request user :get 409 "apps/demo/bundle")))))))))
(deftest superuser-can-manage-and-view-test
(mt/test-helpers-set-global-values!
(mt/with-premium-features #{:data-apps}

52
enterprise/backend/test/metabase_enterprise/data_apps/models/data_app_test.clj
View File

@@ -6,6 +6,7 @@
[metabase-enterprise.data-apps.db :as data-apps.db]
[metabase-enterprise.data-apps.models.data-app]
[metabase.api.common :as api]
[metabase.events.core :as events]
[metabase.models.interface :as mi]
[metabase.test :as mt]
[metabase.util.json :as json]
@@ -40,9 +41,58 @@
(deftest insert-creates-the-resources-the-app-owns-test
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(let [{:keys [resource_collection_id permission_group_id]} (t2/select-one :model/DataApp (insert-app!))]
(is (t2/exists? :model/Collection :id resource_collection_id))
(is (=? {:name "Data App: m" :namespace :data-apps :location "/"}
(t2/select-one :model/Collection :id resource_collection_id))
"the collection is the app's own, in the data-apps namespace")
(is (t2/exists? :model/PermissionsGroup :id permission_group_id :is_data_app_group true)))))
(deftest insert-publishes-the-creation-of-the-collection-test
(testing "the collection is created the way any collection is, so what listens to collection events (remote sync,
the activity log) sees it"
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(let [created (atom [])]
(mt/with-dynamic-fn-redefs [events/publish-event! (fn [topic {:keys [object]}]
(when (= :event/collection-create topic)
(swap! created conj (:id object))))]
(let [id (insert-app!)]
(is (= [(t2/select-one-fn :resource_collection_id :model/DataApp id)] @created))))))))
(deftest insert-keeps-the-collection-it-is-given-test
(testing "an import names the collection its manifest references, so the insert links that one rather than creating"
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(mt/with-temp [:model/Collection {collection-id :id} {:name "Data App: m" :namespace :data-apps}]
(let [before (t2/count :model/Collection)
app (t2/select-one :model/DataApp (insert-app! :resource_collection_id collection-id))]
(is (= collection-id (:resource_collection_id app)))
(is (= before (t2/count :model/Collection)) "no second collection is created"))))))
(deftest the-resource-collection-cannot-change-test
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(mt/with-temp [:model/Collection {other-id :id} {:name "Other" :namespace :data-apps}]
(let [id (insert-app!)
collection-id (t2/select-one-fn :resource_collection_id :model/DataApp id)]
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"resource collection cannot be changed"
(t2/update! :model/DataApp id {:resource_collection_id other-id})))
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"resource collection cannot be changed"
(t2/update! :model/DataApp id {:resource_collection_id nil})))
(is (= collection-id (t2/select-one-fn :resource_collection_id :model/DataApp id)))
(testing "an update that names the same collection is not a change"
(t2/update! :model/DataApp id {:resource_collection_id collection-id :display_name "Renamed"})
(is (= "Renamed" (t2/select-one-fn :display_name :model/DataApp id))))
(testing "an app whose collection was deleted out from under it may be given one again"
(t2/delete! :model/Collection :id collection-id)
(is (nil? (t2/select-one-fn :resource_collection_id :model/DataApp id)) "the reference was cleared")
(t2/update! :model/DataApp id {:resource_collection_id other-id})
(is (= other-id (t2/select-one-fn :resource_collection_id :model/DataApp id))))))))
(deftest delete-removes-the-resources-the-app-owns-test
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(let [id (insert-app!)
{:keys [resource_collection_id permission_group_id]} (t2/select-one :model/DataApp id)]
(t2/delete! :model/DataApp id)
(is (not (t2/exists? :model/Collection :id resource_collection_id)))
(is (not (t2/exists? :model/PermissionsGroup :id permission_group_id))))))
(deftest writes-are-normalized-and-validated-against-the-schema-test
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(let [id (insert-app! :display_name " M " :bundle_path "./dist/index.js"

40
enterprise/backend/test/metabase_enterprise/data_apps/resources_test.clj
View File

@@ -17,30 +17,19 @@
:display_name name
:bundle_path (format "data_apps/%s/index.js" name)}))
(deftest ensure-resources-restores-a-collection-trashed-through-an-ancestor-test
(testing "an app collection filed under another collection is archived indirectly when that
ancestor is trashed, and the ancestor stays there — so it is restored to the root
rather than to a parent that would reject it"
(deftest the-resource-collection-stays-at-the-root-of-its-namespace-test
(testing "an app collection lives in the data-apps namespace, so it can't be filed under a regular collection,
where trashing the ancestor would take the app's copies with it"
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(mt/with-test-user :crowberto
(mt/with-temp [:model/Collection {ancestor-id :id} {:name "Filed under" :location "/"}]
(let [app (create-data-app! "wrens")
{:keys [resource_collection_id]} (data-app.resources/ensure-resources! app)]
(collection/move-collection!
(t2/select-one :model/Collection :id resource_collection_id)
(collection/children-location (t2/select-one :model/Collection :id ancestor-id)))
(collection/archive-or-unarchive-collection!
(t2/select-one :model/Collection :id ancestor-id)
{:archived true})
(is (true? (t2/select-one-fn :archived :model/Collection :id resource_collection_id))
"precondition: the ancestor took the app collection with it")
(data-app.resources/ensure-resources! (t2/select-one :model/DataApp :id (:id app)))
(is (false? (t2/select-one-fn :archived :model/Collection :id resource_collection_id))
"the app collection is usable again")
(is (= "/" (t2/select-one-fn :location :model/Collection :id resource_collection_id))
"and sits at the root, not under the ancestor still in the trash")
(is (true? (t2/select-one-fn :archived :model/Collection :id ancestor-id))
"the ancestor is left where the admin put it")))))))
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"same namespace as its parent"
(collection/move-collection!
(t2/select-one :model/Collection :id resource_collection_id)
(collection/children-location (t2/select-one :model/Collection :id ancestor-id)))))
(is (= "/" (t2/select-one-fn :location :model/Collection :id resource_collection_id)))))))))
(deftest ensure-resources-restores-a-trashed-collection-test
(testing "trashing the resource collection archives the copies the app is served from,
@@ -65,6 +54,19 @@
(is (false? (t2/select-one-fn :archived :model/Card :id card-id))
"the copy the app serves is readable again")))))))
(deftest ensure-resources-recreates-a-deleted-collection-test
(testing "the reference is nullable so the collection can be deleted on its own; the next import gives the app
a collection again rather than leaving it broken"
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(let [app (create-data-app! "finches")]
(t2/delete! :model/Collection :id (:resource_collection_id app))
(let [{:keys [resource_collection_id]} (data-app.resources/ensure-resources! app)]
(is (pos-int? resource_collection_id))
(is (not= (:resource_collection_id app) resource_collection_id))
(is (=? {:name "Data App: finches" :namespace :data-apps :location "/"}
(t2/select-one :model/Collection :id resource_collection_id)))
(is (= resource_collection_id (t2/select-one-fn :resource_collection_id :model/DataApp :id (:id app)))))))))
(deftest ensure-resources-blocks-the-app-groups-view-data-test
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(let [app (create-data-app! "birds")

52
enterprise/backend/test/metabase_enterprise/data_apps/serialization_test.clj
View File

@@ -22,8 +22,16 @@
:allowed_hosts ["https://api.example.com"]}
extra)))
(defn- export! [dir]
(serialization/store! (serdes/extract-all "DataApp" {}) (serialization/file-writer dir)))
(defn- export!
"Export the data apps, with the collection each owns (its serdes descendant) when `with-collections?`."
[dir & {:keys [with-collections?]}]
(serialization/store! (if with-collections?
(extract/extract {:targets (mapv (fn [id] ["DataApp" id])
(t2/select-pks-vec :model/DataApp :draft false))
:no-settings true
:no-data-model true})
(serdes/extract-all "DataApp" {}))
(serialization/file-writer dir)))
(defn- import! [dir]
(serialization/load-metabase! (serialization/ingest-yaml dir)))
@@ -62,7 +70,8 @@
:name "Sales Ops"
:description "Pipeline health"
:path "dist/index.js"
:allowed_hosts ["https://api.example.com"]}
:allowed_hosts ["https://api.example.com"]
:collection (t2/select-one-fn :entity_id :model/Collection :id (:resource_collection_id app))}
(dissoc (yaml/from-file (io/file dump-dir "data_apps" "sales-ops" "data_app.yaml"))
:created_at))))
(testing "the bundle is a plain file at its path next to the manifest"
@@ -78,8 +87,14 @@
(mt/with-premium-features #{:data-apps}
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(ts/with-random-dump-dir [dump-dir "data-app-round-trip-"]
(let [app (insert-app!)]
(export! dump-dir)
(let [app (insert-app!)
collection-entity-id (t2/select-one-fn :entity_id :model/Collection :id (:resource_collection_id app))]
(export! dump-dir :with-collections? true)
(testing "the app's collection is exported with it, like any collection"
(is (= [collection-entity-id]
(->> (file-seq (io/file dump-dir "collections"))
(filter #(.isFile ^java.io.File %))
(map (comp :entity_id yaml/from-file))))))
(t2/delete! :model/DataApp (:id app))
(import! dump-dir)
(let [imported (t2/select-one :model/DataApp :entity_id (:entity_id app))]
@@ -91,10 +106,33 @@
:draft false}
imported))
(is (= "console.log(1)" (bundle-text imported)))
(testing "the import creates the collection and permission group the app owns"
(is (t2/exists? :model/Collection :id (:resource_collection_id imported)))
(testing "the import links the collection the manifest names and creates the permission group"
(is (=? {:entity_id collection-entity-id :namespace :data-apps}
(t2/select-one :model/Collection :id (:resource_collection_id imported))))
(is (t2/exists? :model/PermissionsGroup :id (:permission_group_id imported))))))))))
(deftest import-refuses-a-manifest-that-names-another-collection-test
(mt/with-premium-features #{:data-apps}
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(ts/with-random-dump-dir [dump-dir "data-app-switch-"]
(mt/with-temp [:model/Collection {other-entity-id :entity_id} {:name "Other" :namespace :data-apps}]
(let [app (insert-app!)]
(write-app-files! dump-dir "sales-ops" (app-yaml (:entity_id app) "sales-ops" :collection other-entity-id)
{"dist/index.js" "B"})
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"Failed to load" (import! dump-dir)))
(is (= (:resource_collection_id app)
(t2/select-one-fn :resource_collection_id :model/DataApp :id (:id app))))))))))
(deftest import-refuses-a-manifest-naming-a-collection-the-repository-lacks-test
(mt/with-premium-features #{:data-apps}
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]
(ts/with-random-dump-dir [dump-dir "data-app-no-collection-"]
(write-app-files! dump-dir "x" (app-yaml "pZrj7PDuz3vSWYYi0QFhd" "x" :collection "nosuchcollection00001")
{"dist/index.js" "B"})
(is (thrown-with-msg? clojure.lang.ExceptionInfo #"Collection 'nosuchcollection00001' was not found"
(import! dump-dir)))
(is (not (t2/exists? :model/DataApp :entity_id "pZrj7PDuz3vSWYYi0QFhd")))))))
(deftest import-updates-in-place-and-keeps-local-state-test
(mt/with-premium-features #{:data-apps}
(mt/with-model-cleanup [:model/DataApp :model/Collection :model/PermissionsGroup]

14
enterprise/frontend/src/metabase-enterprise/data_apps/admin/components/DataAppActionsMenu/DataAppActionsMenu.tsx
View File

@@ -63,14 +63,12 @@ export const DataAppActionsMenu = ({ app, canRemove = false }: Props) => {
</Menu.Target>
<Menu.Dropdown>
{app.resource_collection_id != null && (
<Menu.Item
component={Link}
to={`/collection/${app.resource_collection_id}`}
>
{t`View resources`}
</Menu.Item>
)}
<Menu.Item
component={Link}
to={`/collection/${app.resource_collection_id}`}
>
{t`View resources`}
</Menu.Item>
{app.permission_group_id != null && (
<Menu.Item

12
enterprise/frontend/src/metabase-enterprise/data_apps/admin/components/DataAppActionsMenu/DataAppActionsMenu.unit.spec.tsx
View File

@@ -15,7 +15,7 @@ const setup = ({
}: {
enabled?: boolean;
canRemove?: boolean;
resourceCollectionId?: number | null;
resourceCollectionId?: number;
permissionGroupId?: number | null;
} = {}) => {
const app = createMockDataApp({
@@ -64,16 +64,6 @@ describe("DataAppActionsMenu", () => {
expect(menuItems[2]).toHaveTextContent("Disable");
});
it("does not show the collection link before an app has a collection", async () => {
setup({ resourceCollectionId: null });
await openMenu();
expect(
screen.queryByRole("menuitem", { name: "View resources" }),
).not.toBeInTheDocument();
});
it("does not show the group link before an app has a permission group", async () => {
setup({ permissionGroupId: null });

10
enterprise/frontend/src/metabase-enterprise/data_apps/components/DataAppView/DataAppView.tsx
View File

@@ -264,16 +264,6 @@ export function DataAppView() {
);
}
if (status === 409) {
return (
<CenteredEmptyState
title={t`This data app isn’t published yet`}
message={t`An administrator needs to publish this data app before it can be opened.`}
illustrationElement={<Icon name="hourglass" size={64} />}
/>
);
}
if (status === 403) {
return (
<CenteredEmptyState

14
enterprise/frontend/src/metabase-enterprise/data_apps/components/DataAppView/DataAppView.unit.spec.tsx
View File

@@ -90,20 +90,6 @@ describe("DataAppView", () => {
).not.toBeInTheDocument();
});
it("shows a not-published screen when the app has no resource collection yet", () => {
setup({ error: { status: 409 } });
expect(
screen.getByText("This data app isn’t published yet"),
).toBeInTheDocument();
expect(
screen.getByText(
"An administrator needs to publish this data app before it can be opened.",
),
).toBeInTheDocument();
});
it("shows the API's outdated message when the app 409s with the outdated error code", () => {
setup({
error: {

2
frontend/src/metabase-types/api/data-app.ts
View File

@@ -27,7 +27,7 @@ export interface DataApp {
/** Whether the app is a draft that reserves its slug before the app is created. */
draft: boolean;
/** The collection that contains this app's saved questions and models. */
resource_collection_id: number | null;
resource_collection_id: number;
/** The group that grants users access to this data app. */
permission_group_id: number | null;
/** Tables used by the last successful resource synchronization. */

32
resources/migrations/065/20261006_data_app_resource_collection.yaml Normal file
View File

@@ -0,0 +1,32 @@
## Every data app owns a resource collection, a root collection of the `data-apps` namespace, from its insert on.
## Apps created before the namespace existed hold theirs in the default namespace, and an app whose collection
## was deleted has none: give the first the namespace and the second a collection.
databaseChangeLog:
## MySQL, MariaDB, and H2 index a foreign key's column themselves; Postgres doesn't. On MySQL a second index
## would take over the constraint and refuse to be dropped on rollback, so this one is Postgres-only.
- changeSet:
id: v65.2026-10-06T00:00:00
author: timofei
comment: Index data apps by their resource collection (PostgreSQL)
dbms: postgresql
changes:
- createIndex:
tableName: data_app
indexName: idx_data_app_resource_collection_id
columns:
- column:
name: resource_collection_id
rollback:
- dropIndex:
tableName: data_app
indexName: idx_data_app_resource_collection_id
- changeSet:
id: v65.2026-10-06T00:00:01
author: timofei
comment: Put every data app's resource collection in the data-apps namespace, creating the missing ones
changes:
- customChange:
class: metabase.app_db.custom_migrations.MoveDataAppResourceCollectionsToTheirNamespace
rollback: []

5
src/metabase/actions/models.clj
View File

@@ -104,6 +104,11 @@
(throw (ex-info (tru "Implicit actions are not supported for models with clauses.")
{:status-code 400}))))
;; A data app's resource collection holds the copies of the actions the app runs.
(defmethod collection/allowed-namespaces :model/Action
[_]
(conj collection/default-allowed-namespaces collection/data-apps-ns))
(defn- check-collection-content
"Throws unless an Action may go in the Collection with `collection-id`."
[collection-id]

26
src/metabase/app_db/custom_migrations.clj
View File

@@ -31,6 +31,7 @@
[metabase.app-db.quartz]
[metabase.app-db.setting :as mdb.setting]
[metabase.config.core :as config]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
[metabase.util.encryption :as encryption]
[metabase.util.honey-sql-2 :as h2x]
@@ -2311,3 +2312,28 @@
;; revoking their tokens would instead force them through the refresh-failure path. The changeset
;; stays so that instances which already ran it can still roll back past it.
nil)
(define-migration MoveDataAppResourceCollectionsToTheirNamespace
;; Every data app owns a resource collection, a root collection of the `data-apps` namespace, from its insert
;; on (see `metabase-enterprise.data-apps.models.data-app`). Collections created before the namespace existed sit
;; in the default namespace; an app whose collection was deleted has none. Both are brought to the invariant here.
(t2/query {:update :collection
:set {:namespace "data-apps"}
:where [:exists ^:allow-subquery {:select [1]
:from [:data_app]
:where [:= :data_app.resource_collection_id :collection.id]}]})
(run! (fn [{:keys [id name]}]
(let [collection-name (str "Data App: " name)
;; slugified as the Collection model does (`collection-slug-max-length`), so a later rename changes nothing
collection-id (t2/insert-returning-pk! :collection {:name collection-name
:slug (u/slugify collection-name {:max-length 510})
:location "/"
:namespace "data-apps"
:entity_id (u/generate-nano-id)
:created_at :%now})]
(t2/query {:update :data_app
:set {:resource_collection_id collection-id}
:where [:= :id id]})))
(t2/reducible-query {:select [:id :name]
:from [:data_app]
:where [:= :resource_collection_id nil]})))

4
src/metabase/collections/core.clj
View File

@@ -41,6 +41,8 @@
remote-synced-collection?
remote-synced-dependents
shared-tenant-collection?
transforms-ns]
transforms-ns
data-apps-ns
default-allowed-namespaces]
[metabase.collections.util
annotate-dashboards])

10
src/metabase/collections/models/collection.clj
View File

@@ -94,6 +94,10 @@
"Namespace for snippets"
:snippets)
(def data-apps-ns
"Namespace for the resource collections of data apps"
:data-apps)
(defn- trash-collection* []
(collections.db/collection-of-type trash-collection-type))
@@ -2251,9 +2255,13 @@
{:arglists '([model])}
t2.protocols/dispatch-value)
(def default-allowed-namespaces
"The Collection namespaces most models may go in: the default namespace and the ones that hold the same content."
#{nil :analytics :shared-tenant-collection :tenant-specific})
(defmethod allowed-namespaces :default
[_]
#{nil :analytics :shared-tenant-collection :tenant-specific})
default-allowed-namespaces)
(defn check-collection-namespace
"Check that object's `:collection_id` refers to a Collection in an allowed namespace (see

5
src/metabase/queries/models/card.clj
View File

@@ -1482,6 +1482,11 @@
:else base)))
;; A data app's resource collection holds the saved questions and metric copies the app runs.
(defmethod collection/allowed-namespaces :model/Card
[_]
(conj collection/default-allowed-namespaces collection/data-apps-ns))
(defmethod serdes/make-spec "Card"
[_model-name _opts]
{:copy [:archived :archived_directly :collection_position :collection_preview :description :display

29
test/metabase/app_db/custom_migrations_test.clj
View File

@@ -3088,3 +3088,32 @@
(testing "tokens already carrying a v2 tool scope keep working"
(is (not (revoked? :oauth_access_token v2-access)))
(is (not (revoked? :oauth_refresh_token v2-refresh))))))))
(deftest move-data-app-resource-collections-to-their-namespace-test
(testing "v65.2026-10-06T00:00:01: every data app's resource collection is in the data-apps namespace"
(impl/test-migrations ["v65.2026-10-06T00:00:01"] [migrate!]
(let [insert-app! (fn [slug collection-id]
(t2/insert-returning-pk! :data_app {:name slug
:display_name slug
:bundle_path "dist/index.js"
:entity_id (str slug "Entity0000000000")
:resource_collection_id collection-id
:created_at :%now
:updated_at :%now}))
old-coll (t2/insert-returning-pk! :collection {:name "Data App: sales"
:slug "data_app__sales"
:location "/"
:entity_id "salesCollection000001"
:created_at :%now})
with-coll (insert-app! "sales" old-coll)
without (insert-app! "ops" nil)]
(migrate!)
(testing "a collection from before the namespace existed is moved into it"
(is (= "data-apps" (t2/select-one-fn :namespace :collection :id old-coll)))
(is (= old-coll (t2/select-one-fn :resource_collection_id :data_app :id with-coll))))
(testing "an app without a collection gets one, at the root of the namespace"
(let [collection-id (t2/select-one-fn :resource_collection_id :data_app :id without)]
(is (some? collection-id))
(is (=? {:name "Data App: ops" :slug "data_app__ops" :location "/" :namespace "data-apps"}
(t2/select-one :collection :id collection-id)))
(is (= 21 (count (t2/select-one-fn :entity_id :collection :id collection-id))))))))))

2
test/metabase/collections/models/collection_test.clj
View File

@@ -1586,7 +1586,7 @@
(mt/with-temp [:model/Collection {collection-id :id} {:namespace "x"}]
(is (thrown-with-msg?
clojure.lang.ExceptionInfo
#"A Card can only go in Collections in the \"default\" or :shared-tenant-collection or :tenant-specific or :analytics namespace."
#"A Card can only go in Collections in the \"default\" or :shared-tenant-collection or :tenant-specific or :analytics or :data-apps namespace."
(collection/check-collection-namespace :model/Card collection-id)))))
(testing "Should throw exception if Collection does not exist"
(is (thrown-with-msg?