Auto-Save Changes in the Workflow Composer

[JazzyMichael]

Closes #993

Adds a checkbox in the Workflow Composer to toggle "Auto-Save". When enabled, all changes in the Workflow Composer are debounced by 1 second and then automatically saved. This is an added enhancement to the save button and keyboard shortcut.

https://user-images.githubusercontent.com/46300738/163027284-9fb884ea-345d-4e80-9ebb-62a07d746c81.mov

[amanda11]

Thanks for the contribution.

I'm wondering if its possible to make this something you can switch on when you want. Often I don't want changes automatically saved when I'm editing workflows as I might be experimenting, or showing how a workflow could be altered - so to have it automatically save is NOT what I would want it to do.

So I think this would need to be something that you enable either on a system-wide or per edit of a workflow.

[mickmcgrath13]

Needs rebase/merge from master

[JazzyMichael]

A toggle to enable/disable autosave is a great idea! I can add that in the workflow composer and rebase.

[amanda11]

A toggle to enable/disable autosave is a great idea! I can add that in the workflow composer and rebase.

Thanks. I think it's safest to have disabled by default,as if you are editing a workflow that is called by a rule - and say making a couple of changes - auto save would make them live part way through - which could be bad.

But if you are editing a brand new workflow or one not live - then you may want to enable auto save.

Probably safest to make auto save be something the user makes conscious choice to enable.

[JazzyMichael]

@amanda11 Thanks for your feedback! I added a toggle for the autosave functionality with a debounce time of 3 seconds. It's indifferent to the undo/redo functionality; clicking "undo" after an autosave will not revert the save, it will apply the next change in the undo stack, unaffected by the save.

[rush-skills]

I feel that this feature would be more useful if it is opt-in (user can toggle auto-save to ON in the workflow composer) and then it is rather instantaneous (1 sec?) rather than a random wait (3-25 seconds?), since the wait might make the experience inconsistent (I made changes thinking they will be autosaved but closed the tab before the autosave triggered).

[mickmcgrath13]

@rush-skills I agree it should be opt-in. @Jappzy can you confirm whether or not the latest updates are "off by default and opt-in" ?

As far as the debounce, I think we want a small wait so that it's not overloading the server with api calls due to a request for every . 1-2 second might be okay. @Jappzy and @amanda11 , thoughts on dropping the timeout to 1s?

Best case scenario would be to make it configurable via the st2config, but maybe that's work for a future ticket as it'd be more effort.

[amanda11]

I'd be worried about 1s being a bit too much load. My concern @rush-skills @mickmcgrath13 is that from what I read (@Jappzy to confirm) it's going to send those requests every 1s whether or not you have changed the workflow - so if we make it a more often refresh, then that's going to be quite often even you aren't doing anything. So in that case, does it need to be more clever and know if something has changed before it re-sends the request? I guess if you are only viewing you wouldn't switch it to auto-save, so maybe 1s is ok in that case.

[JazzyMichael]

@amanda11 @rush-skills @mickmcgrath13 Can you elaborate on the repeating requests every 1 second? This PR has the autosave feature off by default and debounces all of the change requests for 3 seconds after each change. There are no requests every second and it only updates when something changed. There is a screen recording demo in the description showcasing the functionality.

[mickmcgrath13]

@Jappzy That's How I understood it:

• auto-save off by default
• toggle auto-save on, and
  • making a change causes a timer for 3 seconds - after which the save is triggered
  • additional changes that happen within the 3 seconds resets the timer (i.e. debounce)

What I was referring to is essentially to reset the "timer" to be 1s vs 3s.

I think the current functionality is more desirable than a "save every X seconds regardless of change"; I think the only question is "how long after a change should the save happen?"

[mickmcgrath13]

@amanda11 does that alleviate your concerns?

[rush-skills]

@Jappzy If it is off-by-default and does not send a request unless the workflow is actually changed, it is good to me.

For the 1s vs 3s question - I don't think it changes any thing on the server wrt load (does 1 sec mean more writes during the editing phase? is that problematic?), and I will prefer 1 second over 3 seconds just because it seems more UX friendly (I will be less likely to face a situation where I turn auto-save on, make changes, close the tab soonish, and don't have the changes persisted because it was less than 3 secs).

[amanda11]

@amanda11 does that alleviate your concerns?

Yes the design sounds great. I've not had a chance to install and have a play to test it out. But design sounds good, and am ok with the 1second proposal as well.

[JazzyMichael]

@amanda11 great! I changed the debounce timer to 1 second so it should be good to test now. Thanks for the review!

[mickmcgrath13]

@Jappzy Can you take a look at @amanda11's latest comment?

[CLAassistant]

All committers have signed the CLA.

[WestonVincze]

Thanks for the detailed feedback @amanda11 .

I've fixed the first issue by adding a call to the save function when the autosave checkbox is turned on. I also added the autosave functionality to the browser editors (metadata and workflows).

[amanda11]

@Jappzy I had some time to test today, but unfortunatley I can't download the package as the CircleCI tests are failing. We need to look at why we are getting incompatible versions of node being used for the CircleCI runs. It's not particular to your PR its failing on all st2web PRs, but if you have a chance to look that would be great.

[amanda11]

think I've found the problem with st2web, and I'm looking at a PR to fix.

[amanda11]

@Jappzy Can you look at why the lint checks are failing on this PR? I'd like to get it so we've got a built instance I can install on a test box. Thanks.

[WestonVincze]

@amanda11 fixed the CI issues. You should be able to build it now.

[nzlosh]

@amanda11 Have your review requirements been satisfied?

[amanda11]

@amanda11 Have your review requirements been satisfied?

I missed the lint had been fixed, I'll try and install and test this week.

[amanda11]

@Jappzy @WestonVincze I've found different problems with the auto-save now, please see the review comments. Worth double-checking after any change that its auto-saving when you change anything in the UI - the list of ones where it isn't, might not be exhaustive.

[arm4b]

@amanda11 This PR had some changes and is ready for your review again. I'll test it too, but you may know the previous places where it didn't work as expected. Thanks for your feedback!

[amanda11]

@amanda11 This PR had some changes and is ready for your review again.

I'll test it too, but you may know the previous places where it didn't work as expected.

Thanks for your feedback!

Thanks @armab - should have time later in week to test this, and I will report back on ticket

[amanda11]

@mickmcgrath13 @armab @cded I just found one scenario where it's not autosaving - see review comment.

[cded]

@amanda11 Thanks for the catch, I updated the PR to handle auto save on adding, editing and deleting parameters. I also fixed an issue with the auto save checkbox where toggling the details panel didn't save the checkbox state.

[arm4b]

@amanda11 Thanks a lot for all the reviews 👍 Based on Slack https://stackstorm-community.slack.com/archives/C020U70RZRR/p1666360017941239, looks like we're good to merge, - appreciate the follow-up with @nzlosh!