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

All: Support for note history (WIP) #1415

Merged
merged 27 commits into from May 6, 2019

Conversation

Projects
None yet
9 participants
@laurent22
Copy link
Owner

commented Apr 12, 2019

This branch is to more easily manage this big feature, which is rather complicated (though I thought it would be easier).

The goals are:

  • To allow restoring previous versions of a note (the service can potentially version any Joplin object but for now only notes are supported).
  • Make the app more trustable. A lot of complains about Evernote is that it works in an opaque way - sometimes notes are changed or they disappear and it's not clear why - it could be a user error, or some bug, but regardless it makes it hard to trust the app with thousands of notes. So this feature give some transparency over what's happening - if some note seems to be gone or even if there's a bug, the redundant data allows investigating the issue and restoring content.
  • Eventually, to allow the implementation of a recycle bin. At the backend level, it's going to be supported right away since deleted notes keep their revisions, but some front end will need to be added too.

laurent22 added some commits Apr 4, 2019

wip
@tessus

This comment has been minimized.

Copy link
Collaborator

commented Apr 22, 2019

This looks like a great addition to Joplin, but I have a few questions:

How does the architecture look like? How many revisions are stored? Is the number of revisions configurable?
Is there a way to turn this off?

I don't want that every single change (every key stroke) creates a new revision. When exactly is a revision created?

@Agent-D

This comment has been minimized.

Copy link

commented Apr 25, 2019

Can't wait for this. I've been using Simplenote for years. It is rudimentary and buggy, and scales poorly; but the note history is its absolute killer feature. Once Joplin gets history implemented, I'll be switching everything over.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented Apr 25, 2019

Yes, quite keen to get this completed, and there's not too much left to be done.

@tessus, by default it will keep revisions for 6 months (configurable), and the feature can be disabled. It's designed to be efficient in terms of storage space as it only keeps diff of changes.

It will save revisions every 10 minutes.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented Apr 26, 2019

@laurent22 Thanks for the info. Will the revisions also be synced or will they only exist in the local database?

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented Apr 26, 2019

They will be synced too.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

I've noticed the following behavior when an existing note is changed:

Let's say you have a note with the following text.

Line 1

You change the note to look like this:

Line 1
Line 2

In the history the first revision never shows up. It basically starts at revision number 2. Maybe there should be a check, when you activate the revision system. If it is activated an initial revision for all notes should be made. What do you think?

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

Also, what happens, if you uncheck Enable note history? Are all revisions deleted?
If yes, there should be a warning that this will happen.
If no, there should be a way to delete them.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 7, 2019

In the history the first revision never shows up. It basically starts at revision number 2. Maybe there should be a check, when you activate the revision system. If it is activated an initial revision for all notes should be made. What do you think?

Auto-creating revisions for potentially thousands of notes would be too heavy, but there's a mechanism to auto-create one when a note is changed, and that note existed before the revision service. Was that note definitely an old one?

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 7, 2019

Also, what happens, if you uncheck Enable note history? Are all revisions deleted?

They are kept, but no new revision is created.

If yes, there should be a warning that this will happen.
If no, there should be a way to delete them.

They'll still be deleted after the number of days specified. So I guess if you set it to delete after 1 day, they'll be all gone after 24h.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 7, 2019

but there's a mechanism to auto-create one when a note is changed

Hmm, but it seems that only the note after the change was captured, not the one before. Otherwise I would have a revision that only shows Line 1. But maybe I hit an edge case. I will do some more tests later.

They are kept, but no new revision is created.

Ok, I see.

They'll still be deleted after the number of days specified

Ok, makes sense.

What would happen, if you set the value to 0? You can't use the up and down to set it to 0, but you can enter 0 manually.
Does 0 mean forever?

P.S.: Sorry for all these questions, but I guess the SRE in me is showing.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 7, 2019

Hmm, but it seems that only the note after the change was captured, not the one before.

Was the note recently modified?

Because I think it's related to a glitch that I've just noticed. Basically the service will save a revision with the original content of a note, when that note existed before the service. To do that it relies on a property (basically item_changes.before_change_item) in the database that keep the previous content of a note.

The problem is that this property item_changes.before_change_item has only been added at the same time as the revision service. So if a note was modified shortly before the revision service was installed, there would be an "item_changes" row for this change. The service will process this and create a revision but since the previous content was not saved in "before_change_item", it will just save a revision with the current content.

This should only happen in that particular case. Any old note that gets modified from then on should get its original content saved as expected. I'm going to double check though.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 7, 2019

What would happen, if you set the value to 0? You can't use the up and down to set it to 0, but you can enter 0 manually.

It means keep revisions for 0 days so everything will be deleted. Might have to fix this.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 8, 2019

Was the note recently modified?

I pulled the latest changes and used the ./run.sh script to run my test Joplin. I have a few test notes in there. So I changed one and waited to see, if there's a new history snapshot. After a while I saw that there was, but it didn't show the previous note (before the change), but the current one in the history.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 8, 2019

This is working as expected: there are two cases:

  • If the note was changed within the last 7 days, the created revision is simply the current content of the note.

  • if the note was last changed more than 7 days ago, and you change it now, the revision will contain the content before the change.

The reason for that is that we save revisions every 10 minutes, but if you make many changes within a few minutes and then stop modifying the note, the final revision will not contain the current content of the note. Basically at one point the service will see there's a revision from less than 10 minutes, and will not save a new one.

That's why when you change the note again more than 7 days later, we save that revision that wasn't saved now. The logic is a bit complicated but the goal is to preserve the last significant state of a note. I consider that if you make many changes to a note then stop editing it for several months, the last significant state was at the end of that series of edits, so we need to save that.

Sorry for the long post, and I guess I should also add this info somewhere while it's still fresh in my mind!

@whisperdancer

This comment has been minimized.

Copy link

commented May 12, 2019

Thank you for this great feature! I noticed Android App updated and Win Desktop. However, IOS App not updated to support this feature so I receive a sync error "unknown type 13. When will the IOS App get the update?

Thank you.

@schemen

This comment has been minimized.

Copy link

commented May 13, 2019

Ah, found the code as well, this means this is probably also related to #1517 as the ios app is not yet approved by apple probably (?)

@subi54

This comment has been minimized.

Copy link

commented May 13, 2019

@schemen, you are correct in your assumption.

@laurent22 wrote this over at discourse, 15 hrs ago: “The iOS version has been submitted and should be available tomorrow once it’s been reviewed by Apple.”

Source

@Dasymi

This comment has been minimized.

Copy link

commented May 13, 2019

Because of this I haven’t been able to use Joplin on IOS all day - not a very good advert. We definitely need to remember NOT to update desktop version until mobile updates are also available.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 13, 2019

Yes that's not good. With the desktop, Android and terminal apps, I can do instant releases and fix issues. With iOS, we need to wait a long time till they review and even then it's not sure they'll approve. Next time there's a sync change like this, I'll add a warning.

@mgrandi

This comment has been minimized.

Copy link

commented May 13, 2019

Random comment: the code here makes references to a 'revision viewer GUI', i don't see any way to bring this up, is that enabled yet?

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 13, 2019

@Dasymi Apple is really bad with validating updates to applications in their AppStore. It takes them forever to do anything. And this new notarizing madness makes it even more complicated on macOS. This is why I dislike this freaking AppStore monopoly so much. They decide what is allowed on your systems.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 13, 2019

@mgrandi click the info toolbar button and then on Previous versions of this note

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 14, 2019

@laurent22 maybe a bit off-topic, but I was wondering, if there wasn't a way to make it work across 2 database versions. I'm just thinking about the technical options here, but let's say you have a new statement that includes new columns. but the old statement with the old database structure are still valid, so I'm not sure why the old version wouldn't work. When it comes to syncing, just sync unknown types and the old version is going to ignore them anyway. So I'm a bit puzzled why using 2 different versions wouldn't work. I'm just curious for an answer, I'm not saying that Joplin must support sync across different database versions. (I just don't understand what the problem is.)

@whisperdancer

This comment has been minimized.

Copy link

commented May 14, 2019

How do I access the note history using android app? Thank you.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 14, 2019

@laurent22 maybe a bit off-topic, but I was wondering, if there wasn't a way to make it work across 2 database versions. I'm just thinking about the technical options here, but let's say you have a new statement that includes new columns. but the old statement with the old database structure are still valid, so I'm not sure why the old version wouldn't work. When it comes to syncing, just sync unknown types and the old version is going to ignore them anyway. So I'm a bit puzzled why using 2 different versions wouldn't work. I'm just curious for an answer, I'm not saying that Joplin must support sync across different database versions. (I just don't understand what the problem is.)

It's mostly for safety: if sync encounters an item it doesn't know, it just stops there to avoid any data corruption. I'm not currently testing how the sync process and db would handle unknown items, and it's not really worth it since it's potentially a lot of additional complexity, just for rare one-off events like this one.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 14, 2019

How do I access the note history using android app? Thank you.

The mobile app also creates note revisions, however the tool to inspect them is only available on desktop at the moment. Note that all apps (mobile, desktop, etc.) share the same history so if you notice some issue on mobile, you can simply inspect the history on desktop. Since it's a bit of an advanced tool, it's possible it will be only available on desktop.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 14, 2019

@laurent22

It's mostly for safety

Thanks for the info.

Since it's a bit of an advanced tool, it's possible it will be only available on desktop.

In that case we should probably restrict the setting to the desktop app.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 14, 2019

In that case we should probably restrict the setting to the desktop app.

Revisions are still being saved on mobile, and these settings are useful to define how they should be saved.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 14, 2019

Revisions are still being saved on mobile

Ok, but I thought that revisions are synced. So what will happen, if you set the desktop to keep 10 days and the Android to keep 5 days? This probably means that after 5 days older revisions will be deleted and no longer available on desktop.
Somehow this should be documented somewhere. This can be very confusing.

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 14, 2019

Ok, but I thought that revisions are synced. So what will happen, if you set the desktop to keep 10 days and the Android to keep 5 days? This probably means that after 5 days older revisions will be deleted and no longer available on desktop.

That's a good point, it's a setting that's kind of global to all the apps, but there's no such concept in Joplin. I'm not sure what would be a good workaround - I think for now I'll just add a description below "Warning: this setting applies to all the application" or something like this.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 14, 2019

That's why I thought limiting the setting to the desktop app (since it is the only one that read the history snapshots) would make sense. On the mobile apps we could add the message that the day count can only be set on the desktop app, or omit all history settings. It was just a thought, it doesn't mean that it was a good one. :-)

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 16, 2019

@laurent22 one more question that came to mind: what will happen, if note history is enabled on one client, but disabled on another?

@laurent22

This comment has been minimized.

Copy link
Owner Author

commented May 16, 2019

@laurent22 one more question that came to mind: what will happen, if note history is enabled on one client, but disabled on another?

The notes revisions will not be created on the client when it's disabled, but everything else will be the same.

@tessus

This comment has been minimized.

Copy link
Collaborator

commented May 16, 2019

@laurent22 so I will still have a note history on the machine where it is enabled (and no history where it is disabled)? in this case only the days-to-keep is a "global" one. ok, just had to be sure.

@phlind

This comment has been minimized.

Copy link

commented May 18, 2019

@laurent22 Is there already a possibility to view the revision history or the actual differences in the command line version of Joplin?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.