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

Update note list on opening the app #406

Closed
jancborchardt opened this issue May 30, 2018 · 10 comments
Closed

Update note list on opening the app #406

jancborchardt opened this issue May 30, 2018 · 10 comments
Labels

Comments

@jancborchardt
Copy link
Member

I just had the case where I edited a lot of stuff in a note on the computer (via a text editor, syncing through the desktop client). Then I turned the computer off – it was synced, as I had been working for an hour and saving always – and went elsewhere.

I had more thoughts on the matter, opened the Notes app on my phone and opened the note, which was on the top of the list. To my horror the changes were not there, it was a much earlier version of the file.

(So I went back to the laptop where I’m writing this issue. ;)

It seems like on opening the app, the note list should be updated. Seamlessly so it doesn’t show an empty list, just updating in case something new came in. cc @stefan-niedermann @korelstar

@korelstar
Copy link
Member

When opening the app, synchronization is started, already. However, it is possible that you opened the note before synchronization has finished. This would explain the observed behavior.

Please see also #161 for a discussion about this problem.

@jancborchardt
Copy link
Member Author

Yeah, I saw that thread but was thinking it might be a bit different since it’s not really about concurrent editing. Sure – once I open the issue I entered the death trap of concurrency, but the point is that in this case (where I only use one device at a time) we could prevent that from happening.

So I consider resolving concurrent edits, and this here, different issues. But yeah, very related.

@korelstar
Copy link
Member

"Concurrency" doesn't mean that the things happen in parallel. From the computer science perspective, this is clearly a problem of concurrency -- and we have to address this issue (as stated in #161).

However, I don't understand, what's the difference to #161, that you see. Can you explain this in more detail, please?

A possible patch before implementing the "big" solution (see nextcloud/notes#56) would be to notify the EditNoteActivity about (remotely) changed notes and then apply the new content to the edit view. However, if you've changed the note in the EditNoteActivity already, this will generate a conflict (your changes will be lost). So this would be a small improvement, only.

Do you have any other ideas?

@jancborchardt
Copy link
Member Author

Ah ok, thanks for the clarification.

The improvement you suggest seems good. Another very simple enhancement would be that when you open the app and the initial sync is done, show the same feedback spinner as is shown when you pull to refresh. Then people know what's happening.

@korelstar
Copy link
Member

Another very simple enhancement would be that when you open the app and the initial sync is done, show the same feedback spinner as is shown when you pull to refresh. Then people know what's happening.

I'm fine with this. But IIRC there was a discussion a long time ago, with a contrary statement.

What do you think, @stefan-niedermann ?

@stefan-niedermann
Copy link
Member

I am fine with this change, too. Phew, do you know the issue where this discussion happened? I can not remember the reasons why we decided to hide it.

I can not see a valid reason against it, so yeah :)

@korelstar
Copy link
Member

No, couldn't find it. Therefore, I implemented this one (see #410):

Another very simple enhancement would be that when you open the app and the initial sync is done, show the same feedback spinner as is shown when you pull to refresh. Then people know what's happening.

Interesting: with this change, I found out, that the app indeed doesn't always sync on start! On my phone, auto-sync on start works perfectly. But on the emulator, it doesn't! Sorry that I didn't believed you, @jancborchardt !

Now, we have to find out, why auto-sync doesn't work always ... (but for today, I'm out ...)

@korelstar korelstar added the bug label Jun 7, 2018
@jancborchardt
Copy link
Member Author

@korelstar all good :D thank you so much for the fix!

@haarp

This comment has been minimized.

@stefan-niedermann
Copy link
Member

Closing this, because:

  • Sync is triggered when the app is opened
  • Spinner is shown, even on the first sync
  • There are several other issues to avoid and resolve concurrent edits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants