Cover Offset() in aggregations with e2e tests (#42455)

* Add repro for 32323 * Fix offset not working in case * Make offset function return any * Add a repro for #42377 * Fix order of adjustments * Revert unrelated changes * Remove commented code * Revert unrelated changes * Refactor test * Type offset aggregation * Add a test for no order by or breakout clause * Add a basic test for breakout clause * Fix assertion * Remove problematic dependency * Fix uuids generation * Remove redundant limit * Add a test for multiple breakouts * Update test names * Extract OFFSET_SUM_TOTAL_AGGREGATION * Add a repro for #42509 * Add a test for multiple aggregations and breakouts * Remove unused intercept * Move uuid utils to separate file - it wasn't possible to import the utils file in e2e tests without it * Make isUuid a type guard * Add tests for sorting * Tag the test * Add extra assertion to verify expression parsing * Add a complex scenario * Reverse isFirst logic
metabase · May 13, 2024 · f596f0b · f596f0b
1 parent 38d540a
commit f596f0b
Show file tree

Hide file tree

Showing 9 changed files with 413 additions and 55 deletions.
diff --git a/e2e/test/scenarios/question/offset.cy.spec.ts b/e2e/test/scenarios/question/offset.cy.spec.ts
diff --git a/frontend/src/metabase-types/api/query.ts b/frontend/src/metabase-types/api/query.ts
@@ -155,6 +155,12 @@ type StdDevAgg = ["stddev", ConcreteFieldReference];
 type SumAgg = ["sum", ConcreteFieldReference];
 type MinAgg = ["min", ConcreteFieldReference];
 type MaxAgg = ["max", ConcreteFieldReference];
+type OffsetAgg = [
+ "offset",
+ { "lib/uuid": string; name: string; "display-name": string },
+ Aggregation,
+ number,
+];
 
 type CommonAggregation =
  | CountAgg
@@ -166,7 +172,8 @@ type CommonAggregation =
  | StdDevAgg
  | SumAgg
  | MinAgg
- | MaxAgg;
+ | MaxAgg
+ | OffsetAgg;
 
 type MetricAgg = ["metric", MetricId];
 

diff --git a/frontend/src/metabase/admin/settings/components/widgets/CustomGeoJSONWidget.jsx b/frontend/src/metabase/admin/settings/components/widgets/CustomGeoJSONWidget.jsx
@@ -12,7 +12,7 @@ import Select, { Option } from "metabase/core/components/Select";
 import AdminS from "metabase/css/admin.module.css";
 import ButtonsS from "metabase/css/components/buttons.module.css";
 import CS from "metabase/css/core/index.css";
-import { uuid } from "metabase/lib/utils";
+import { uuid } from "metabase/lib/uuid";
 import { SettingsApi, GeoJSONApi } from "metabase/services";
 import LeafletChoropleth from "metabase/visualizations/components/LeafletChoropleth";
 

diff --git a/frontend/src/metabase/dashboard/utils.ts b/frontend/src/metabase/dashboard/utils.ts
@@ -3,7 +3,8 @@ import _ from "underscore";
 
 import { IS_EMBED_PREVIEW } from "metabase/lib/embed";
 import { SERVER_ERROR_TYPES } from "metabase/lib/errors";
-import { isUUID, isJWT } from "metabase/lib/utils";
+import { isJWT } from "metabase/lib/utils";
+import { isUuid } from "metabase/lib/uuid";
 import {
  getGenericErrorMessage,
  getPermissionErrorMessage,
@@ -182,7 +183,7 @@ export function getDashboardType(id: unknown) {
  if (id == null || typeof id === "object") {
  // HACK: support inline dashboards
  return "inline";
- } else if (isUUID(id)) {
+ } else if (isUuid(id)) {
  return "public";
  } else if (isJWT(id)) {
  return "embed";

diff --git a/frontend/src/metabase/lib/utils.ts b/frontend/src/metabase/lib/utils.ts
@@ -6,12 +6,6 @@ import { PLUGIN_IS_EE_BUILD } from "metabase/plugins";
 const EMAIL_REGEX =
  /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
 
-function s4() {
- return Math.floor((1 + Math.random()) * 0x10000)
- .toString(16)
- .substring(1);
-}
-
 export function isEmpty(str: string | null) {
  if (str != null) {
  str = String(str);
@@ -41,30 +35,6 @@ export function numberToWord(num: number) {
  }
 }
 
-export function uuid() {
- return (
- s4() +
- s4() +
- "-" +
- s4() +
- "-" +
- s4() +
- "-" +
- s4() +
- "-" +
- s4() +
- s4() +
- s4()
- );
-}
-
-export function isUUID(uuid: unknown) {
- return (
- typeof uuid === "string" &&
- /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/.test(uuid)
- );
-}
-
 export function isJWT(string: unknown) {
  return (
  typeof string === "string" &&

diff --git a/frontend/src/metabase/lib/uuid.ts b/frontend/src/metabase/lib/uuid.ts
@@ -0,0 +1,29 @@
+export function uuid() {
+ return (
+ s4() +
+ s4() +
+ "-" +
+ s4() +
+ "-" +
+ s4() +
+ "-" +
+ s4() +
+ "-" +
+ s4() +
+ s4() +
+ s4()
+ );
+}
+
+function s4() {
+ return Math.floor((1 + Math.random()) * 0x10000)
+ .toString(16)
+ .substring(1);
+}
+
+export function isUuid(uuid: unknown): uuid is string {
+ return (
+ typeof uuid === "string" &&
+ /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/.test(uuid)
+ );
+}
diff --git a/frontend/src/metabase/query_builder/components/template_tags/TagEditorHelp/TagEditorHelp.tsx b/frontend/src/metabase/query_builder/components/template_tags/TagEditorHelp/TagEditorHelp.tsx
@@ -7,7 +7,7 @@ import ExternalLink from "metabase/core/components/ExternalLink";
 import CS from "metabase/css/core/index.css";
 import { useSelector } from "metabase/lib/redux";
 import MetabaseSettings from "metabase/lib/settings";
-import { uuid } from "metabase/lib/utils";
+import { uuid } from "metabase/lib/uuid";
 import { getShowMetabaseLinks } from "metabase/selectors/whitelabel";
 import type Database from "metabase-lib/v1/metadata/Database";
 import type { DatabaseId, NativeDatasetQuery } from "metabase-types/api";

diff --git a/...sualizations/visualizations/SmartScalar/SettingsComponents/SmartScalarSettingsWidgets.tsx b/...sualizations/visualizations/SmartScalar/SettingsComponents/SmartScalarSettingsWidgets.tsx
@@ -1,21 +1,20 @@
 import type { DragEndEvent } from "@dnd-kit/core";
-import { DndContext, useSensor, PointerSensor } from "@dnd-kit/core";
+import { DndContext, PointerSensor, useSensor } from "@dnd-kit/core";
 import {
- restrictToVerticalAxis,
  restrictToParentElement,
+ restrictToVerticalAxis,
 } from "@dnd-kit/modifiers";
 import {
+ SortableContext,
  arrayMove,
  verticalListSortingStrategy,
- SortableContext,
 } from "@dnd-kit/sortable";
 import { useCallback } from "react";
 import { usePreviousDistinct } from "react-use";
 import { t } from "ttag";
-import _ from "underscore";
 
 import { Sortable } from "metabase/core/components/Sortable";
-import { uuid } from "metabase/lib/utils";
+import { uuid } from "metabase/lib/uuid";
 import { Stack } from "metabase/ui";
 import type { DatasetColumn, SmartScalarComparison } from "metabase-types/api";
 

diff --git a/frontend/src/metabase/visualizations/visualizations/SmartScalar/utils.ts b/frontend/src/metabase/visualizations/visualizations/SmartScalar/utils.ts
@@ -4,7 +4,7 @@ import _ from "underscore";
 
 import { formatNumber } from "metabase/lib/formatting/numbers";
 import { measureText } from "metabase/lib/measure-text";
-import { uuid } from "metabase/lib/utils";
+import { uuid } from "metabase/lib/uuid";
 import { isEmpty } from "metabase/lib/validate";
 import { isDate, isNumeric } from "metabase-lib/v1/types/utils/isa";
 import type {