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.

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

typing in the first line of a notes generates 2 activity entries for (almost) each character #489

Closed
violoncelloCH opened this issue Apr 12, 2020 · 12 comments · Fixed by #555
Labels
needs discussion Need to clarify if and how we should implement this
Milestone

Comments

@violoncelloCH
Copy link
Member

violoncelloCH commented Apr 12, 2020

Describe the bug (this is not really a bug, but anyways to stick to the template as much as possible)
When I type in the first line of a note (e.g. when starting a new note), on every character change the file gets saved (first activity entry) and the filename changes (second activity entry).

To Reproduce
Steps to reproduce the behavior:

  1. type/make changes in the first line of a note (e.g. when starting a new one)
  2. go to Nextcloud's Activity app/section
  3. see a lot of entries for the just edited note (in fact two for (almost) every changed character)

Expected behavior
While it obviously makes sense to store the changes (on server side) as soon as possible to avoid data loss if e.g. the client disconnects due to network outage, those changes should only be written to disk once editing has "finished" (e.g. no changes for 5 seconds?). This way changes to the first line of a note can be done without "flooding" the activity stream.

Server (please complete the following information):

  • Notes app version: 3.2.0
  • Nextcloud version: 18.0.3

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@violoncelloCH
Copy link
Member Author

violoncelloCH commented Apr 12, 2020

okay I've just found #190 and #474 ... with that change I guess this issue here is (depending on how the creation of the first filename is implemented) already improved... however I guess my suggestion remains in general for edits to notes (and therefore the creation of "file changed" activities)...

@korelstar
Copy link
Member

Yes, that's annoying. I've disabled activities for my own changes a long time ago, so this problem doesn't occur to me. But it's a legitimate issue.

I'm not sure how to fix this. Currently, we save the note after one second, if it was changed. We should not increase this value too much in order to reduce data-loss.

On the other hand, I don't know if it is possible to manipulate the activity stream in order to reduce the impact. However, then the activity stream would not represent the truth, anymore.

Maybe @nickvergessen has an idea, since he seems to be the maintainer of the activity app (?).

@nickvergessen
Copy link
Member

Not really into the activity app anymore, but there is no replacement for the maintainer either :/

I guess what could change is that you debounce the saving in the UI until you didnt type for 1 second instead of saving every second when the content changed?

@violoncelloCH
Copy link
Member Author

can't the Notes app backend handle this debouncing effect? From looking at the network connection, the frontend doesn't make direct request to the webdav endpoint, so wouldn't it be possible to send changes more or less immediately (like it is now) to the Notes app backend and keep those there, but only save them to the disk after a delay of no input?
Kind of how onlyoffice does it, where changes get saved to the Nextcloud storage once the user exits the document or when the connection breaks for more than 10 seconds (or something like that)...

@korelstar
Copy link
Member

@nickvergessen

I guess what could change is that you debounce the saving in the UI until you didnt type for 1 second instead of saving every second when the content changed?

Thought about that, too. But if someone writes much text for a long time without pause, there is no save in the meantime. Therefore, I really don't want to change this behavior.

@violoncelloCH

can't the Notes app backend handle this debouncing effect? From looking at the network connection, the frontend doesn't make direct request to the webdav endpoint, so wouldn't it be possible to send changes more or less immediately (like it is now) to the Notes app backend and keep those there, but only save them to the disk after a delay of no input?

Keep those where? This must be a save place.


But honestly, I don't want to make notes saving that complicate just because another app is doing to much logging. I think it would be better if the activity app would consolidate multiple file changes if they appear in a short time period. Wouldn't that be a better solution?

@korelstar korelstar added the needs discussion Need to clarify if and how we should implement this label Apr 18, 2020
@nickvergessen
Copy link
Member

But honestly, I don't want to make notes saving that complicate just because another app is doing to much logging. I think it would be better if the activity app would consolidate multiple file changes if they appear in a short time period.

I can feel your frustration and see the issue too. The problem is somewhere in the middle.
Just as explanation: The activity app logs every time a file is written, as we never know whether a write is the first, the middle or the last one.

Later on they are grouped while rendering when:

  1. Happened either by the 1 user on N files, or N users on 1 file
  2. They are not further away than 3 hours
  3. No one did something else in between
  4. Not more than 30 entries are combined.

But well if you send a request to the server for every character that is mostlikely to result in multiple entries.

Maybe the debounce approach can be extended with a setTimeout for 30 seconds or something. so you start to debounce and start a 30 seconds timeout. If the user keeps typing all the time you save after 30 seconds, otherwise the debounce stops and you kill the timeout.

@violoncelloCH
Copy link
Member Author

Keep those where? This must be a save place.

In memory server side? Or even cached on disk on the server?

  1. No one did something else in between

I guess this why it's not working for this scenario, because we have two types of activity, filechanges and renamings every time...?

@korelstar
Copy link
Member

  1. No one did something else in between

I guess this why it's not working for this scenario, because we have two types of activity, filechanges and renamings every time...?

Yes, I also think that this is the source of the problem. "Normal" writes (i.e. those that don't cause a rename) are already grouped in the activity app.

While I really don't want to (increasingly) debounce content changes, we maybe can debounce the rename process. Losing the content is much more worse than loosing the latest title renaming.

Nevertheless, we have to ensure that saving the content is done before the user switches to another Nextcloud app or closes the browser tab. Currently, there is only a check for the content, since all other attributes are saved immediately. If we debounce saving the title, we have to add this to the "unsaved" check. Therefore, a rework of the save process (JavaScript) may be required. I can try to realize this in conjunction with #495. But this will take some time ...

@korelstar korelstar mentioned this issue May 17, 2020
2 tasks
@korelstar
Copy link
Member

@violoncelloCH I just released Notes 3.3.1 which includes a relaxed autosave. This will not completely fix this issue, but it should reduce the problem. Can you please test the new version and give some feedback if this is sufficient for now?

@Enrico204

This comment has been minimized.

@korelstar

This comment has been minimized.

@korelstar korelstar added this to the 3.6.0 milestone Jun 27, 2020
@violoncelloCH
Copy link
Member Author

sorry for the late feedback @korelstar
I tested it and it's much better now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs discussion Need to clarify if and how we should implement this
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants