From 19680bf09b5ad0e2d42fd3124c567d44e37e6ac4 Mon Sep 17 00:00:00 2001
From: lbrdnk <lbrdnk@users.noreply.github.com>
Date: Wed, 30 Oct 2024 02:35:58 +0100
Subject: [PATCH] Select only compatible fields from idFields (Mongo specific)
(#49262)
* Select only compatible fields from idFields
* Use isComparableWith to filter target fields
* Remove filter-comparable-fields
* Add test
---
.../src/metabase-lib/v1/metadata/Field.ts | 14 +++++++++
.../v1/metadata/Field.unit.spec.ts | 31 +++++++++++++++++++
.../SemanticTypeAndTargetPicker.tsx | 11 ++++---
3 files changed, 52 insertions(+), 4 deletions(-)
diff --git a/frontend/src/metabase-lib/v1/metadata/Field.ts b/frontend/src/metabase-lib/v1/metadata/Field.ts
index 6f733a7bea5..7b87b5aa19a 100644
--- a/frontend/src/metabase-lib/v1/metadata/Field.ts
+++ b/frontend/src/metabase-lib/v1/metadata/Field.ts
@@ -287,6 +287,20 @@ class FieldInner extends Base {
);
}
+ /**
+ * Predicate to decide whether `this` is comparable with `field`.
+ *
+ * Currently only the MongoBSONID erroneous case is ruled out to fix the issue #49149. To the best of my knowledge
+ * there's no logic on FE to reliably decide whether two columns are comparable. Trying to come up with that in ad-hoc
+ * manner could disable some cases that users may depend on.
+ */
+ isComparableWith(field) {
+ return this.effective_type === "type/MongoBSONID" ||
+ field.effective_type === "type/MongoBSONID"
+ ? this.effective_type === field.effective_type
+ : true;
+ }
+
/**
* @param {Field} field
*/
diff --git a/frontend/src/metabase-lib/v1/metadata/Field.unit.spec.ts b/frontend/src/metabase-lib/v1/metadata/Field.unit.spec.ts
index c2e59f8bfec..f43c75fae3f 100644
--- a/frontend/src/metabase-lib/v1/metadata/Field.unit.spec.ts
+++ b/frontend/src/metabase-lib/v1/metadata/Field.unit.spec.ts
@@ -559,4 +559,35 @@ describe("Field", () => {
});
});
});
+
+ describe("isComparableWith", () => {
+ const field = setup({
+ fields: [
+ createMockField({
+ id: FIELD_ID,
+ effective_type: "type/MongoBSONID",
+ }),
+ createMockField({
+ id: 2,
+ effective_type: "type/Integer",
+ }),
+ ],
+ });
+ const metadata = field.metadata;
+ const mongoField = metadata?.field(FIELD_ID);
+ const integerField = metadata?.field(2);
+
+ it("should return true for 2 MongoBSONID fields", () => {
+ expect(mongoField?.isComparableWith(mongoField)).toBe(true);
+ });
+
+ it("should return true for 2 non-MongoBSONID fields", () => {
+ expect(integerField?.isComparableWith(integerField)).toBe(true);
+ });
+
+ it("should return false for MongoBSONID field and other field", () => {
+ expect(mongoField?.isComparableWith(integerField)).toBe(false);
+ expect(integerField?.isComparableWith(mongoField)).toBe(false);
+ });
+ });
});
diff --git a/frontend/src/metabase/admin/datamodel/metadata/components/SemanticTypeAndTargetPicker/SemanticTypeAndTargetPicker.tsx b/frontend/src/metabase/admin/datamodel/metadata/components/SemanticTypeAndTargetPicker/SemanticTypeAndTargetPicker.tsx
index 916223759e8..ce1c3bb5ede 100644
--- a/frontend/src/metabase/admin/datamodel/metadata/components/SemanticTypeAndTargetPicker/SemanticTypeAndTargetPicker.tsx
+++ b/frontend/src/metabase/admin/datamodel/metadata/components/SemanticTypeAndTargetPicker/SemanticTypeAndTargetPicker.tsx
@@ -56,8 +56,11 @@ const SemanticTypeAndTargetPicker = ({
hasSeparator,
onUpdateField,
}: SemanticTypeAndTargetPickerProps) => {
- const hasIdFields = idFields.length > 0;
- const includeSchema = hasMultipleSchemas(idFields);
+ const comparableIdFields = idFields.filter((idField: Field) =>
+ field.isComparableWith(idField),
+ );
+ const hasIdFields = comparableIdFields.length > 0;
+ const includeSchema = hasMultipleSchemas(comparableIdFields);
const showFKTargetSelect = field.isFK();
const showCurrencyTypeSelect = field.isCurrency();
@@ -148,11 +151,11 @@ const SemanticTypeAndTargetPicker = ({
hasSeparator ? CS.mt0 : CS.mt1,
className,
)}
- placeholder={getFkFieldPlaceholder(field, idFields)}
+ placeholder={getFkFieldPlaceholder(field, comparableIdFields)}
searchProp={SEARCH_PROPS}
value={field.fk_target_field_id}
onChange={handleChangeTarget}
- options={idFields}
+ options={comparableIdFields}
optionValueFn={getFieldId}
optionNameFn={includeSchema ? getFieldNameWithSchema : getFieldName}
optionIconFn={getFieldIcon}
--
GitLab