search

PASTA-ELN / pasta-eln

PASTA-ELN with new frontend
https://pasta-eln.github.io/pasta-eln/
Other
7 stars 2 forks source link

Autosave functionality #236

Closed HTsybenko closed 2 months ago

HTsybenko commented 4 months ago

At this point, there are no ways to restore the comment from the unsaved form (the comment does not get automatically restored, is not copied to buffer to be pasted, etc.). How does autosave work then and for which kinds of metadata?

SteffenBrinckmann commented 4 months ago

Bug: comment is not saved, for what-ever reason, to .pastaELN.temp. Debug form.py and in it creation of that file

SteffenBrinckmann commented 3 months ago

Debugged. How does it work: when you do an autosave (whether implicitly every 5min or explicit after cancel the form and then save), the content incl. comment, name,... (but not tags) is save in a temporary location. When you open the next time, this temporary information is found, and the user is alerted to it. He should open the previously edited item and then the content reappears there. If she opens another item, the temporary content reappears there. The main use-case: you edit something long and type a great comment, and then suddenly your laptop turns off because the battery is empty. Then wen you reopen, not all your work is lost. Does that make sense, @HTsybenko ?

HTsybenko commented 3 months ago

I will test it when the new release is available for installing.

HTsybenko commented 3 months ago

The autosave works now, as described.

However, in the following scenario, its implementation seems to be confusing:

  1. Open an item to edit.
  2. Add a comment.
  3. Click "Cancel" to trigger a warning pop up “You will loose all new data. Do you want to save it for next time?”. Click “Yes”.
  4. Open any item with already existing comment to edit.

In the comment editing field, the existing comment will be overwritten by the one that was saved to the buffer.

This means that no previously existing comments can be viewed or edited, until user a) saves any form with the autosaved comment, potentially overwriting the existing one. b) triggers the “You will loose all new data. Do you want to save it for next time?” and selects "No".

HTsybenko commented 3 months ago

Related bug:

  1. Open an item to edit.
  2. Add a comment.
  3. Click "Cancel" to trigger a warning pop up “You will loose all new data. Do you want to save it for next time?”. Click “Yes”.
  4. Open any item. The autosaved comment appears in the comment editing field. Save the form.
  5. Repeat the steps 1-4 with another item.
  6. Afterwards, open and close multiple items (without editing, close the items via "X"), reopen the project, etc. (it is difficult to recreate the exact steps, as the same actions may cause or not the bug).

At some point, the existing comment of the opened item will be overwritten with the autosaved one.

pastaELN issue.log

SteffenBrinckmann commented 3 months ago

@HTsybenko I knew that this issue would exist, I just did not consider it a "bug" What do you think about the solution: overwrite the saved comment from the buffer only if it is the same document? In all other cases do nothing. If a new document, then the comment from the saved buffer is used for the next new document. Would that make sense?

HTsybenko commented 3 months ago

@SteffenBrinckmann I could imagine that this implementation would still be confusing for the new users. Ideally, the comment field could have another button "paste the last autosaved comment" that would append it to the old comment.

Please also look into the last comment of mine (Related bug), as this is definitely an unexpected behavior.

HTsybenko commented 3 months ago

Also, I would change the warning pop up to “You will lose the entered comment. Do you want to save it to clipboard?”. It will then be clear which data is meant.

When the PASTA is started again, it is not clear which data/content/temporary data is meant in this form: "There is data from a prematurely closed form. Do you want to use it? -if you choose yes, please reopen that form and content will be reloaded. -if you choose no, this temporary data will be removed now".

It could be changed to "There is an unsaved comment from a prematurely closed form. Do you want to restore it? -if you choose yes, please reopen that form and the comment will be reloaded. -if you choose no, the comment will be removed from the clipboard", or removed completely (always a yes), as I would expect my unsaved comments to be still in buffer if the autosave feature is turned on.

SteffenBrinckmann commented 3 months ago

@HTsybenko OK, I changed the text after clicking the cancel button to "You will lose the entered information. Do you want to save everything (except tags) to a temporary location?" if you click yes, it is saved. When you start pasta afterwards, nothing appears. Only when you come back to your previously edited item, then the question is asked "There is an unsaved information from a prematurely closed form. Do you want to restore it?\nIf you decline, the unsaved information 'will be removed" Why did I change the text: well the data is not saved to the clipboard (specific term). And the name, etc is also saved. Does that make sense?

HTsybenko commented 3 months ago

@SteffenBrinckmann As for now, I only noticed that the autosave feature restores the comment. Name changes, tags (as you mentioned), content in procedures, and links are not restored when the item is opened again. Therefore, it would make more sense for me to specify exactly what is saved. "Everything (except tags)" may sound a bit vague, in this sense.

"Temporary location" makes more sense, of course, and makes it easier for the users to understand what happens.

We should continue discussing this issue next time we have an online meeting, as it is clear that some discussion points would be missed otherwise.

As this issue does not affect the overall stability of PASTA, it can be moved to the next milestone, if you would like to make an official release sooner.

SteffenBrinckmann commented 3 months ago

@HTsybenko ok, I move this into 2.5.2 (which I newly created)

HTsybenko commented 3 months ago

@SteffenBrinckmann Here is the summary of changes and fixes for this issue that we agreed on:

1) By default / upon installation, the autosave feature has to be turned off.

2) Autosave should restore the temporary data only for exactly those items it was initially saved for. Only one (last) temporary data version is saved per item.

3) When opening those prematurely closed items, a pop-up "Do you want to restore unsaved data?" should be added.

4) The warning text changes to "You will lose the entered information. Do you want to save everything (except tags) to a temporary location?"

5) The pop-up "There is data from a prematurely closed form. Do you want to use it?" when opening PASTA should be removed.

6) Bug 1: Not all content is restored by the autosave. Only comment is restored currently.

7) Bug 2: Do the following sequence twice: "Open form, add comment, "Cancel", respond "Yes" to "Do you want to save...?", open form again, autofill, "Save". Then open and close (via "X") multiple items. At some point, the comment fields of opened items will be autofilled with old temporary comments or the comments of other items.

If needed, new issues can be created for these changes and fixes.

HTsybenko commented 3 months ago

The changes 1-5 were implemented, and the bugs 6-7 fixed.

Related comments:

a) There are three ways to answer the question whether to autosave the data: Cancel, No, and Yes. Since Cancel works in the same way as "X", is it necessary to keep it?

b) Please fix the typos in the message "There is an unsaved information from a prematurely closed form. Do you want to restore it? If you decline, the unsaved informationwill be removed" –> "There is unsaved information from a prematurely closed form. Do you want to restore it? If you decline, the unsaved information will be removed".

c) Bug:

  1. "Open the form, add a comment, "Cancel," and respond "Yes" to "Do you want to save...?" - do it for two items.
  2. Open one of the items again, autofill and "Save".
  3. Open the other item for which the information was previously autosaved.

There will be no pop-up window asking if information has to be restored. This behavior is not expected.

  1. Close the item.
  2. Open this item again.

Now, the pop-up asking if information has to be restored will appear.

  1. Click "Yes"

The comment will not be restored. This is a bug.

SteffenBrinckmann commented 3 months ago

a) It is a requirement of the QT framework: the "x" takes the "least-Yes" answer. If there is a cancel, take that. If there is no cancel, then take the "no" and that is not what we want. b) did that correction, thank you c) fixed and manually verified

HTsybenko commented 2 months ago

@SteffenBrinckmann I just noticed that autosave does not restore unsaved links. Is this an issue, or does it work as intended?

SteffenBrinckmann commented 2 months ago

@HTsybenko for me, links are not difficult enough to re-create to warrant an autosave feature for them. Comments might be large texts that should be saved automatically. If autosave reopens, it shows the non-links and then the user can fix them. That was the idea of the autosave in the first place. If you see it differently, please create a new feature request to also autosave links. The new feature request can then be queued into the next version, depending on their priority.

HTsybenko commented 2 months ago

@SteffenBrinckmann Not autosaving the links is not a game-breaking issue; that is true. I just wanted to make sure that you are aware and understand what is meant by "Do you want to save everything to a temporary location?"