Skip to content

Commit

Permalink
UI-9526 - Ensures that the behaviorOnError flag is respected (#128)
Browse files Browse the repository at this point in the history 
* Errored widgets are left untouched

* Add test for usage of the keep-last-successful-version behaviorOnError flag

* Small edits

* Update src/4.3_to_5.0/__test_resources__/smallLegacyUIFolderWithInvalidWidget.ts

* Update src/migrateContentServer.test.ts

Co-authored-by: Benjamin Amelot <[email protected]>

* Update src/migrateContentServer.test.ts

Co-authored-by: Benjamin Amelot <[email protected]>

* Update src/migrateContentServer.test.ts

Co-authored-by: Benjamin Amelot <[email protected]>

* UI-9526 - Undo changes to the migrate_43_to_50 file since conserving the bookmark was already handled

* Update src/migrateContentServer.test.ts

---------

Co-authored-by: Benjamin Amelot <[email protected]>
  • Loading branch information
@NZepeda @Nouzbe
NZepeda and Nouzbe authored Aug 1, 2024
1 parent c883371 commit 5e3020e
Show file tree
Hide file tree
Showing 4 changed files with 292 additions and 2 deletions.
186 changes: 186 additions & 0 deletions src/4.3_to_5.0/__test_resources__/smallLegacyUIFolderWithInvalidWidget.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,186 @@
/**
* The shortened version of the content of the /ui folder on a Content Server, useful for unit tests.
* Contains a valid widget, a widget whose `containerKey` is not valid, and another widget with a filter on a hierarchy that does not exist in the test cube.
*/
export const smallLegacyUIFolderWithInvalidWidget = {
entry: {
isDirectory: true,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879725132,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
children: {
bookmarks: {
entry: {
isDirectory: true,
owners: ["ROLE_CS_ROOT"],
readers: ["ROLE_USER"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
children: {
content: {
entry: {
isDirectory: true,
owners: ["ROLE_USER"],
readers: ["ROLE_USER"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
children: {
"158": {
entry: {
content:
'{"description":"Widget with invalid container key","name":"Invalid widget","type":"container","value":{"style":{},"showTitleBar":false,"containerKey":"invalid-container-key","body":{"serverUrl":"","mdx":"SELECT NON EMPTY [Measures].[contributors.COUNT] ON COLUMNS FROM [EquityDerivativesCube] WHERE [Geography].[City].[ALL].[AllMember].[New York] CELL PROPERTIES VALUE, FORMATTED_VALUE, BACK_COLOR, FORE_COLOR, FONT_FLAGS","contextValues":{},"updateMode":"once","ranges":{}}}}',
isDirectory: false,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
"1231": {
entry: {
content:
'{"description":"Widget with filter on invalid hierarchy","name":"Invalid widget","type":"container","value":{"style":{},"showTitleBar":false,"containerKey":"pivot-table","body":{"serverUrl":"","mdx":"SELECT NON EMPTY [Measures].[contributors.COUNT] ON COLUMNS FROM [EquityDerivativesCube] WHERE [Geography].[InvalidHierarchy].[ALL].[AllMember].[Member] CELL PROPERTIES VALUE, FORMATTED_VALUE, BACK_COLOR, FORE_COLOR, FONT_FLAGS","contextValues":{},"updateMode":"once","ranges":{}}}}',
isDirectory: false,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
"777": {
entry: {
content:
'{"description": "Valid widget","name": "Valid widget","type": "container", "value": {"style": {},"showTitleBar": false,"containerKey": "pivot-table","body": {"serverUrl": "","mdx": "SELECT NON EMPTY [Measures].[contributors.COUNT] ON COLUMNS FROM [EquityDerivativesCube] WHERE [Geography].[City].[ALL].[AllMember].[New York] CELL PROPERTIES VALUE, FORMATTED_VALUE, BACK_COLOR, FORE_COLOR, FONT_FLAGS","contextValues": {},"updateMode": "once", "ranges": {}}}}',
isDirectory: false,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
},
},
i18n: {
entry: {
isDirectory: true,
owners: ["ROLE_CS_ROOT"],
readers: ["ROLE_USER"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
children: {
"en-US": {
entry: {
isDirectory: true,
owners: ["ROLE_CS_ROOT"],
readers: ["ROLE_USER"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
"fr-FR": {
entry: {
isDirectory: true,
owners: ["ROLE_CS_ROOT"],
readers: ["ROLE_USER"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
},
},
structure: {
entry: {
isDirectory: true,
owners: ["ROLE_USER"],
readers: ["ROLE_USER"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
children: {
"158": {
entry: {
isDirectory: true,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
"1231": {
entry: {
isDirectory: true,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
"777": {
entry: {
isDirectory: true,
owners: ["admin"],
readers: ["admin"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
},
},
},
},
settings: {
entry: {
isDirectory: true,
owners: ["ROLE_CS_ROOT"],
readers: ["ROLE_USER"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
version: {
entry: {
content: '{"package":"4.3.8","contentServerApi":"0.1.0"}',
isDirectory: false,
owners: ["ROLE_CS_ROOT"],
readers: ["ROLE_USER"],
timestamp: 1607879735685,
lastEditor: "admin",
canRead: true,
canWrite: true,
},
},
},
};
2 changes: 1 addition & 1 deletion src/bin.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ yargs
More generically, assuming that the error occurred at step p out of a total of n, you can choose one of the following behaviors:
\n- "keep-original": keep the original item untouched, as before the whole migration.
\n- "keep-last-successful-version": keep the version of the item obtained after the first p-1 successful steps.
\n- "keep-going": try to apply the n-p remaining steps to the version of the item obtained after step p, despite the error. Note that the remaining steps are likely to fail too, and in that case the result will be the same as "keep-last-succesful-version".
\n- "keep-going": try to apply the n-p remaining steps to the version of the item obtained after step p, despite the error. Note that the remaining steps are likely to fail too, and in that case the result will be the same as "keep-last-successful-version".
`,
});
args.implies("stack", "debug");
Expand Down
2 changes: 1 addition & 1 deletion src/getMigrateSavedWidgets.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ export const getMigrateSavedWidgets =

for (const fileId in content.children) {
if (errorReport.widgets?.[fileId] && behaviorOnError !== "keep-going") {
return;
continue;
}

if (!filesAncestry[fileId]) {
Expand Down
104 changes: 104 additions & 0 deletions src/migrateContentServer.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { smallLegacyPivotFolder } from "./4.3_to_5.0/__test_resources__/smallLeg
import { smallLegacyUIFolder } from "./4.3_to_5.0/__test_resources__/smallLegacyUIFolder";
import { migrateContentServer } from "./migrateContentServer";
import _cloneDeep from "lodash/cloneDeep";
import { smallLegacyUIFolderWithInvalidWidget } from "./4.3_to_5.0/__test_resources__/smallLegacyUIFolderWithInvalidWidget";

jest.mock(`./4.3_to_5.0/generateId`, () => {
let counter = 0;
Expand Down Expand Up @@ -232,4 +233,107 @@ describe("migrateContentServer", () => {
undefined,
);
});

it("keeps the original item untouched, as before the whole migration when the item cannot be migrated due to an error and the `behaviorOnError` flag is set to `keep-original`.", async () => {
const contentServer: ContentRecord = {
children: {
ui: smallLegacyUIFolderWithInvalidWidget,
pivot: smallLegacyPivotFolder,
},
entry: {
owners: [],
readers: [],
isDirectory: true,
canRead: true,
canWrite: false,
lastEditor: "Freddie Mercury",
timestamp: 0xbeef,
},
};

const contentServerBeforeMigration = _cloneDeep(contentServer);

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

// In 4.3 the content of the widget is kept in the `body` which is within the bookmark's `content` string.
// If the migration of the widget fails, we keep the content of the widget intact but move its location in order to maintain the structure expected in 5.0/5.1.
const savedWidgetContentBeforeMigration = JSON.stringify(
JSON.parse(
contentServerBeforeMigration.children?.ui.children?.bookmarks.children
?.content.children?.["158"].entry.content,
).value.body,
);

const savedWidgetContentAfterMigration =
contentServer?.children?.ui?.children?.widgets?.children?.content
?.children?.["158"].entry.content;

// The widget with id "158" has an invalid widget key and its migration will therefore fail.
// Since `behaviorOnError` is set to "keep-original", the output content still contains the state of this saved widget exactly as it was before the migration.
expect(savedWidgetContentBeforeMigration).toBe(
savedWidgetContentAfterMigration,
);
});

it("keeps the 5.0 version of the widget, when migrating from 4.3 to 5.1 with `behaviorOnError` set to `keep-last-successful-version` and the 5.0 to 5.1 step fails", async () => {
const contentServer: ContentRecord = {
children: {
ui: smallLegacyUIFolderWithInvalidWidget,
pivot: smallLegacyPivotFolder,
},
entry: {
owners: [],
readers: [],
isDirectory: true,
canRead: true,
canWrite: false,
lastEditor: "Freddie Mercury",
timestamp: 0xbeef,
},
};

const contentServerBeforeMigration = _cloneDeep(contentServer);

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

const savedWidgetContentBeforeMigration =
contentServerBeforeMigration.children?.ui.children?.bookmarks.children
?.content.children?.["1231"].entry.content;

expect(savedWidgetContentBeforeMigration.filters).not.toBeDefined();

const savedWidgetContentAfterMigration = JSON.parse(
contentServer?.children?.ui?.children?.widgets?.children?.content
?.children?.["1231"].entry.content,
);

expect(savedWidgetContentAfterMigration.filters).toBeDefined();
expect(savedWidgetContentAfterMigration.filters).not.toHaveLength(0);

const migratedWidgetFilter = savedWidgetContentAfterMigration.filters[0];

// The widget with id "1231" contains a filter referring to a hierarchy which does not exist in the cube.
// This leads to an error in the 5.0 -> 5.1 step.
// As `behaviorOnError` is set to "keep-last-successful-version", the output content contains the "5.0" version of this widget.
// The filters in 5.0 are represented as strings.
expect(typeof migratedWidgetFilter.mdx).toBe("string");
});
});

0 comments on commit 5e3020e

Please sign in to comment.