Skip to content

Commit

Permalink
UI-8742 - Migration tool wrongly changes the distinct count calculate…
Browse files Browse the repository at this point in the history
…d measure (#113)

* First draft

* lint

* Update with continous build

* Remove previous commit

* Update comment

* Review
  • Loading branch information
mailys-ds authored Sep 25, 2023
1 parent 4fdf917 commit 9dccacd
Show file tree
Hide file tree
Showing 5 changed files with 1,414 additions and 1,372 deletions.
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -28,14 +28,14 @@
"trailingComma": "all"
},
"devDependencies": {
"@activeviam/activeui-sdk-5.0": "npm:@activeviam/[email protected].28",
"@activeviam/activeui-sdk-5.0": "npm:@activeviam/[email protected].29-20230915115910-fc34c05",
"@activeviam/activeui-sdk-5.1": "npm:@activeviam/[email protected]",
"@activeviam/activeui-sdk-scripts": "5.0.28",
"@activeviam/content-server-initialization-5.0": "npm:@activeviam/[email protected].28",
"@activeviam/activeui-sdk-scripts": "5.0.29-20230915115910-fc34c05",
"@activeviam/content-server-initialization-5.0": "npm:@activeviam/[email protected].29-20230915115910-fc34c05",
"@activeviam/dashboard-base-5.1": "npm:@activeviam/[email protected]",
"@activeviam/data-model-5.0": "npm:@activeviam/[email protected].28",
"@activeviam/data-model-5.0": "npm:@activeviam/[email protected].29-20230915115910-fc34c05",
"@activeviam/data-model-5.1": "npm:@activeviam/[email protected]",
"@activeviam/mdx-5.0": "npm:@activeviam/[email protected].28",
"@activeviam/mdx-5.0": "npm:@activeviam/[email protected].29-20230915115910-fc34c05",
"@activeviam/mdx-5.1": "npm:@activeviam/[email protected]",
"@swc/core": "^1.3.31",
"@swc/jest": "^0.2.24",
Expand Down
33 changes: 28 additions & 5 deletions src/4.3_to_5.0/_cleanupDescendants.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { parse, stringify } from "@activeviam/mdx-5.0";
import { MdxSelect, parse, stringify } from "@activeviam/mdx-5.0";
import { dataModelsForTests } from "@activeviam/data-model-5.0";

import { _cleanupDescendants } from "./_cleanupDescendants";
Expand All @@ -7,7 +7,7 @@ const cube = dataModelsForTests.sandbox.catalogs[0].cubes[0];

describe("_cleanupDescendants", () => {
it("replaces a Descendants function by its input set if the shallowest level expressed in the set is the same as the target one or above", () => {
const mdx = parse(`
const mdx = parse<MdxSelect>(`
SELECT
NON EMPTY Hierarchize(
Descendants(
Expand All @@ -31,7 +31,7 @@ describe("_cleanupDescendants", () => {
});

it("replaces a useless Descendants function called on a set from a multilevel hierarchy", () => {
const mdx = parse(`
const mdx = parse<MdxSelect>(`
SELECT
NON EMPTY Hierarchize(
Descendants(
Expand All @@ -55,7 +55,7 @@ describe("_cleanupDescendants", () => {
});

it("replaces a useless Descendants function when the second argument is a level index", () => {
const mdx = parse(`
const mdx = parse<MdxSelect>(`
SELECT
NON EMPTY Hierarchize(
Descendants(
Expand All @@ -79,7 +79,7 @@ describe("_cleanupDescendants", () => {
});

it("replaces a useless Descendants function called on a set with members from the leaf level, without a second argument", () => {
const mdx = parse(`
const mdx = parse<MdxSelect>(`
SELECT
NON EMPTY Hierarchize(
Descendants(
Expand All @@ -100,4 +100,27 @@ describe("_cleanupDescendants", () => {
FROM [EquityDerivativesCube]"
`);
});

it("does not cleanup the descendants of a hierarchy's current member", () => {
const mdxString = `
WITH
Member [Measures].[City] AS Count(
Descendants(
[Geography].[City].CurrentMember,
[Geography].[City].[City]
),
EXCLUDEEMPTY
), FORMAT_STRING = "#,###.##"
SELECT
NON EMPTY {
[Measures].[City]
} ON COLUMNS,
NON EMPTY [Geography].[City].[City].Members ON ROWS
FROM [EquityDerivativesCube]
CELL PROPERTIES BACK_COLOR, FONT_FLAGS, FORE_COLOR, FORMATTED_VALUE, VALUE
`;
const mdx = parse<MdxSelect>(mdxString);
const cleanMdx = _cleanupDescendants(mdx, cube);
expect(cleanMdx).toStrictEqual(mdx);
});
});
13 changes: 11 additions & 2 deletions src/4.3_to_5.0/_cleanupDescendants.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { Mdx, Cube } from "@activeviam/activeui-sdk-5.0";
import { MdxDrillthrough, MdxSelect, Cube } from "@activeviam/activeui-sdk-5.0";
import { getHierarchy, getLevelIndex } from "@activeviam/data-model-5.0";
import {
MdxFunction,
Expand All @@ -22,6 +22,12 @@ const canDescendantsBeReplacedByItsFirstArgument = (
) => {
const [set, downToLevel] = descendantsNode.arguments;
const levelsInSet = getLevels(set, { cube });

// If no levels are found in the input set, for instance when it consists only in the `CurrentMember` of a hierarchy (which could be on any level), the `Descendants` function should not be removed.
if (levelsInSet.length === 0) {
return false;
}

const shallowestLevelIndexInSet = Math.min(
...levelsInSet.map((levelCoordinates) =>
getLevelIndex({ cube, ...levelCoordinates }),
Expand Down Expand Up @@ -67,7 +73,10 @@ const canDescendantsBeReplacedByItsFirstArgument = (
* Descendants([Currency].[Currency].[ALL].[AllMember].[EUR], [Currency].[Currency].[Currency]) => [Currency].[Currency].[ALL].[AllMember].[EUR]
*
*/
export function _cleanupDescendants<T extends Mdx>(mdx: T, cube: Cube): T {
export function _cleanupDescendants<T extends MdxSelect | MdxDrillthrough>(
mdx: T,
cube: Cube,
): T {
return produce(mdx, (draft) => {
let nextNodeToCleanup;
while (
Expand Down
2 changes: 1 addition & 1 deletion src/4.3_to_5.0/_migrateQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export interface LegacyQuery {

/**
* Returns an array whose first item contains the query, filters and context corresponding to the given legacy query.
* The second item indicates wheter an unsupported update mode was used in the legacy query.
* The second item indicates whether an unsupported update mode was used in the legacy query.
*/
export const _migrateQuery = <T extends MdxSelect | MdxDrillthrough>({
legacyQuery,
Expand Down
Loading

0 comments on commit 9dccacd

Please sign in to comment.