Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI-9566 - Ensure a best effort migration of the KPI titles even when some titles could not be successfully migrated. #129

Merged
merged 23 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
dd3f796
UI-9566 - Ensure kpis with titles corresponding to empty measures hav…
NZepeda Jul 29, 2024
620462a
Minor improvement
NZepeda Jul 30, 2024
88bab6b
Drop kpi titles that have an empty location.
NZepeda Jul 30, 2024
5e66d80
Update src/4.3_to_5.0/getMigratedKpiTitles.ts
NZepeda Jul 30, 2024
206fc72
Update test resource
NZepeda Jul 30, 2024
fd9896d
Merge branch 'task/nze/UI-9566-support-migrating-empty-titles' of git…
NZepeda Jul 30, 2024
3876da1
Apply suggestions from code review
NZepeda Jul 31, 2024
85700d8
Update tests
NZepeda Jul 31, 2024
96a710a
UI-9566 - Ensure a best effort migration of the kpi titles. If there …
NZepeda Jul 31, 2024
6f50c45
Update src/migrateContentServer.test.ts
NZepeda Jul 31, 2024
71fbb2d
Fixups
NZepeda Jul 31, 2024
1d5c86c
Merge branch 'task/nze/UI-9566-support-migrating-empty-titles' of git…
NZepeda Jul 31, 2024
643e807
Merge main
NZepeda Aug 1, 2024
53e4964
Use an empty folder test resource and a generator function to create …
NZepeda Aug 14, 2024
22b5547
Update previously added tests to utilize the new test resource function
NZepeda Aug 14, 2024
31e1754
Remove the now unnecessary test resource files
NZepeda Aug 14, 2024
bdcdec4
Move the location of addLegacyBookmarkToUIFolder to the test resource…
NZepeda Sep 18, 2024
1eb0b41
The empty legacy ui folder test resource is truly empty
NZepeda Sep 18, 2024
22ecfc2
Split out the invalid legacy bookmarks and their folder into their ow…
NZepeda Sep 18, 2024
1cde91f
Split out the legacy kpi bookmark and folder into their own resource …
NZepeda Sep 18, 2024
2280099
Create the smallLegacy ui folder with the new add legacy bookmark to …
NZepeda Sep 18, 2024
26adf4b
Apply suggestions from code review
NZepeda Sep 18, 2024
dee3129
Ensure the test snapshots have not lost data
NZepeda Sep 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 54 additions & 3 deletions src/4.3_to_5.0/__snapshots__/migrate_43_to_50.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2497,7 +2497,24 @@ exports[`migrate_43_to_50 returns a valid ActiveUI5 /ui folder on a small input
"widgets": {
"children": {
"content": {
"children": {},
"children": {
"kpi": {
"entry": {
"canRead": true,
"canWrite": true,
"content": "{"query":{"mdx":"SELECT NON EMPTY Hierarchize(AddCalculatedMembers(Descendants({[Geography].[City].[ALL].[AllMember]}, 1, SELF_AND_BEFORE))) ON ROWS, NON EMPTY {[Measures].[contributors.COUNT]} ON COLUMNS FROM [EquityDerivativesCube]","updateMode":"once"},"filters":["[Geography].[City].[ALL].[AllMember].[New York]"],"queryContext":[],"mapping":{"rows":["[Geography].[City].[City]"],"columns":[],"measures":["[Measures].[contributors.COUNT]"]},"serverKey":"my-server","titles":{"[Measures].[contributors.COUNT]":"Custom title for contributors.COUNT"}}",
"isDirectory": false,
"lastEditor": "admin",
"owners": [
"admin",
],
"readers": [
"admin",
],
"timestamp": 1607879735685,
},
},
},
"entry": {
"isDirectory": true,
"owners": [
Expand All @@ -2509,7 +2526,41 @@ exports[`migrate_43_to_50 returns a valid ActiveUI5 /ui folder on a small input
},
},
"structure": {
"children": {},
"children": {
"kpi": {
"children": {
"kpi_metadata": {
"entry": {
"canRead": true,
"canWrite": true,
"content": "{"name":"KPI","widgetKey":"kpi"}",
"isDirectory": true,
"lastEditor": "admin",
"owners": [
"admin",
],
"readers": [
"admin",
],
"timestamp": 1607879735685,
},
},
},
"entry": {
"canRead": true,
"canWrite": true,
"isDirectory": true,
"lastEditor": "admin",
"owners": [
"admin",
],
"readers": [
"admin",
],
"timestamp": 1607879735685,
},
},
},
"entry": {
"isDirectory": true,
"owners": [
Expand Down Expand Up @@ -2568,7 +2619,7 @@ exports[`migrate_43_to_50 returns a valid ActiveUI5 /ui folder on a small input
"failed": 0,
"partial": 0,
"removed": 0,
"success": 0,
"success": 1,
},
}
`;
Expand Down
3 changes: 3 additions & 0 deletions src/4.3_to_5.0/__test_resources__/legacyKpi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ export const legacyKpi: LegacyWidgetState = {
configuration: {
featuredValues: {
locations: {
"": {
title: "Empty measure title",
},
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this fix, I went with dropping titles that have an empty tuple key and ensuring the rest of the titles are still migrated. However, I'm not 100% sure how one can even land in this state. It doesn't seem like it's valid so I dropped it, but if it is better to ensure this kind of custom title is supported, I can adapt the change to ensure it's also migrated.

// "EUR, USD" is not a member of the sandbox, but it is used here to check that the script supports member names including ",".
"[Currency].[Currency].[AllMember].[EUR, USD],[Measures].[contributors.COUNT]":
{
Expand Down
24 changes: 24 additions & 0 deletions src/4.3_to_5.0/__test_resources__/smallLegacyUIFolder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,19 @@ export const smallLegacyUIFolder = {
canWrite: true,
},
},
kpi: {
entry: {
content:
'{"description": "A KPI","name":"KPI","type":"container","value": {"style": {},"showTitleBar": true,"body": {"serverUrl": "","mdx": "SELECT NON EMPTY Hierarchize(AddCalculatedMembers(Descendants({[Geography].[City].[ALL].[AllMember]},1,SELF_AND_BEFORE))) ON ROWS,NON EMPTY {[Measures].[contributors.COUNT]} ON COLUMNS FROM (SELECT[Geography].[City].[ALL].[AllMember].[New York] ON COLUMNS FROM [EquityDerivativesCube])","contextValues": {},"updateMode": "once","ranges": {"row": {},"column": {}},"configuration": {"featuredValues": {"locations":{"":{"title": "Title with empty location"},"[Measures].[contributors.COUNT]": {"title": "Custom title for contributors.COUNT"}}}}},"containerKey":"featured-values"},"writable": true}',
isDirectory: false,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
},
},
i18n: {
Expand Down Expand Up @@ -106,6 +119,17 @@ export const smallLegacyUIFolder = {
canWrite: true,
},
},
kpi: {
entry: {
isDirectory: true,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
},
},
},
Expand Down
9 changes: 8 additions & 1 deletion src/4.3_to_5.0/getMigratedKpiTitles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
getMeasuresPositionOnAxis,
MdxSelect,
} from "@activeviam/mdx-5.0";
import _isEmpty from "lodash/isEmpty";

interface LegacyKpiTitle {
title: string;
Expand Down Expand Up @@ -43,6 +44,9 @@ function _getLegacyKpiTitles(
} = {};
const memberUniqueNames = tupleKey.split(/,(?![^\[]*\])/);
for (const memberUniqueName of memberUniqueNames) {
if (!memberUniqueName) {
continue;
}
const compoundIdentifier =
parse<MdxUnknownCompoundIdentifier>(memberUniqueName);
const specificCompoundIdentifier = getSpecificCompoundIdentifier(
Expand All @@ -61,7 +65,10 @@ function _getLegacyKpiTitles(
tuple[hierarchyUniqueName] = specificCompoundIdentifier.path;
}
}
legacyTitles.push({ title, tuple });

if (!_isEmpty(tuple)) {
legacyTitles.push({ title, tuple });
}
}

return legacyTitles;
Expand Down
38 changes: 38 additions & 0 deletions src/migrateContentServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,42 @@ describe("migrateContentServer", () => {
undefined,
);
});

it("migrates the kpi custom titles when the original version contains a custom title with a tupleKey that is empty.", async () => {
NZepeda marked this conversation as resolved.
Show resolved Hide resolved
const contentServer: ContentRecord = {
children: { ui: smallLegacyUIFolder, pivot: smallLegacyPivotFolder },
entry: {
owners: [],
readers: [],
isDirectory: true,
canRead: true,
canWrite: false,
lastEditor: "Freddie Mercury",
Nouzbe marked this conversation as resolved.
Show resolved Hide resolved
timestamp: 0xbeef,
Nouzbe marked this conversation as resolved.
Show resolved Hide resolved
},
};

await migrateContentServer({
contentServer,
servers,
fromVersion: "4.3",
toVersion: "5.1",
keysOfWidgetPluginsToRemove: [],
doesReportIncludeStacks: false,
shouldUpdateFiltersMdx: true,
behaviorOnError: "keep-original",
});

const migratedKpiContent = JSON.parse(
contentServer.children?.ui.children?.widgets.children?.content.children?.[
"kpi"
].entry.content,
);
NZepeda marked this conversation as resolved.
Show resolved Hide resolved

expect(migratedKpiContent?.titles).toMatchInlineSnapshot(`
{
"[Measures].[contributors.COUNT]": "Custom title for contributors.COUNT",
}
`);
});
});
Loading