Fix for delete entries should ask user

[shawn-jj]

Fixes #10509

Mandatory checks

• [x] Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
• [ ] Tests created for changes (if applicable)
• [x] Manually tested changed features in running JabRef (always required)
• [x] Screenshots added in PR description (for UI changes)
• [x] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
• [x] 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.

Implemented the following features

• Create issue #471 for user documentation

1. When user delete entries, files which linked to selected entries are also delete

2. When user cut entries, keep files unchanged

3. Solved the access error caused by repeated deletion of files when one file is attached to multiple entries

4. A pop-up dialog box to confirm whether the user wants to delete attached files from selected entry

5. Keep track of user preference: if the user prefers not show confirmation dialog when deleting entries, then delete the files without displaying the dialog box

   

6. Add a preference option in File>Preference>Linked Files>Attached files so that users can manage preference about external files deletion

   

[shawn-jj]

I am trying to fix the pipeline, but OpenRewrite test is always failing. The log is provided below, and it appears to be connected to the recent code modifications I made in LibraryTab.java. However, I'm having difficulty finding the exact source of the problem.

Greatly appreciate it if someone could give me some hints!

> Task :rewriteDryRun
Validating active recipes
Scanning sources in project :
Using active styles [org.openrewrite.java.Checkstyle]
Failed to resolve dependencies from ::implementation. Some type information may be incomplete
Failed to resolve dependencies from ::testImplementation. Some type information may be incomplete
All sources parsed, running active recipes: org.jabref.config.rewrite.cleanup
These recipes would make changes to src/main/java/org/jabref/gui/LibraryTab.java:
    org.jabref.config.rewrite.cleanup
        org.openrewrite.staticanalysis.IsEmptyCallOnCollections

Report available:
FAILURE: Build failed with an exception.
    /home/runner/work/jabref/jabref/build/reports/rewrite/rewrite.patch

Run 'gradle rewriteRun' to apply the recipes.
* What went wrong:

Execution failed for task ':rewriteDryRun'.
> Task :rewriteDryRun FAILED
> java.lang.RuntimeException: Applying recipes would make changes. See logs for more details.

[koppor]

I am trying to fix the pipeline, but OpenRewrite test is always failing. The log is provided below, and it appears to be connected to the recent code modifications I made in LibraryTab.java. However, I'm having difficulty finding the exact source of the problem.

Please execute the rewriteRun task. This does the change in the file.

[Siedlerchr]

TODO: Unify the different dialogs Find a way to get around viewMode/LInkedFiles etc in LIbraryTab

[shawn-jj]

Not sure why, but I may have modified these files unintentionally. I tried to git clone from styles, but still could not restore this modification. May someone give me some hints?😥

[Siedlerchr]

the styles is a git submodule, you have to unselect/ignore it when commiting Delete it, then run git submodule init and git submodule update

[shawn-jj]

Problem solved! Ready for another code review :D

[koppor]

@shawn-jj Sorry for the long delay. We are afraid that JabRef might delete too much data (see https://discourse.jabref.org/t/your-zero-warning-auto-renaming-of-library-files-destroyed-years-of-work-in-seconds/4110 for a rant by a user - we are still investigating). Thus, we need clear heads for review and to check whether there could be some undesired behavior.

I am thinking of possible test cases with TestFX, but I cannot say anything concrete. Just wanted to state that.

[koppor]

I continued with the PR. Added support for "Trash" and removed code duplicates.

Issue:

Single file display

It should be displayed

c:\git-repositories...\minimal-note.pdf

. Without any scrolling. Thus, no softwrap, but use "...", The shown characters should be more in case the dialog is widened. And less, if it is shrunk.

Multiple file display

List all filenames, instead of the long text.

[koppor]

[koppor]

Test: Open

src\test\resources\org\jabref\logic\search

test-library-with-attached-files.bib

and delete files.

[koppor]

If anyone tests this, DO NOT RESET the existing preferences. We would like to know that happens when upgrading from a stable (or older dev version) of JabRef.

[koppor]

Requirement from above is implemented

Open issue: When pressing cancel in the context of deletion of an entry, the whole entry should not be deleted.

I wanted to introduce a "short" boolean flag to check for the result value. @calixtus proposed some more architectural cleaner approach. With callbacks etc. Think, this is harder to implement. Let's see.

[koppor]

DevCall decision: resolve conflict and test. If everything works fine: go The "Cancel" button thing can come later.

[ThiloteE]

Test with default preferences from older version / main.

1. Select entry

2. Delete entry via key "Delete" or right-click context menu and choose "Delete entry"

3. Triggers:

4. • [x] Choose "Keep entry". Result: Entry is kept. Attached file is not deleted. 👍
5. • [x] Choose "Delete entry" Result: Entry is deleted. Next popup dialogue triggers. 👍
     
6. • [x] Choose "Cancel" Result: Entry is deleted. Attached file is not deleted. 👍
7. • [x] Press "CTRL + Z" for Undo after step 6. Result: Entry will re-emerge. Attached file will continue to be attached to the entry. Requirement: Only works, if pressing "ctrl + z" TWICE. Could be better, by only having to press ctrl + z once, but good enough for release, so 👎 👍
8. • [x] Choose "Remove from entry" Result: Entry is deleted. Attached file is not deleted. 👍
9. • [x] Press "CTRL + Z" for Undo after step 8. Result: Similar results as in Step 7. 👎 👍
10. • [ ] Choose "Delete from Disc" Result: Entry is deleted. Attached file is deleted from disk. Actually, file is in trash, even though users default preference is Expected behaviour with these settings: File should not be in trash, but permanently deleted. So, kinda 👎 ❌ 👍
11. • [ ] Press "CTRL+Z" for Undo twice after Step 10. Result: Entry re-emerges. 👍 Attached file is still deleted from disk. (is in trash) 👎 ❌ Expected behaviour: File should not be in trash, but back in the folder and attached to the file.
12. • [x] Enable the following preference: Move deleted files to trash (Instead of deleting them), then choose "Delete from disk" Result: Entry is deleted. Attached file is deleted from disk. Actually, file is in trash. 👍
13. • [x] Select multiple entries and choose "delete from disk" Result: Entries are deleted. Attached file is deleted from disk. Actually, file is in trash. 👍 Undo works too.

Final observations / recommendation:

• Enabling or disabling the preference does not seem to change anything, so that should either be fixed or the preference be removed.
• The default preference should be to move attached files upon deletion into the trash, to prevent dataloss. Therefore, Move deleted files to trash (Instead of deleting them) should be enabled by default, not disabled.
• You may have to press "undo" multiple times in certain circumstances.

[koppor]

@ThiloteE Thank you for testing. I had the fear that "Trash" is not activated upon upgrade to this development version. We need to check.

[koppor]

New dialog (question more generic, because of Trash)

[koppor]

@ThiloteE I missed to bind the preferences UI correctly with the internal preferences. Thus, the UI setting had no effect. I fixed it. -- It should be good to go. @Siedlerchr will also test it on MacOS and then we will merge this in. -- Regarding the undo, I created an internal follow-up issue at https://github.com/JabRef/jabref-issue-melting-pot/issues/358

[Siedlerchr]

Works on mac