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.

Sign up for GitHub

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

feat: format SharedDB operations as human-readable list of changes #2877

Merged
merged 9 commits into from
Apr 19, 2024
Merged

feat: format SharedDB operations as human-readable list of changes #2877

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2c0cccf
pretty print operations
jessicamcinchak Mar 11, 2024
efdd614
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Apr 11, 2024
aa51801
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Apr 12, 2024
0a4338f
rebase & display last formatted op
jessicamcinchak Apr 12, 2024
2dd3841
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Apr 17, 2024
d2dc1a0
add unit tests
jessicamcinchak Apr 17, 2024
a0ab1e3
add UNDO feature flag
jessicamcinchak Apr 18, 2024
9962a6d
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Apr 19, 2024
9635f25
restructure formatOps function and better types
jessicamcinchak Apr 19, 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
18 changes: 9 additions & 9 deletions editor.planx.uk/src/@planx/graph/__tests__/formatOps.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,8 @@ describe("Insert operations", () => {
});
});

describe("Delete operations", () => {
test("Deleting a node from the graph", () => {
describe("Remove operations", () => {
test("Removing a node from the graph", () => {
const ops = [
{
p: ["_root", "edges", 1],
Expand Down Expand Up @@ -196,14 +196,14 @@ describe("Delete operations", () => {
];

expect(formatOps(flowWithChecklist, ops)).toEqual([
'Deleted Answer "Blueberry"',
'Deleted Answer "Orange"',
'Deleted Answer "Banana"',
'Deleted Checklist "Which fruits?"',
'Removed Answer "Blueberry"',
'Removed Answer "Orange"',
'Removed Answer "Banana"',
'Removed Checklist "Which fruits?"',
]);
});

test("Deleting a child of a node", () => {
test("Removing a child of a node", () => {
const ops = [
{
p: ["FW5G3EMBI3", "edges"],
Expand All @@ -223,11 +223,11 @@ describe("Delete operations", () => {

expect(formatOps(flowWithChecklist, ops)).toEqual([
"Updated order of Checklist edges",
'Deleted Answer "Orange"',
'Removed Answer "Orange"',
]);
});

test.todo("Deleting a data property of an existing node");
test.todo("Removing a data property of an existing node");
});

const emptyFlow: Graph = {
Expand Down
145 changes: 67 additions & 78 deletions editor.planx.uk/src/@planx/graph/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,87 +504,76 @@ export const sortIdsDepthFirst =
* Translates a list of ShareDB operations into a human-readable change summary.
* See https://github.com/ottypes/json0?tab=readme-ov-file#summary-of-operations
*/
export const formatOps = (graph: Graph, ops: any[]): string[] => {
export const formatOps = (graph: Graph, ops: Array<OT.Op>): string[] => {
const output: string[] = [];

// Updating a node or its properties (update = delete + insert)
const handleUpdate = (node: Node, op: OT.Object.Replace) => {
if (op.od.type && op.oi.type) {
output.push(
`Replaced ${TYPES[op.od.type]} "${op.od.data?.title || op.od.data?.text
}" with ${TYPES[op.oi.type]} "${op.oi.data?.title || op.oi.data?.text
}"`,
);
} else if (op.p.includes("data")) {
output.push(
`Updated ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} from "${op.od}" to "${op.oi
}"`,
);
} else if (op.p.includes("edges")) {
output.push(
`Updated order of ${node.type ? TYPES[node.type] : "graph"} edges`,
);
}
};

// Updating the _root list (update = list insert or list delete)
const handleRootUpdate = (op: OT.Array.Replace) => {
if (op.p.includes("edges") && op.p.includes("_root")) {
output.push(`Re-ordered the graph`);
}
};

// Adding (inserting) a node or its properties
const handleAdd = (node: Node, op: OT.Object.Add) => {
if (op.oi.type) {
output.push(
`Added ${TYPES[op.oi.type]} "${op.oi.data?.title || op.oi.data?.text
}"`,
);
} else if (op.p.includes("data")) {
output.push(`Added ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} "${op.oi}"`);
} else if (op.p.includes("edges")) {
output.push(`Added ${node.type ? TYPES[node.type] : "node"} to branch`);
}
};

// Removing (deleting) a node or its properties
const handleRemove = (node: Node, op: OT.Object.Remove) => {
if (op.od.type) {
output.push(
`Removed ${TYPES[op.od.type]} "${op.od.data?.title || op.od.data?.text
}"`,
);
} else if (op.p.includes("data")) {
output.push(`Removed ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} "${op.od}"`);
} else if (op.p.includes("edges")) {
output.push(`Removed ${node.type ? TYPES[node.type] : "node"} from branch`);
}
};

ops.map((op) => {
const node = graph[op.p?.[0]];
// Updating an object or its properties (update = delete + insert)
if (Object.keys(op).includes("od") && Object.keys(op).includes("oi")) {
if (op.od.type && op.oi.type) {
output.push(
`Replaced ${TYPES[op.od.type]} "${
op.od.data?.title || op.od.data?.text
}" with ${TYPES[op.oi.type]} "${
op.oi.data?.title || op.oi.data?.text
}"`,
);
} else if (op.p.includes("data")) {
if (node.type) {
output.push(
`Updated ${TYPES[node.type]} ${op.p?.[2]} from "${op.od}" to "${
op.oi
}"`,
);
} else {
output.push(
`Updated node ${op.p?.[2]} from "${op.od}" to "${op.oi}"`,
);
}
} else if (op.p.includes("edges")) {
output.push(
`Updated order of ${node.type ? TYPES[node.type] : "graph"} edges`,
);
}
// Adding (inserting) an object or its properties
} else if (Object.keys(op).includes("oi")) {
if (op.oi.type) {
output.push(
`Added ${TYPES[op.oi.type]} "${
op.oi.data?.title || op.oi.data?.text
}"`,
);
} else if (op.p.includes("data")) {
if (node.type) {
output.push(`Added ${TYPES[node.type]} ${op.p?.[2]} "${op.oi}"`);
} else {
output.push(`Added node ${op.p?.[2]} "${op.oi}"`);
}
} else if (op.p.includes("edges")) {
if (node.type) {
output.push(`Added ${TYPES[node.type]} to branch`);
} else {
output.push(`Added node to branch`);
}
}
// Deleting an object or its properties
} else if (Object.keys(op).includes("od")) {
if (op.od.type) {
output.push(
`Deleted ${TYPES[op.od.type]} "${
op.od.data?.title || op.od.data?.text
}"`,
);
} else if (op.p.includes("data")) {
if (node.type) {
output.push(`Deleted ${TYPES[node.type]} ${op.p?.[2]} "${op.od}"`);
} else {
output.push(`Deleted node ${op.p?.[2]} "${op.od}"`);
}
} else if (op.p.includes("edges")) {
if (node.type) {
output.push(`Deleted ${TYPES[node.type]} from branch`);
} else {
output.push(`Deleted node from branch`);
}
}
// Updating the _root list (update = list insert or list delete)
} else if (
Object.keys(op).includes("li") &&
Object.keys(op).includes("ld")
) {
if (op.p.includes("edges") && op.p.includes("_root")) {
output.push(`Re-ordered the graph`);
}
const operationTypes = Object.keys(op);

if (operationTypes.includes("od") && operationTypes.includes("oi")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: If we made this a type guard, we could avoid the casting here if we wanted to -

const isUpdate = (op: OT.Op): op is OT.Object.Replace => {
  const operationTypes = Object.keys(op);
  return operationTypes.includes("od") && operationTypes.includes("oi");
}

Copy link
Member Author

Choose a reason for hiding this comment

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

This is super useful to keep in mind & I'll definitley come back to it - will personally feel more confident full type-guarding after more testing of various operation scenarios !

handleUpdate(node, op as OT.Object.Replace);
} else if (operationTypes.includes("oi")) {
handleAdd(node, op as OT.Object.Add);
} else if (operationTypes.includes("od")) {
handleRemove(node, op as OT.Object.Remove);
} else if (operationTypes.includes("li") && operationTypes.includes("ld")) {
handleRootUpdate(op as OT.Array.Replace);
}
});

Expand Down
3 changes: 2 additions & 1 deletion editor.planx.uk/src/pages/FlowEditor/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { gql, useSubscription } from "@apollo/client";
import Box from "@mui/material/Box";
import Typography from "@mui/material/Typography";
import { formatOps } from "@planx/graph";
import { OT } from "@planx/graph/types";
import { format } from "date-fns";
import { hasFeatureFlag } from "lib/featureFlags";
import React, { useRef } from "react";
Expand All @@ -21,7 +22,7 @@ interface Operation {
firstName: string;
lastName: string;
};
data: Record<string, any>[]; // consider OT types via src/@planx/graph in future
data: Array<OT.Op>;
}

export const LastEdited = () => {
Expand Down
Loading