Skip to content
Snippets Groups Projects
Unverified Commit 240bd6f1 authored by Anton Kulyk's avatar Anton Kulyk Committed by GitHub
Browse files

Don't allow mapping a model FK to another model (#32146)

* Add `getFields` method to `Database`

* Ensure only real fields returned by `getIdFields`

* Rework to iterate over fields

* Add repro
parent aba2062f
No related branches found
No related tags found
No related merge requests found
import { appBar, main, popover, restore } from "e2e/support/helpers";
import { SAMPLE_DB_ID } from "e2e/support/cypress_data";
import { SAMPLE_DATABASE } from "e2e/support/cypress_sample_database";
const { ORDERS_ID, PRODUCTS_ID } = SAMPLE_DATABASE;
// Should be removed once proper model FK support is implemented
describe("issue 31663", () => {
beforeEach(() => {
restore();
cy.signInAsAdmin();
cy.intercept("POST", "/api/dataset").as("dataset");
cy.intercept("GET", `/api/database/${SAMPLE_DB_ID}/idfields`).as(
"idFields",
);
cy.createQuestion({
name: "Orders Model",
dataset: true,
query: { "source-table": ORDERS_ID },
});
cy.createQuestion(
{
name: "Products Model",
dataset: true,
query: { "source-table": PRODUCTS_ID },
},
{ visitQuestion: true },
);
});
it("shouldn't list model IDs as possible model FK targets (metabase#31663)", () => {
// It's important to have product model's metadata loaded to reproduce this
appBar().findByText("Our analytics").click();
main().findByText("Orders Model").click();
cy.wait("@dataset");
cy.findByLabelText("Move, archive, and more...").click();
popover().findByText("Edit metadata").click();
cy.findByTestId("TableInteractive-root").findByText("Product ID").click();
cy.wait("@idFields");
cy.findByLabelText("Foreign key target").click();
popover().within(() => {
cy.findByText("Orders Model → ID").should("not.exist");
cy.findByText("Products Model → ID").should("not.exist");
cy.findByText("Orders → ID").should("exist");
cy.findByText("People → ID").should("exist");
cy.findByText("Products → ID").should("exist");
cy.findByText("Reviews → ID").should("exist");
});
});
});
......@@ -65,6 +65,10 @@ class Metadata {
return Object.values(this.tables);
}
fieldsList(): Field[] {
return Object.values(this.fields);
}
/**
* @deprecated this won't be sorted or filtered in a meaningful way
* @returns {Metric[]}
......
......@@ -20,6 +20,8 @@ import {
getMetadataUnfiltered,
} from "metabase/selectors/metadata";
import { isVirtualCardId } from "metabase-lib/metadata/utils/saved-questions";
// OBJECT ACTIONS
export const FETCH_DATABASE_METADATA =
"metabase/entities/database/FETCH_DATABASE_METADATA";
......@@ -81,11 +83,15 @@ const Databases = createEntity({
_.any(Databases.selectors.getList(state, props), db => db.is_sample),
getIdFields: createSelector(
[state => getMetadata(state).fields, (state, props) => props.databaseId],
[
state => getMetadata(state).fieldsList(),
(state, props) => props.databaseId,
],
(fields, databaseId) =>
Object.values(fields).filter(f => {
const { db_id } = f.table || {}; // a field's table can be null
return db_id === databaseId && f.isPK();
fields.filter(field => {
const dbId = field?.table?.db_id;
const isRealField = !isVirtualCardId(field.table_id);
return dbId === databaseId && isRealField && field.isPK();
}),
),
},
......
......@@ -93,6 +93,9 @@ function FKTargetPicker({
onChange={onChange}
searchable
searchProp={SEARCH_PROPERTIES}
buttonProps={{
"aria-label": t`Foreign key target`,
}}
optionValueFn={getOptionValue}
optionNameFn={getFieldName}
optionIconFn={getOptionIcon}
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment