Add compare button to duplicates in Citation relations tab by KumaLuo · Pull Request #11915 · JabRef/jabref

KumaLuo · 2024-10-11T11:30:05Z

Added a "Compare" button for duplicate entries in the CitationRelationsTab.

screenshot before:

screenshot after:

Implemented functionality that the compare button will open the "Possible duplicate entries" window

screen recording preview:

preview.mov

This PR fixes #11192

Mandatory checks

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.

…ionsTab. - Added localization key for 'Compare with duplicate entries'.

Siedlerchr · 2024-10-11T13:41:20Z

For opening the compare window check out the MergeDialogEntriesDialog

…r comparing and merging duplicate entries. - Updated changelog for compare button feature.

InAnYan · 2024-10-12T07:52:12Z

 - We added a different background color to the search bar to indicate when the search syntax is wrong. [#11658](https://github.com/JabRef/jabref/pull/11658)
 - We added a setting which always adds the literal "Cited on pages" text before each JStyle citation. [#11691](https://github.com/JabRef/jabref/pull/11732)
 - We added a new plain citation parser that uses LLMs. [#11825](https://github.com/JabRef/jabref/issues/11825)
+- Added a compare button to the duplicates in the citation relations tab to open the "Possible duplicate entries" window. [#11192](https://github.com/JabRef/jabref/issues/11192)


"We added" 😅

😂 I will fix that

InAnYan · 2024-10-12T08:03:22Z

+                        Button compareButton = IconTheme.JabRefIcons.MERGE_ENTRIES.asButton();
+                        compareButton.setTooltip(new Tooltip(Localization.lang("Compare with duplicate entries")));
+                        compareButton.setOnMouseClicked(event -> {
+                            openPossibleDuplicateEntriesWindow(entry);


Does this mean that every entry is potentially a duplicate?

In the broader context of the code, there’s a DuplicateCheck object that checks if an entry in the database has a duplicate. When the comparison button is created, the button is only shown when a potential duplicate is detected. This happens because of the duplicateCheck.containsDuplicate() method. If this method detects a duplicate for a given entry, the compareButton will allow the user to compare the entries. I'm sorry.I may not understand the point of the question, do I need to make any changes?😅

Ah, CitationRelationItem.isLocal() returns whether it's a duplicate or not?

I think CitationRelationItem.isLocal() returns true if entries in citation relations tab have been detected there is a possible duplicate entry in local current database.

… into fix-for-issue-11192

koppor

Good first step.

After clicking "merge", the current entry should be kept selected - and not the merged entry be selected.

However, does not always work. It uses the entry of the citation relation if the entry was added to the library before

Open citationr relations for paper P1
Add citing paper P2 to library
Edit P2 using JabRef
Go back toi P1: Citatoin relations
See that P2 is used - and not the paper retrieved by citation relatations

This could be another issue -- if you don't see how to fix it. - I think, there "only" needs to be a clone made when adding an entry to the library.

koppor · 2024-10-12T09:36:25Z

                        vContainer.getChildren().add(jumpTo);
+
+                        Button compareButton = IconTheme.JabRefIcons.MERGE_ENTRIES.asButton();
+                        compareButton.setTooltip(new Tooltip(Localization.lang("Compare with duplicate entries")));


I think "duplicate" is not a good term. It is about an existing entry in the library

Suggested change

compareButton.setTooltip(new Tooltip(Localization.lang("Compare with duplicate entries")));

compareButton.setTooltip(new Tooltip(Localization.lang("Compare with existing entries")));

- Fixed bug that when adding an entry to the library from Citation Relations Tab by adding the entry's clone instead of itself. - Optimised openPossibleDuplicateEntriesWindow function in CitationRelationsTab.java to show result of undo and redo. - Implemented that After clicking "merge", the current entry will be kept selected - and not the merged entry be selected.

KumaLuo · 2024-10-13T05:42:46Z

Hi@koppor, I think my approach to change focus is a bit stupid, by reading the code in the LibraryTab.java and MainTable.java, it seems like that new added entry will always be selected, and my approach won't affect other functionalities. Any better suggestions? Thank you very much for your help!

KumaLuo · 2024-10-13T05:50:56Z

test failed, I will try to fix🥲

…ationsRelationsTabViewModelTest with clone entries.

… into fix-for-issue-11192

koppor

IMHO you should add code comments on the "why" of some things. I think, the five lines I commented on can just be removed without any change in functionality.

koppor · 2024-10-13T13:50:56Z

+        BibEntry current = libraryTab.getEntryEditor().getCurrentlyEditedEntry();
+        stateManager.getActiveDatabase().get().getDatabase().removeEntry(current);
+        stateManager.getActiveDatabase().get().getDatabase().insertEntry(current);


Why this? These three lines remove and add the same entry? Can these lines just be removed?

😅As I said, this is a stupid method to change selected entry from merged entry to original one. And I have tested that org.jabref.gui.LibraryTab#clearAndSelect seems to have nothing to do with merge entries, the operation of merge entries should be related to Maintable#clearAndSelect, which will clear the current selection and select the entry just added. So I remove and add the original entry to make sure the original entry be selected after merge. I think I have found a better way. I'm working on it！🥰

koppor · 2024-10-13T13:52:59Z

    }

    public void importCleanedEntries(List<BibEntry> entries) {
+        entries = entries.stream().map(entry -> (BibEntry) entry.clone()).filter(Objects::nonNull).toList();


Why this filter(Objects::nonNull)? I think, this can be removed. -- the list of entries should not contain null

(This code fragment seems to be AI generated - otherwise, it is OK)

koppor · 2024-10-13T13:53:52Z

+        libraryTab.showAndEdit(current);
+        libraryTab.clearAndSelect(current);


Here you change to the new entry. What happens if you remove these two lines?

koppor · 2024-10-13T14:01:54Z

it seems like that new added entry will always be selected, and my approach won't affect other functionalities. Any better suggestions? Thank you very much for your help!

This is part of the journey!

Suggestion add a breakpoint at org.jabref.gui.LibraryTab#clearAndSelect and inspect the call hiearchy to see why the entry is selected.

…t original entry be selected after citation item merge. - Modified MainTable#clearAndSelect to ensure original entry be selected after citation item merge. - Added some comments.

koppor

Two things; I think, they correlate. If one is fixed, the other might be fixed, too.

Jumping to an entry does not work any more

Cllick on the button just scrolls to another position in the "Citation relations".

Keep scroll position

After merging of an entry, the scroll bar of "Citation relations" moves up.

Before:

After:

My test entry:

@Article{Thames2016,
  author       = {Thames, Lane and Schaefer, Dirk},
  date         = {2016},
  journaltitle = {Procedia CIRP},
  title        = {Software-defined Cloud Manufacturing for Industry 4.0},
  doi          = {10.1016/j.procir.2016.07.041},
  issn         = {2212-8271},
  pages        = {12--17},
  volume       = {52},
  abstract     = {Procedia CIRP, 52 (2016) 12-17. doi:10.1016/j.procir.2016.07.041},
  publisher    = {Elsevier BV},
}

…in MainTable.java to let original entry get selected after merge instead of selecting new merged entry.

… changed to new merged entry, need to refresh selected citation relation item to ensure that the item link to current local entry instead of the old one.

koppor · 2024-10-14T17:03:02Z

                        vContainer.getChildren().add(jumpTo);
+
+                        Button compareButton = IconTheme.JabRefIcons.MERGE_ENTRIES.asButton();
+                        compareButton.setTooltip(new Tooltip(Localization.lang("Compare with existing entries")));


I think, it compares with a single entry – string should be adapted!

…issue-11192

koppor · 2024-10-15T07:50:13Z

+        MergeEntriesDialog dialog = new MergeEntriesDialog(localEntry, duplicateEntry, preferences);
+        dialog.setTitle(Localization.lang("Possible duplicate entries"));
+
+        Optional<EntriesMergeResult> mergeResultOpt = dialogService.showCustomDialogAndWait(dialog);


Do you really need the Opt suffix? - You can name it entriesMergeResult and one line below use mergeResult if you like shorter variables.

koppor · 2024-10-15T07:51:11Z

+
+        Optional<EntriesMergeResult> mergeResultOpt = dialogService.showCustomDialogAndWait(dialog);
+        mergeResultOpt.ifPresentOrElse(entriesMergeResult -> {
+            entriesMerge = entriesMergeResult.mergedEntry();


Please rename to mergedEntry. -- The variable name should state what's stored inside. It are not multiple entries. Moreover, consistency with a method name is nice.

koppor · 2024-10-15T07:52:12Z


    private long lastKeyPressTime;
    private String columnSearchTerm;
+    private boolean citationMerge = false; // citation merge flag


The comment could be directed from the variable name. The "why" is missing. Either add a comment why this is required - or remove the comment (both is OK for me)

Name the variable citationMergeMode.

koppor · 2024-10-15T07:53:00Z

                    .findFirst();
    }
+
+    public void setCitationMerge(boolean merge) {


Maybe name it setCitationMergeMode

koppor · 2024-10-15T07:55:36Z

+            BibDatabase database = stateManager.getActiveDatabase().get().getDatabase();
+
+            database.removeEntry(entriesMergeResult.originalLeftEntry());
+            database.insertEntry(entriesMergeResult.mergedEntry());


Right before this line, the setCitationMergeMode should be called.

koppor · 2024-10-15T07:56:41Z

+                            // let main table know this is a citation merge
+                            libraryTab.getMainTable().setCitationMerge(true);


Wrong position - because if I click "cancel" on the merge, the mode is not reset - I show you a better position in the code below.

- fixed wrong position codes: codes that updating citation relation item and setting state of citationMergeMode now be moved into CitationRelationsTab#openPossibleDuplicateEntriesWindow - modified some comments and Java doc

koppor

The column headings of the merge entries dialog are not self-explanatory

What is "left", what is "right"?`

Proposal:

Swap left and right - because the left is the new thing flowing into the middle thing and then the output is the merged entry
Set "Citation" and "Library" as titles for the columns

I hope this is possible with the current base offered by JabRef

I moved the statement libraryTab.getMainTable().setCitationMergeMode(true); close to insertEntry, because it is needed right before there. (fbbd5f0)

Other

… more self-explanatory.

koppor · 2024-10-16T08:00:51Z

Finally approved. Thank you for the fast follow-ups! I hope you will take another issue to improve JabRef even more!

KumaLuo · 2024-10-16T08:07:15Z

@koppor Thank you for the approval and your continuous support throughout the process! 🙌 It was a pleasure contributing to JabRef, and I appreciate your helpful guidance along the way. I look forward to tackling more issues and helping improve JabRef even further! 🚀

outdated

- Added a "Compare" button for duplicate entries in the CitationRelat…

c0f54ec

…ionsTab. - Added localization key for 'Compare with duplicate entries'.

KumaLuo changed the title ~~Add compare button to duplicates in Citation relations tab~~ [WIP]Add compare button to duplicates in Citation relations tab Oct 11, 2024

KumaLuo changed the title ~~[WIP]Add compare button to duplicates in Citation relations tab~~ [WIP] Add compare button to duplicates in Citation relations tab Oct 11, 2024

- Implemented functionality to open a "Possible duplicate entries" fo…

2edb8b6

…r comparing and merging duplicate entries. - Updated changelog for compare button feature.

KumaLuo changed the title ~~[WIP] Add compare button to duplicates in Citation relations tab~~ Add compare button to duplicates in Citation relations tab Oct 12, 2024

Merge branch 'main' into fix-for-issue-11192

e3c759c

KumaLuo marked this pull request as ready for review October 12, 2024 02:50

InAnYan previously requested changes Oct 12, 2024

View reviewed changes

KumaLuo added 2 commits October 12, 2024 19:42

Updated changelog

ea4bb0e

Merge branch 'fix-for-issue-11192' of https://github.com/KumaLuo/jabref…

a708875

… into fix-for-issue-11192

koppor requested changes Oct 12, 2024

View reviewed changes

KumaLuo and others added 2 commits October 13, 2024 16:37

Merge branch 'main' into fix-for-issue-11192

95c1362

KumaLuo added 2 commits October 13, 2024 18:17

Modified CitationsRelationsTabViewModel and ImportHandler to pass Cit…

8bec936

…ationsRelationsTabViewModelTest with clone entries.

Merge branch 'fix-for-issue-11192' of https://github.com/KumaLuo/jabref…

8fc4f91

… into fix-for-issue-11192

KumaLuo requested a review from koppor October 13, 2024 07:28

koppor requested changes Oct 13, 2024

View reviewed changes

KumaLuo and others added 2 commits October 14, 2024 03:02

- Deleted old code in CitationRelationTab.java which to implement tha…

5eed679

…t original entry be selected after citation item merge. - Modified MainTable#clearAndSelect to ensure original entry be selected after citation item merge. - Added some comments.

Merge branch 'main' into fix-for-issue-11192

99fa500

KumaLuo requested a review from koppor October 13, 2024 16:16

koppor requested changes Oct 14, 2024

View reviewed changes

KumaLuo and others added 3 commits October 14, 2024 22:27

Optimised citation relations item merge: Added a citation merge flag …

4bd4872

…in MainTable.java to let original entry get selected after merge instead of selecting new merged entry.

Optimised MainTable#clearAndSelect: make the code look more concise.

4bf1f44

Merge branch 'main' into fix-for-issue-11192

0710c3d

koppor mentioned this pull request Oct 14, 2024

Added double click to jump to entry in CitationRelationsTab #11955

Merged

6 tasks

Fixed bug: As original local entry of citation relation item has been…

9566980

… changed to new merged entry, need to refresh selected citation relation item to ensure that the item link to current local entry instead of the old one.

Merge branch 'main' into fix-for-issue-11192

272e686

KumaLuo requested a review from koppor October 14, 2024 16:16

koppor reviewed Oct 14, 2024

View reviewed changes

KumaLuo added 2 commits October 15, 2024 12:34

Adapted tool tip of compare button.

f8736fc

Merge branch 'main' of https://github.com/JabRef/jabref into fix-for-…

6cfb3b9

…issue-11192

KumaLuo requested a review from koppor October 15, 2024 01:56

Merge branch 'main' into fix-for-issue-11192

447f220

koppor requested changes Oct 15, 2024

View reviewed changes

- renamed some variables to make them more understandable

080e80f

- fixed wrong position codes: codes that updating citation relation item and setting state of citationMergeMode now be moved into CitationRelationsTab#openPossibleDuplicateEntriesWindow - modified some comments and Java doc

KumaLuo requested a review from koppor October 15, 2024 10:49

koppor added 2 commits October 16, 2024 06:39

Merge branch 'main' into fix-for-issue-11192

5f4264c

Move statement closer to intended use

fbbd5f0

koppor requested changes Oct 16, 2024

View reviewed changes

Modified the column headings of the merge entries dialog to make them…

2631280

… more self-explanatory.

KumaLuo requested a review from koppor October 16, 2024 05:52

koppor approved these changes Oct 16, 2024

View reviewed changes

koppor enabled auto-merge October 16, 2024 08:00

KumaLuo requested a review from InAnYan October 16, 2024 08:09

koppor added this pull request to the merge queue Oct 16, 2024

Merged via the queue into JabRef:main with commit ec2059a Oct 16, 2024

LoayGhreeb mentioned this pull request Oct 29, 2024

Fix inserting new entry #12123

Merged

7 tasks

	compareButton.setTooltip(new Tooltip(Localization.lang("Compare with duplicate entries")));
	compareButton.setTooltip(new Tooltip(Localization.lang("Compare with existing entries")));

		libraryTab.showAndEdit(current);
		libraryTab.clearAndSelect(current);

		// let main table know this is a citation merge
		libraryTab.getMainTable().setCitationMerge(true);

Uh oh!

Conversation

KumaLuo commented Oct 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Mandatory checks

Uh oh!

Siedlerchr commented Oct 11, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KumaLuo Oct 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KumaLuo commented Oct 13, 2024

Uh oh!

KumaLuo commented Oct 13, 2024

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koppor commented Oct 13, 2024

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Jumping to an entry does not work any more

Keep scroll position

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

koppor left a comment

Choose a reason for hiding this comment

Uh oh!

koppor commented Oct 16, 2024

Uh oh!

KumaLuo commented Oct 16, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

KumaLuo commented Oct 11, 2024 •

edited

Loading

KumaLuo Oct 12, 2024 •

edited

Loading