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

Data apps: actions without a model (#83607)

Browse Source
This commit is contained in:
Alexander Polyankin
2026-10-05 12:11:30 -04:00
committed by GitHub
parent 6aa6b1560f
commit 6c54ed5234
49 changed files with 1289 additions and 1946 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
.clj-kondo/config/modules/config.edn
View File

@@ -3091,6 +3091,7 @@
:model/Database
:model/Field
:model/Measure
:model/QueryAction
:model/Table}}
typed-schemas.rest

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

@@ -1,14 +1,15 @@
import { USER_GROUPS } from "e2e/support/cypress_data";
import { USER_GROUPS, WRITABLE_DB_ID } from "e2e/support/cypress_data";
import * as Urls from "metabase/urls/data-apps";
import type {
CardId,
Collection,
CollectionId,
CollectionPermission,
CollectionPermissionsGraph,
DataApp,
WritebackAction,
} from "metabase-types/api";
import { createTestNativeQuery } from "./api";
import type { DataAppTestEnv } from "./data-app-test-env";
import { getIframeBody } from "./e2e-embedding-helpers";
import { LOCAL_GIT_PATH } from "./e2e-remote-sync-helpers";
@@ -197,28 +198,66 @@ export function setDataAppCollectionAccess(
});
}
export function moveDataAppModelToCollection({
modelId,
/** Creates a collection the non-admin groups hold `access` to. */
export function createDataAppCollection({
name,
access,
}: {
modelId: CardId;
name: string;
access: CollectionPermission;
}) {
return cy
.request<Collection>("POST", "/api/collection", { name })
.then(({ body: collection }) => {
cy.request("PUT", `/api/card/${modelId}`, {
collection_id: collection.id,
});
setDataAppCollectionAccess(collection.id, access);
return cy.wrap(collection, { log: false });
});
}
/** Creates a query action without a model that inserts a team into `scoreboard_actions`. */
export function createDataAppScoreboardAction({
name = "Add team",
collectionId = null,
}: { name?: string; collectionId?: CollectionId | null } = {}) {
return createTestNativeQuery({
database: WRITABLE_DB_ID,
query:
"INSERT INTO scoreboard_actions (team_name, score) VALUES ({{team_name}}, {{score}})",
templateTags: {
team_name: { type: "text", "display-name": "Team name", required: true },
score: { type: "number", "display-name": "Score", required: true },
},
}).then((datasetQuery) =>
cy
.request<WritebackAction>("POST", "/api/action", {
name,
type: "query",
database_id: WRITABLE_DB_ID,
collection_id: collectionId,
dataset_query: datasetQuery,
parameters: [
{
id: "team_name",
slug: "team_name",
name: "Team name",
type: "string/=",
target: ["variable", ["template-tag", "team_name"]],
required: true,
},
{
id: "score",
slug: "score",
name: "Score",
type: "number/=",
target: ["variable", ["template-tag", "score"]],
required: true,
},
],
})
.then(({ body: action }) => cy.wrap(action, { log: false })),
);
}
/**
* The dev host app is a real vite data app with the published SDK installed, so
* synchronizing it exercises the package an author actually consumes rather than

457
e2e/test-host-app/data-apps/sync-resources-actions.cy.spec.ts
View File

@@ -2,26 +2,28 @@ import { USERS, WRITABLE_DB_ID } from "e2e/support/cypress_data";
import {
addUserToGroup,
createDataAppApiKey,
createDataAppCollection,
createDataAppScoreboardAction,
dataAppHostAppRoot,
dataAppPermissionGroupId,
declareDataAppActions,
moveDataAppModelToCollection,
removeDataAppActionDeclaration,
resetDataAppHostAppSources,
syncDataAppResources,
} from "e2e/support/helpers";
import type { Card } from "metabase-types/api";
import type { WritebackAction } from "metabase-types/api";
const { H } = cy;
const TEST_TABLE = "scoreboard_actions";
const MODEL_NAME = "Scoreboard model";
/** The `slug` the app's `data_app.yaml` declares. */
const APP_SLUG = "vite-6-data-app-host-app";
const APP_ROOT = () => dataAppHostAppRoot();
const PARAMETERS = { team_name: "Data App FC", score: 7 };
/**
* Drives the real `sync-resources` CLI against a live instance, so the copies are
* checked against Metabase rather than a fake of its API. The unit suite covers
@@ -39,10 +41,6 @@ describe(
H.resetTestTable({ type: "postgres", table: TEST_TABLE });
H.resyncDatabase({ dbId: WRITABLE_DB_ID, tableName: TEST_TABLE });
H.setActionsEnabledForDB(WRITABLE_DB_ID);
H.createModelFromTableName({
tableName: TEST_TABLE,
modelName: MODEL_NAME,
});
resetDataAppHostAppSources();
createDataAppApiKey().as("apiKey");
@@ -60,22 +58,21 @@ describe(
});
});
/** The models Metabase holds in the app's own collection. */
const copiedModels = () =>
/** The actions Metabase holds in the app's own collection. */
const copiedActions = () =>
cy.request(`/api/apps/${APP_SLUG}`).then(({ body: app }) => {
expect(app.resource_collection_id, "resource collection").to.be.a(
"number",
);
return cy
.request(
`/api/collection/${app.resource_collection_id}/items?models=dataset`,
)
.then(({ body }) => body.data);
.request<WritebackAction[]>("/api/action")
.then(({ body }) =>
body.filter(
(action) => action.collection_id === app.resource_collection_id,
),
);
});
const actionsOnModel = (modelId: number) =>
cy.request(`/api/action?model-id=${modelId}`).then(({ body }) => body);
/** Runs a sync that must be refused, and returns the message it refused with. */
const syncExpectingRefusal = (message: string) =>
cy.get<string>("@apiKey").then((apiKey) => {
@@ -85,48 +82,34 @@ describe(
});
});
/** Declares one implicit action on the fixture model, without syncing yet. */
/** Declares one action, without syncing yet. */
const declareOneAction = () =>
cy.get<number>("@modelId").then((modelId) =>
H.createImplicitAction({ model_id: modelId, kind: "create" }).then(
({ body: action }) => {
createDataAppScoreboardAction().then((action) => {
declareDataAppActions(APP_ROOT(), [action.id]);
return cy.wrap({ modelId, action }, { log: false });
},
),
);
return cy.wrap(action, { log: false });
});
/** Syncs one declared action and hands back the copies Metabase now holds. */
/** Syncs one declared action and hands back the copy Metabase now holds. */
const syncOneAction = () =>
declareOneAction().then(({ modelId, action }) => {
declareOneAction().then((action) => {
sync();
return copiedModels().then(([copiedModel]) =>
actionsOnModel(copiedModel.id).then(([copiedAction]) =>
cy.wrap(
{ modelId, action, copiedModel, copiedAction },
{ log: false },
),
),
return copiedActions().then(([copiedAction]) =>
cy.wrap({ action, copiedAction }, { log: false }),
);
});
it("copies the model once for two actions, then deletes each copy as its declaration is removed", () => {
cy.get<number>("@modelId").then((modelId) => {
H.createImplicitAction({ model_id: modelId, kind: "create" }).then(
({ body: create }) => {
H.createImplicitAction({ model_id: modelId, kind: "update" }).then(
({ body: update }) => {
declareDataAppActions(APP_ROOT(), [create.id, update.id]);
it("copies each declared action into the app collection, then deletes each copy as its declaration is removed", () => {
createDataAppScoreboardAction({ name: "Add team" }).then((add) => {
createDataAppScoreboardAction({ name: "Add rival" }).then((rival) => {
declareDataAppActions(APP_ROOT(), [add.id, rival.id]);
sync();
copiedModels().then((models) => {
expect(models).to.have.length(1);
const copiedModelId = models[0].id;
expect(copiedModelId).not.to.eq(modelId);
actionsOnModel(copiedModelId).then((actions) => {
copiedActions().then((actions) => {
expect(actions).to.have.length(2);
actions.forEach((copy) => {
expect(copy.model_id, "a copy belongs to no model").to.eq(null);
expect([add.id, rival.id]).not.to.include(copy.id);
});
});
@@ -138,125 +121,67 @@ describe(
cy.readFile(`${APP_ROOT()}/resources_metadata.json`).then(
(lockfile) => {
expect(lockfile.models).to.have.length(1);
expect(lockfile.models[0].actions).to.have.length(2);
expect(lockfile.actions).to.have.length(2);
},
);
sync();
copiedModels().then((models) => {
expect(models).to.have.length(1);
actionsOnModel(models[0].id).then((actions) => {
expect(actions).to.have.length(2);
});
});
copiedActions().should("have.length", 2);
// The model survives: its sibling still needs it.
removeDataAppActionDeclaration(APP_ROOT(), update.id);
removeDataAppActionDeclaration(APP_ROOT(), rival.id);
sync();
copiedActions().should("have.length", 1);
copiedModels().then((models) => {
expect(models).to.have.length(1);
actionsOnModel(models[0].id).then((actions) => {
expect(actions).to.have.length(1);
});
});
removeDataAppActionDeclaration(APP_ROOT(), create.id);
removeDataAppActionDeclaration(APP_ROOT(), add.id);
sync();
copiedModels().should("have.length", 0);
copiedActions().should("have.length", 0);
cy.readFile(`${APP_ROOT()}/resources_metadata.json`).then(
(lockfile) => {
expect(lockfile.models).to.have.length(0);
},
);
},
);
expect(lockfile.actions).to.have.length(0);
},
);
});
});
});
it("updates the copies in place when the source model or action changes", () => {
cy.get<number>("@modelId").then((modelId) => {
H.createImplicitAction({ model_id: modelId, kind: "create" }).then(
({ body: action }) => {
declareDataAppActions(APP_ROOT(), [action.id]);
sync();
copiedModels().then(([copy]) => {
cy.request("PUT", `/api/card/${modelId}`, {
name: `${MODEL_NAME} renamed`,
});
it("updates the copy in place when the source action changes", () => {
syncOneAction().then(({ action, copiedAction }) => {
cy.request("PUT", `/api/action/${action.id}`, {
description: "now documented",
});
sync();
copiedModels().then((models) => {
expect(models).to.have.length(1);
expect(models[0].id).to.eq(copy.id);
expect(models[0].name).to.eq(`${MODEL_NAME} renamed`);
actionsOnModel(models[0].id).then((actions) => {
copiedActions().then((actions) => {
expect(actions).to.have.length(1);
expect(actions[0].id).to.eq(copiedAction.id);
expect(actions[0].description).to.eq("now documented");
});
});
});
},
);
});
});
it("leaves the copies untouched when nothing changed", () => {
syncOneAction().then(({ copiedModel, copiedAction }) => {
it("leaves the copy untouched when nothing changed", () => {
syncOneAction().then(({ copiedAction }) => {
sync();
copiedModels().then(([model]) => {
expect(model.id).to.eq(copiedModel.id);
expect(model.updated_at, "the model copy was not rewritten").to.eq(
copiedModel.updated_at,
);
actionsOnModel(model.id).then(([action]) => {
copiedActions().then(([action]) => {
expect(action.id).to.eq(copiedAction.id);
expect(
action.updated_at,
"the action copy was not rewritten",
).to.eq(copiedAction.updated_at);
});
});
});
});
it("restores a copied model edited directly in Metabase", () => {
syncOneAction().then(({ copiedModel }) => {
cy.request("PUT", `/api/card/${copiedModel.id}`, {
name: "Edited by hand",
});
sync();
copiedModels().then((models) => {
expect(models).to.have.length(1);
expect(models[0].id).to.eq(copiedModel.id);
expect(models[0].name).to.eq(MODEL_NAME);
expect(action.updated_at, "the copy was not rewritten").to.eq(
copiedAction.updated_at,
);
});
});
});
it("restores a copied action edited directly in Metabase", () => {
syncOneAction().then(({ copiedModel, copiedAction }) => {
it("restores a copy edited directly in Metabase", () => {
syncOneAction().then(({ copiedAction }) => {
cy.request("PUT", `/api/action/${copiedAction.id}`, {
name: "Edited by hand",
});
sync();
actionsOnModel(copiedModel.id).then((actions) => {
copiedActions().then((actions) => {
expect(actions).to.have.length(1);
expect(actions[0].id).to.eq(copiedAction.id);
expect(actions[0].name).to.eq(copiedAction.name);
@@ -271,24 +196,26 @@ describe(
return cy.wrap(groupId, { log: false });
});
// The copy is the whole point of the model copy: an app's viewers hold
// read on the app's collection, so only the copy is reachable to them.
// The copy is the whole point: an app's viewers hold read on the app's
// collection, so only the copy is reachable to them.
it("lets the app's group execute the copy but not the action it was copied from", () => {
syncOneAction().then(({ modelId, action, copiedAction }) => {
// An action is readable through its model, and the fixture model sits
// in the root collection the normal user's groups can read.
moveDataAppModelToCollection({
modelId,
name: "Source models",
createDataAppCollection({
name: "Source actions",
access: "none",
});
}).then((collection) => {
createDataAppScoreboardAction({
collectionId: collection.id,
}).then((action) => {
declareDataAppActions(APP_ROOT(), [action.id]);
sync();
joinAppGroup();
copiedActions().then(([copiedAction]) => {
cy.signInAsNormalUser();
cy.request({
method: "POST",
url: `/api/action/${copiedAction.id}/execute`,
body: { parameters: { team_name: "Data App FC", score: 7 } },
body: { parameters: PARAMETERS },
})
.its("status")
.should("be.oneOf", [200, 204]);
@@ -297,20 +224,15 @@ describe(
cy.request({
method: "POST",
url: `/api/action/${action.id}/execute`,
body: { parameters: { team_name: "Data App FC", score: 7 } },
failOnStatusCode: false,
})
.its("status")
.should("eq", 403);
cy.request({
url: `/api/action/${action.id}`,
body: { parameters: PARAMETERS },
failOnStatusCode: false,
})
.its("status")
.should("eq", 403);
});
});
});
});
it("reports a copy that was deleted in Metabase without being re-synced", () => {
syncOneAction().then(({ copiedAction }) => {
@@ -324,45 +246,23 @@ describe(
cy.request({
method: "POST",
url: `/api/action/${copiedAction.id}/execute`,
body: { parameters: { team_name: "Data App FC", score: 7 } },
body: { parameters: PARAMETERS },
failOnStatusCode: false,
})
.its("status")
.should("eq", 404);
});
});
it("does not widen access to the model the copy was made from", () => {
syncOneAction().then(({ modelId, copiedModel }) => {
// The fixture model is created in the root collection, which the
// normal user's groups can read, so close it before asking whether
// the app's group opened anything.
moveDataAppModelToCollection({
modelId,
name: "Source models",
access: "none",
});
joinAppGroup();
cy.signInAsNormalUser();
cy.request(`/api/card/${copiedModel.id}`)
.its("body.id")
.should("eq", copiedModel.id);
cy.request({ url: `/api/card/${modelId}`, failOnStatusCode: false })
.its("status")
.should("eq", 403);
});
});
});
describe("recovery", () => {
it("reuses the existing copies when copiedActionId is missing from the source", () => {
syncOneAction().then(({ action, copiedModel, copiedAction }) => {
it("reuses the existing copy when copiedActionId is missing from the source", () => {
syncOneAction().then(({ action, copiedAction }) => {
// Rewriting the declarations drops the injected ID, as a bad merge would.
declareDataAppActions(APP_ROOT(), [action.id]);
sync();
actionsOnModel(copiedModel.id).then((actions) => {
copiedActions().then((actions) => {
expect(actions).to.have.length(1);
expect(actions[0].id, "the copy is reused, not replaced").to.eq(
copiedAction.id,
@@ -375,16 +275,12 @@ describe(
});
});
it("recreates the copied action but keeps its model when the action is deleted in Metabase", () => {
syncOneAction().then(({ copiedModel, copiedAction }) => {
it("recreates the copy when it is deleted in Metabase", () => {
syncOneAction().then(({ copiedAction }) => {
cy.request("DELETE", `/api/action/${copiedAction.id}`);
sync();
copiedModels().then((models) => {
expect(models, "the model survives").to.have.length(1);
expect(models[0].id).to.eq(copiedModel.id);
actionsOnModel(models[0].id).then((actions) => {
copiedActions().then((actions) => {
expect(actions).to.have.length(1);
expect(actions[0].id).not.to.eq(copiedAction.id);
});
@@ -392,216 +288,69 @@ describe(
});
});
it("brings back a copied model that was trashed on its own", () => {
syncOneAction().then(({ copiedModel }) => {
cy.request("PUT", `/api/card/${copiedModel.id}`, { archived: true });
sync();
cy.request(`/api/card/${copiedModel.id}`).then(
({ body: restored }) => {
expect(restored.archived, "the copy is out of the trash").to.eq(
false,
);
expect(restored.collection_id).to.eq(copiedModel.collection_id);
},
);
});
});
it("recreates the copied model after it is deleted in Metabase", () => {
syncOneAction().then(({ copiedModel }) => {
cy.request("DELETE", `/api/card/${copiedModel.id}`);
sync();
copiedModels().then((models) => {
expect(models).to.have.length(1);
expect(models[0].id).not.to.eq(copiedModel.id);
});
});
});
});
describe("refusals", () => {
// `GET /api/action/:id` filters archived actions out, so an archived source
// is unreadable rather than readable-and-flagged.
it("copies nothing when the action's model is in the trash", () => {
declareOneAction().then(({ modelId, action }) => {
cy.request("PUT", `/api/card/${modelId}`, { archived: true });
syncExpectingRefusal(`Could not read action ${action.id}`);
copiedModels().should("have.length", 0);
});
});
it("copies nothing when a declared action is archived and cannot be read", () => {
declareOneAction().then(({ action }) => {
declareOneAction().then((action) => {
cy.request("PUT", `/api/action/${action.id}`, { archived: true });
syncExpectingRefusal(`Could not read action ${action.id}`);
copiedModels().should("have.length", 0);
copiedActions().should("have.length", 0);
});
});
it("copies nothing when a declared action belongs to a model", () => {
H.createModelFromTableName({
tableName: TEST_TABLE,
modelName: "Scoreboard model",
});
cy.get<number>("@modelId").then((modelId) =>
H.createImplicitAction({ model_id: modelId, kind: "create" }).then(
({ body: action }) => {
declareDataAppActions(APP_ROOT(), [action.id]);
syncExpectingRefusal("is not a query action without a model");
copiedActions().should("have.length", 0);
},
),
);
});
// Validation itself is unit-tested; what matters here is that a rejected
// lockfile stops the CLI before it mutates anything.
it("refuses to sync when resources_metadata.json is corrupt, leaving the copies alone", () => {
syncOneAction().then(({ copiedModel, copiedAction }) => {
it("refuses to sync when resources_metadata.json is corrupt, leaving the copy alone", () => {
syncOneAction().then(({ copiedAction }) => {
cy.writeFile(`${APP_ROOT()}/resources_metadata.json`, "{ not json");
syncExpectingRefusal("Could not read resources_metadata.json");
copiedModels().then((models) => {
expect(models, "the copies are left alone").to.have.length(1);
expect(models[0].id).to.eq(copiedModel.id);
actionsOnModel(models[0].id).then((actions) => {
expect(actions).to.have.length(1);
copiedActions().then((actions) => {
expect(actions, "the copy is left alone").to.have.length(1);
expect(actions[0].id).to.eq(copiedAction.id);
});
});
});
});
it("refuses to sync when a copied model was moved out of the app collection", () => {
syncOneAction().then(({ copiedModel }) => {
it("refuses to sync when a copy was moved out of the app collection", () => {
syncOneAction().then(({ copiedAction }) => {
cy.request("POST", "/api/collection", { name: "Elsewhere" }).then(
({ body: collection }) => {
cy.request("PUT", `/api/card/${copiedModel.id}`, {
cy.request("PUT", `/api/action/${copiedAction.id}`, {
collection_id: collection.id,
});
syncExpectingRefusal(`Move card ${copiedModel.id} back to`);
// Refusing is only worth anything if the card is left alone.
cy.request(`/api/card/${copiedModel.id}`)
syncExpectingRefusal(
`Action ${copiedAction.id} is the copy of action`,
);
// Refusing is only worth anything if the copy is left alone.
cy.request(`/api/action/${copiedAction.id}`)
.its("body.collection_id")
.should("eq", collection.id);
},
);
});
});
it("refuses to sync when a copied model was converted to a question", () => {
syncOneAction().then(({ copiedModel }) => {
cy.request("PUT", `/api/card/${copiedModel.id}`, {
type: "question",
});
syncExpectingRefusal("is no longer a model");
});
});
// Converting the parent model to a question deletes its implicit actions
// and archives the rest (see `card.clj`), so the source becomes unreadable
// rather than readable-with-a-non-model-parent.
it("copies nothing when the source model was converted to a question", () => {
declareOneAction().then(({ modelId, action }) => {
cy.request("PUT", `/api/card/${modelId}`, { type: "question" });
syncExpectingRefusal(`Could not read action ${action.id}`);
copiedModels().should("have.length", 0);
});
});
it("refuses to update a copied action that now belongs to another model", () => {
syncOneAction().then(({ modelId, copiedAction }) => {
// Still declared, but repointed at a model the app does not own:
// replacing it would abandon an action nothing tracks.
cy.request("PUT", `/api/action/${copiedAction.id}`, {
model_id: modelId,
});
syncExpectingRefusal(
`Action ${copiedAction.id} is the copy of action`,
);
cy.request(`/api/action/${copiedAction.id}`)
.its("body.model_id")
.should("eq", modelId);
});
});
it("refuses to delete a copied action that now belongs to another model", () => {
cy.get<number>("@modelId").then((modelId) => {
H.createImplicitAction({ model_id: modelId, kind: "create" }).then(
({ body: create }) => {
H.createImplicitAction({
model_id: modelId,
kind: "update",
}).then(({ body: update }) => {
declareDataAppActions(APP_ROOT(), [create.id, update.id]);
sync();
copiedModels().then(([copiedModel]) => {
actionsOnModel(copiedModel.id).then((actions) => {
const copyOfUpdate = actions.find(
(candidate: { name: string }) =>
candidate.name === "Update",
);
// Repointed at the source model, which the app does not
// own — deleting it would destroy someone else's action.
cy.request("PUT", `/api/action/${copyOfUpdate.id}`, {
model_id: modelId,
});
removeDataAppActionDeclaration(APP_ROOT(), update.id);
syncExpectingRefusal(
`no longer hangs off copied model ${copiedModel.id}`,
);
cy.request(`/api/action/${copyOfUpdate.id}`)
.its("status")
.should("eq", 200);
});
});
});
},
);
});
});
it("does not copy models that are based on saved questions", () => {
declareOneAction();
cy.log("create a saved question");
cy.get<number>("@modelId")
.then((modelId) => cy.request<Card>(`/api/card/${modelId}`))
.then(({ body: model }) =>
cy.request<Card>("POST", "/api/card", {
name: "Scoreboard source question",
display: "table",
visualization_settings: {},
dataset_query: model.dataset_query,
}),
)
.its("body.id")
.as("sourceQuestionId");
cy.log("change the model to use that saved question as its source");
cy.get<number>("@sourceQuestionId").then((sourceQuestionId) => {
cy.get<number>("@modelId").then((modelId) => {
cy.request("PUT", `/api/card/${modelId}`, {
dataset_query: {
database: WRITABLE_DB_ID,
type: "query",
query: { "source-table": `card__${sourceQuestionId}` },
},
});
});
});
cy.request("/api/action").its("body").as("actionsBeforeSync");
syncExpectingRefusal(
"depends on saved questions and cannot be synchronized",
);
cy.log("sync should not copy models that are based on saved questions");
copiedModels().should("have.length", 0);
cy.get("@actionsBeforeSync").then((actions) => {
cy.request("/api/action").its("body").should("deep.equal", actions);
});
});
});
},
);

2
e2e/test-host-app/data-apps/sync-resources.cy.spec.ts
View File

@@ -364,7 +364,7 @@ describe("Embedding SDK: data-app sync-resources (queries)", () => {
it("rebuilds a missing lockfile entry from the ID left in the source", () => {
syncOneQuery().then((card) => {
cy.writeFile(LOCKFILE(), { queries: [], models: [] });
cy.writeFile(LOCKFILE(), { queries: [], actions: [] });
sync();

35
e2e/test/scenarios/data-apps/actions.cy.spec.ts
View File

@@ -2,7 +2,8 @@ import { WRITABLE_DB_ID } from "e2e/support/cypress_data";
import {
DATA_APP_DISPLAY_NAME as APP_DISPLAY_NAME,
DATA_APP_NAME as APP_NAME,
moveDataAppModelToCollection,
createDataAppCollection,
createDataAppScoreboardAction,
visitDataAppRoute as visitAppRoute,
} from "e2e/support/helpers";
import type { CollectionPermission } from "metabase-types/api";
@@ -12,7 +13,6 @@ import { DATA_APP_TEST_ENV as TEST_ENV } from "./helpers";
const { H } = cy;
const TEST_TABLE = "scoreboard_actions";
const MODEL_NAME = "Scoreboard model";
const EXISTING_TEAM = "Amorous Aardvarks";
const RESOURCE_COLLECTION = "Data App resources";
@@ -28,24 +28,15 @@ describe(
H.resetTestTable({ type: "postgres", table: TEST_TABLE });
H.resyncDatabase({ dbId: WRITABLE_DB_ID, tableName: TEST_TABLE });
H.setActionsEnabledForDB(WRITABLE_DB_ID);
H.createModelFromTableName({
tableName: TEST_TABLE,
modelName: MODEL_NAME,
});
});
const setupActionsApp = (actionParams: Record<string, string | number>) =>
cy.get<number>("@modelId").then((modelId) => {
H.createImplicitAction({
model_id: modelId,
kind: "create",
}).then(({ body: action }) => {
createDataAppScoreboardAction().then((action) => {
H.mockDataApp(APP_NAME, {
displayName: APP_DISPLAY_NAME,
testEnv: { ...TEST_ENV, actionId: action.id, actionParams },
});
});
});
it("executes the action, exposing isExecuting and result, and resets", () => {
cy.intercept("POST", "/api/action/*/execute", (req) => {
@@ -107,17 +98,10 @@ describe(
describe("collection permissions", () => {
const setupRestrictedApp = (access: CollectionPermission) =>
cy.get<number>("@modelId").then((modelId) => {
moveDataAppModelToCollection({
modelId,
name: RESOURCE_COLLECTION,
access,
});
H.createImplicitAction({
model_id: modelId,
kind: "create",
}).then(({ body: action }) => {
createDataAppCollection({ name: RESOURCE_COLLECTION, access }).then(
(collection) =>
createDataAppScoreboardAction({ collectionId: collection.id }).then(
(action) => {
H.mockDataApp(APP_NAME, {
displayName: APP_DISPLAY_NAME,
testEnv: {
@@ -126,8 +110,9 @@ describe(
actionParams: { team_name: "Data App FC", score: 7 },
},
});
});
});
},
),
);
const assertNothingWritten = () =>
H.queryWritableDB(

27
e2e/test/scenarios/data-apps/sync-resources-production-actions.cy.spec.ts
View File

@@ -2,6 +2,7 @@ import { USERS, WRITABLE_DB_ID } from "e2e/support/cypress_data";
import {
addUserToGroup,
createDataAppApiKey,
createDataAppScoreboardAction,
dataAppIframe,
dataAppPermissionGroupId,
mockDataApp,
@@ -11,7 +12,6 @@ import {
const { H } = cy;
const TEST_TABLE = "scoreboard_actions";
const MODEL_NAME = "Scoreboard model";
const APP_SLUG = "synced-actions-app";
const APP_DISPLAY_NAME = "Synced Actions App";
@@ -49,8 +49,8 @@ const declaration = (sourceActionId: number) =>
/**
* The action half of the production path. Outside the dev preview
* `toExecutableActionId` runs `copiedActionId` — the copy hanging off the copied
* model — because that is the only action an app's viewers may execute.
* `toExecutableActionId` runs `copiedActionId` — the copy in the app's collection —
* because that is the only action an app's viewers may execute.
*/
describe(
"scenarios > data apps > sync-resources in production (actions)",
@@ -71,10 +71,6 @@ describe(
H.resetTestTable({ type: "postgres", table: TEST_TABLE });
H.resyncDatabase({ dbId: WRITABLE_DB_ID, tableName: TEST_TABLE });
H.setActionsEnabledForDB(WRITABLE_DB_ID);
H.createModelFromTableName({
tableName: TEST_TABLE,
modelName: MODEL_NAME,
});
restoreAuthoredFixture();
createDataAppApiKey().as("apiKey");
@@ -84,11 +80,9 @@ describe(
restoreAuthoredFixture();
});
/** Declares the model's action, synchronizes, and returns both action IDs. */
/** Declares an action, synchronizes, and returns both action IDs. */
const syncApp = () =>
cy.get<number>("@modelId").then((modelId) =>
H.createImplicitAction({ model_id: modelId, kind: "create" }).then(
({ body: action }) => {
createDataAppScoreboardAction().then((action) => {
cy.writeFile(ACTION_FILE(), declaration(action.id));
return cy.get<string>("@apiKey").then((apiKey) =>
@@ -99,13 +93,10 @@ describe(
return cy
.readFile(`${APP_ROOT()}/resources_metadata.json`)
.then((lockfile) => {
const copiedActionId =
lockfile.models?.[0]?.actions?.[0]?.copiedActionId;
const copiedActionId = lockfile.actions?.[0]?.copiedActionId;
if (typeof copiedActionId !== "number") {
throw new Error(
"The sync wrote no action to the lockfile.",
);
throw new Error("The sync wrote no action to the lockfile.");
}
return cy.wrap(
@@ -115,9 +106,7 @@ describe(
});
}),
);
},
),
);
});
it("executes the synchronized copy rather than the authored action", () => {
syncApp().then(({ sourceActionId, copiedActionId }) => {

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

@@ -89,7 +89,7 @@ middleware's lookup doesn't pull in route code.
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, action models, table-sourced metrics) and a **permissions group** its users belong to.
questions, actions, table-sourced metrics) and a **permissions group** its users belong to.
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

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

@@ -417,7 +417,7 @@
:- QueryTableDependenciesResponse
"Return the tables read by already-saved queries, including ones reached only through an implicit join.
Sync copies models, actions and metrics whose queries it never resolves through `/query`, and only
Sync copies actions and metrics whose queries it never resolves through `/query`, and only
this metadata-based lookup sees an implicit join: the id of a table reached through a foreign key
appears nowhere in the query itself."
[{:keys [slug]} :- [:map {:closed true} [:slug ms/NonBlankString]]

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

@@ -140,6 +140,11 @@
[]
(t2/select-pks-set :model/PermissionsGroup :is_data_app_group true))
(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]))
(defn databases-with-legacy-permissions
"Database IDs with legacy View Data permissions from groups not owned by apps."
[database-ids]

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

@@ -188,3 +188,9 @@
:feature :none
[]
(data-apps.db/data-app-group-ids))
(defenterprise data-app-collection-ids
"The resource collections of the data apps, which hold the copies `sync-resources` makes."
:feature :none
[]
(data-apps.db/resource-collection-ids))

43
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/README.md
View File

@@ -18,7 +18,7 @@ Two directories at the data-app root, beside `package.json`:
```
queries/orders.query.ts export const RevenueQuery = defineQuery({ source: schema.tables.orders })
actions/orders.action.ts export const CreateOrder = defineAction({ action: schema.models.orders.actions.create })
actions/orders.action.ts export const CreateOrder = defineAction({ action: schema.actions.shipOrder })
```
Nothing else is scanned — definitions under `src/` are invisible to sync, which is a common
@@ -33,29 +33,16 @@ authoring mistake.
yet, plus its collection and permission group.
3. **Reconcile queries** (`reconcile.ts`) — each authored query becomes a saved question in the
collection, and its ID is injected back into the source as `savedQuestionSourceId`.
4. **Reconcile models** (`reconcile-models.ts`) — each declared action's model is copied into the
collection, the action is copied onto that copy, and the copy's ID is injected back as
4. **Reconcile actions** (`reconcile-actions.ts`) — each declared action, a query action that
belongs to no model, is copied into the collection, and the copy's ID is injected back as
`copiedActionId`.
## Why actions copy a whole model
## Actions
An action's permissions resolve through its parent model's collection
(`metabase.actions.models`, `perms-objects-set`). Copying a card does **not** copy its actions —
`create-card!` writes one row and nothing traverses Card → Action — so making an action reachable
means copying its model _and_ recreating the action on that copy.
Models are therefore reference-counted by the actions that need them. The lockfile entry holds the
set of actions, not a count, because a count drifts across repeated runs while a set converges:
| Event | Model | Action |
| ------------------------------------- | --------------- | ------------------------- |
| action declared, model not yet copied | copy it | recreate on the copy |
| action declared, model already copied | reuse | recreate just this action |
| declaration dropped, siblings remain | keep | delete that copied action |
| last declaration dropped | delete the copy | cascade removes the rest |
The last row is free: `action.model_id` is `ON DELETE CASCADE`, so deleting the copied model takes
its copied actions with it.
An action without a model resolves its permissions through its own collection, so copying the
action into the app collection is what makes it runnable by the app's viewers. Each lockfile entry
maps a source action to its copy; a copy is updated when its source changes, recreated when it was
deleted, and deleted when its declaration goes away. An action that belongs to a model is refused.
## Discovery is deliberately strict
@@ -76,30 +63,22 @@ injects sequentially and an earlier injection into the same file shifts every of
"queries": [
{ "tableId": 1, "hash": "v1:sha256:…", "savedQuestionSourceId": 54 },
],
"models": [
{
"sourceModelId": 5,
"copiedModelId": 80,
"hash": "v1:sha256:…",
"actions": [
{ "sourceActionId": 51, "copiedActionId": 91, "hash": "v1:sha256:…" },
],
},
],
}
```
It is not a cache. It is the only evidence that a card or action in Metabase belongs to this app,
and every mutating path checks it first. The `hash` fields fingerprint the **source** payload — the
only way to notice that someone edited the upstream model or action, since nothing in the app's own
source changes when they do.
and every mutating path checks it first. The `hash` fields fingerprint the payload last copied; sync
compares it against the live copy, which catches both an edited source and a copy edited in Metabase.
## Invariants
These hold across both reconcilers, and are what the tests are mostly about:
- **Nothing is mutated or deleted without lockfile proof**, plus a live ownership check — still a
question/model, still in the app's collection, action still hanging off the copied model. Anything
question, still in the app's collection, an action still without a model in the app's collection. Anything
that has moved is refused with recovery instructions rather than silently touched.
- **The lockfile flushes after every single mutation**, so a crash mid-run leaves resumable state.
- **`404` is the only recoverable failure.** Any other status rethrows; recreate-after-404

2
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/discover.ts
View File

@@ -194,7 +194,7 @@ export async function discoverActions(
if (!isRecord(action) || !isPositiveInteger(action.id)) {
throw new Error(
`${location} must reference a generated action, such as \`schema.models.<model>.actions.<action>\`.`,
`${location} must reference a generated action, such as \`schema.actions.<action>\`.`,
);
}

42
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/lockfile.ts
View File

@@ -5,7 +5,6 @@ import { isPositiveInteger, isRecord } from "./guards";
import type {
ActionLockEntry,
MetricLockEntry,
ModelLockEntry,
QueryLockEntry,
ResourceLockfile,
} from "./types";
@@ -36,17 +35,6 @@ function isActionLockEntry(value: unknown): value is ActionLockEntry {
);
}
function isModelLockEntry(value: unknown): value is ModelLockEntry {
return (
isRecord(value) &&
isPositiveInteger(value.sourceModelId) &&
isPositiveInteger(value.copiedModelId) &&
isHash(value.hash) &&
Array.isArray(value.actions) &&
value.actions.every(isActionLockEntry)
);
}
function isMetricLockEntry(value: unknown): value is MetricLockEntry {
return (
isRecord(value) &&
@@ -105,49 +93,37 @@ function parseQueries(value: unknown): QueryLockEntry[] {
return queries;
}
function parseModels(value: unknown): ModelLockEntry[] {
function parseActions(value: unknown): ActionLockEntry[] {
if (value === undefined) {
return [];
}
if (
!Array.isArray(value) ||
value.some((entry) => !isModelLockEntry(entry))
value.some((entry) => !isActionLockEntry(entry))
) {
throw new Error(`${RESOURCE_LOCKFILE} contains an invalid model entry.`);
throw new Error(`${RESOURCE_LOCKFILE} contains an invalid action entry.`);
}
const models: ModelLockEntry[] = value;
const actions: ActionLockEntry[] = value;
assertUnique(
models.map((entry) => entry.sourceModelId),
"source model ID",
);
assertUnique(
models.map((entry) => entry.copiedModelId),
"copied model ID",
);
assertUnique(
models.flatMap((entry) =>
entry.actions.map(({ sourceActionId }) => sourceActionId),
),
actions.map(({ sourceActionId }) => sourceActionId),
"source action ID",
);
assertUnique(
models.flatMap((entry) =>
entry.actions.map(({ copiedActionId }) => copiedActionId),
),
actions.map(({ copiedActionId }) => copiedActionId),
"copied action ID",
);
return models;
return actions;
}
export function readResourceLockfile(appRoot: string): ResourceLockfile {
const lockfilePath = path.join(appRoot, RESOURCE_LOCKFILE);
if (!fs.existsSync(lockfilePath)) {
return { queries: [], models: [], metrics: [] };
return { queries: [], actions: [], metrics: [] };
}
let value: unknown;
@@ -173,7 +149,7 @@ export function readResourceLockfile(appRoot: string): ResourceLockfile {
? undefined
: { collectionId: value.collectionId }),
queries: parseQueries(value.queries ?? []),
models: parseModels(value.models),
actions: parseActions(value.actions),
metrics: parseMetrics(value.metrics),
};
}

34
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/metabase-client.ts
View File

@@ -176,13 +176,6 @@ export class MetabaseClient {
return this.request<void>(`card/${id}`, { method: "DELETE" });
}
createModel(input: ModelInput) {
return this.request<MetabaseCard>("card", {
method: "POST",
body: JSON.stringify(modelBody(input)),
});
}
createMetric(input: MetricInput) {
return this.request<MetabaseCard>("card", {
method: "POST",
@@ -197,13 +190,6 @@ export class MetabaseClient {
});
}
updateModel(id: number, input: ModelInput) {
return this.request<MetabaseCard>(`card/${id}`, {
method: "PUT",
body: JSON.stringify(modelBody(input)),
});
}
getAction(id: number) {
return this.request<MetabaseAction>(`action/${id}`);
}
@@ -227,15 +213,6 @@ export class MetabaseClient {
}
}
interface ModelInput {
name: string;
collectionId: number;
datasetQuery: Record<string, unknown>;
display: string;
visualizationSettings: Record<string, unknown>;
description: string | null;
}
interface MetricInput {
name: string;
collectionId: number;
@@ -245,17 +222,6 @@ interface MetricInput {
description: string | null;
}
const modelBody = (input: ModelInput) => ({
name: input.name,
type: "model",
archived: false,
dataset_query: input.datasetQuery,
display: input.display,
visualization_settings: input.visualizationSettings,
description: input.description,
collection_id: input.collectionId,
});
const metricBody = (input: MetricInput) => ({
name: input.name,
type: "metric",

237
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/reconcile-actions.ts Normal file
View File

@@ -0,0 +1,237 @@
import { ACTION_DEFINITIONS, injectGeneratedId } from "./ast/query-source";
import { getPayloadFingerprint } from "./canonical";
import { isPositiveInteger } from "./guards";
import { writeResourceLockfile } from "./lockfile";
import { getErrorMessage, getRelativeDefinitionLocation } from "./messages";
import type { MetabaseClient } from "./metabase-client";
import { orNullOn404 } from "./metabase-client";
import type {
ActionLockEntry,
DiscoveredAction,
MetabaseAction,
ResourceLockfile,
} from "./types";
export interface ReconcileActionsOptions {
appRoot: string;
slug: string;
collectionId: number;
actions: DiscoveredAction[];
lockfile: ResourceLockfile;
client: MetabaseClient;
log: (message: string) => void;
}
interface ResolvedAction {
action: DiscoveredAction;
source: MetabaseAction;
}
function pickDefined(source: Record<string, unknown>) {
return Object.fromEntries(
Object.entries(source).filter(([, value]) => value !== undefined),
);
}
/** Also the fingerprint input, so drift is measured over exactly what gets copied. */
function actionCopyFields(source: MetabaseAction) {
return pickDefined({
name: source.name,
type: source.type,
description: source.description,
visualization_settings: source.visualization_settings,
parameters: source.parameters,
parameter_mappings: source.parameter_mappings,
dataset_query: source.dataset_query,
database_id: source.database_id,
});
}
async function resolveActions({
appRoot,
collectionId,
actions,
lockfile,
client,
}: ReconcileActionsOptions): Promise<ResolvedAction[]> {
const copiedIds = new Set(
lockfile.actions.map(({ copiedActionId }) => copiedActionId),
);
return Promise.all(
actions.map(async (action) => {
const location = getRelativeDefinitionLocation(appRoot, action);
let source: MetabaseAction;
try {
// `GET /api/action/:id` filters archived actions out, so an archived
// source surfaces here as a 404 rather than a readable payload.
source = await client.getAction(action.sourceActionId);
} catch (error) {
throw new Error(
`Could not read action ${action.sourceActionId} for ${location}: ${getErrorMessage(error)}`,
);
}
if (isPositiveInteger(source.model_id) || source.type !== "query") {
throw new Error(
`${location} references action ${action.sourceActionId}, which is not a query action without a model. A data app runs only query actions that belong to no model.`,
);
}
// A copy is synchronized state: declaring it as a source would make the
// next run delete it once its own source is dropped.
if (copiedIds.has(source.id) || source.collection_id === collectionId) {
throw new Error(
`${location} references action ${action.sourceActionId}, which is a copy synchronized into this data app. Declare the action it was copied from instead.`,
);
}
return { action, source };
}),
);
}
function isOwnedActionCopy(copy: MetabaseAction, collectionId: number) {
return (
!isPositiveInteger(copy.model_id) && copy.collection_id === collectionId
);
}
function assertOwnedActionCopy(
copy: MetabaseAction,
sourceActionId: number,
collectionId: number,
) {
if (!isOwnedActionCopy(copy, collectionId)) {
throw new Error(
`Action ${copy.id} is the copy of action ${sourceActionId} but is no longer in data app collection ${collectionId}, so it was left untouched. Move it back or delete it manually, then run sync-resources again.`,
);
}
}
async function reconcileAction(
{ appRoot, collectionId, client, lockfile, log }: ReconcileActionsOptions,
{ action, source }: ResolvedAction,
) {
const fields = actionCopyFields(source);
const hash = getPayloadFingerprint(fields);
let mapping: ActionLockEntry | undefined = lockfile.actions.find(
({ sourceActionId }) => sourceActionId === source.id,
);
let copy = mapping
? await orNullOn404(client.getAction(mapping.copiedActionId))
: null;
if (mapping && copy) {
assertOwnedActionCopy(copy, source.id, collectionId);
}
// A lost lockfile entry: adopt the copy the declaration still names rather
// than leaving it behind in the app collection beside a new one.
if (!mapping && isPositiveInteger(action.copiedActionId)) {
const named = await orNullOn404(client.getAction(action.copiedActionId));
const claimed = lockfile.actions.some(
({ copiedActionId }) => copiedActionId === action.copiedActionId,
);
if (named && !claimed && isOwnedActionCopy(named, collectionId)) {
mapping = { sourceActionId: source.id, copiedActionId: named.id, hash };
lockfile.actions.push(mapping);
writeResourceLockfile(appRoot, lockfile);
log(`adopted action: action ${source.id} -> action ${named.id}`);
copy = named;
}
}
if (mapping && !copy) {
lockfile.actions.splice(lockfile.actions.indexOf(mapping), 1);
mapping = undefined;
}
if (!mapping || !copy) {
const created = await client.createAction({
...fields,
collection_id: collectionId,
});
if (!isPositiveInteger(created.id)) {
throw new Error("The Action API did not return a valid action ID.");
}
lockfile.actions.push({
sourceActionId: source.id,
copiedActionId: created.id,
hash,
});
injectGeneratedId(action, ACTION_DEFINITIONS, created.id);
writeResourceLockfile(appRoot, lockfile);
log(`copied action: action ${source.id} -> action ${created.id}`);
return;
}
// Fingerprint the copy rather than trusting the lockfile: that also catches
// a copy edited directly in Metabase, which the source hash cannot see.
if (getPayloadFingerprint(actionCopyFields(copy)) !== hash) {
await client.updateAction(mapping.copiedActionId, fields);
mapping.hash = hash;
writeResourceLockfile(appRoot, lockfile);
log(`updated action: action ${mapping.copiedActionId}`);
}
if (action.copiedActionId !== mapping.copiedActionId) {
injectGeneratedId(action, ACTION_DEFINITIONS, mapping.copiedActionId);
log(
`restored action ID: ${action.exportName} -> action ${mapping.copiedActionId}`,
);
}
}
async function removeUnusedActions(
{ appRoot, collectionId, client, lockfile, log }: ReconcileActionsOptions,
desiredIds: Set<number>,
) {
for (const mapping of [...lockfile.actions]) {
if (desiredIds.has(mapping.sourceActionId)) {
continue;
}
const copy = await orNullOn404(client.getAction(mapping.copiedActionId));
if (copy) {
assertOwnedActionCopy(copy, mapping.sourceActionId, collectionId);
await client.deleteAction(mapping.copiedActionId);
log(`deleted action: action ${mapping.copiedActionId}`);
}
lockfile.actions.splice(lockfile.actions.indexOf(mapping), 1);
writeResourceLockfile(appRoot, lockfile);
}
}
/**
* Makes the data app collection hold a copy of exactly the actions the app
* declares, copying an action when its declaration appears and deleting the
* copy when the declaration goes away. Returns the tables those actions read.
*/
export async function reconcileActions(options: ReconcileActionsOptions) {
const { slug, client } = options;
const resolved = await resolveActions(options);
for (const entry of resolved) {
await reconcileAction(options, entry);
}
await removeUnusedActions(
options,
new Set(resolved.map(({ source }) => source.id)),
);
return client.resolveTableDependencies(
slug,
resolved.flatMap(({ source }) =>
source.dataset_query ? [source.dataset_query] : [],
),
);
}

453
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/reconcile-models.ts
View File

@@ -1,453 +0,0 @@
import { ACTION_DEFINITIONS, injectGeneratedId } from "./ast/query-source";
import { getPayloadFingerprint } from "./canonical";
import { isPositiveInteger } from "./guards";
import { writeResourceLockfile } from "./lockfile";
import { getErrorMessage, getRelativeDefinitionLocation } from "./messages";
import type { MetabaseClient } from "./metabase-client";
import { orNullOn404 } from "./metabase-client";
import { getSavedCardSourceIds } from "./saved-card-sources";
import type {
ActionLockEntry,
DiscoveredAction,
MetabaseAction,
MetabaseCard,
ModelLockEntry,
ResourceLockfile,
} from "./types";
export interface ReconcileModelsOptions {
appRoot: string;
slug: string;
collectionId: number;
actions: DiscoveredAction[];
lockfile: ResourceLockfile;
client: MetabaseClient;
log: (message: string) => void;
}
interface ResolvedAction {
action: DiscoveredAction;
source: MetabaseAction;
}
interface ModelContext {
appRoot: string;
collectionId: number;
client: MetabaseClient;
lockfile: ResourceLockfile;
log: (message: string) => void;
}
function pickDefined(source: Record<string, unknown>) {
return Object.fromEntries(
Object.entries(source).filter(([, value]) => value !== undefined),
);
}
/** Also the fingerprint input, so drift is measured over exactly what gets copied. */
function modelCopyInput(source: MetabaseCard) {
return {
name: source.name,
datasetQuery: source.dataset_query,
display: source.display ?? "table",
visualizationSettings: source.visualization_settings ?? {},
description: source.description ?? null,
};
}
/**
* Implicit actions omit `parameters` and `parameter_mappings`: Metabase derives
* those from the model's fields on every read.
*/
function actionCopyFields(source: MetabaseAction) {
const common = {
name: source.name,
type: source.type,
description: source.description,
visualization_settings: source.visualization_settings,
};
if (source.type === "implicit") {
return pickDefined({ ...common, kind: source.kind });
}
if (source.type === "query") {
return pickDefined({
...common,
parameters: source.parameters,
parameter_mappings: source.parameter_mappings,
dataset_query: source.dataset_query,
database_id: source.database_id,
});
}
return pickDefined(common);
}
async function resolveActions(
appRoot: string,
actions: DiscoveredAction[],
client: MetabaseClient,
): Promise<ResolvedAction[]> {
return Promise.all(
actions.map(async (action) => {
let source: MetabaseAction;
try {
// `GET /api/action/:id` filters archived actions out, so an archived
// source surfaces here as a 404 rather than a readable payload.
source = await client.getAction(action.sourceActionId);
} catch (error) {
throw new Error(
`Could not read action ${action.sourceActionId} for ${getRelativeDefinitionLocation(appRoot, action)}: ${getErrorMessage(error)}`,
);
}
if (!isPositiveInteger(source.model_id)) {
throw new Error(
`${getRelativeDefinitionLocation(appRoot, action)} references action ${action.sourceActionId}, which does not belong to a model.`,
);
}
return { action, source };
}),
);
}
async function fetchSourceModels(
appRoot: string,
desired: Map<number, ResolvedAction[]>,
client: MetabaseClient,
): Promise<Map<number, MetabaseCard>> {
const models = await Promise.all(
[...desired].map(async ([id, actions]) => {
const location = getRelativeDefinitionLocation(
appRoot,
actions[0].action,
);
let card: MetabaseCard;
try {
card = await client.getCard(id);
} catch (error) {
throw new Error(
`Could not read model ${id} for ${location}: ${getErrorMessage(error)}`,
);
}
if (card.type !== "model") {
throw new Error(
`${location} references an action on card ${id}, which is not a model.`,
);
}
// A trashed action reads as a 404 and stops the run; a trashed model is
// still readable, so refuse it here rather than copying from something
// its author deleted.
if (card.archived === true) {
throw new Error(
`${location} references an action on model ${id}, which is in the trash. Restore the model or remove the declaration, then run sync-resources again.`,
);
}
if (getSavedCardSourceIds(card.dataset_query).size > 0) {
throw new Error(
`Model ${id} depends on saved questions and cannot be synchronized. Use a model without saved question sources or joins.`,
);
}
return [id, card] as const;
}),
);
return new Map(models);
}
function assertOwnedActionCopy(
action: MetabaseAction,
sourceActionId: number,
copiedModelId: number,
) {
if (action.model_id !== copiedModelId) {
throw new Error(
`Action ${action.id} is the copy of action ${sourceActionId} but no longer hangs off copied model ${copiedModelId}, so it was left untouched. Move it back or delete it manually, then run sync-resources again.`,
);
}
}
function assertOwnedModelCopy(card: MetabaseCard, collectionId: number) {
if (card.type !== "model") {
throw new Error(
`Card ${card.id} belongs to a synchronized model but is no longer a model, so it was left untouched. Change it back to a model in data app collection ${collectionId} or delete it manually, then run sync-resources again.`,
);
}
// A trashed copy reports the Trash as its collection, so its real one says
// nothing here. Only the restore path reaches this with one, and it puts both
// the card and its collection back.
if (card.archived !== true && card.collection_id !== collectionId) {
throw new Error(
`Card ${card.id} belongs to a synchronized model but is no longer in the data app collection, so it was left untouched. Move card ${card.id} back to data app collection ${collectionId} or delete it manually, then run sync-resources again.`,
);
}
}
async function reconcileModelActions(
context: ModelContext,
entry: ModelLockEntry,
desiredActions: ResolvedAction[],
) {
const { appRoot, client, lockfile, log } = context;
const desiredIds = new Set(desiredActions.map(({ source }) => source.id));
for (const { action, source } of desiredActions) {
const fields = actionCopyFields(source);
const hash = getPayloadFingerprint(fields);
let mapping: ActionLockEntry | undefined = entry.actions.find(
({ sourceActionId }) => sourceActionId === source.id,
);
const copiedAction = mapping
? await orNullOn404(client.getAction(mapping.copiedActionId))
: null;
if (mapping && copiedAction) {
assertOwnedActionCopy(copiedAction, source.id, entry.copiedModelId);
}
if (mapping && !copiedAction) {
entry.actions.splice(entry.actions.indexOf(mapping), 1);
mapping = undefined;
}
if (!mapping || !copiedAction) {
const created = await client.createAction({
...fields,
model_id: entry.copiedModelId,
});
if (!isPositiveInteger(created.id)) {
throw new Error("The Action API did not return a valid action ID.");
}
entry.actions.push({
sourceActionId: source.id,
copiedActionId: created.id,
hash,
});
injectGeneratedId(action, ACTION_DEFINITIONS, created.id);
writeResourceLockfile(appRoot, lockfile);
log(`copied action: action ${source.id} -> action ${created.id}`);
continue;
}
// Fingerprint the copy rather than trusting the lockfile: that also catches
// a copy edited directly in Metabase, which the source hash cannot see.
if (getPayloadFingerprint(actionCopyFields(copiedAction)) !== hash) {
await client.updateAction(mapping.copiedActionId, fields);
mapping.hash = hash;
writeResourceLockfile(appRoot, lockfile);
log(`updated action: action ${mapping.copiedActionId}`);
}
if (action.copiedActionId !== mapping.copiedActionId) {
injectGeneratedId(action, ACTION_DEFINITIONS, mapping.copiedActionId);
log(
`restored action ID: ${action.exportName} -> action ${mapping.copiedActionId}`,
);
}
}
for (const mapping of [...entry.actions]) {
if (desiredIds.has(mapping.sourceActionId)) {
continue;
}
const copiedAction = await orNullOn404(
client.getAction(mapping.copiedActionId),
);
if (copiedAction && copiedAction.model_id !== entry.copiedModelId) {
throw new Error(
`Action ${mapping.copiedActionId} belongs to a removed declaration but no longer hangs off copied model ${entry.copiedModelId}, so it was left untouched. Move it back or delete it manually, then run sync-resources again.`,
);
}
if (copiedAction) {
await client.deleteAction(mapping.copiedActionId);
log(`deleted action: action ${mapping.copiedActionId}`);
}
entry.actions.splice(entry.actions.indexOf(mapping), 1);
writeResourceLockfile(appRoot, lockfile);
}
}
async function reconcileModel(
context: ModelContext,
sourceModel: MetabaseCard,
desiredActions: ResolvedAction[],
) {
const { appRoot, collectionId, client, lockfile, log } = context;
const hash = getPayloadFingerprint(modelCopyInput(sourceModel));
const index = lockfile.models.findIndex(
({ sourceModelId }) => sourceModelId === sourceModel.id,
);
const previous = index >= 0 ? lockfile.models[index] : undefined;
const copy = previous
? await orNullOn404(client.getCard(previous.copiedModelId))
: null;
if (previous && copy) {
assertOwnedModelCopy(copy, collectionId);
// `archived` is deliberately not part of the fingerprint: it describes the
// copy's state, not the payload, and folding it in would invalidate every
// lockfile written before this.
if (
copy.archived === true ||
getPayloadFingerprint(modelCopyInput(copy)) !== hash
) {
await client.updateModel(previous.copiedModelId, {
...modelCopyInput(sourceModel),
collectionId,
});
previous.hash = hash;
writeResourceLockfile(appRoot, lockfile);
log(`updated model: card ${previous.copiedModelId}`);
}
await reconcileModelActions(context, previous, desiredActions);
return;
}
if (previous) {
lockfile.models.splice(index, 1);
}
const created = await client.createModel({
...modelCopyInput(sourceModel),
collectionId,
});
if (!isPositiveInteger(created.id)) {
throw new Error("The Card API did not return a valid model ID.");
}
const entry: ModelLockEntry = {
sourceModelId: sourceModel.id,
copiedModelId: created.id,
hash,
actions: [],
};
lockfile.models.push(entry);
writeResourceLockfile(appRoot, lockfile);
log(
previous
? `recreated model: card ${previous.copiedModelId} -> card ${created.id}`
: `copied model: model ${sourceModel.id} -> card ${created.id}`,
);
await reconcileModelActions(context, entry, desiredActions);
}
async function removeUnusedModels(
context: ModelContext,
previousModels: ModelLockEntry[],
desiredModelIds: Set<number>,
) {
const { appRoot, collectionId, client, lockfile, log } = context;
for (const entry of previousModels) {
if (desiredModelIds.has(entry.sourceModelId)) {
continue;
}
const index = lockfile.models.findIndex(
({ sourceModelId }) => sourceModelId === entry.sourceModelId,
);
if (index < 0) {
continue;
}
const copy = await orNullOn404(client.getCard(entry.copiedModelId));
if (copy?.archived === true) {
// The trash masks a card's collection, so this reads the same as a copy
// moved out and trashed afterwards. Deleting is permanent, and the card is
// already where someone put it to be deleted.
log(`left in the trash: card ${copy.id}`);
} else if (copy) {
assertOwnedModelCopy(copy, collectionId);
// Deleting the copy cascades to the actions copied onto it.
await client.deleteCard(copy.id);
log(`deleted model: card ${copy.id}`);
}
lockfile.models.splice(index, 1);
writeResourceLockfile(appRoot, lockfile);
}
}
/**
* Makes the data app collection hold exactly the models its actions belong to,
* copying a model when its first action appears and deleting it when its last
* action goes away. Returns the tables those models and actions read.
*/
export async function reconcileModels({
appRoot,
slug,
collectionId,
actions,
lockfile,
client,
log,
}: ReconcileModelsOptions) {
const previousModels = [...lockfile.models];
const resolved = await resolveActions(appRoot, actions, client);
const desired = new Map<number, ResolvedAction[]>();
for (const entry of resolved) {
desired.set(entry.source.model_id, [
...(desired.get(entry.source.model_id) ?? []),
entry,
]);
}
const sourceModels = await fetchSourceModels(appRoot, desired, client);
const context: ModelContext = {
appRoot,
collectionId,
client,
lockfile,
log,
};
for (const [sourceModelId, desiredActions] of desired) {
const sourceModel = sourceModels.get(sourceModelId);
if (sourceModel) {
await reconcileModel(context, sourceModel, desiredActions);
}
}
await removeUnusedModels(context, previousModels, new Set(desired.keys()));
const modelQueries = [...sourceModels.values()].map(
(model) => model.dataset_query,
);
const actionQueries = resolved.flatMap(({ source }) =>
source.dataset_query ? [source.dataset_query] : [],
);
return client.resolveTableDependencies(slug, [
...modelQueries,
...actionQueries,
]);
}

32
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/sync.ts
View File

@@ -11,7 +11,7 @@ import {
} from "./lockfile";
import { MetabaseClient, orNullOn404 } from "./metabase-client";
import { reconcileQueries } from "./reconcile";
import { reconcileModels } from "./reconcile-models";
import { reconcileActions } from "./reconcile-actions";
import type { DiscoveredAction, ResourceLockfile } from "./types";
export interface SyncResourcesOptions {
@@ -72,9 +72,7 @@ function checkActionsSynchronized(
lockfile: ResourceLockfile,
) {
const byId = new Map(
lockfile.models.flatMap((model) =>
model.actions.map((entry) => [entry.sourceActionId, entry] as const),
),
lockfile.actions.map((entry) => [entry.sourceActionId, entry] as const),
);
for (const action of actions) {
@@ -86,10 +84,8 @@ function checkActionsSynchronized(
}
const liveIds = new Set(actions.map(({ sourceActionId }) => sourceActionId));
const hasRemovedAction = lockfile.models.some(
(model) =>
model.actions.length === 0 ||
model.actions.some(({ sourceActionId }) => !liveIds.has(sourceActionId)),
const hasRemovedAction = lockfile.actions.some(
({ sourceActionId }) => !liveIds.has(sourceActionId),
);
if (hasRemovedAction) {
@@ -131,7 +127,6 @@ async function moveCopiesToAppCollection(
if (previousCollectionId !== undefined) {
const copiedCardIds = [
...lockfile.queries.map((entry) => entry.savedQuestionSourceId),
...lockfile.models.map((entry) => entry.copiedModelId),
...lockfile.metrics.map((entry) => entry.copiedMetricId),
];
@@ -145,6 +140,21 @@ async function moveCopiesToAppCollection(
await client.moveCardToCollection(cardId, collectionId);
log(`moved card ${cardId} into data app collection ${collectionId}`);
}
for (const { copiedActionId } of lockfile.actions) {
const action = await orNullOn404(client.getAction(copiedActionId));
if (action?.collection_id !== previousCollectionId) {
continue;
}
await client.updateAction(copiedActionId, {
collection_id: collectionId,
});
log(
`moved action ${copiedActionId} into data app collection ${collectionId}`,
);
}
}
lockfile.collectionId = collectionId;
@@ -191,7 +201,7 @@ export async function syncResources({
client,
log,
});
const modelTableIds = await reconcileModels({
const actionTableIds = await reconcileActions({
appRoot,
slug,
collectionId: app.resource_collection_id,
@@ -201,7 +211,7 @@ export async function syncResources({
log,
});
const tableIds = [...new Set([...queryTableIds, ...modelTableIds])].sort(
const tableIds = [...new Set([...queryTableIds, ...actionTableIds])].sort(
(a, b) => a - b,
);

65
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/tests/lockfile.unit.spec.ts
View File

@@ -17,21 +17,8 @@ function query(savedQuestionSourceId: number) {
return { tableId: 1, hash: FAKE_HASH, savedQuestionSourceId };
}
function model(
sourceModelId: number,
copiedModelId: number,
actions: Array<[number, number]> = [],
) {
return {
sourceModelId,
copiedModelId,
hash: FAKE_HASH,
actions: actions.map(([sourceActionId, copiedActionId]) => ({
sourceActionId,
copiedActionId,
hash: FAKE_HASH,
})),
};
function action(sourceActionId: number, copiedActionId: number) {
return { sourceActionId, copiedActionId, hash: FAKE_HASH };
}
describe("resource lockfile", () => {
@@ -40,16 +27,16 @@ describe("resource lockfile", () => {
it("reads an absent lockfile as empty", () => {
expect(readResourceLockfile(makeApp())).toEqual({
queries: [],
models: [],
actions: [],
metrics: [],
});
});
it("round-trips queries and models", () => {
it("round-trips queries and actions", () => {
const appRoot = makeApp();
const lockfile: ResourceLockfile = {
queries: [query(40)],
models: [model(5, 80, [[51, 91]])],
actions: [action(51, 91)],
metrics: [],
};
write(appRoot, lockfile);
@@ -57,13 +44,13 @@ describe("resource lockfile", () => {
expect(readResourceLockfile(appRoot)).toEqual(lockfile);
});
it("defaults a missing models key rather than failing", () => {
it("defaults a missing actions key rather than failing", () => {
const appRoot = makeApp();
write(appRoot, { queries: [query(40)] });
expect(readResourceLockfile(appRoot)).toEqual({
queries: [query(40)],
models: [],
actions: [],
metrics: [],
});
});
@@ -73,7 +60,7 @@ describe("resource lockfile", () => {
it.each([
["unparseable JSON", "{ not json", "Could not read"],
// A bare array is the shape no version writes; accepting it would let a
// hand-edited file silently drop every model entry.
// hand-edited file silently drop every action entry.
["a bare array", [query(40)], "contains an invalid entry"],
[
"a malformed hash",
@@ -85,18 +72,10 @@ describe("resource lockfile", () => {
{ queries: [{ ...query(40), savedQuestionSourceId: 1.5 }] },
"contains an invalid entry",
],
[
"a model entry missing its actions",
{ queries: [], models: [{ sourceModelId: 5, copiedModelId: 80 }] },
"contains an invalid model entry",
],
[
"a malformed action mapping",
{
queries: [],
models: [{ ...model(5, 80), actions: [{ sourceActionId: 51 }] }],
},
"contains an invalid model entry",
{ queries: [], actions: [{ sourceActionId: 51 }] },
"contains an invalid action entry",
],
[
"a duplicate saved question ID",
@@ -104,29 +83,13 @@ describe("resource lockfile", () => {
"duplicate saved question ID",
],
[
"a duplicate source model ID",
{ queries: [], models: [model(5, 80), model(5, 81)] },
"duplicate source model ID",
],
[
"a duplicate copied model ID",
{ queries: [], models: [model(5, 80), model(6, 80)] },
"duplicate copied model ID",
],
[
"a source action ID claimed by two models",
{
queries: [],
models: [model(5, 80, [[51, 91]]), model(6, 81, [[51, 92]])],
},
"a duplicate source action ID",
{ queries: [], actions: [action(51, 91), action(51, 92)] },
"duplicate source action ID",
],
[
"a copied action ID claimed by two models",
{
queries: [],
models: [model(5, 80, [[51, 91]]), model(6, 81, [[52, 91]])],
},
"a duplicate copied action ID",
{ queries: [], actions: [action(51, 91), action(52, 91)] },
"duplicate copied action ID",
],
])("rejects %s", (_name, value, message) => {

268
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/tests/reconcile-actions.unit.spec.ts Normal file
View File

@@ -0,0 +1,268 @@
import fs from "node:fs";
import { discoverActions } from "../discover";
import { MetabaseApiError, MetabaseClient } from "../metabase-client";
import { reconcileActions } from "../reconcile-actions";
import type { MetabaseAction, ResourceLockfile } from "../types";
import {
FAKE_HASH,
makeApp,
setupResourceSyncTests,
writeAction,
} from "./setup";
const COLLECTION_ID = 35;
const SOURCE: MetabaseAction = {
id: 51,
name: "Ship order",
type: "query",
model_id: null,
collection_id: 7,
description: null,
parameters: [{ id: "id", type: "number/=" }],
visualization_settings: {},
dataset_query: { database: 1, stages: [] },
database_id: 1,
};
const notFound = () => new MetabaseApiError(404, "Not found");
const createMockClient = () => {
const client = new MetabaseClient("http://localhost:3000", "test-key");
return Object.assign(client, {
getAction: jest.spyOn(client, "getAction"),
createAction: jest.spyOn(client, "createAction"),
updateAction: jest.spyOn(client, "updateAction").mockResolvedValue(SOURCE),
deleteAction: jest.spyOn(client, "deleteAction").mockResolvedValue(),
resolveTableDependencies: jest
.spyOn(client, "resolveTableDependencies")
.mockResolvedValue([4]),
});
};
function emptyLockfile(): ResourceLockfile {
return { queries: [], actions: [], metrics: [] };
}
async function run(
appRoot: string,
lockfile: ResourceLockfile,
client: ReturnType<typeof createMockClient>,
) {
const log = jest.fn();
const tableIds = await reconcileActions({
appRoot,
slug: "orders",
collectionId: COLLECTION_ID,
actions: await discoverActions(appRoot),
lockfile,
client,
log,
});
return { tableIds, log };
}
describe("action reconciliation", () => {
setupResourceSyncTests();
it("copies a declared action into the app collection and injects the copy's id", async () => {
const appRoot = makeApp();
const file = writeAction(
appRoot,
`export const Ship = defineAction({ action: { id: 51, parameters: [] } });`,
);
const client = createMockClient();
client.getAction.mockResolvedValueOnce(SOURCE);
client.createAction.mockResolvedValue({ ...SOURCE, id: 91 });
const lockfile = emptyLockfile();
const { tableIds } = await run(appRoot, lockfile, client);
expect(client.createAction).toHaveBeenCalledWith({
name: "Ship order",
type: "query",
description: null,
visualization_settings: {},
parameters: [{ id: "id", type: "number/=" }],
dataset_query: { database: 1, stages: [] },
database_id: 1,
collection_id: COLLECTION_ID,
});
expect(lockfile.actions).toEqual([
{ sourceActionId: 51, copiedActionId: 91, hash: expect.any(String) },
]);
expect(fs.readFileSync(file, "utf8")).toContain("copiedActionId: 91");
expect(client.resolveTableDependencies).toHaveBeenCalledWith("orders", [
SOURCE.dataset_query,
]);
expect(tableIds).toEqual([4]);
});
it("refuses an action that belongs to a model", async () => {
const appRoot = makeApp();
writeAction(
appRoot,
`export const Ship = defineAction({ action: { id: 51, parameters: [] } });`,
);
const client = createMockClient();
client.getAction.mockResolvedValueOnce({ ...SOURCE, model_id: 80 });
await expect(run(appRoot, emptyLockfile(), client)).rejects.toThrow(
"which is not a query action without a model",
);
expect(client.createAction).not.toHaveBeenCalled();
});
it("updates a copy that drifted from its source", async () => {
const appRoot = makeApp();
writeAction(
appRoot,
`export const Ship = defineAction({ copiedActionId: 91, action: { id: 51, parameters: [] } });`,
);
const client = createMockClient();
client.getAction.mockResolvedValueOnce(SOURCE).mockResolvedValueOnce({
...SOURCE,
id: 91,
name: "Old name",
collection_id: COLLECTION_ID,
});
const lockfile: ResourceLockfile = {
...emptyLockfile(),
actions: [{ sourceActionId: 51, copiedActionId: 91, hash: FAKE_HASH }],
};
await run(appRoot, lockfile, client);
expect(client.updateAction).toHaveBeenCalledWith(
91,
expect.objectContaining({ name: "Ship order" }),
);
expect(client.createAction).not.toHaveBeenCalled();
});
it("recreates a copy that was deleted in Metabase", async () => {
const appRoot = makeApp();
writeAction(
appRoot,
`export const Ship = defineAction({ copiedActionId: 91, action: { id: 51, parameters: [] } });`,
);
const client = createMockClient();
client.getAction
.mockResolvedValueOnce(SOURCE)
.mockRejectedValueOnce(notFound());
client.createAction.mockResolvedValue({ ...SOURCE, id: 92 });
const lockfile: ResourceLockfile = {
...emptyLockfile(),
actions: [{ sourceActionId: 51, copiedActionId: 91, hash: FAKE_HASH }],
};
await run(appRoot, lockfile, client);
expect(lockfile.actions).toEqual([
{ sourceActionId: 51, copiedActionId: 92, hash: expect.any(String) },
]);
});
it("leaves a copy that moved out of the app collection untouched", async () => {
const appRoot = makeApp();
writeAction(
appRoot,
`export const Ship = defineAction({ copiedActionId: 91, action: { id: 51, parameters: [] } });`,
);
const client = createMockClient();
client.getAction
.mockResolvedValueOnce(SOURCE)
.mockResolvedValueOnce({ ...SOURCE, id: 91, collection_id: 8 });
const lockfile: ResourceLockfile = {
...emptyLockfile(),
actions: [{ sourceActionId: 51, copiedActionId: 91, hash: FAKE_HASH }],
};
await expect(run(appRoot, lockfile, client)).rejects.toThrow(
"is no longer in data app collection 35",
);
expect(client.updateAction).not.toHaveBeenCalled();
});
it("deletes the copy of an action whose declaration was removed", async () => {
const appRoot = makeApp();
const client = createMockClient();
client.getAction.mockResolvedValueOnce({
...SOURCE,
id: 91,
collection_id: COLLECTION_ID,
});
const lockfile: ResourceLockfile = {
...emptyLockfile(),
actions: [{ sourceActionId: 51, copiedActionId: 91, hash: FAKE_HASH }],
};
await run(appRoot, lockfile, client);
expect(client.deleteAction).toHaveBeenCalledWith(91);
expect(lockfile.actions).toEqual([]);
});
it("refuses a declared action that is one of the app's own copies", async () => {
const appRoot = makeApp();
writeAction(
appRoot,
`export const Ship = defineAction({ action: { id: 91, parameters: [] } });`,
);
const client = createMockClient();
client.getAction.mockResolvedValueOnce({
...SOURCE,
id: 91,
collection_id: COLLECTION_ID,
});
await expect(run(appRoot, emptyLockfile(), client)).rejects.toThrow(
"which is a copy synchronized into this data app",
);
expect(client.createAction).not.toHaveBeenCalled();
expect(client.deleteAction).not.toHaveBeenCalled();
});
it("adopts the copy a declaration names when its lockfile entry was lost", async () => {
const appRoot = makeApp();
writeAction(
appRoot,
`export const Ship = defineAction({ copiedActionId: 91, action: { id: 51, parameters: [] } });`,
);
const client = createMockClient();
client.getAction.mockResolvedValueOnce(SOURCE).mockResolvedValueOnce({
...SOURCE,
id: 91,
collection_id: COLLECTION_ID,
});
const lockfile = emptyLockfile();
await run(appRoot, lockfile, client);
expect(client.createAction).not.toHaveBeenCalled();
expect(lockfile.actions).toEqual([
{ sourceActionId: 51, copiedActionId: 91, hash: expect.any(String) },
]);
});
it("leaves a copy that moved out of the app collection when its declaration is removed", async () => {
const appRoot = makeApp();
const client = createMockClient();
client.getAction.mockResolvedValueOnce({
...SOURCE,
id: 91,
collection_id: 8,
});
const lockfile: ResourceLockfile = {
...emptyLockfile(),
actions: [{ sourceActionId: 51, copiedActionId: 91, hash: FAKE_HASH }],
};
await expect(run(appRoot, lockfile, client)).rejects.toThrow(
"is no longer in data app collection 35",
);
expect(client.deleteAction).not.toHaveBeenCalled();
});
});

16
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/tests/reconcile-metrics.unit.spec.ts
View File

@@ -35,7 +35,7 @@ describe("metric reconciliation", () => {
const lockfile: ResourceLockfile = {
queries: [],
models: [],
actions: [],
metrics: [{ sourceMetricId: 251, copiedMetricId: 404, hash: HASH }],
};
@@ -80,7 +80,7 @@ describe("metric reconciliation", () => {
const lockfile: ResourceLockfile = {
queries: [],
models: [],
actions: [],
metrics: [],
};
@@ -145,7 +145,7 @@ describe("metric reconciliation", () => {
const lockfile: ResourceLockfile = {
queries: [],
models: [],
actions: [],
metrics: [{ sourceMetricId: 251, copiedMetricId: 404, hash: HASH }],
};
@@ -195,7 +195,7 @@ describe("metric reconciliation", () => {
const lockfile: ResourceLockfile = {
queries: [],
models: [],
actions: [],
metrics: [{ sourceMetricId: 251, copiedMetricId: 404, hash: HASH }],
};
@@ -250,7 +250,7 @@ describe("metric reconciliation", () => {
metrics: [],
},
],
lockfile: { queries: [], models: [], metrics: [] },
lockfile: { queries: [], actions: [], metrics: [] },
client,
log: jest.fn(),
}),
@@ -295,7 +295,7 @@ describe("metric reconciliation", () => {
],
},
],
lockfile: { queries: [], models: [], metrics: [] },
lockfile: { queries: [], actions: [], metrics: [] },
client,
log: jest.fn(),
}),
@@ -321,7 +321,7 @@ describe("metric reconciliation", () => {
const lockfile: ResourceLockfile = {
queries: [],
models: [],
actions: [],
metrics: [{ sourceMetricId: 251, copiedMetricId: 404, hash: HASH }],
};
@@ -372,7 +372,7 @@ describe("metric reconciliation", () => {
lockfile = {
queries: [],
models: [],
actions: [],
metrics: [{ sourceMetricId: 251, copiedMetricId: 404, hash: HASH }],
};
});

2
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/tests/setup.ts
View File

@@ -78,7 +78,7 @@ export function writeQueryLockfile(
) {
fs.writeFileSync(
path.join(appRoot, "resources_metadata.json"),
JSON.stringify({ queries, models: [] }),
JSON.stringify({ queries, actions: [] }),
);
}

21
enterprise/frontend/src/embedding-sdk-package/data-app-query-sync/types.ts
View File

@@ -10,18 +10,6 @@ export interface ActionLockEntry {
hash: string;
}
/**
* A model copied into the data app collection. `actions` is both the
* reference count that keeps the copy alive and the mapping the app's
* definitions address.
*/
export interface ModelLockEntry {
sourceModelId: number;
copiedModelId: number;
hash: string;
actions: ActionLockEntry[];
}
export interface MetricLockEntry {
sourceMetricId: number;
copiedMetricId: number;
@@ -32,7 +20,7 @@ export interface ResourceLockfile {
/** Where the copies were last synchronized, so a moved app reads apart from a moved copy. */
collectionId?: number;
queries: QueryLockEntry[];
models: ModelLockEntry[];
actions: ActionLockEntry[];
metrics: MetricLockEntry[];
}
@@ -78,16 +66,13 @@ export interface MetabaseAction {
id: number;
name: string;
type: string;
model_id: number;
model_id: number | null;
collection_id?: number | null;
archived?: boolean;
description?: string | null;
parameters?: unknown[] | null;
parameter_mappings?: Record<string, unknown> | null;
visualization_settings?: Record<string, unknown> | null;
/** Present on implicit actions. */
kind?: string | null;
/** Present on query actions. */
dataset_query?: Record<string, unknown> | null;
/** Present on query actions. */
database_id?: number | null;
}

14
enterprise/frontend/src/embedding-sdk-package/hooks/public/use-metabase-query/tests/fixtures.ts
View File

@@ -160,15 +160,12 @@ export const TEST_SCHEMA = {
sourceCardId: 41,
},
},
models: {
orders: {
actions: {
create: {
createOrder: {
kind: "action" as const,
id: 51,
name: "Create Order",
type: "implicit" as const,
implicitKind: "row/create" as const,
type: "query" as const,
parameters: [
{
slug: "status",
@@ -177,12 +174,11 @@ export const TEST_SCHEMA = {
},
],
},
update: {
updateOrder: {
kind: "action" as const,
id: 52,
name: "Update Order",
type: "implicit" as const,
implicitKind: "row/update" as const,
type: "query" as const,
parameters: [
{
slug: "id",
@@ -193,8 +189,6 @@ export const TEST_SCHEMA = {
],
},
},
},
},
questions: {
// Generated question columns carry only `type`, `name`, and `jsType`; the
// display name is rendered as a comment, not as runtime metadata.

12
enterprise/frontend/src/embedding-sdk-package/hooks/public/use-metabase-query/tests/type-contract-invalid.ts
View File

@@ -31,7 +31,7 @@ defineQuery({
const actionWithInvalidActionSourceId = {
copiedActionId: "91",
action: TEST_SCHEMA.models.orders.actions.create,
action: TEST_SCHEMA.actions.createOrder,
} as const;
// @ts-expect-error generated action source IDs are numeric
@@ -41,11 +41,11 @@ defineAction(actionWithInvalidActionSourceId);
defineAction({ action: TEST_SCHEMA.tables.orders });
const CreateOrder = defineAction({
action: TEST_SCHEMA.models.orders.actions.create,
action: TEST_SCHEMA.actions.createOrder,
});
const UpdateOrder = defineAction({
action: TEST_SCHEMA.models.orders.actions.update,
action: TEST_SCHEMA.actions.updateOrder,
});
// --------
@@ -150,7 +150,7 @@ function InvalidTypeFixtures() {
useMetabaseQueryObject({ ...staticQuery, limit: 10 });
// @ts-expect-error a data app's action is a `defineAction` export from `actions/`, never an inline object
useDataAppAction({ action: TEST_SCHEMA.models.orders.actions.create });
useDataAppAction({ action: TEST_SCHEMA.actions.createOrder });
// @ts-expect-error a copy of a definition is not the definition
useDataAppAction({ ...CreateOrder });
@@ -159,10 +159,10 @@ function InvalidTypeFixtures() {
useDataAppAction(51);
// @ts-expect-error pass the `defineAction` export, not the schema entry
useDataAppAction(TEST_SCHEMA.models.orders.actions.create);
useDataAppAction(TEST_SCHEMA.actions.createOrder);
// @ts-expect-error the SDK hook takes a definition object or an id, not the schema entry
useAction(TEST_SCHEMA.models.orders.actions.create);
useAction(TEST_SCHEMA.actions.createOrder);
// @ts-expect-error the definition's parameter slugs are the only keys
useAction(CreateOrder).execute({ stauts: "shipped" });

18
enterprise/frontend/src/embedding-sdk-package/hooks/public/use-metabase-query/tests/type-contract-valid.ts
View File

@@ -29,7 +29,7 @@ const _queryLimit: 10 = revenueQuery.limit;
const CreateOrder = defineAction({
copiedActionId: 91,
action: TEST_SCHEMA.models.orders.actions.create,
action: TEST_SCHEMA.actions.createOrder,
});
const _copiedActionId: 91 = CreateOrder.copiedActionId;
@@ -37,7 +37,7 @@ const _sourceActionId: 51 = CreateOrder.action.id;
// A definition is authored without a generated ID; synchronization writes one.
const UpdateOrder = defineAction({
action: TEST_SCHEMA.models.orders.actions.update,
action: TEST_SCHEMA.actions.updateOrder,
});
// --------
@@ -70,10 +70,10 @@ function ValidTypeFixtures() {
void createOrder.execute({ status: "shipped" });
const createdRow: RowValue | undefined =
createOrder.result?.["created-row"].ID;
const createdCount: number | undefined =
createOrder.result?.["rows-affected"];
void createdRow;
void createdCount;
const updateOrder = useDataAppAction(UpdateOrder);
@@ -81,16 +81,16 @@ function ValidTypeFixtures() {
// The SDK hook takes a plain definition object, and a `defineAction` export too.
const sdkCreateOrder = useAction({
action: TEST_SCHEMA.models.orders.actions.create,
action: TEST_SCHEMA.actions.createOrder,
});
void sdkCreateOrder.execute({ status: "shipped" });
void useAction(CreateOrder).execute({ status: "shipped" });
const updatedRows: readonly RowValue[] | undefined =
updateOrder.result?.["rows-updated"];
const updatedCount: number | undefined =
updateOrder.result?.["rows-affected"];
void updatedRows;
void updatedCount;
// A raw id types nothing, so the generics still stand in for a definition.
const rawAction = useAction<{ status: string }, "create">(51);

2
frontend/src/embedding-sdk-bundle/lib/execute-action.ts
View File

@@ -53,7 +53,7 @@ const isActionDefinition = (
/**
* The action that actually runs. Outside the dev preview the synchronized copy
* replaces the authored action: the copy is what grants an app's viewers
* permission to run it, through the collection its model lives in.
* permission to run it, through the app's collection.
*/
function toExecutableActionId(input: SdkActionInput): SdkActionId {
if (!isActionDefinition(input)) {

34
skills/metabase-data-app-actions/SKILL.md
View File

@@ -5,24 +5,19 @@ description: Use when a Metabase data app needs to trigger a write or mutation
# Triggering actions from a Metabase data app
A Metabase **action** is a server-defined write operation against the data warehouse — either a basic CRUD operation (insert / update / delete) on a model, or a custom SQL command. Actions are configured ahead of time on the Metabase instance, with their parameters, model bindings, and permissions already set. A data app's job is to **invoke** an action with the right parameters when the user does something — clicking a button, submitting a form, confirming a destructive prompt.
A Metabase **action** is a saved, parameterized SQL write against the data warehouse — an `INSERT`, `UPDATE`, or `DELETE`. Actions are configured ahead of time on the Metabase instance, with their parameters and permissions already set. A data app's job is to **invoke** an action with the right parameters when the user does something — clicking a button, submitting a form, confirming a destructive prompt.
## The mental model
Actions belong to a model. They mutate that model's rows. Concretely:
- Every action has a parent model. In the schema it appears as `schema.models.<modelName>.actions.<actionName>`.
- Model entries are included here only to catalog actions. Do not render models as questions, pass model ids to `InteractiveQuestion`, or fetch model rows; use semantic-layer queries/questions for read views.
- An action's `type` is either `"implicit"` (CRUD on the model) or `"query"` (custom SQL the user authored).
- Implicit actions have an `implicitKind` that says what they do: `"row/create"`, `"row/update"`, `"row/delete"`, or `"bulk/*"` variants.
- Every action the app can run appears in the schema as `schema.actions.<actionName>`, with `type: "query"`.
- Each action publishes a `parameters` list. Each parameter has a `slug` (the key the Data App sends), a `jsType` (`"string"` / `"number"` / `"Date"` / `"boolean"` / `"unknown"`), and an optional `required` flag.
- Use `action.parameters` to know which fields to render and submit. A create result may include `result["created-row"]`, but that row is only typed as `Record<string, RowValue>`; use it for lightweight confirmation, then refresh the existing table/question/query data already used by the page. Do not fetch or render the parent model itself.
- Use `action.parameters` to know which fields to render and submit. The result reports `result["rows-affected"]`; use it for lightweight confirmation, then refresh the table/question/query data the page already shows.
## What's in the schema (and what isn't)
Action entries are only generated when the typed schema includes models. Before writing action-invoking code, make sure the schema was generated with `include-models=true`; with `database=<name-or-id>&include-models=true`, Metabase includes models/actions for that database only. Do not rely on `question-collections` for actions; question collections only add saved questions.
Action entries are only generated when the typed schema includes actions. Before writing action-invoking code, make sure the schema was generated with `include-actions=true`; with `database=<name-or-id>&include-actions=true`, Metabase includes the actions for that database only.
Before writing any action-invoking code, look at the schema and enumerate what's available under `schema.models.<m>.actions` across the models the app cares about. The schema is your **complete** catalog of actions for the instance, not a catalog of model data to display. If a model's `actions` entry has `create`, `update`, `delete`, those are the actions invokable. Anything not present doesn't exist as far as the Data App is concerned.
Before writing any action-invoking code, enumerate what's available under `schema.actions`. The schema is your **complete** catalog of the actions the app can run. Anything not present doesn't exist as far as the Data App is concerned: when the app needs a write the schema lacks, ask the user to create it as a query action without a model (`POST /api/action` with `type: "query"` and no `model_id`; actions created in the Metabase UI belong to a model and are never listed), on a database with actions enabled, then regenerate the schema.
## The hook
@@ -33,8 +28,8 @@ const { execute, isExecuting, result, error, reset } = useAction(MyAction);
```
- **Import it from `@metabase/embedding-sdk-react/data-app`.** That entry's `useAction` is the data-app form: it accepts a `defineAction(...)` export or `null` and nothing else. The main entry's `useAction` is the general SDK hook, which also takes plain objects and raw ids, so it cannot enforce the definition; a data app that imports it from the main entry compiles with an unsynchronized action and fails in production.
- **The argument** is the `defineAction(...)` export itself, declared in the app's root-level `actions/` directory (one `<topic>.action.ts` file per topic, beside `package.json`, never under `src/`) — pass `MyAction`, not `MyAction.copiedActionId`. The hook accepts nothing else in a data app: an inline `{ action: ... }` object, the schema entry, or a spread copy of a definition fails to compile with `Property 'definedWithDefineAction' is missing`. Fix that by adding the export to `actions/`, not with a cast and not by calling `defineAction(...)` at the hook, which compiles but is never synchronized. A production build then runs the synchronized copy, whose model sits in the app's own collection; the dev preview keeps running the authored action, so an app works before it has ever been synchronized. Do **not** pass `schema.models.<model>.actions.<action>` or its `.id`: the entry is a compile error, and the raw id belongs to the original model, which the app's users cannot read, so the call fails on permissions in production.
- **Don't write the generics.** The definition carries its schema entry, so the hook infers both the parameters object and the discriminated `result` from it: `parameters[]` becomes a keyed object (`required: true` entries are required keys, each value typed from its `jsType`), and `implicitKind` / `type` become the kind (`"row/create"` → `"create"`, `"row/update"` → `"update"`, `"row/delete"` → `"delete"`, any `"bulk/*"` → `"bulk"`, `type === "query"` → `"sql"`). The raw-id form exists only on the SDK's `useAction` from the main entry, where `useAction<TParameters, TKind>(42)` needs them spelled out since an id describes nothing; a data app never names an action by id.
- **The argument** is the `defineAction(...)` export itself, declared in the app's root-level `actions/` directory (one `<topic>.action.ts` file per topic, beside `package.json`, never under `src/`) — pass `MyAction`, not `MyAction.copiedActionId`. The hook accepts nothing else in a data app: an inline `{ action: ... }` object, the schema entry, or a spread copy of a definition fails to compile with `Property 'definedWithDefineAction' is missing`. Fix that by adding the export to `actions/`, not with a cast and not by calling `defineAction(...)` at the hook, which compiles but is never synchronized. A production build then runs the synchronized copy in the app's own collection; the dev preview keeps running the authored action, so an app works before it has ever been synchronized. Do **not** pass `schema.actions.<action>` or its `.id`: the entry is a compile error, and the raw id names the authored action, which the app's users cannot read, so the call fails on permissions in production.
- **Don't write the generics.** The definition carries its schema entry, so the hook infers both the parameters object and the discriminated `result` from it: `parameters[]` becomes a keyed object (`required: true` entries are required keys, each value typed from its `jsType`), and `type: "query"` makes the kind `"sql"`. The raw-id form exists only on the SDK's `useAction` from the main entry, where `useAction<TParameters, TKind>(42)` needs them spelled out since an id describes nothing; a data app never names an action by id.
- **`execute(parameters)`** — triggers the action. Parameters object is keyed by parameter `slug`; parameters declared `required: true` are required keys, everything else optional. Returns the response body on success AND throws on failure (the error is also written to `error` state for render-time consumers). Resolves to `null` (without making a request) when `actionId` is `null` or the SDK is not yet initialized — guard the call site if those cases are reachable.
- **No `enabled` / `options` argument.** The hook only ever runs when `execute(...)` is called, so a gate option would be redundant. Skip the action by branching in the event handler:
```ts
@@ -44,7 +39,7 @@ const { execute, isExecuting, result, error, reset } = useAction(MyAction);
};
```
- **`isExecuting`** — `true` between the call and its resolution. Drive button `disabled` from this so the user can't double-click into duplicate requests.
- **`result`** — the response body, discriminated by `TKind` (or the `AnyActionResult` union when `TKind` is omitted). `null` before the first call and after `reset()`. Use it for lightweight confirmation (`result?.["created-row"]` after an insert, `result?.["rows-affected"]` after a SQL action), but do not treat create rows as richly typed model rows; refresh surrounding data instead — see *After an action runs*.
- **`result`** — the response body, discriminated by `TKind` (or the `AnyActionResult` union when `TKind` is omitted). `null` before the first call and after `reset()`. Use it for lightweight confirmation (`result?.["rows-affected"]`), then refresh surrounding data — see *After an action runs*.
- **`error`** — the last thrown error, typed `ActionExecuteError | null`. Read fields directly with no cast: `error?.data?.message`, `error?.status`, `error?.isCancelled`.
- **`reset()`** — clears `result` and `error` back to `null`. Useful after the user acknowledges success or dismisses an error.
@@ -90,7 +85,7 @@ function AddPersonForm({ onCreated }: { onCreated: () => void }) {
## Showing the error message
When `execute(...)` fails, surface both error layers. The hook's `error` is typed `ActionExecuteError | null` — shape `{ status?, data: { message?, errors? }, isCancelled }`. `error.data.message` is the whole-request failure. `error.data.errors` is a per-field validation map keyed by parameter slug (`{ <slug>: <message> }`), or `{}` for whole-request failures. Read both directly, no cast:
When `execute(...)` fails, surface both error layers. The hook's `error` is typed `ActionExecuteError | null` — shape `{ status?, data: { message?, errors? }, isCancelled }`. `error.data.message` is the whole-request failure. `error.data.errors` is a per-field validation map keyed by parameter slug (`{ <slug>: <message> }`), or `{}` for whole-request failures — which is every failure of the SQL itself. Read both directly, no cast:
```tsx
const fieldErrors = error?.data.errors ?? {};
@@ -159,7 +154,7 @@ Each `parameters[]` entry on a schema action exposes `slug`, `displayName`, `jsT
- **`required: true` parameters cannot be omitted.** Reflected in the TS type: required keys are required.
- **`displayName`** is for labels; never use it as a key.
For implicit actions, the slugs match the model's column names (slugified). For custom SQL actions, slugs are whatever the action's author named the SQL parameters in Metabase. Either way, the schema is the source of truth.
Slugs are whatever the action's author named the SQL parameters in Metabase; the schema is the source of truth.
## Form-side validation — match the main app
@@ -188,17 +183,16 @@ When an action appears to succeed but the screen doesn't update, or a call fails
1. Log `result`, `error`, and `isExecuting` after `await execute(...)` to confirm the request actually went through and succeeded.
2. Confirm `useAction` was called with the `defineAction(...)` export imported from `actions/`. Passing the schema entry, an inline object, or a spread copy is a compile error; passing its `.id` compiles but leaves `execute` untyped and 403s in production, since the authored action is not the one an app's users can run. Calling `defineAction(...)` inside the component compiles too, but `sync-resources` scans only `actions/`, so that action is never copied and also 403s.
3. Log the object passed to `execute({ ... })`. Every key must match a parameter `slug` from `schema.models.<model>.actions.<action>.parameters`; every value must match its declared `jsType`.
4. List every data view on the screen that reads from the mutated model. Confirm each one's data hook is mounted ABOVE the action trigger so its refresh callback can be passed down.
3. Log the object passed to `execute({ ... })`. Every key must match a parameter `slug` from `schema.actions.<action>.parameters`; every value must match its declared `jsType`.
4. List every data view on the screen that reads the rows the action writes. Confirm each one's data hook is mounted ABOVE the action trigger so its refresh callback can be passed down.
5. Confirm the refresh callback is called AFTER `await execute(...)` AND that the refresh itself is awaited. When multiple refreshes apply, confirm they're awaited together (`Promise.all`).
6. For implicit actions, confirm the right `implicitKind` matches what you intend (`row/create` vs `row/update` vs `row/delete`) — picking the wrong action publishes the wrong write.
7. If the schema doesn't list an action you expect, the action isn't defined on the instance or the schema file is stale. Regenerate the schema.
6. If the schema doesn't list an action you expect, the action isn't defined on the instance or the schema file is stale. Regenerate the schema.
## Common Mistakes
- Importing `useAction` from `@metabase/embedding-sdk-react` instead of `@metabase/embedding-sdk-react/data-app`. The main entry's hook is the general SDK one and accepts anything, so the definition check never runs.
- Declaring the action anywhere but root-level `actions/`: inline at the hook, wrapped in `defineAction(...)` inside a component, or under `src/actions/`. Only `actions/` is synchronized.
- Passing `schema.models.<model>.actions.<action>.id` to `useAction` instead of the `defineAction(...)` export. `execute` then accepts any `Record<string, unknown>`, typos like `{ wrongKey: 1 }` slip through, and production 403s because the authored action is not the one an app's users can run.
- Passing `schema.actions.<action>.id` to `useAction` instead of the `defineAction(...)` export. `execute` then accepts any `Record<string, unknown>`, typos like `{ wrongKey: 1 }` slip through, and production 403s because the authored action is not the one an app's users can run.
- Forgetting to refresh after a successful action. The UI keeps rendering stale data with no error or warning.
- Rendering `"Failed"` / `"Something went wrong"` / `String(error)` instead of the real backend message. Always extract `error.data.message` / `error.data.errors` (the diagnostic the user needs is in there) and render it verbatim — see *Showing the error message*.
- Calling the refresh callback without `await`ing it. The modal dismisses or the form clears before fresh data arrives, leaving the user staring at the stale view for a beat.

18
skills/metabase-data-app-semantic-layer/SKILL.md
View File

@@ -58,14 +58,14 @@ Scope parameters:
- `include-data-library=true` for the whole `Library / Data` tree.
- `include-metric-library=true` for the whole `Library / metrics` tree.
- `library-collections=<id-or-entity-id>[,<id-or-entity-id>]` for specific Data or metrics library subcollections.
- `include-models=true` for readable models that have actions. When combined with `database=<name-or-id>`, it includes models with actions for that database only.
- `include-actions=true` for the actions the app can run, under `schema.actions`. When combined with `database=<name-or-id>`, it includes that database's actions only.
- `database=<name-or-id>` when the app should use tables from one database. Use it separately from library scopes; the API rejects that combination.
Combine library scopes when the app needs both tables and curated metrics.
Use `include-models=true` when the app needs any saved action under `schema.models.<model>.actions`; it includes all readable models with executable actions, unless `database` scopes them to one database. Models without executable actions are omitted to keep generated schemas compact. It can be combined with `library-collections`, `include-data-library`, or `include-metric-library` so one schema can include selected tables/metrics plus all readable actions.
Use `include-actions=true` when the app needs any saved action under `schema.actions`; it includes all readable actions, unless `database` scopes them to one database. It can be combined with `library-collections`, `include-data-library`, or `include-metric-library` so one schema can include selected tables/metrics plus all readable actions.
If the user asks for any mutation-like flow, such as creating, updating, deleting, submitting, approving, executing an action, or running a write operation, include `include-models=true` in the typed-schema URL. Do this even when the user names one specific model/action, because actions are only discoverable through generated model entries.
If the user asks for any mutation-like flow, such as creating, updating, deleting, submitting, approving, executing an action, or running a write operation, include `include-actions=true` in the typed-schema URL. Do this even when the user names one specific action, because actions are only discoverable through the generated schema.
The Metabase URL and API key live in the **repo-root** `.env.local` as
`DATA_APP_MB_URL` and `DATA_APP_MB_API_KEY` (one file per repo, usually two levels up
@@ -109,7 +109,7 @@ fi
After a successful export, verify that the schema contains every entity needed for the requested app. If any are missing, revise the scope using available context or ask for the missing context before building the UI.
If schema generation fails while building a selected model or model action, do not hide, paraphrase away, or retry past the error. Surface the typed-schema error to the user, including the failing `card-id` / `card-name` / `card-type`, `model-id` / `model-name`, dropped action ids, and message when present.
If schema generation fails, do not hide, paraphrase away, or retry past the error. Surface the typed-schema error to the user, including the failing ids, names, and message when present.
## Synchronize every query and action
@@ -124,11 +124,11 @@ export const RevenueQuery = defineQuery({ source: schema.tables.orders });
// actions/orders.action.ts
export const CreateOrder = defineAction({
action: schema.models.orders.actions.create,
action: schema.actions.createOrder,
});
```
One `sync-resources` run reconciles both. For a query it materializes the authored table query as a saved question and injects `savedQuestionSourceId`. For an action it copies the action's parent model into the app collection, copies the action onto that copy, and injects `copiedActionId`; a model is copied once no matter how many of its actions the app declares, siblings reuse that copy, and it disappears with the last declaration. Never copy a model into the app collection by hand.
One `sync-resources` run reconciles both. For a query it materializes the authored table query as a saved question and injects `savedQuestionSourceId`. For an action it copies the action into the app collection and injects `copiedActionId`; the copy disappears with its declaration. Never copy an action into the app collection by hand.
Pass the definition itself to the hook and let the SDK resolve what runs — a production build runs the copy, while the dev preview runs the authored table or action, so an app works before its first synchronization:
@@ -140,13 +140,13 @@ const { data } = useMetabaseQuery(RevenueQuery, {
const { execute, isExecuting, error } = useAction(CreateOrder);
```
Never pass an inline table-source query (not even a read-only, filter-option, or helper query), a raw action id, `savedQuestionSourceId`, `copiedActionId`, or a hand-built `{ source: { type: "card", id } }`, and never spread a definition into a new object. Each defeats the swap; the authored ids also bypass the permission boundary. TypeScript rejects most of these: the hooks accept only what `defineQuery`/`defineAction` returned, so an inline object, a `satisfies`-typed object, a spread copy, and `schema.models.<model>.actions.<action>` all fail to compile. When `tsc` reports `Property 'definedWithDefineQuery' is missing` or `Property 'definedWithDefineAction' is missing`, the argument is not a definition: move it into `queries/` or `actions/` and import the export. Do not silence it with a cast or by wrapping the inline object in `defineQuery(...)` at the call site, which compiles but leaves the query unsynchronized. Keep fixed permission-boundary filters, aggregations, and breakouts inside `defineQuery` — synchronization bakes them into the saved question, so don't apply them again outside it, and put runtime clauses in the hook's second argument (see *Static and dynamic query parts*). `useAction` needs no generics: the definition types `execute`'s parameters and `result`.
Never pass an inline table-source query (not even a read-only, filter-option, or helper query), a raw action id, `savedQuestionSourceId`, `copiedActionId`, or a hand-built `{ source: { type: "card", id } }`, and never spread a definition into a new object. Each defeats the swap; the authored ids also bypass the permission boundary. TypeScript rejects most of these: the hooks accept only what `defineQuery`/`defineAction` returned, so an inline object, a `satisfies`-typed object, a spread copy, and `schema.actions.<action>` all fail to compile. When `tsc` reports `Property 'definedWithDefineQuery' is missing` or `Property 'definedWithDefineAction' is missing`, the argument is not a definition: move it into `queries/` or `actions/` and import the export. Do not silence it with a cast or by wrapping the inline object in `defineQuery(...)` at the call site, which compiles but leaves the query unsynchronized. Keep fixed permission-boundary filters, aggregations, and breakouts inside `defineQuery` — synchronization bakes them into the saved question, so don't apply them again outside it, and put runtime clauses in the hook's second argument (see *Static and dynamic query parts*). `useAction` needs no generics: the definition types `execute`'s parameters and `result`.
Wire `package.json` with `"sync-resources": "embedding-sdk-react data-apps sync-resources"` and `"build": "npm run sync-resources && vite build"`, then run `npm run build` after adding, changing, renaming, or removing any definition; run `sync-resources` directly only to inspect generated state before a build. It reads `DATA_APP_MB_URL` and `DATA_APP_MB_API_KEY` from the repo-root `.env.local`.
Inline generated IDs and `resources_metadata.json` are generated state: never delete or hand-edit either. A missing ID is restored automatically when the definition still identifies its resource — a query by its table and authored hash matching one unclaimed lockfile entry, an action by naming the same action — while a duplicated ID fails the run. Do not test or hand off the app until `npm run build` succeeds, every live definition carries a positive generated ID, and `resources_metadata.json` holds its matching entry. Commit every generated change. The build stops before bundling when synchronization fails.
Inline generated IDs and `resources_metadata.json` are generated state: never delete or hand-edit either. A missing ID is restored automatically when the definition still identifies its resource — a query by its table and authored hash matching one unclaimed lockfile entry, an action by declaring the same source action — while a duplicated ID fails the run. Do not test or hand off the app until `npm run build` succeeds, every live definition carries a positive generated ID, and `resources_metadata.json` holds its matching entry. Commit every generated change. The build stops before bundling when synchronization fails.
If synchronization fails, surface the exact error and stop. Fix local shape, serialization, duplicate-ID, or lockfile errors before retrying. A confirmed `404` is recovered automatically; authentication, permission, network, server, collection-ownership, and Card-type failures must not trigger manual Card creation, deletion, ID replacement, or lockfile editing. Treat a successful run that discovers nothing as a failure when the app has queries or actions. Synchronization copies actions but never creates them, so an action the app needs must already exist in Metabase and be picked up by a regenerated schema; if the run reports that actions are not enabled for the database, stop and tell the user to enable them rather than working around it.
If synchronization fails, surface the exact error and stop. Fix local shape, serialization, duplicate-ID, or lockfile errors before retrying. A confirmed `404` is recovered automatically; authentication, permission, network, server, collection-ownership, and Card-type failures must not trigger manual Card or action creation, deletion, ID replacement, or lockfile editing. Treat a successful run that discovers nothing as a failure when the app has queries or actions. Synchronization copies actions but never creates them, so an action the app needs must already exist in Metabase, without a model, and be picked up by a regenerated schema; if the run reports that actions are not enabled for the database, stop and tell the user to enable them rather than working around it.
## Standard pattern

10
skills/metabase-data-app-setup/template/actions/README.md
View File

@@ -14,7 +14,7 @@ import { defineAction } from "@metabase/embedding-sdk-react/data-app";
import schema from "../src/metabase.data";
export const CreateOrder = defineAction({
action: schema.models.orders.actions.create,
action: schema.actions.createOrder,
});
```
@@ -55,11 +55,11 @@ Rules:
(`npm run sync-resources`, run by `npm run build`) scans only `queries/` and
`actions/`, so a definition anywhere else is never synchronized, and the
authored action is refused for the app's viewers in production.
- Actions exist only when the generated schema includes models
(`include-models=true`). Synchronization copies actions; it never creates
them.
- Actions exist only when the generated schema includes actions
(`include-actions=true`). Synchronization copies actions; it never creates
them, and a missing one is created as a query action without a model.
- Pass the export itself to `useAction`. Never pass
`schema.models.<model>.actions.<action>` or its `.id`.
`schema.actions.<action>` or its `.id`.
- `copiedActionId` is written by synchronization. Never add, edit, or remove it
by hand; commit it together with `resources_metadata.json`.
- After `execute` resolves, refresh every query on screen the action could have

1
src/metabase/permissions/core.clj
View File

@@ -41,6 +41,7 @@
at-least-as-permissive?
batch-delete-permissions!
batch-insert-permissions!
data-app-collection-ids
data-app-group-ids
data-app-view-data-permission-level
disable-perms-cache

6
src/metabase/permissions/models/data_permissions.clj
View File

@@ -1342,6 +1342,12 @@
[]
#{})
(defenterprise data-app-collection-ids
"Ids of the collections data apps own, which hold the copies an app runs. OSS has none."
metabase-enterprise.data-apps.models.data-app
[]
#{})
;;; ---------------------------------------- Bulk permission functions ------------------------------------------------
;; These functions set permissions for newly-created entities (groups, databases, tables) using batch SQL operations
;; instead of per-row mutations. They are intended to be called from within a coarse cluster lock.

51
src/metabase/typed_schemas/build.clj
View File

@@ -8,7 +8,6 @@
docstring for the pipeline shape and the separation rules."
(:require
[clojure.set :as set]
[medley.core :as m]
[metabase.system.core :as system]
[metabase.typed-schemas.common :as common]
[metabase.typed-schemas.source :as source]
@@ -47,21 +46,19 @@
[:boolean {:description "Whether to include the root data library."}]]
[:include-metric-library? {:optional true}
[:boolean {:description "Whether to include the root metrics library."}]]
[:include-models? {:optional true}
[:boolean {:description (str "Include models with actions. Database scope filters models; "
"library scope does not. Without a scope, returns models only.")}]]])
[:include-actions? {:optional true}
[:boolean {:description (str "Include query actions that belong to no model. Database scope filters them; "
"library scope does not. Without a scope, returns actions only.")}]]])
(def Items
"Fetched schema entities, ready for pure assembly by [[create-schema]].
Each entry holds entity maps shaped by the `metabase.typed-schemas.schema.*`
builders; `:key` on each entity seeds the generated object keys. `:errors`
describes entities that could not be built (currently only models)."
builders; `:key` on each entity seeds the generated object keys."
[:map {:closed true}
[:models [:sequential :map]]
[:actions [:sequential :map]]
[:tables [:sequential :map]]
[:metrics [:sequential :map]]
[:errors [:sequential :map]]])
[:metrics [:sequential :map]]])
(defn- invalid-options!
[message]
@@ -103,38 +100,35 @@
(fetch-items options source/app-db-source))
([options source]
(let [{:keys [database library-collection-refs
include-data-library? include-metric-library? include-models?]
include-data-library? include-metric-library? include-actions?]
:or {library-collection-refs []
include-data-library? false
include-metric-library? false
include-models? false}} options]
include-actions? false}} options]
(validate-options! (assoc options
:library-collection-refs library-collection-refs
:include-data-library? include-data-library?
:include-metric-library? include-metric-library?
:include-models? include-models?))
:include-actions? include-actions?))
(let [library-scope (source/library-scope source
{:library-collection-refs library-collection-refs
:include-data-library? include-data-library?
:include-metric-library? include-metric-library?})
database-ids (source/database-ids source database)
{model-schemas :models
model-errors :errors} (if include-models?
;; database-ids scopes the models; nil reads all of them
(source/models source database-ids)
{:models [] :errors []})]
action-schemas (if include-actions?
;; database-ids scopes the actions; nil reads all of them
(source/actions source database-ids)
[])]
(if (or library-scope
(and include-models? (nil? database-ids)))
(and include-actions? (nil? database-ids)))
(let [{:keys [tables metrics]} (when library-scope
(library-items source library-scope))]
{:models (vec model-schemas)
{:actions (vec action-schemas)
:tables (vec tables)
:metrics (vec metrics)
:errors (vec model-errors)})
{:models (vec model-schemas)
:metrics (vec metrics)})
{:actions (vec action-schemas)
:tables (source/tables source database-ids nil)
:metrics (source/metrics source database-ids nil)
:errors (vec model-errors)})))))
:metrics (source/metrics source database-ids nil)})))))
(defn create-schema
"Assembles fetched [[Items]] into the semantic schema value rendered by
@@ -144,14 +138,11 @@
to the current time and the configured site URL."
([items]
(create-schema items nil))
([{:keys [models tables metrics errors]} {:keys [generated-at instance-url]}]
(m/assoc-some
([{:keys [actions tables metrics]} {:keys [generated-at instance-url]}]
(array-map
:schemaVersion 2
:generatedAt (str (or generated-at (Instant/now)))
:metabase {:instanceUrl (or instance-url (system/site-url))}
:models (common/keyed-model-map models)
:actions (common/keyed-map actions)
:tables (common/keyed-map tables)
:metrics (common/keyed-map metrics))
;; Omit when empty so healthy responses carry no `errors` key.
:errors (not-empty (vec errors)))))
:metrics (common/keyed-map metrics))))

34
src/metabase/typed_schemas/common.clj
View File

@@ -88,26 +88,32 @@
(cond-> key
(duplicate-key? key->count key) (str (or key-disambiguator table-id id))))
(defn- unique-keys
"Returns `ks` with every key that is still shared by several `entities` suffixed with its entity id until all are
unique, so no entity overwrites another."
[entities ks]
(let [key->count (frequencies ks)]
(if (every? #(= 1 %) (vals key->count))
ks
(recur entities (mapv (fn [entity k]
(cond-> k (duplicate-key? key->count k) (str "_" (:id entity))))
entities ks)))))
(defn keyed-map
"Returns a sorted map keyed by each entity key, disambiguating duplicate keys."
[entities]
(let [entities (vec entities)
base-key->count (frequencies (map :key entities))
candidate-keys (mapv (partial keyed-map-candidate-key base-key->count) entities)
candidate->count (frequencies candidate-keys)]
candidate->count (frequencies candidate-keys)
ks (unique-keys entities
(mapv (fn [entity candidate-key]
(cond-> candidate-key
(duplicate-key? candidate->count candidate-key) (str (:id entity))))
entities candidate-keys))]
(into (sorted-map)
(map (fn [[entity candidate-key]]
(let [key (cond-> candidate-key
(duplicate-key? candidate->count candidate-key) (str (:id entity)))]
(map (fn [entity key]
[key (-> entity
(dissoc :keyDisambiguator)
(assoc :key key))])))
(map vector entities candidate-keys))))
(defn keyed-model-map
"Returns a sorted model map keyed by model key, exposing only action namespaces."
[models]
(reduce-kv (fn [model-map model-key model]
(assoc model-map model-key (select-keys model [:actions])))
(sorted-map)
(keyed-map models)))
(assoc :key key))])
entities ks))))

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

@@ -1,6 +1,6 @@
(ns metabase.typed-schemas.core
"Typed schemas: TypeScript modules that describe questions, models (with
executable actions), tables and metrics to coding agents. Runtime objects
"Typed schemas: TypeScript modules that describe questions, actions, tables
and metrics to coding agents. Runtime objects
feed the Lib.createTestQuery DSL; `//` comments carry context for humans and
agents.

27
src/metabase/typed_schemas/db.clj
View File

@@ -6,7 +6,6 @@
[metabase.lib.schema.id :as lib.schema.id]
[metabase.queries.core :as queries]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
[metabase.warehouse-schema-overlay.core :as warehouse-schema-overlay]
[toucan2.core :as t2]))
@@ -56,12 +55,26 @@
[table-ids :- [:sequential ::lib.schema.id/table]]
(t2/select [:model/Table :id :db_id :name :display_name] :id [:in table-ids] {:from [(warehouse-schema-overlay/table-query)]}))
(mu/defn model-actions
"The id, model id, name, and type of the unarchived Actions of the model Cards with `model-ids`."
[model-ids :- [:set ms/PositiveInt]]
(t2/select [:model/Action :id :model_id :name :type]
:model_id [:in model-ids]
:archived false))
(mu/defn model-less-query-action-ids
"The ids of the unarchived query Actions without a model in collections the current user can see, outside
`excluded-collection-ids`, among `database-ids` (nil for unscoped), in name then id order."
[database-ids :- [:maybe [:set ::lib.schema.id/database]]
excluded-collection-ids :- [:set ::lib.schema.id/collection]]
(t2/select-pks-vec :model/Action
{:where [:and
[:= :model_id nil]
[:= :type "query"]
[:= :archived false]
(collection/visible-collection-filter-clause :collection_id)
(when (seq excluded-collection-ids)
[:or [:= :collection_id nil] [:not-in :collection_id excluded-collection-ids]])
(when database-ids
[:exists ^:allow-subquery {:select [1]
:from [[(t2/table-name :model/QueryAction) :qa]]
:where [:and
[:= :qa.action_id :action.id]
(scope-filter-clause database-ids :qa.database_id)]}])]
:order-by [[:name :asc] [:id :asc]]}))
(mu/defn field-table-id
"The Table id of the Field with `field-id`, or nil."

4
src/metabase/typed_schemas/render.clj
View File

@@ -356,14 +356,14 @@
(vals (:metrics schema)))))
(def ^:private top-level-keys
[:questions :models :tables :metrics])
[:questions :actions :tables :metrics])
(defn- section->node
"Converts one top-level schema section into an object expression."
[section-key section]
(case section-key
:questions (keyed-entities->obj section :question)
:models (generic->node section)
:actions (generic->node section)
:tables (keyed-entities->obj section :table)
:metrics (keyed-entities->obj section :metric)))

92
src/metabase/typed_schemas/schema/action.clj Normal file
View File

@@ -0,0 +1,92 @@
(ns metabase.typed-schemas.schema.action
"Typed schema generation for actions that belong to no model."
(:require
[medley.core :as m]
[metabase.actions.core :as actions]
[metabase.lib.core :as lib]
[metabase.lib.schema.common :as lib.schema.common]
[metabase.permissions.core :as perms]
[metabase.typed-schemas.common :as common]
[metabase.typed-schemas.db :as typed-schemas.db]
[metabase.typed-schemas.schema.common :as schema.common]
[metabase.util :as u]))
(set! *warn-on-reflection* true)
(defn- param-type->js-type
"Returns the JS type for a Metabase action parameter type."
[param-type]
(when ((some-fn keyword? string?) param-type)
(let [param-type-keyword (lib.schema.common/normalize-keyword param-type)
prefix (or (namespace param-type-keyword) (name param-type-keyword))]
(case prefix
("number" "numeric") "number"
("text" "string") "string"
"date" "Date"
"boolean" "boolean"
nil))))
(defn- template-tag-value-type
"Returns the type a template tag's values carry: a field filter's `:type` is always `:dimension`, so its
`:widget-type` (e.g. `:date/single`) is what tells us the value type."
[{:keys [type widget-type]}]
(if (= type :dimension)
widget-type
type))
(defn- template-tag-types
"Returns template-tag value types keyed by tag name, or nil when the action has no usable query."
[{:keys [dataset_query]}]
;; a stored query that failed to deserialize comes back as {}, which Lib rejects
(some-> dataset_query not-empty lib/all-template-tags-map (update-vals template-tag-value-type)))
(defn- parameter-schema
"Returns the schema for an action execute parameter."
[tag-types {:keys [id slug name display-name type target required]}]
(let [resolved-slug (or slug
(some-> id clojure.core/name)
(some-> name u/slugify))
resolved-type (or (param-type->js-type type)
(param-type->js-type
(get tag-types (some-> target lib/parameter-target-template-tag-name)))
"unknown")]
(m/assoc-some
{:slug resolved-slug
:displayName (or display-name name resolved-slug)
:jsType resolved-type}
:required (when required true))))
(defn- hidden-parameter-ids
"The ids of the parameters the action's form hides; execute refuses a value for them."
[{:keys [visualization_settings]}]
(into #{} (keep #(when (:hidden %) (:id %))) (vals (:fields visualization_settings))))
(defn action-schema
"Returns the typed schema entry for a query action, leaving out the parameters its form hides."
[{:keys [id name description parameters entity_id] :as action}]
(let [tag-types (template-tag-types action)
hidden? (hidden-parameter-ids action)]
(m/assoc-some
{:kind "action"
:key (common/generated-key name id)
:id id
:name name
:type "query"
:parameters (into [] (comp (remove (comp hidden? :id))
(map #(parameter-schema tag-types %)))
parameters)}
:description description
:entityId entity_id)))
(defn action-schemas
"Returns schema entries for the readable query actions without a model among `database-ids` (nil for unscoped),
leaving out the copies data apps own."
[database-ids]
(let [ids (typed-schemas.db/model-less-query-action-ids database-ids (set (perms/data-app-collection-ids)))
details-by-id (when (seq ids) (u/index-by :id (actions/select-actions-for-ids nil ids)))
details (keep details-by-id ids)
destination-ids (schema.common/destination-db-ids (into #{} (keep :database_id) details))]
(into []
(comp (remove #(contains? destination-ids (:database_id %)))
(map action-schema))
details)))

296
src/metabase/typed_schemas/schema/model.clj
View File

@@ -1,296 +0,0 @@
(ns metabase.typed-schemas.schema.model
"Typed schema generation for models and actions."
(:require
[clojure.string :as str]
[medley.core :as m]
[metabase.actions.core :as actions]
[metabase.lib.core :as lib]
[metabase.lib.schema.common :as lib.schema.common]
[metabase.typed-schemas.common :as common]
[metabase.typed-schemas.db :as typed-schemas.db]
[metabase.typed-schemas.schema.common :as schema.common]
[metabase.util :as u]))
(set! *warn-on-reflection* true)
(defn- keyword-or-string?
"Returns true for values that can be normalized to keywords."
[value]
((some-fn keyword? string?) value))
(defn- keyword-name
"Returns a display string for Toucan-normalized action type/kind keywords."
[value]
(when (keyword? value)
(u/qualified-name value)))
(defn- action-type-name
"Returns a display-safe action type name."
[action]
(or (keyword-name (:type action)) "unknown"))
(defn- model-action-error-data
"Returns structured error data for model action schema failures."
([model error-data]
(m/assoc-some
{:model-id (:id model)
:model-name (:name model)}
:status-code (:status-code error-data)))
([model action error-data]
(m/assoc-some
(assoc (model-action-error-data model error-data)
:action-id (:id action)
:action-name (:name action)
:action-type (:type action))
:status-code (:status-code error-data))))
(defn- action-rows
"Returns raw rows so we can detect actions that cannot be resolved to action details."
[model-ids]
(typed-schemas.db/model-actions model-ids))
(defn- unresolved-action-rows
"Returns action rows missing from resolved action details."
[action-rows action-details]
(let [action-ids (set (map :id action-details))]
(not-empty
(remove #(contains? action-ids (:id %)) action-rows))))
(defn- unresolved-action-rows-message
"Returns the error message for action rows that cannot be resolved."
[model unresolved-action-rows]
(format "Failed to build action schemas for model \"%s\" (card %s): action rows could not be resolved: %s"
(or (:name model) "Untitled")
(:id model)
(str/join ", "
(for [action unresolved-action-rows]
(format "%s (action %s, type %s)"
(or (:name action) "Untitled")
(:id action)
(action-type-name action))))))
(defn- unresolved-action-rows-data
"Returns structured error data for action rows that cannot be resolved."
[model unresolved-action-rows]
(assoc (model-action-error-data model nil)
:unresolved-action-rows (mapv #(select-keys % [:id :name :type]) unresolved-action-rows)))
(defn- throw-if-unresolved-action-rows!
"Surfaces action rows that cannot be resolved into action details."
[model action-rows action-details]
(when-let [unresolved-rows (unresolved-action-rows action-rows action-details)]
(throw (ex-info (unresolved-action-rows-message model unresolved-rows)
(unresolved-action-rows-data model unresolved-rows)))))
(defn- param-type->js-type
"Returns the JS type for a Metabase action parameter type."
[param-type]
(when (keyword-or-string? param-type)
(let [param-type-keyword (lib.schema.common/normalize-keyword param-type)
prefix (or (namespace param-type-keyword) (name param-type-keyword))]
(case prefix
("number" "numeric") "number"
("text" "string") "string"
"date" "Date"
"boolean" "boolean"
nil))))
(defn- template-tag-value-type
"Returns the type a template tag's values carry: a field filter's `:type` is always `:dimension`, so its
`:widget-type` (e.g. `:date/single`) is what tells us the value type."
[{:keys [type widget-type]}]
(if (= type :dimension)
widget-type
type))
(defn- query-action-template-tag-types
"Returns template-tag value types for query action parameters, or nil when the action has no usable query."
[{:keys [type dataset_query]}]
(when (= (lib.schema.common/normalize-keyword type) :query)
;; a stored query that failed to deserialize comes back as {}, which Lib rejects
(some-> dataset_query not-empty lib/all-template-tags-map (update-vals template-tag-value-type))))
(defn- model-action-error-message
"Returns the error message for model action schema failures."
([model error-message]
(format "Failed to build action schemas for model \"%s\" (card %s): %s"
(or (:name model) "Untitled")
(:id model)
(or error-message "unknown error")))
([model action error-message]
(format "Failed to build action schema for action \"%s\" (action %s, type %s) on model \"%s\" (card %s): %s"
(or (:name action) "Untitled")
(:id action)
(action-type-name action)
(or (:name model) "Untitled")
(:id model)
(or error-message "unknown error"))))
(defn- error-data-with-cause-message
"Adds the original exception message to structured error data."
[error-data exception]
(assoc error-data :cause-message (ex-message exception)))
(defn- action-parameter-schema
"Returns the schema for an action execute parameter."
[tag-types
{:keys [id slug name display-name type target required]}]
(let [resolved-slug (or slug
(some-> id clojure.core/name)
(some-> name u/slugify))
resolved-type (or (param-type->js-type type)
(param-type->js-type
(get tag-types
(some-> target lib/parameter-target-template-tag-name)))
"unknown")]
(m/assoc-some
{:slug resolved-slug
:displayName (or display-name name resolved-slug)
:jsType resolved-type}
:required (when required true))))
(defn- action-detail-schema
"Returns the typed schema entry for resolved action details."
[{:keys [id name description type kind parameters entity_id] :as action}]
(let [tag-types (query-action-template-tag-types action)
implicit? (= (lib.schema.common/normalize-keyword type) :implicit)]
(m/assoc-some
{:kind "action"
:key (common/generated-key name id)
:id id
:name name
:type (keyword-name type)
:parameters (mapv #(action-parameter-schema tag-types %) parameters)}
:description description
:entityId entity_id
:implicitKind (when implicit? (keyword-name kind)))))
(defn- resolved-action-details
"Returns action details from the actions module, preserving lookup error context."
[model]
(try
(actions/select-actions-for-models [model] [(:id model)])
(catch Exception exception
(throw (ex-info (model-action-error-message model (ex-message exception))
(error-data-with-cause-message
(model-action-error-data model (ex-data exception))
exception)
exception)))))
(defn- resolved-action-details-for-models
"Returns resolved action details for selected models."
[models]
(let [model-ids (set (map :id models))]
(try
(actions/select-actions-for-models models (vec model-ids))
(catch Exception exception
(throw (ex-info (format "Failed to build action schemas for selected models: %s" (ex-message exception))
(error-data-with-cause-message
{:model-ids model-ids
:status-code (:status-code (ex-data exception))}
exception)
exception))))))
(defn- model-action-schema
"Returns an action schema, preserving which model/action failed to render."
[model action]
(try
(action-detail-schema action)
(catch Exception exception
(throw (ex-info (model-action-error-message model action (ex-message exception))
(error-data-with-cause-message
(model-action-error-data model action (ex-data exception))
exception)
exception)))))
(defn- model-action-schemas
"Returns action schemas for a model, preserving action lookup error context."
([model]
(model-action-schemas model
(action-rows #{(:id model)})
(resolved-action-details model)))
([model action-rows action-details]
(throw-if-unresolved-action-rows! model action-rows action-details)
(not-empty
(mapv #(model-action-schema model %) action-details))))
(defn model-schema
"Returns the model schema with actions, or nil when the model has no executable actions."
([model]
(model-schema model (model-action-schemas model)))
([{:keys [id name]} action-schemas]
(when (seq action-schemas)
{:key (common/generated-key name id)
:keyDisambiguator id
:actions (common/keyed-map action-schemas)})))
(defn- interrupted-exception?
"Returns true when `exception`, or one of its causes, is an InterruptedException."
[exception]
(or (instance? InterruptedException exception)
(some-> (ex-cause exception) interrupted-exception?)))
(defn- rethrow-if-interrupted!
"Rethrows `exception` when interrupted, so cancellations are not swallowed as errors."
[exception]
(when (interrupted-exception? exception)
(throw exception)))
(defn- model-error-entry
"Returns a schema error entry for a model that could not be built."
[model exception]
(m/assoc-some
{:type "modelError"
:modelId (:id model)
:message (or (ex-message exception) "unknown error")}
:modelName (:name model)))
(defn- bulk-action-schema-builder
"Returns a `model -> action-schemas` function backed by one bulk action lookup,
or nil when that lookup fails so [[model-schemas]] resolves each model on its own."
[models model-ids]
(try
(let [action-rows-by-model-id (group-by :model_id (action-rows model-ids))
action-details-by-model-id (group-by :model_id (resolved-action-details-for-models models))]
(fn [model]
(model-action-schemas model
(get action-rows-by-model-id (:id model))
(get action-details-by-model-id (:id model)))))
;; Broad on purpose: the per-model fallback recomputes the same result, so degrading masks nothing.
(catch Exception exception
(rethrow-if-interrupted! exception)
nil)))
(defn- collect-model-schema
"Reduces one model into `{:models [...] :errors [...]}`.
Only the structured `ExceptionInfo` a bad model raises becomes an error entry;
anything else is an unexpected bug and propagates instead of masking it."
[acc build-action-schemas model]
(let [{:keys [schema error]}
(try
{:schema (model-schema model (build-action-schemas model))}
(catch clojure.lang.ExceptionInfo exception
(rethrow-if-interrupted! exception)
{:error (model-error-entry model exception)}))]
(cond-> acc
schema (update :models conj schema)
error (update :errors conj error))))
(defn model-schemas
"Returns `{:models [...] :errors [...]}`, with optional database and collection scopes.
A model that cannot be built becomes an `:errors` entry instead of failing the rest."
[database-ids collection-ids]
(let [models (schema.common/select-schema-cards :model database-ids collection-ids)]
(if (seq models)
(let [model-ids (set (map :id models))
;; The bulk lookup avoids N+1s; when a broken model makes it throw we
;; fall back to resolving each model's actions on its own.
build-action-schemas (or (bulk-action-schema-builder models model-ids)
model-action-schemas)]
(reduce (fn [acc model]
(collect-model-schema acc build-action-schemas model))
{:models [] :errors []}
models))
{:models [] :errors []})))

14
src/metabase/typed_schemas/source.clj
View File

@@ -17,8 +17,7 @@
All methods return the shaped schema entities produced by the
`metabase.typed-schemas.schema.*` builders, except [[library-tables]], which
returns raw table rows (only `:id` is consumed), and [[models]], which returns
`{:models [...] :errors [...]}` so broken models surface as data.
returns raw table rows (only `:id` is consumed).
When the pipeline needs to read something new, add a protocol method and its
[[app-db-source]] implementation here — do not call `t2`/`metabot` directly
@@ -26,8 +25,8 @@
the module's data-access surface enumerable and every downstream stage
testable with literal values."
(:require
[metabase.typed-schemas.schema.action :as schema.action]
[metabase.typed-schemas.schema.metric :as schema.metric]
[metabase.typed-schemas.schema.model :as schema.model]
[metabase.typed-schemas.schema.question :as schema.question]
[metabase.typed-schemas.schema.table :as schema.table]
[metabase.typed-schemas.scope :as scope]))
@@ -49,9 +48,8 @@
collection refs and include flags, or nil when none are requested.")
(questions [source database-ids collection-ids]
"Question schema entities.")
(models [source database-ids]
"Model schemas as `{:models [...] :errors [...]}`: entities for models with
executable actions, plus errors for models that could not be built.")
(actions [source database-ids]
"Schema entities for query actions that belong to no model.")
(metrics [source database-ids collection-ids]
"Metric schema entities.")
(tables [source database-ids table-ids]
@@ -70,8 +68,8 @@
(scope/library-scope scope-options))
(questions [_ database-ids collection-ids]
(vec (schema.question/question-schemas database-ids collection-ids)))
(models [_ database-ids]
(schema.model/model-schemas database-ids nil))
(actions [_ database-ids]
(schema.action/action-schemas database-ids))
(metrics [_ database-ids collection-ids]
(vec (schema.metric/metric-schemas database-ids collection-ids)))
(tables [_ database-ids table-ids]

12
src/metabase/typed_schemas_rest/api.clj
View File

@@ -34,9 +34,9 @@
[:include-metric-library {:optional true}
[:maybe {:description "Whether to include the entire metric library."}
:boolean]]
[:include-models {:optional true}
[:maybe {:description (str "Include models with actions. Database scope filters models; "
"library scope does not. Without a scope, returns models only.")}
[:include-actions {:optional true}
[:maybe {:description (str "Include query actions that belong to no model. Database scope filters them; "
"library scope does not. Without a scope, returns actions only.")}
:boolean]]])
(api.macros/defendpoint :get "/v1/typescript" :- :any
@@ -44,10 +44,14 @@
[_route-params
query-params :- TypedSchemaQueryParams
_body-params
{{question-collections "question-collections"} :query-params}]
{{question-collections "question-collections"
include-models "include-models"} :query-params}]
(when (some? question-collections)
(throw (ex-info "The question-collections query parameter is not supported."
{:status-code 400})))
(when (some? include-models)
(throw (ex-info "The include-models query parameter is not supported; use include-actions."
{:status-code 400})))
{:status 200
:headers typescript-response-headers
:body (-> query-params

2
src/metabase/typed_schemas_rest/api/query_params.clj
View File

@@ -42,4 +42,4 @@
:library-collection-refs (comma-separated-references (:library-collections query-params))
:include-data-library? (enabled? (:include-data-library query-params))
:include-metric-library? (enabled? (:include-metric-library query-params))
:include-models? (enabled? (:include-models query-params))})
:include-actions? (enabled? (:include-actions query-params))})

7
test/metabase/typed_schemas/common_test.clj
View File

@@ -84,3 +84,10 @@
(let [card-ids (into #{} (map :id) (schema.common/select-schema-cards :metric nil nil))]
(is (contains? card-ids open-card-id))
(is (not (contains? card-ids destination-card-id))))))))
(deftest ^:parallel keyed-map-never-drops-an-entity-test
(testing "a disambiguated key that equals another entity's own key is disambiguated again rather than overwriting it"
(let [entities [{:key "step", :id 1} {:key "step", :id 23} {:key "step1", :id 7} {:key "step11", :id 40}]
result (typed-schemas.common/keyed-map entities)]
(is (= 4 (count result)))
(is (= #{1 23 7 40} (into #{} (map :id) (vals result)))))))

95
test/metabase/typed_schemas/core_test.clj
View File

@@ -42,18 +42,18 @@
(is (= {:schemaVersion 2
:generatedAt "2026-01-01T00:00:00Z"
:metabase {:instanceUrl "https://metabase.example.com"}
:models {"orders" {:actions {"create" {:kind "action", :id 9}}}}
:actions {"shipOrder" {:kind "action", :key "shipOrder", :id 11}}
:tables {"orders" {:type "table", :key "orders", :id 3}}
:metrics {"revenue" {:type "metric", :key "revenue", :id 2}}}
(build/create-schema
{:models [{:key "orders", :name "Orders", :actions {"create" {:kind "action", :id 9}}}]
{:actions [{:kind "action", :key "shipOrder", :id 11}]
:tables [{:type "table", :key "orders", :id 3}]
:metrics [{:type "metric", :key "revenue", :id 2}]}
test-info))))
(deftest create-schema-disambiguates-duplicate-keys-test
(let [schema (build/create-schema
{:models []
{:actions []
:tables [{:type "table", :key "orders", :id 3}
{:type "table", :key "orders", :id 4}]
:metrics []}
@@ -62,7 +62,7 @@
(is (= ["orders3" "orders4"] (map :key (vals (:tables schema)))))))
(deftest create-schema-defaults-info-test
(let [schema (build/create-schema {:models [], :tables [], :metrics []})]
(let [schema (build/create-schema {:actions [], :tables [], :metrics []})]
(is (string? (:generatedAt schema)))
(is (contains? (:metabase schema) :instanceUrl))))
@@ -70,13 +70,13 @@
"A [[source/SchemaSource]] over literal values. `tables` are filtered by the
requested table ids so tests can see which tables fetching asked for."
[{:keys [database-ids collection-ids library-scope library-tables
questions models model-errors metrics tables]}]
questions actions metrics tables]}]
(reify source/SchemaSource
(database-ids [_ _] database-ids)
(collection-ids [_ _] collection-ids)
(library-scope [_ _] library-scope)
(questions [_ _ _] (vec questions))
(models [_ _] {:models (vec models) :errors (vec model-errors)})
(actions [_ _] (vec actions))
(metrics [_ _ _] (vec metrics))
(tables [_ _ table-ids] (cond->> (vec tables)
table-ids (filterv #(contains? table-ids (:id %)))))
@@ -93,11 +93,10 @@
:tables [{:id 10, :type "table", :key "publishedTable"}
{:id 42, :type "table", :key "mappedTable"}
{:id 99, :type "table", :key "notInScope"}]})]
(is (= {:models []
(is (= {:actions []
:tables [{:id 10, :type "table", :key "publishedTable"}
{:id 42, :type "table", :key "mappedTable"}]
:metrics [{:type "metric", :key "revenue", :id 1, :mappedTableIds [42]}]
:errors []}
:metrics [{:type "metric", :key "revenue", :id 1, :mappedTableIds [42]}]}
(build/fetch-items {:include-data-library? true} source)))))
(deftest options-reject-question-collection-refs-test
@@ -105,54 +104,29 @@
#"Invalid semantic schema options\."
(build/fetch-items {:question-collection-refs [{:id 1}]}))))
(deftest fetch-items-threads-model-errors-test
(testing "model errors from the source land in Items :errors alongside the built models"
(let [source (literal-source
{:models [{:key "orders", :name "Orders"
:actions {"create" {:kind "action", :id 9}}}]
:model-errors [{:type "modelError", :modelId 7, :modelName "Broken"
:message "Failed to build action schemas for model \"Broken\" (card 7): boom"}]})]
(is (= {:models [{:key "orders", :name "Orders"
:actions {"create" {:kind "action", :id 9}}}]
:tables []
:metrics []
:errors [{:type "modelError", :modelId 7, :modelName "Broken"
:message "Failed to build action schemas for model \"Broken\" (card 7): boom"}]}
(build/fetch-items {:include-models? true} source))))))
(deftest fetch-items-includes-models-only-when-requested-test
(let [model-calls (atom [])
(deftest fetch-items-includes-actions-only-when-requested-test
(let [action-calls (atom [])
action {:kind "action", :key "shipOrder", :id 11}
source (reify source/SchemaSource
(database-ids [_ _] #{26})
(database-ids [_ database-ref] (when database-ref #{26}))
(collection-ids [_ _] nil)
(library-scope [_ _] nil)
(library-scope [_ {:keys [include-data-library?]}]
(when include-data-library? {:data-collection-ids #{10}}))
(questions [_ _ _] [])
(models [_ database-ids]
(swap! model-calls conj database-ids)
{:models [] :errors []})
(actions [_ database-ids]
(swap! action-calls conj database-ids)
[action])
(metrics [_ _ _] [])
(tables [_ _ _] [])
(library-tables [_ _] []))]
(testing "database scope does not include models by default"
(is (= {:models [], :tables [], :metrics [], :errors []}
(build/fetch-items {:database {:id 26}} source)))
(is (empty? @model-calls)))
(testing "include-models scopes models to the database"
(is (= {:models [], :tables [], :metrics [], :errors []}
(build/fetch-items {:database {:id 26}
:include-models? true}
source)))
(is (= [#{26}] @model-calls)))))
(deftest create-schema-includes-errors-when-present-test
(testing "errors are omitted from a healthy schema and included when present"
(let [base-items {:models [], :tables [], :metrics []}]
(is (not (contains? (build/create-schema base-items test-info) :errors)))
(is (= [{:type "modelError", :modelId 7, :modelName "Broken", :message "boom"}]
(:errors (build/create-schema
(assoc base-items :errors [{:type "modelError", :modelId 7
:modelName "Broken", :message "boom"}])
test-info)))))))
(testing "no action is read unless asked for"
(is (=? {:actions []} (build/fetch-items {:database {:id 26}} source)))
(is (empty? @action-calls)))
(testing "a database scope scopes the actions to it"
(is (=? {:actions [action]} (build/fetch-items {:database {:id 26}, :include-actions? true} source))))
(testing "a library scope reads every readable action"
(is (=? {:actions [action]} (build/fetch-items {:include-data-library? true, :include-actions? true} source))))
(is (= [#{26} nil] @action-calls))))
;; One end-to-end test over the real test-data dataset: cards for every entity
;; kind on real synced tables, run through the whole pipeline to TypeScript.
@@ -183,10 +157,13 @@
:field_ref [:field (mt/id :orders :total) nil]
:id (mt/id :orders :total)}]}
:model/Action action {:name "Update order", :model_id (:id model), :type :implicit}
:model/ImplicitAction _ {:action_id (:id action), :kind "row/update"}]
:model/ImplicitAction _ {:action_id (:id action), :kind "row/update"}
:model/Action standalone {:name "Discount order", :type :query}
:model/QueryAction _ {:action_id (:id standalone)
:dataset_query (lib/native-query mp "UPDATE orders SET discount = 0 WHERE id = {{id}}")}]
(mt/with-current-user (mt/user->id :crowberto)
(let [schema (typed-schemas/build-semantic-schema {:database {:id (mt/id)}
:include-models? true}
:include-actions? true}
test-info)
body (typed-schemas/render-typescript schema)]
(testing "every entity kind lands in the schema with its real relationships"
@@ -196,16 +173,18 @@
:metrics {"orderRevenue" {:mappedTableIds [(mt/id :orders)]
:columns [{:displayName "Sum of Total"
:jsType "number"}]}}
:models {"orderModel" {:actions {"updateOrder" {:kind "action"}}}}}
:actions {"discountOrder" {:kind "action", :id (:id standalone), :type "query"}}}
schema)))
(testing "saved questions are absent from the schema"
(is (not (contains? schema :questions))))
(testing "only the temp metric and model are in scope for the dataset database"
(is (= {:metrics ["orderRevenue"], :models ["orderModel"]}
(update-vals (select-keys schema [:metrics :models])
(testing "an action that belongs to a model stays out"
(is (not (str/includes? body "updateOrder"))))
(testing "only the temp metric and action are in scope for the dataset database"
(is (= {:metrics ["orderRevenue"], :actions ["discountOrder"]}
(update-vals (select-keys schema [:metrics :actions])
(comp vec keys)))))
(testing "the rendered module carries the real entities"
(is (str/includes? body "orders: {"))
(is (str/includes? body "name: \"Order revenue\""))
(is (str/includes? body "updateOrder: {"))
(is (str/includes? body "discountOrder: {"))
(is (str/ends-with? body "export default schema;\n")))))))))))

22
test/metabase/typed_schemas/render_test.clj
View File

@@ -159,28 +159,6 @@
(is (= [:lit "string"] (obj-entry field-node :jsType)))
(is (nil? (obj-entry field-node :displayName))))))
(def ^:private schema-with-errors
{:schemaVersion 2
:metabase {:instanceUrl "https://metabase.example.com"}
:questions {}
:models {}
:tables {}
:metrics {}
:errors [{:type "modelError"
:modelId 7
:modelName "Broken model"
:message "Failed to build action schemas for model \"Broken model\" (card 7): boom"}]})
(deftest typescript-renderer-emits-model-errors-test
(let [body (typed-schemas/render-typescript schema-with-errors)]
;; Broken models surface as runtime data so agents can tell users which
;; models are bad instead of the whole response failing.
(is (str/includes? body "errors: ["))
(is (str/includes? body "type: \"modelError\""))
(is (str/includes? body "modelId: 7"))
(is (str/includes? body "modelName: \"Broken model\""))
(is (str/includes? body "message: \"Failed to build action schemas"))))
(deftest typescript-renderer-omits-pick-fields-helper-for-raw-dimensions-test
(let [body (typed-schemas/render-typescript raw-dimensions-schema)]
;; Dimensions that cannot be resolved to table fields stay as raw fields, so

64
test/metabase/typed_schemas/schema/action_test.clj Normal file
View File

@@ -0,0 +1,64 @@
(ns metabase.typed-schemas.schema.action-test
(:require
[clojure.test :refer :all]
[metabase.actions.core :as actions]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.test :as mt]
[metabase.typed-schemas.schema.action :as schema.action]))
(defn- standalone-action-schema
"The schema entry for a model-less query action created from `action` and `dataset-query`."
[action dataset-query]
(mt/with-temp [:model/Action {action-id :id} (merge {:type :query, :name "Update bird"} action)
:model/QueryAction _ {:action_id action-id, :dataset_query dataset-query}]
(schema.action/action-schema (actions/select-action :id action-id))))
(deftest action-schema-types-a-parameter-from-its-template-tag-test
(testing ":category has no JS type of its own, so the persisted template tag's type decides it"
(is (=? {:kind "action"
:key "updateBird"
:type "query"
:parameters [{:slug "name", :displayName "Name", :jsType "string"}]}
(standalone-action-schema
{:parameters [{:id "name", :name "Name", :type :category, :target [:variable [:template-tag "name"]]}]}
(lib/native-query (mt/metadata-provider) "UPDATE birds SET name = {{name}}"))))))
(deftest action-schema-resolves-field-filter-widget-type-test
(testing "a field filter's value type comes from its widget type, through a :dimension parameter target"
(let [mp (mt/metadata-provider)]
(is (=? {:parameters [{:slug "created_at", :displayName "Created At", :jsType "Date"}]}
(standalone-action-schema
{:parameters [{:id "created_at"
:name "Created At"
:type :category
:target [:dimension [:template-tag "created_at"]]}]}
(-> (lib/native-query mp "UPDATE birds SET name = 'x' WHERE {{created_at}}")
(lib/with-template-tags
{"created_at" {:name "created_at"
:display-name "Created At"
:type :dimension
:widget-type :date/single
:dimension (lib/ref (lib.metadata/field mp (mt/id :categories :name)))}}))))))))
(deftest action-schema-tolerates-an-empty-query-test
(testing "an action whose stored query degraded to {} still builds, typing parameters from their own types"
(is (=? {:parameters [{:slug "name", :displayName "Name", :jsType "string"}]}
(schema.action/action-schema {:id 7, :name "Update bird", :dataset_query {}
:parameters [{:id "name", :name "Name", :type :text}]})))))
(deftest action-schema-leaves-out-hidden-parameters-test
(testing "a parameter the action's form hides is left out, since execute refuses a value for it"
(is (=? {:parameters [{:slug "name"}]}
(schema.action/action-schema
{:id 7
:name "Update bird"
:dataset_query {}
:parameters [{:id "name", :name "Name", :type :text}
{:id "updated_by", :name "Updated by", :type :text, :required true}]
:visualization_settings {:fields {"updated_by" {:id "updated_by", :hidden true}}}})))
(is (= ["name"]
(map :slug (:parameters (schema.action/action-schema
{:id 7, :name "Update bird", :dataset_query {}
:parameters [{:id "name", :type :text} {:id "updated_by", :type :text}]
:visualization_settings {:fields {"updated_by" {:id "updated_by", :hidden true}}}})))))))

259
test/metabase/typed_schemas/schema/model_test.clj
View File

@@ -1,259 +0,0 @@
(ns metabase.typed-schemas.schema.model-test
(:require
[clojure.test :refer :all]
[metabase.actions.core :as actions]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.test :as mt]
[metabase.typed-schemas.schema.common :as schema.common]
[metabase.typed-schemas.schema.model :as schema.model]))
(comment mt/keep-me)
(deftest model-schema-includes-actions-test
(mt/with-dynamic-fn-redefs [schema.model/model-action-schemas
(constantly [{:kind "action", :key "create", :id 5}])]
(is (= {:key "ordersModel"
:keyDisambiguator 42
:actions {"create" {:kind "action", :key "create", :id 5}}}
(schema.model/model-schema
{:id 42
:name "Orders model"})))))
(deftest model-schema-supports-persisted-mbql5-template-tags-test
(let [mp (mt/metadata-provider)
action-query (lib/native-query mp "UPDATE birds SET name = {{name}}")]
(mt/with-actions [model {:name "Bird model"
:type :model
:dataset_query (lib/query mp (lib.metadata/table mp (mt/id :categories)))}
{action-id :action-id} {:name "Update bird"
:database_id (mt/id)
:dataset_query action-query
;; :category has no JS type of its own, so the result depends on the
;; persisted template tag's type
:parameters [{:id "name"
:name "Name"
:type :category
:target [:variable [:template-tag "name"]]}]}]
(let [action (actions/select-action :id action-id)]
(is (sequential? (get-in action [:dataset_query :stages 0 :template-tags])))
(is (=? [{:slug "name", :displayName "Name", :jsType "string"}]
(get-in (schema.model/model-schema model)
[:actions "updateBird" :parameters])))))))
(deftest model-schema-resolves-field-filter-widget-type-test
(testing "a field filter's value type comes from its widget type, through a :dimension parameter target"
(let [mp (mt/metadata-provider)
action-query (-> (lib/native-query mp "UPDATE birds SET name = 'x' WHERE {{created_at}}")
(lib/with-template-tags
{"created_at" {:name "created_at"
:display-name "Created At"
:type :dimension
:widget-type :date/single
:dimension (lib/ref (lib.metadata/field mp (mt/id :categories :name)))}}))]
(mt/with-actions [model {:name "Bird model"
:type :model
:dataset_query (lib/query mp (lib.metadata/table mp (mt/id :categories)))}
{action-id :action-id} {:name "Update bird"
:database_id (mt/id)
:dataset_query action-query
:parameters [{:id "created_at"
:name "Created At"
:type :category
:target [:dimension [:template-tag "created_at"]]}]}]
(is (= :dimension (get-in (actions/select-action :id action-id) [:dataset_query :stages 0 :template-tags 0 :type])))
(is (=? [{:slug "created_at", :displayName "Created At", :jsType "Date"}]
(get-in (schema.model/model-schema model)
[:actions "updateBird" :parameters])))))))
(deftest model-schema-tolerates-empty-action-query-test
(testing "an action whose stored query degraded to {} still builds instead of failing the whole model"
(mt/with-actions [model {:name "Bird model"
:type :model
:dataset_query (let [mp (mt/metadata-provider)]
(lib/query mp (lib.metadata/table mp (mt/id :categories))))}
{action-id :action-id} {:name "Update bird"
:database_id (mt/id)
:dataset_query {}
:parameters [{:id "name", :name "Name", :type :text}]}]
(is (= {} (:dataset_query (actions/select-action :id action-id))))
(is (=? [{:slug "name", :displayName "Name", :jsType "string"}]
(get-in (schema.model/model-schema model)
[:actions "updateBird" :parameters]))))))
(deftest model-schemas-includes-only-actionable-models-test
(with-redefs [schema.common/select-schema-cards
(constantly [{:id 42 :name "Model 42"}
{:id 43 :name "Model 43"}])
schema.model/action-rows
(constantly [{:id 5 :model_id 42 :name "Create" :type :query}])
actions/select-actions-for-models
(constantly [{:id 5
:model_id 42
:name "Create"
:type :query
:parameters []}])]
;; only model 42 has an action, so model 43 is omitted
(is (= ["model42"]
(map :key (:models (schema.model/model-schemas #{1} nil)))))))
; Ensures we are not doing N+1 queries for action rows and details
(deftest model-schemas-bulk-loads-actions-test
(let [models [{:id 42 :name "Model 42"}
{:id 43 :name "Model 43"}]
action-rows-calls (atom [])
action-details-calls (atom [])]
(with-redefs [schema.common/select-schema-cards (constantly models)
schema.model/action-rows (fn [model-ids]
(swap! action-rows-calls conj model-ids)
[])
actions/select-actions-for-models (fn [known-models model-ids]
(swap! action-details-calls conj [known-models model-ids])
[])]
(is (= {:models [] :errors []} (schema.model/model-schemas #{1} nil)))
(is (= [#{42 43}] @action-rows-calls))
(is (= [[models #{42 43}]]
(map (fn [[known-models model-ids]] [known-models (set model-ids)]) @action-details-calls))))))
(deftest model-schemas-collects-broken-model-errors-test
(testing "a broken model becomes an :errors entry while healthy models still build"
(with-redefs [schema.common/select-schema-cards
(constantly [{:id 42 :name "Model 42"}
{:id 43 :name "Broken model"}])
schema.model/action-rows
(constantly [{:id 5 :model_id 42 :name "Create" :type :query}
;; model 43's action row resolves to no action details
{:id 6 :model_id 43 :name "Broken action" :type :broken}])
actions/select-actions-for-models
(constantly [{:id 5
:model_id 42
:name "Create"
:type :query
:parameters []}])]
(let [{:keys [models errors]} (schema.model/model-schemas #{1} nil)]
(is (= ["model42"] (map :key models)))
(is (=? [{:type "modelError"
:modelId 43
:modelName "Broken model"
:message #".*Broken model.*could not be resolved.*"}]
errors))))))
(deftest model-schemas-falls-back-when-bulk-lookup-fails-test
(testing "a broken model poisoning the bulk action lookup does not hide other models"
(with-redefs [schema.common/select-schema-cards
(constantly [{:id 42 :name "Model 42"}
{:id 43 :name "Broken model"}])
;; bulk lookup blows up for the whole batch
actions/select-actions-for-models
(fn [_known-models model-ids]
(cond
(< 1 (count model-ids))
(throw (ex-info "bulk lookup exploded" {}))
(= (set model-ids) #{42})
[{:id 5 :model_id 42 :name "Create" :type :query :parameters []}]
:else
(throw (ex-info "action lookup failed" {:status-code 500}))))
;; per-model fallback: model 42 resolves, model 43 still fails
schema.model/action-rows
(fn [model-ids]
(if (contains? model-ids 42)
[{:id 5 :model_id 42 :name "Create" :type :query}]
[]))]
(let [{:keys [models errors]} (schema.model/model-schemas #{1} nil)]
(is (= ["model42"] (map :key models)))
(is (=? [{:type "modelError"
:modelId 43
:modelName "Broken model"
:message #".*Broken model.*action lookup failed.*"}]
errors))))))
(deftest model-schemas-does-not-swallow-interruption-test
(testing "an interruption while bulk-resolving actions propagates instead of collecting an error"
(with-redefs [schema.common/select-schema-cards
(constantly [{:id 42 :name "Model 42"}])
schema.model/action-rows (constantly [])
actions/select-actions-for-models
(fn [& _] (throw (InterruptedException. "cancelled")))]
(let [thrown (is (thrown? clojure.lang.ExceptionInfo
(schema.model/model-schemas #{1} nil)))]
(is (instance? InterruptedException (ex-cause thrown))))))
(testing "an interruption wrapped in the structured error propagates instead of collecting an error"
(with-redefs [schema.common/select-schema-cards
(constantly [{:id 42 :name "Model 42"}])
schema.model/action-rows (constantly [])
actions/select-actions-for-models (constantly [])
schema.model/model-action-schemas
(fn [& _] (throw (ex-info "cancelled" {} (InterruptedException. "cancelled"))))]
(let [thrown (is (thrown? clojure.lang.ExceptionInfo
(schema.model/model-schemas #{1} nil)))]
(is (instance? InterruptedException (ex-cause thrown)))))))
(deftest model-schemas-propagates-unexpected-errors-test
(testing "an unexpected, unstructured failure propagates instead of downgrading to partial data"
(with-redefs [schema.common/select-schema-cards
(constantly [{:id 42 :name "Model 42"}])
schema.model/action-rows (constantly [])
actions/select-actions-for-models (constantly [])
schema.model/model-action-schemas
(fn [& _] (throw (NullPointerException. "boom")))]
(is (thrown? NullPointerException
(schema.model/model-schemas #{1} nil))))))
(deftest model-schema-surfaces-action-selection-errors-test
(with-redefs [schema.model/action-rows (constantly [])
actions/select-actions-for-models
(fn [& _]
(throw (ex-info "action lookup failed"
{:status-code 500})))]
(let [exception (is (thrown? clojure.lang.ExceptionInfo
(schema.model/model-schema {:id 100
:name "Broken model"})))]
(is (= "Failed to build action schemas for model \"Broken model\" (card 100): action lookup failed"
(ex-message exception)))
(is (=? {:model-id 100
:model-name "Broken model"
:status-code 500
:cause-message "action lookup failed"}
(ex-data exception))))))
(deftest model-schema-surfaces-action-rendering-errors-test
(with-redefs [actions/select-actions-for-models
(constantly [{:id 200
:name "Broken action"
:type :query}])
schema.model/action-rows (constantly [{:id 200
:name "Broken action"
:type :query}])
schema.model/action-detail-schema (fn [& _]
(throw (ex-info "action parameters are invalid"
{:status-code 500})))]
(let [exception (is (thrown? clojure.lang.ExceptionInfo
(schema.model/model-schema {:id 100
:name "Broken model"})))]
(is (= "Failed to build action schema for action \"Broken action\" (action 200, type query) on model \"Broken model\" (card 100): action parameters are invalid"
(ex-message exception)))
(is (=? {:model-id 100
:model-name "Broken model"
:action-id 200
:action-name "Broken action"
:action-type :query
:status-code 500
:cause-message "action parameters are invalid"}
(ex-data exception))))))
(deftest model-schema-surfaces-unresolved-action-row-errors-test
(with-redefs [schema.model/action-rows (constantly [{:id 200
:name "Broken action"
:type :broken}])
actions/select-actions-for-models (constantly [])]
(let [exception (is (thrown? clojure.lang.ExceptionInfo
(schema.model/model-schema {:id 100
:name "Broken model"})))]
(is (= "Failed to build action schemas for model \"Broken model\" (card 100): action rows could not be resolved: Broken action (action 200, type broken)"
(ex-message exception)))
(is (=? {:model-id 100
:model-name "Broken model"
:unresolved-action-rows [{:id 200, :name "Broken action", :type :broken}]}
(ex-data exception))))))

46
test/metabase/typed_schemas/source_test.clj
View File

@@ -5,8 +5,11 @@
cheap in-memory tests; these tests are the seam where typed schemas meet the
database."
(:require
[clojure.set :as set]
[clojure.test :refer :all]
[metabase.collections.models.collection :as collection]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.typed-schemas.scope :as scope]
@@ -98,3 +101,46 @@
(source/tables source/app-db-source #{(:id db)} nil))))
(testing "an empty table-id scope matches nothing"
(is (= [] (source/tables source/app-db-source nil #{})))))))
(defn- touch-category-query []
(let [query (lib/native-query (mt/metadata-provider) "UPDATE categories SET name = name WHERE id = {{id}}")]
(lib/with-template-tags query (mapv #(assoc % :type :number) (lib/template-tags query)))))
(def ^:private id-parameter
{:id "id", :slug "id", :type :number/=, :target [:variable [:template-tag "id"]], :required true})
(deftest actions-test
(mt/with-temp [:model/Database {other-db-id :id} {}
:model/Collection {hidden-coll-id :id} {:name "Hidden actions"}
:model/Card {model-id :id} {:type :model
:dataset_query (lib/query (mt/metadata-provider)
(lib.metadata/table (mt/metadata-provider)
(mt/id :categories)))}
:model/Action {standalone :id} {:type :query, :name "Touch category", :parameters [id-parameter]}
:model/QueryAction _ {:action_id standalone, :dataset_query (touch-category-query)}
:model/Action {archived :id} {:type :query, :name "Archived", :archived true}
:model/QueryAction _ {:action_id archived, :dataset_query (touch-category-query)}
:model/Action {hidden :id} {:type :query, :name "Hidden", :collection_id hidden-coll-id}
:model/QueryAction _ {:action_id hidden, :dataset_query (touch-category-query)}
:model/Action {on-model :id} {:type :query, :name "On a model", :model_id model-id}
:model/QueryAction _ {:action_id on-model, :dataset_query (touch-category-query)}]
(let [action-ids (fn [database-ids] (into #{} (map :id) (source/actions source/app-db-source database-ids)))
ours #{standalone archived hidden on-model}]
(mt/with-current-user (mt/user->id :crowberto)
(testing "a database scope returns that database's unarchived actions without a model"
(is (= #{standalone hidden} (set/intersection ours (action-ids #{(mt/id)})))))
(testing "no scope reads every database"
(is (= #{standalone hidden} (set/intersection ours (action-ids nil)))))
(testing "a scope of another database leaves them out"
(is (= #{} (set/intersection ours (action-ids #{other-db-id})))))
(testing "an action renders with its parameters typed from the template tag"
(is (=? [{:kind "action"
:key "touchCategory"
:id standalone
:type "query"
:parameters [{:slug "id", :jsType "number", :required true}]}]
(filter #(= standalone (:id %)) (source/actions source/app-db-source #{(mt/id)}))))))
(testing "an action in a collection the user cannot read is left out"
(mt/with-non-admin-groups-no-collection-perms hidden-coll-id
(mt/with-current-user (mt/user->id :rasta)
(is (not (contains? (action-ids #{(mt/id)}) hidden)))))))))

6
test/metabase/typed_schemas_rest/api/query_params_test.clj
View File

@@ -8,18 +8,18 @@
:library-collection-refs [{:id 10} {:id 20}]
:include-data-library? false
:include-metric-library? false
:include-models? true}
:include-actions? true}
(query-params/query-params->options
{:database " Boba "
:library-collections " 10, 20 "
:include-models true}))))
:include-actions true}))))
(deftest ^:parallel query-params->options-coerces-values-and-applies-defaults-test
(is (= {:database {:id 1}
:library-collection-refs [{:id 3}]
:include-data-library? true
:include-metric-library? true
:include-models? false}
:include-actions? false}
(query-params/query-params->options
{:database "1"
:library-collections "3"

40
test/metabase/typed_schemas_rest/api_test.clj
View File

@@ -4,6 +4,7 @@
[clojure.test :refer :all]
[metabase.lib.core :as lib]
[metabase.lib.metadata :as lib.metadata]
[metabase.permissions.core :as perms]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.typed-schemas.core :as typed-schemas]
@@ -38,10 +39,10 @@
:crowberto
:get
200
"typed-schemas/v1/typescript?include-models=true&library-collections=1,2")
"typed-schemas/v1/typescript?include-actions=true&library-collections=1,2")
:body
read-string)]
(is (true? (:include-models? response)))
(is (true? (:include-actions? response)))
(is (= [{:id 1} {:id 2}] (:library-collection-refs response))))))
(deftest database-filter-test
@@ -65,7 +66,7 @@
(is (str/includes? missing-schema "const tables = { }"))
(is (str/includes? missing-schema "const metrics = { }")))))
(deftest database-scope-includes-models-only-when-requested-test
(deftest database-scope-includes-actions-only-when-requested-test
(mt/with-actions-enabled
(mt/with-temp [:model/Database other-db {}
:model/Card model {:name "Order model", :database_id (mt/id), :table_id (mt/id :orders)
@@ -77,16 +78,35 @@
:field_ref [:field (mt/id :orders :total) nil]
:id (mt/id :orders :total)}]}
:model/Action action {:name "Update order", :model_id (:id model), :type :implicit}
:model/ImplicitAction _ {:action_id (:id action), :kind "row/update"}]
:model/ImplicitAction _ {:action_id (:id action), :kind "row/update"}
:model/Action standalone {:name "Discount order", :type :query}
:model/QueryAction _ {:action_id (:id standalone)
:dataset_query (lib/native-query (mt/metadata-provider)
"UPDATE orders SET discount = 0")}
:model/Collection copies {:name "Data App: orders"}
:model/Action copy {:name "Copied order", :type :query, :collection_id (:id copies)}
:model/QueryAction _ {:action_id (:id copy)
:dataset_query (lib/native-query (mt/metadata-provider)
"UPDATE orders SET discount = 0")}]
(let [schema (fn [& query-params]
(:body (apply mt/user-http-request-full-response
:crowberto :get 200 "typed-schemas/v1/typescript" query-params)))]
(testing "a database scope leaves models out unless asked for"
(is (not (str/includes? (schema :database (mt/id)) "orderModel"))))
(testing "include-models adds the scoped database's models"
(is (str/includes? (schema :database (mt/id) :include-models true) "orderModel")))
(testing "include-models keeps models of other databases out"
(is (not (str/includes? (schema :database (:id other-db) :include-models true) "orderModel"))))))))
(testing "a database scope leaves actions out unless asked for"
(is (not (str/includes? (schema :database (mt/id)) "discountOrder"))))
(testing "include-actions adds the scoped database's actions without a model, and never a model"
(let [body (schema :database (mt/id) :include-actions true)]
(is (str/includes? body "discountOrder"))
(is (not (str/includes? body "updateOrder")))))
(testing "include-actions keeps actions of other databases out"
(is (not (str/includes? (schema :database (:id other-db) :include-actions true) "discountOrder"))))
(testing "include-models is refused"
(is (str/includes? (str (mt/user-http-request :crowberto :get 400 "typed-schemas/v1/typescript"
:include-models true))
"use include-actions")))
(testing "a copy a data app owns stays out"
(is (str/includes? (schema :database (mt/id) :include-actions true) "copiedOrder"))
(mt/with-dynamic-fn-redefs [perms/data-app-collection-ids (constantly #{(:id copies)})]
(is (not (str/includes? (schema :database (mt/id) :include-actions true) "copiedOrder")))))))))
(deftest collection-and-database-query-params-are-mutually-exclusive-test
(mt/user-http-request-full-response