Allow users to review backup changes before restoring them or merge them selectively #9311

HoussemNasri · 2022-10-25T14:44:56Z

Notes

The majority of the changed files are the result of changing terminology from 'ExternalChange' to 'DatabaseChange.' 'DatabseChange' now makes more sense because it can represent a change from a backup database too (rather than just external files).

Description

The goal of this feature is to prevent data loss in the event of a corrupted backup (see #9063). Users can review the changes that would happen to their database when they choose to restore the backup. Alternatively, they can selectively merge some of the changes from the backup database into their database.

Feature Workflow

Review backup changes before restoring it
a. User opens a library
b. JabRef detects a difference between the library and the latest backup
c. JabRef launches the restore backup dialog
d. User selects Review backup to review backup changes
e. User has done reviewing and chooses to close the Review backup dialog
f. If the user approves the changes then he can select Restore from backup, otherwise he can select Ignore backup.
Selectively merge backup
a. User opens a library
b. JabRef detects a difference between the library and the latest backup
c. JabRef launches the restore backup dialog
d. User selects Review backup to review backup changes
e. User resolves the changes by selecting Accept or Deny for each change
f. User resolved all changes and the dialog is closed
g. JabRef applies the accepted changes to the current library

Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

- This logic will be used for diffing backup and current library, so it make sense to move it from ChangeScanner to its own class.

…to reuse it

* Included localization prefs in GeneralPreferences * Convertet getLanguage to new prefs pattern * Extracted localization from MetaDataDiff in logic * Refactored FileHistory and converted getFileHistory to new prefs model * Partial localization without restart * Convert list to EnumSet * Used pattern matching and applied IDE suggestions * Fixed comparison of optionals

* Fix for issue #9157. Added method that capitalizes first letter of every word, including words after hyphens. * Added test cases for CapitalizeFormatter

I was attempting to handle the dialog actions within the dialog class, but encountered some roadblocks. For example, when the user clicked restore from backup, I wanted to run the restore backup code without blocking the FX thread, so I wrapped it in a BackgroundTask#wrap. However, the database loading task in OpenDatabaseAction#loadDatabase will continue because the dialog is closed once the action is clicked, and the JVM continues running the code at BackupUIManager#showRestoreBackupDialog.

- An external change used to represent a database modification received from an external program. After this pr, an external change can also be a modification received from a backup library, and maybe later someone will use it to compare two libraries . For the above reasons, DatabaseChange is a better name.

- Because 'applyChanges' doesn't perform any IO operations or heavy computing, wrapping it in 'Platform.runLater' is unnecessary.

src/main/java/org/jabref/gui/collab/metedatachange/MetadataChangeDetailsView.java

src/main/java/org/jabref/gui/dialogs/BackupUIManager.java

HoussemNasri · 2022-11-01T17:32:18Z

Since JabRef saves 10 backup files, it would be nice to have a way choose the backup file (e.g. an older one) that should be restored and compared with status quo. Not sure if this goes beyond the scope of this pr though.

This is a great feature! However, I believe the 'Backup found' dialog is not the best place to access the feature. Thinking about it from the users' perspective, they would want to see their library as soon as possible. They may have lost one entry or all of them; they can't know (Maybe we should load the library before showing the dialog?). Browsing and comparing backups can take time, and it isn't necessary at that point because the backup libraries aren't going anywhere. At that point, it is preferable to have a simple UI with limited options. Once the library loads and they have done reviewing the damage, they can browse the backups and select the appropriate backup.

I really like the idea, but it is unfortunately out of scope. I would even argue that developing it would require more lines of code than this PR. This would make an excellent university project, in my opinion. ~~I'll open an issue for it.~~ I created a feature request: https://discourse.jabref.org/t/users-should-be-able-to-browse-and-merge-older-backups/3607

koppor · 2022-11-07T21:02:00Z

New dialog:

We discussed, it would be maybe good to use a text without any filename.

"A backup file was found. This could indicate that JabRef ..."

Then an additional button "Open backup folder". There, the backupped file should be highlighted.

(Would that be possible with low effort?)

HoussemNasri · 2022-11-07T21:08:55Z

Then an additional button "Open backup folder". There, the backupped file should be highlighted.

I don't believe it's the best place to have the open backup folder feature. Perhaps we should include an 'Open backup folder' and an 'Open application data folder' button in the about dialog.

HoussemNasri · 2022-11-07T21:14:51Z

In addition, we can make the backup file name a hyperlink, when it's clicked it opens the backup folder and selects the latest backup.

calixtus · 2022-11-08T21:52:16Z

In addition, we can make the backup file name a hyperlink, when it's clicked it opens the backup folder and selects the latest backup.

Sounds somewhat nice, but does it fit in our current design pattern? Or should it be a button?

HoussemNasri · 2022-11-08T22:42:42Z

@calixtus Four buttons/options seem excessive and overwhelming for one dialog. I don't recall seeing hyperlinks for opening folders in JabRef, so this could be a new pattern. If we can have a non-blocking dialog like the one shown below from Adobe PDF Reader, users will be able to access their backups from the about dialog if they wanted to.

src/main/java/org/jabref/gui/Base.css

HoussemNasri · 2022-11-12T23:05:02Z

Please don't merge yet. I recently discovered a bug in which manually merging backup changes using the three-way merge does not work. However, clicking 'Accept' seems to work.

…e dialog - In line 98, calling getVisibleChanges().remove(oldChange) might trigger the accepted changes to be applied and the dialog to close, thus, any change marked as accepted after that line will not be applied.

# Conflicts: # src/main/resources/l10n/JabRef_en.properties

# Conflicts: # src/main/java/org/jabref/gui/collab/ChangeScanner.java # src/main/java/org/jabref/gui/collab/DatabaseChangeResolverFactory.java # src/main/java/org/jabref/gui/collab/entrychange/EntryChangeResolver.java

Siedlerchr

At a first glance looks good. Will test it and take a other look tomorrow

src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java

Siedlerchr · 2022-12-04T18:55:52Z

src/main/java/org/jabref/gui/Base.css

@@ -256,7 +255,7 @@
 }

 .unchanged {
-    -rtfx-background-color:#0000;
+    -rtfx-background-color: #0000;


Extract that to a color constant?

Siedlerchr

Tested locally and works fine.! Hyperlink is cool feature!

* upstream/main: (37 commits) Update database context in state manager after loading (JabRef#9450) Bump classgraph from 4.8.151 to 4.8.152 (JabRef#9448) Bump appleboy/ssh-action from 0.1.5 to 0.1.6 (JabRef#9443) Bump Pendect/action-rsyncer from 1.1.0 to 2.0.0 (JabRef#9444) Bump jackson-dataformat-yaml from 2.14.0 to 2.14.1 (JabRef#9445) Bump unirest-java from 3.14.0 to 3.14.1 (JabRef#9447) Bump postgresql from 42.5.0 to 42.5.1 (JabRef#9446) New Crowdin updates (JabRef#9435) Return absolute path in case an absolute one is given (JabRef#9433) New Crowdin updates (JabRef#9434) Fix for issue: right click menu 6601 (JabRef#9271) Fix modernizer and refactor protected terms (JabRef#9427) Observable Preferences (OpenOffice) (JabRef#9422) Allow users to review backup changes before restoring them or merge them selectively (JabRef#9311) Bump slf4j-api from 2.0.4 to 2.0.5 (JabRef#9428) Bump archunit-junit5-api from 1.0.0 to 1.0.1 (JabRef#9429) Bump jackson-datatype-jsr310 from 2.14.0 to 2.14.1 (JabRef#9430) Bump lucene-highlighter from 9.4.1 to 9.4.2 (JabRef#9431) Fix weird checkbox styling (JabRef#9425) New translations JabRef_en.properties (Italian) (JabRef#9424) ...

HoussemNasri and others added 18 commits October 25, 2022 15:05

Create ExternalChangeList to share the library diffing logic

9073e21

- This logic will be used for diffing backup and current library, so it make sense to move it from ChangeScanner to its own class.

Remove unused local variable: generalPreferences

7d17159

Make the ExternalChangesResolverDialog title configurable to be able …

1325f23

…to reuse it

Experiment with moving the RestoreBackupDialog's UI to its own class

09db449

"Capitalize" capitalizes words after hyphens (#9186)

898e034

* Fix for issue #9157. Added method that capitalizes first letter of every word, including words after hyphens. * Added test cases for CapitalizeFormatter

Handle backup dialog actions in BackupUIManager

8611d64

Cleanup

e76a0f7

Convert 'dialogTitle' field to a local constructor variable

8ca7d28

Document DatabaseChangeList#compareAndGetChanges method

3b8d700

Run the 'applyChanges' method on the FX thread.

2e753a7

- Because 'applyChanges' doesn't perform any IO operations or heavy computing, wrapping it in 'Platform.runLater' is unnecessary.

Show the real backup changes in the DatabaseChangesResolverDialog

a9a5d39

Apply backup

48ffc12

Compute the parserResult recursively when the user chooses review Backup

86cd0b1

Remove System.out.println

58f89f0

Merge branch 'main' into merge-backup-manually

ddd0dcb

github-actions bot reviewed Oct 25, 2022

View reviewed changes

src/main/java/org/jabref/gui/collab/metedatachange/MetadataChangeDetailsView.java Show resolved Hide resolved

Sync with main

ae154cc

HoussemNasri changed the title ~~Allow users to review backup before restoring it with selective merging~~ Allow users to review backup changes before restoring them or merge them selectively Oct 25, 2022

Siedlerchr reviewed Oct 25, 2022

View reviewed changes

src/main/java/org/jabref/gui/dialogs/BackupUIManager.java Outdated Show resolved Hide resolved

ThiloteE added export / save duplicateFinder labels Oct 26, 2022

HoussemNasri added backup and removed export / save labels Oct 26, 2022

HoussemNasri added 3 commits October 29, 2022 10:10

Log exception

e0ef258

Fix failing localization test

7b57f08

Delete RestoreBackupDialog.java

8fb4ed0

HoussemNasri marked this pull request as ready for review October 31, 2022 14:17

HoussemNasri added 2 commits November 12, 2022 22:12

Replace textblock with normal string because it can't be localized

f881edd

Add a hyperlink to open the backups folder

9cf07fe

HoussemNasri commented Nov 12, 2022

View reviewed changes

src/main/java/org/jabref/gui/Base.css Show resolved Hide resolved

Fix localization test

e288cde

HoussemNasri added 6 commits December 3, 2022 21:21

Keep the universal styling for TextFlow children

190210a

Merge branch 'main' into merge-backup-manually

da6ed9a

# Conflicts: # src/main/resources/l10n/JabRef_en.properties

Merge branch 'main' into merge-backup-manually

6a23aec

# Conflicts: # src/main/java/org/jabref/gui/collab/ChangeScanner.java # src/main/java/org/jabref/gui/collab/DatabaseChangeResolverFactory.java # src/main/java/org/jabref/gui/collab/entrychange/EntryChangeResolver.java

Resolve conflict

3d0229b

Pass unit tests

f67fb8e

HoussemNasri requested review from Siedlerchr, koppor and calixtus December 3, 2022 23:23

HoussemNasri added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 3, 2022

Siedlerchr reviewed Dec 3, 2022

View reviewed changes

src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java Outdated Show resolved Hide resolved

Improve code readability

2639f2f

Siedlerchr reviewed Dec 4, 2022

View reviewed changes

Siedlerchr approved these changes Dec 4, 2022

View reviewed changes

koppor approved these changes Dec 5, 2022

View reviewed changes

calixtus approved these changes Dec 5, 2022

View reviewed changes

calixtus merged commit e86839a into main Dec 5, 2022

calixtus deleted the merge-backup-manually branch December 5, 2022 19:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow users to review backup changes before restoring them or merge them selectively #9311

Allow users to review backup changes before restoring them or merge them selectively #9311

HoussemNasri commented Oct 25, 2022 •

edited

Loading

HoussemNasri commented Nov 1, 2022 •

edited

Loading

koppor commented Nov 7, 2022

HoussemNasri commented Nov 7, 2022

HoussemNasri commented Nov 7, 2022

calixtus commented Nov 8, 2022

HoussemNasri commented Nov 8, 2022

HoussemNasri commented Nov 12, 2022

Siedlerchr left a comment

Siedlerchr Dec 4, 2022

Siedlerchr left a comment

Allow users to review backup changes before restoring them or merge them selectively #9311

Allow users to review backup changes before restoring them or merge them selectively #9311

Conversation

HoussemNasri commented Oct 25, 2022 • edited Loading

Notes

Description

Feature Workflow

HoussemNasri commented Nov 1, 2022 • edited Loading

koppor commented Nov 7, 2022

HoussemNasri commented Nov 7, 2022

HoussemNasri commented Nov 7, 2022

calixtus commented Nov 8, 2022

HoussemNasri commented Nov 8, 2022

HoussemNasri commented Nov 12, 2022

Siedlerchr left a comment

Choose a reason for hiding this comment

Siedlerchr Dec 4, 2022

Choose a reason for hiding this comment

Siedlerchr left a comment

Choose a reason for hiding this comment

HoussemNasri commented Oct 25, 2022 •

edited

Loading

HoussemNasri commented Nov 1, 2022 •

edited

Loading