diff --git a/e2e/test/scenarios/question/reproductions/42065-mongo-filter-by-id.cy.spec.js b/e2e/test/scenarios/question/reproductions/42065-mongo-filter-by-id.cy.spec.js new file mode 100644 index 0000000000000000000000000000000000000000..27f71a9f50197c5c54ae9bc72258e6e40045f9a2 --- /dev/null +++ b/e2e/test/scenarios/question/reproductions/42065-mongo-filter-by-id.cy.spec.js @@ -0,0 +1,119 @@ +import { WRITABLE_DB_ID } from "e2e/support/cypress_data"; +import { + restore, + openTable, + visualize, + filter, + openNotebook, + popover, +} from "e2e/support/helpers"; + +describe( + "issue 42010 -- Unable to filter by mongo id", + { tags: "@mongo" }, + () => { + beforeEach(() => { + restore("mongo-5"); + cy.signInAsAdmin(); + + cy.request(`/api/database/${WRITABLE_DB_ID}/schema/`).then(({ body }) => { + const tableId = body.find(table => table.name === "orders").id; + openTable({ + database: WRITABLE_DB_ID, + table: tableId, + limit: 2, + }); + }); + }); + + it("should be possible to filter by Mongo _id column (metabase#40770, metabase#42010)", () => { + cy.get("#main-data-grid") + .findAllByRole("gridcell") + .first() + .then($cell => { + // Ids are non-deterministic so we have to obtain the id from the cell, and store its value. + const id = $cell.text(); + + cy.log( + "Scenario 1 - Make sure we can filter directly by clicking on the cell", + ); + cy.wrap($cell).click(); + + cy.findByTestId("filter-pill") + .should("contain", `ID is ${id}`) + .click(); + + cy.findByTestId("question-row-count").should( + "have.text", + "Showing 1 row", + ); + + // This was showing a custom expression editor before the fix! + cy.findByTestId("string-filter-picker").within(() => { + cy.findByLabelText("Filter operator").should("have.value", "Is"); + cy.findByText(id).should("be.visible"); + }); + removeFilter(); + + cy.log( + "Scenario 2 - Make sure the simple mode filter is working correctly (metabase#40770)", + ); + filter(); + + cy.findByRole("dialog").within(() => { + cy.findByPlaceholderText("Search by ID").type(id); + cy.button("Apply filters").click(); + }); + + cy.findByTestId("question-row-count").should( + "have.text", + "Showing 1 row", + ); + removeFilter(); + + cy.log( + "Scenario 3 - Make sure filter is working in the notebook editor (metabase#42010)", + ); + openNotebook(); + filter({ mode: "notebook" }); + + popover() + .findAllByRole("option") + .first() + .should("have.text", "ID") + .click(); + + cy.findByTestId("string-filter-picker").within(() => { + cy.findByLabelText("Filter operator").should("have.value", "Is"); + cy.findByPlaceholderText("Search by ID").type(id); + cy.button("Add filter").click(); + }); + + cy.findByTestId("step-filter-0-0").within(() => { + cy.findByText(`ID is ${id}`); + + cy.log( + "Scenario 3.1 - Trigger the preview to make sure it reflects the filter correctly", + ); + cy.icon("play").click(); + }); + + cy.findByTestId("preview-root") + .should("contain", id) // first row + .and("not.contain", "110.93"); // second row + + cy.log("Scenario 3.2 - Make sure we can visualize the data"); + visualize(); + cy.findByTestId("question-row-count").should( + "have.text", + "Showing 1 row", + ); + }); + }); + }, +); + +function removeFilter() { + cy.findByTestId("filter-pill").findByLabelText("Remove").click(); + cy.findByTestId("question-row-count").should("have.text", "Showing 2 rows"); +} diff --git a/frontend/src/metabase-lib/column_types.ts b/frontend/src/metabase-lib/column_types.ts index 0461d4533e8118c946a66bf2adc7075b1a7d8862..5e6ad91690cd2d0da903a3a32425903e1e9ed357 100644 --- a/frontend/src/metabase-lib/column_types.ts +++ b/frontend/src/metabase-lib/column_types.ts @@ -36,6 +36,8 @@ export const isPrimaryKey: TypeFn = TYPES.primary_key_QMARK_; export const isScope: TypeFn = TYPES.scope_QMARK_; export const isState: TypeFn = TYPES.state_QMARK_; export const isString: TypeFn = TYPES.string_QMARK_; +export const isStringLike: TypeFn = TYPES.string_like_QMARK_; +export const isStringOrStringLike: TypeFn = TYPES.string_or_string_like_QMARK_; export const isSummable: TypeFn = TYPES.summable_QMARK_; export const isTime: TypeFn = TYPES.time_QMARK_; export const isTitle: TypeFn = TYPES.title_QMARK_; diff --git a/frontend/src/metabase-lib/filter.ts b/frontend/src/metabase-lib/filter.ts index 4f9b20ed2daab06ffabe04aebb803d7523f1d2d4..b2d958181ce5804be23fdb405e686ada62491f5e 100644 --- a/frontend/src/metabase-lib/filter.ts +++ b/frontend/src/metabase-lib/filter.ts @@ -8,7 +8,7 @@ import { isTime, isDate, isCoordinate, - isString, + isStringOrStringLike, isNumeric, } from "./column_types"; import { @@ -136,7 +136,7 @@ export function stringFilterParts( const [column, ...values] = args; if ( !isColumnMetadata(column) || - !isString(column) || + !isStringOrStringLike(column) || !isStringLiteralArray(values) ) { return null; diff --git a/frontend/src/metabase-lib/v1/parameters/utils/filters.js b/frontend/src/metabase-lib/v1/parameters/utils/filters.js index 16b10c1cd70f26d493329b71f7651acab4c768d0..9bd942cbbfb8a416fb090ffee1ccd853f45dd2d7 100644 --- a/frontend/src/metabase-lib/v1/parameters/utils/filters.js +++ b/frontend/src/metabase-lib/v1/parameters/utils/filters.js @@ -40,7 +40,8 @@ export function columnFilterForParameter(parameter) { case "number": return column => Lib.isNumber(column) && !Lib.isLocation(column); case "string": - return column => Lib.isString(column) && !Lib.isLocation(column); + return column => + Lib.isStringOrStringLike(column) && !Lib.isLocation(column); } return () => false; diff --git a/frontend/src/metabase/common/utils/columns.ts b/frontend/src/metabase/common/utils/columns.ts index ca09851410ab0762754e84df31a15c53d068c359..1fff200bd996ac4bffd74887a9afecf7d0096942 100644 --- a/frontend/src/metabase/common/utils/columns.ts +++ b/frontend/src/metabase/common/utils/columns.ts @@ -30,7 +30,7 @@ export function getColumnIcon(column: Lib.ColumnMetadata): IconName { if (Lib.isBoolean(column)) { return "io"; } - if (Lib.isString(column)) { + if (Lib.isStringOrStringLike(column)) { return "string"; } if (Lib.isNumeric(column)) { diff --git a/frontend/src/metabase/querying/components/FilterModal/ColumnFilterSection/ColumnFilterSection.tsx b/frontend/src/metabase/querying/components/FilterModal/ColumnFilterSection/ColumnFilterSection.tsx index 02c7f9161711ac8250651beaac0bfbb5dcfe6028..92780a279f45de0ce2c4b805bf5adf41914e7ef2 100644 --- a/frontend/src/metabase/querying/components/FilterModal/ColumnFilterSection/ColumnFilterSection.tsx +++ b/frontend/src/metabase/querying/components/FilterModal/ColumnFilterSection/ColumnFilterSection.tsx @@ -61,7 +61,7 @@ function getFilterWidget(column: Lib.ColumnMetadata) { if (Lib.isNumeric(column)) { return NumberFilterEditor; } - if (Lib.isString(column)) { + if (Lib.isStringOrStringLike(column)) { return StringFilterEditor; } return EmptyFilterEditor; diff --git a/frontend/src/metabase/querying/components/FilterModal/utils/sorting.ts b/frontend/src/metabase/querying/components/FilterModal/utils/sorting.ts index c5928cb51b31fe26fca888ec3ede11aa00a2a1e1..1941f2b1b2826dbb1a35baf701566d6846ea641b 100644 --- a/frontend/src/metabase/querying/components/FilterModal/utils/sorting.ts +++ b/frontend/src/metabase/querying/components/FilterModal/utils/sorting.ts @@ -20,7 +20,7 @@ function isNumberAndNotCoordinate(column: Lib.ColumnMetadata) { } function isShortText(column: Lib.ColumnMetadata) { - return Lib.isString(column) && !isLongText(column); + return Lib.isStringOrStringLike(column) && !isLongText(column); } function isLongText(column: Lib.ColumnMetadata) { diff --git a/frontend/src/metabase/querying/components/FilterPicker/FilterPickerBody/FilterPickerBody.tsx b/frontend/src/metabase/querying/components/FilterPicker/FilterPickerBody/FilterPickerBody.tsx index 1068b24b8e700a8a9f44c6216a6939d7e161cc06..677b0004543a9907143d42729975b0f22526f205 100644 --- a/frontend/src/metabase/querying/components/FilterPicker/FilterPickerBody/FilterPickerBody.tsx +++ b/frontend/src/metabase/querying/components/FilterPicker/FilterPickerBody/FilterPickerBody.tsx @@ -60,7 +60,7 @@ function getFilterWidget(column: Lib.ColumnMetadata) { if (Lib.isNumeric(column)) { return NumberFilterPicker; } - if (Lib.isString(column)) { + if (Lib.isStringOrStringLike(column)) { return StringFilterPicker; } return null; diff --git a/frontend/src/metabase/querying/utils/drills/quick-filter-drill/quick-filter-drill.tsx b/frontend/src/metabase/querying/utils/drills/quick-filter-drill/quick-filter-drill.tsx index a3d2e2e5183ea28ffe81db4db09fe5ccc347e879..af3edc4a58a58a035e358e0d5e7e5c9e88cd5f94 100644 --- a/frontend/src/metabase/querying/utils/drills/quick-filter-drill/quick-filter-drill.tsx +++ b/frontend/src/metabase/querying/utils/drills/quick-filter-drill/quick-filter-drill.tsx @@ -58,7 +58,7 @@ function getActionOverrides( } } - if (Lib.isString(column) && typeof value === "string") { + if (Lib.isStringOrStringLike(column) && typeof value === "string") { const columnName = Lib.displayInfo(query, stageIndex, column).displayName; const valueTitle = getTextValueTitle(value); const action: Partial<ClickAction> = { diff --git a/src/metabase/lib/types/isa.cljc b/src/metabase/lib/types/isa.cljc index abfdce56dceae103732f102286ca26b336e739e5..9e9821cff8dc95d0ff45cae4b7a5ba2a13833c05 100644 --- a/src/metabase/lib/types/isa.cljc +++ b/src/metabase/lib/types/isa.cljc @@ -80,6 +80,16 @@ [column] (field-type? ::lib.types.constants/string column)) +(defn ^:export string-like? + "Is `column` of a temporal type?" + [column] + (field-type? ::lib.types.constants/string_like column)) + +(defn ^:export string-or-string-like? + "Is `column` of a temporal type?" + [column] + (or (string? column) (string-like? column))) + (defn ^:export summable? "Is `column` of a summable type?" [column]