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: undo operations to a flow #3056

Merged
merged 15 commits into from
May 16, 2024
Merged

feat: undo operations to a flow #3056

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
f06f3bd
it works
jessicamcinchak Apr 25, 2024
2faeb09
account for missing actor on local envs
jessicamcinchak Apr 25, 2024
bcb7a4e
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Apr 25, 2024
5c021b1
date as subheader, fix setvalue
jessicamcinchak Apr 25, 2024
3575fec
Merge branch 'main' of github.com:theopensystemslab/planx-new into je…
jessicamcinchak Apr 26, 2024
1a6d80b
tidy formatting
jessicamcinchak Apr 26, 2024
6503713
feat: undo the last N operations to a flow (#3065)
jessicamcinchak May 1, 2024
750d73e
updated styling, restore button scoped to teamEditors
jessicamcinchak May 6, 2024
adca0b0
add allow list of props for tidier formatted op summaries
jessicamcinchak May 6, 2024
90c1aff
move EditHistory to own file
jessicamcinchak May 6, 2024
c737103
better formatOps tests
jessicamcinchak May 6, 2024
f86d0c9
feat: Layout for undo history (#3116)
ianjon3s May 7, 2024
2d948fd
map common props to human readable names in formatOps
jessicamcinchak May 7, 2024
53009bf
remove featureFlag for PO testing
jessicamcinchak May 8, 2024
b7f0bad
rebase and final feedback
jessicamcinchak May 16, 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
1 change: 1 addition & 0 deletions editor.planx.uk/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
"nanoid-good": "^3.1.0",
"natsort": "^2.0.3",
"navi": "^0.15.0",
"ot-json0": "^1.1.0",
jessicamcinchak marked this conversation as resolved.
Show resolved Hide resolved
"postcode": "^5.1.0",
"ramda": "^0.29.1",
"react": "^18.2.0",
Expand Down
3 changes: 3 additions & 0 deletions editor.planx.uk/pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

39 changes: 27 additions & 12 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 @@ -509,16 +509,19 @@ export const formatOps = (graph: Graph, ops: Array<OT.Op>): 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) {
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
`Replaced ${TYPES[op.od.type]} "${
op.od.data?.title || op.od.data?.text || op.od.data?.content
}" with ${TYPES[op.oi.type]} "${
op.oi.data?.title || op.oi.data?.text || op.oi.data?.content
}"`,
);
} else if (op.p.includes("data")) {
output.push(
`Updated ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} from "${op.od}" to "${op.oi
}"`,
`Updated ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} from "${
op.od
}" to "${op.oi}"`,
);
} else if (op.p.includes("edges")) {
output.push(
Expand All @@ -536,29 +539,41 @@ export const formatOps = (graph: Graph, ops: Array<OT.Op>): string[] => {

// Adding (inserting) a node or its properties
const handleAdd = (node: Node, op: OT.Object.Add) => {
if (op.oi.type) {
if (op.oi?.type) {
output.push(
`Added ${TYPES[op.oi.type]} "${op.oi.data?.title || op.oi.data?.text
`Added ${TYPES[op.oi.type]} "${
op.oi.data?.title || op.oi.data?.text || op.oi.data?.content
}"`,
);
} else if (op.p.includes("data")) {
output.push(`Added ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} "${op.oi}"`);
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) {
if (op.od?.type) {
output.push(
`Removed ${TYPES[op.od.type]} "${op.od.data?.title || op.od.data?.text
`Removed ${TYPES[op.od.type]} "${
op.od.data?.title || op.od.data?.text || op.od.data?.content
}"`,
);
} else if (op.p.includes("data")) {
output.push(`Removed ${node.type ? TYPES[node.type] : "node"} ${op.p?.[2]} "${op.od}"`);
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`);
output.push(
`Removed ${node.type ? TYPES[node.type] : "node"} from branch`,
);
}
};

Expand Down
34 changes: 27 additions & 7 deletions 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 @@ -4,6 +4,7 @@ import "./floweditor.scss";
import { gql, useSubscription } from "@apollo/client";
import UndoOutlined from "@mui/icons-material/UndoOutlined";
import Box from "@mui/material/Box";
import IconButton from "@mui/material/IconButton";
import Typography from "@mui/material/Typography";
import { formatOps } from "@planx/graph";
import { OT } from "@planx/graph/types";
Expand All @@ -17,10 +18,11 @@ import PreviewBrowser from "./components/PreviewBrowser";
import { useStore } from "./lib/store";
import useScrollControlsAndRememberPosition from "./lib/useScrollControlsAndRememberPosition";

interface Operation {
id: string;
export interface Operation {
id: number;
createdAt: string;
actor?: {
id: number;
firstName: string;
lastName: string;
};
Expand Down Expand Up @@ -109,7 +111,12 @@ export const LastEdited = () => {
};

export const EditHistory = () => {
const [flowId, flow] = useStore((state) => [state.id, state.flow]);
const [flowId, flow, canUserEditTeam, undoOperation] = useStore((state) => [
state.id,
state.flow,
state.canUserEditTeam,
state.undoOperation,
]);

const { data, loading, error } = useSubscription<{ operations: Operation[] }>(
gql`
Expand All @@ -122,6 +129,7 @@ export const EditHistory = () => {
id
createdAt: created_at
actor {
id
firstName: first_name
lastName: last_name
}
Expand All @@ -142,14 +150,14 @@ export const EditHistory = () => {
}

// Handle missing operations (e.g. non-production data)
if (!loading && !data?.operations) return null;
if (!loading && !data?.operations[0].actor) return null;

return (
<Box>
{loading && !data ? (
<DelayedLoadingIndicator />
) : (
data?.operations?.map((op: Operation) => (
data?.operations?.map((op: Operation, i: number) => (
<Box
key={`container-${op.id}`}
marginBottom={2}
Expand All @@ -165,15 +173,27 @@ export const EditHistory = () => {
>
<Box>
<Typography variant="body1" sx={{ fontWeight: 600 }}>
{`Edited ${formatLastEditDate(op.createdAt)}`}
{`${op.actor ? `Edited` : `Created`} ${formatLastEditDate(
op.createdAt,
)}`}
</Typography>
{op.actor && (
<Typography variant="body2">
{`by ${op.actor?.firstName} ${op.actor?.lastName}`}
</Typography>
)}
</Box>
<UndoOutlined titleAccess="Undo this edit" />
{i === 0 && (
<IconButton
title="Undo"
aria-label="Undo"
onClick={() => undoOperation(op)}
disabled={!canUserEditTeam}
color="primary"
>
<UndoOutlined />
</IconButton>
)}
</Box>
{op.data && (
<Typography variant="body2" component="ul" padding={2}>
Expand Down
9 changes: 9 additions & 0 deletions editor.planx.uk/src/pages/FlowEditor/lib/store/editor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ import {
ROOT_NODE_KEY,
update,
} from "@planx/graph";
import { OT } from "@planx/graph/types";
import axios from "axios";
import { client } from "lib/graphql";
import debounce from "lodash/debounce";
import isEmpty from "lodash/isEmpty";
import omitBy from "lodash/omitBy";
import { customAlphabet } from "nanoid-good";
import en from "nanoid-good/locale/en";
import { type } from "ot-json0";
import { Operation } from "pages/FlowEditor";
import type { StateCreator } from "zustand";

import { FlowLayout } from "../../components/Flow";
Expand Down Expand Up @@ -88,6 +91,7 @@ export interface EditorStore extends Store.Store {
) => Promise<PublishFlowResponse>;
removeNode: (id: Store.nodeId, parent: Store.nodeId) => void;
updateNode: (node: any, relationships?: any) => void;
undoOperation: (operation: Operation) => void;
}

export const editorStore: StateCreator<
Expand Down Expand Up @@ -437,4 +441,9 @@ export const editorStore: StateCreator<
})(get().flow);
send(ops);
},

undoOperation: (op: Operation) => {
const inverseOps: OT.Op[] = type.invert(op.data);
send(inverseOps);
},
Copy link
Member Author

@jessicamcinchak jessicamcinchak Apr 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I considered LOTS of different ways of doing this piece - but eventually landed on this simple Editor store method which hooks into our existing send(ops) - meaning we don't have to directly write any mutations to update operations or flows as the original script does: https://github.com/theopensystemslab/planx-data-recovery/blob/main/index.js

The other main route I was considering was creating an API endpoint to mimic the full set of data-recovery steps (eg /flows/:flowId/undo/:operationId). Ultimately decided against this though in favor of an Editor-only approach for a couple key reasons:

  • the API currently knows very little (nothing?) about operations & ShareDB which makes it hard to access/reuse types, existing methods, etc. Handling "undo" in the Editor store means this logic is co-located with other graph operations like add/remove/etc
  • the mutations required to directly update operations & the flow are very reliant on keeping flow version aligned - and flow version is something that we rarely manually deal with in flow data clients etc. Again, hooking into existing Editor methods means we're just getting that alignment "for free" (similar for user/actor id doing the "undo"!)
  • the original script has a lot of safety nets (eg print original flow data, proposed flow data, diff the JSONs, etc) which we don't actually need to replicate here for an MVP. It would be really nice to figure out an intermediary "ConfirmationDialog" step here in the Editor, but that can be a nice-to-have followup (it's actually sort of cool how seamless it feels as-is! and we don't have confirmation for other Editor deletes)

Copy link
Contributor

Choose a reason for hiding this comment

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

Super helpful explanation here thanks.

});
1 change: 1 addition & 0 deletions editor.planx.uk/types.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ declare module "draftjs-to-html";
declare module "mathjs";
declare module "@opensystemslab/map";
declare module "wkt";
declare module "ot-json0";
Loading