Skip to content
Snippets Groups Projects
Unverified Commit 19680bf0 authored by lbrdnk's avatar lbrdnk Committed by GitHub
Browse files

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
parent d23d7a3a
No related branches found
No related tags found
No related merge requests found
......@@ -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
*/
......
......@@ -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);
});
});
});
......@@ -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}
......
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