feat: undo operations to a flow

[jessicamcinchak]

Behavior:

• Undoes changes (or "restores") to a previous point in the edit history by applying a new inverse operation on the flow
• I can undo any previous edit displayed in the list (last 15), but need to understand that any edits more recent than it will also be reversed in the process. This includes the ability to undo a change originally made by someone else on my team.
• If I only have view-access to this team, I only see a timeline of recent changes without any action

Outstanding:

• Correctly scope all UI elements to canUserEditTeam()
• Improve formatOps function based on feedback
• Remove feature flag & open for PO testing

[jessicamcinchak]

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)

[DafyddLlyr]

Super helpful explanation here thanks.

[github-actions]

Removed vultr server and associated DNS entries

[jessicamcinchak]

Add an 'undo' function in editor

[DafyddLlyr]

Code changes looks solid - great how simple it is 👍

Testing now in some detail and I'll comment on your notes also.

[DafyddLlyr]

Super helpful explanation here thanks.

[DafyddLlyr]

Are there any scenarios NOT working as expected? Rigorously test both formatting & undo functionality

Some testing feedback - most of this is not undo-specific but a few things I hit along the way. Apologies for the wall of text!

Some operations read as [object Object] - these are components with more complex state.

• Filter flag text override
• Changes to grouped checklists
• Reorder FileUploadAndLabel rules
• Changes to TaskList
• Adding an empty content node

We may want to check specifically for undefined, for example when adding a question without a title -

It might be worth trying to capture the deletion of an internal portal a little more explicitly - currently it just lists all children (predictably!)

Some questions that I can foresee users asking -

• Why do internal portals share operation lists, but external portals do not?
• Why isn't publishing shown in my history?

Both of these are easily answerable and the current behaviour is correct, but it's something which may come up and we may need to find a way to express through the UI more clearly in future.

The good news is that undo is working perfectly - not been able to break it 🥇 Super stuff!

[DafyddLlyr]

When un-doing someone else's edit, does the inverse operation correctly attach my user id or is the original actor id still used? Get & set user id from state if necessary before send(inverseOps) is called (can't test locally)

Let's try and get 5 mins to test this on a call maybe just to check! The demo yesterday seemed to work well.

History panel will currently use language of "Created..." and "Edited..." - do we need way to check/say "Undid"/"Reversed" for inverted operations? Or is graph feedback clear enough as is?

I think this is fine as-is currently, we can wait for feedback,

Pick back-up issue of stale timestamps - see comment #3052 (comment)

I think the recent UI changes to de-emphasise the timestamp actually fixes this - the behaviour now matches other timeline examples we can find online.

Error handling! Confirmation dialog?

I'm currently comfortable without a confirmation dialog - this is just as destructive as making changes to the graph which we don't wrap with this protection. Again one to maybe listen out for feedback on.

Spend a bit of time on rendering bug / can we reduce renders? Apollo cache / useMemo operations??

See comments on #3028 - spent a bit more time on this and I'm convinced it's related to react-navi. The /node routes are rendered alongside the actual flow - it's a new instance so likely why it's re-rendering. Might be worth a call and demo to explain this one again?

[jessicamcinchak]

I spent a bit of time thinking about what how we might include "publishes" alongside "operations" in the edit history timeline and there's a few tricky details to work out / good reasons to re-visit this post-MVP I think:

• Unlike query or mutation, GraphQL's subscription can have exactly one root object (meaning I can't simply subscribe to operations & published_flows in the same hook)
• Defining in a meaningful published_flows subscription hook is a bit tricky - the where condition we really want is matching flow id and published date is greater than or equal to the oldest operation that we fetched (basically, simple "last 15" like operations won't make sense)
• We wouldn't be able to display the "restore" icon/action next to "Published by..." items in the history list, but they could serve as static context markers to help understand if you're undo-ing operations to a state before your last publish - this probably means we'll want to come up with a unique style for them!

[DafyddLlyr]

Thanks for the write up here.

I was also thinking about this and couldn't think of a good way to communicate that you can restore operations to a previous version before your last publishing event, but that's not going to undo the publish event - the old content will still be live.

I actually think if there's a user-need for this, it might be a separate thing (in another tab?) called "Publishing history".

Support leaving this as-is for now 👍

[DafyddLlyr]

Hadn't realised sorry that this was still awaiting a final review.

Looking fantastic - only small nits! As always, please feel free to ignore as many as you see fit.