Skip to content

Add File -> Merge action for library comparison#15427

Open
prasanthcp wants to merge 9 commits intoJabRef:mainfrom
prasanthcp:fix/15401-file-merge-menu
Open

Add File -> Merge action for library comparison#15427
prasanthcp wants to merge 9 commits intoJabRef:mainfrom
prasanthcp:fix/15401-file-merge-menu

Conversation

@prasanthcp
Copy link
Copy Markdown

@prasanthcp prasanthcp commented Mar 26, 2026

Related issues and pull requests

Closes #15401

PR Description

This adds a new Merge action under File, so users can compare the currently open library with a selected Bib file. It reviews changes in the existing external changes dialog. With this it is easier to review and apply differences from other bib files without leaving JabRef application.

Steps to test

  1. Merge Action is disabled when no library is selected
step1_JabRef_15401
  1. Open any library (example: Chocolate) in JabRef. Then Go to File -> Merge
step2_JabRef_15401
  1. Select another .bib file
step3_JabRef_15401
  1. Notice the “External Changes Resolver” dialog opens and lists differences. Accept or deny changes and observe that they apply to the current open library
step4_JabRef_15401
  1. Repeat the above process by selecting the same .bib file
step5_JabRef_15401
  1. The dialog now shows that there are no differences and it is empty
step6_JabRef_15401 step7_JabRef_15401

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • I manually tested my changes in running JabRef (always required)
  • I added JUnit tests for changes (if applicable)
  • I added screenshots in the PR description (if change is visible to the user)
  • I added a screenshot in the PR description showing a library with a single entry with me as author and as title the issue number
  • I described the change in CHANGELOG.md in a way that can be understood by the average user (if change is visible to the user)
  • I checked the user documentation for up to dateness and submitted a pull request to our user documentation repository

Test Executed: ./gradlew :jabgui:test --tests org.jabref.gui.collab.ExternalChangesResolverViewModelTest
Output: BUILD SUCCESSFUL in 2m 13s ; 31 actionable tasks: 15 executed, 16 up-to-date

Documentation PR - JabRef/user-documentation#620

@github-actions
Copy link
Copy Markdown
Contributor

Hey @prasanthcp! 👋

Thank you for contributing to JabRef!

We have automated checks in place, based on which you will soon get feedback if any of them are failing. We also use Qodo for review assistance. It will update your pull request description with a review help and offer suggestions to improve the pull request.

After all automated checks pass, a maintainer will also review your contribution. Once that happens, you can go through their comments in the "Files changed" tab and act on them, or reply to the conversation if you have further inputs. You can read about the whole pull request process in our contribution guide.

Please ensure that your pull request is in line with our AI Usage Policy and make necessary disclosures.

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add File → Merge action for library comparison

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add File → Merge action for library comparison
• Allow users to merge external BibTeX files with current library
• Display differences in External Changes Resolver dialog
• Handle empty change lists gracefully in resolver
Diagram
flowchart LR
  User["User opens File menu"]
  Merge["Selects Merge option"]
  Dialog["File dialog opens"]
  Select["Selects BibTeX file"]
  Scan["Scans for changes"]
  Resolver["External Changes Resolver dialog"]
  Accept["User accepts/denies changes"]
  Apply["Changes applied to library"]
  
  User --> Merge
  Merge --> Dialog
  Dialog --> Select
  Select --> Scan
  Scan --> Resolver
  Resolver --> Accept
  Accept --> Apply
Loading

Grey Divider

File Changes

1. jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java ✨ Enhancement +1/-0

Add MERGE_LIBRARY standard action

• Added new MERGE_LIBRARY action enum with "Merge..." label
• Uses MERGE_ENTRIES icon for visual consistency

jabgui/src/main/java/org/jabref/gui/actions/StandardActions.java


2. jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java ✨ Enhancement +127/-0

Implement merge library action logic

• New action class implementing merge functionality
• Opens file dialog to select BibTeX file for merging
• Scans for changes between active and merge database
• Displays changes in resolver dialog and applies accepted changes
• Handles file not found and read errors with user feedback

jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java


3. jabgui/src/main/java/org/jabref/gui/collab/ExternalChangesResolverViewModel.java 🐞 Bug fix +9/-5

Support empty change lists in resolver

• Removed assertion requiring non-empty changes list
• Added conditional logic to handle empty change lists
• Initialize bindings to false when no changes exist
• Maintains original binding behavior for non-empty lists

jabgui/src/main/java/org/jabref/gui/collab/ExternalChangesResolverViewModel.java


View more (3)
4. jabgui/src/main/java/org/jabref/gui/frame/MainMenu.java ✨ Enhancement +3/-0

Integrate merge action into File menu

• Import new MergeLibraryAction class
• Add merge menu item to File menu after save operations
• Instantiate action with required dependencies

jabgui/src/main/java/org/jabref/gui/frame/MainMenu.java


5. jabgui/src/test/java/org/jabref/gui/collab/ExternalChangesResolverViewModelTest.java 🧪 Tests +32/-0

Add resolver view model unit tests

• New test class for ExternalChangesResolverViewModel
• Test empty changes list returns false for all resolution states
• Test non-empty changes list initially unresolved

jabgui/src/test/java/org/jabref/gui/collab/ExternalChangesResolverViewModelTest.java


6. CHANGELOG.md 📝 Documentation +1/-0

Document merge feature in changelog

• Document new Merge action feature in File menu
• Reference issue #15401 for tracking

CHANGELOG.md


Grey Divider

Qodo Logo

@qodo-free-for-open-source-projects
Copy link
Copy Markdown
Contributor

qodo-free-for-open-source-projects bot commented Mar 26, 2026

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. Swing UndoManager in MergeLibraryAction📘 Rule violation ⛨ Security
Description
The new MergeLibraryAction introduces a direct dependency on Swing via
javax.swing.undo.UndoManager. This violates the requirement that new UI code must use JavaFX and
not add Swing usage.
Code

jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[R9-10]

+import javax.swing.undo.UndoManager;
+
Evidence
PR Compliance ID 21 forbids introducing Java Swing in new/changed code. The new class imports and
uses javax.swing.undo.UndoManager.

AGENTS.md
jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[9-10]
jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[39-41]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MergeLibraryAction` introduces Swing usage by depending on `javax.swing.undo.UndoManager`, which is disallowed for new UI code.
## Issue Context
The compliance checklist forbids introducing Java Swing in new/changed code.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[9-10]
- jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[39-41]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Invalid merge file diffed🐞 Bug ≡ Correctness
Description
MergeLibraryAction.scanForChanges() diffs against ParserResult.getDatabaseContext() without checking
ParserResult.isInvalid(), so a parse failure can look like mass deletions/changes and be applied to
the open library if accepted.
Code

jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[R98-105]

+    private List<DatabaseChange> scanForChanges(BibDatabaseContext activeDatabase, Path mergeFile) throws IOException {
+        ImportFormatPreferences importFormatPreferences = preferences.getImportFormatPreferences();
+        ParserResult result = OpenDatabase.loadDatabase(mergeFile, importFormatPreferences, new DummyFileUpdateMonitor());
+        BibDatabaseContext databaseOnDisk = result.getDatabaseContext();
+
+        DatabaseChangeResolverFactory databaseChangeResolverFactory = new DatabaseChangeResolverFactory(dialogService, activeDatabase, preferences, stateManager);
+        return DatabaseChangeList.compareAndGetChanges(activeDatabase, databaseOnDisk, databaseChangeResolverFactory);
+    }
Evidence
OpenDatabase.loadDatabase only throws IOException for I/O; parse problems are represented in
ParserResult (e.g., invalid flag). MergeLibraryAction currently proceeds to compare even if the
ParserResult is invalid, whereas other features guard against invalid/empty ParserResult before
using it.

jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[98-105]
jablib/src/main/java/org/jabref/logic/importer/OpenDatabase.java[16-28]
jablib/src/main/java/org/jabref/logic/importer/ParserResult.java[120-159]
jabgui/src/main/java/org/jabref/gui/externalfiles/PdfMergeDialog.java[62-73]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MergeLibraryAction.scanForChanges` loads the selected BibTeX file and immediately diffs it against the active database. If the import produced a `ParserResult` marked invalid (parse failure), the diff can appear as a large set of deletions/changes and can be applied if the user accepts them.
### Issue Context
`OpenDatabase.loadDatabase(...)` only throws `IOException` for I/O errors. BibTeX parse failures are represented via `ParserResult.isInvalid()` (and sometimes `isEmpty()`), so the merge flow must validate the parse result explicitly.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[98-105]
### Suggested fix
- After `OpenDatabase.loadDatabase(...)`, check `result.isInvalid()` (and decide how to handle `result.isEmpty()`—either block, warn, or require confirmation).
- If invalid, show an error dialog like "Could not parse merge file" and abort (e.g., throw an `IOException` to reuse the existing `.onFailure(...)` handler, or handle it explicitly in `scanForChanges`).
- Optionally include `result.getErrorMessage()` in logs (not necessarily the UI) to aid debugging.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Manual Optional.get() after checks📘 Rule violation ⚙ Maintainability
Description
The new code uses Optional.isEmpty() followed by Optional.get() for control flow, instead of
using ifPresent/ifPresentOrElse/map chains. This goes against the preferred Optional usage
patterns and can lead to less idiomatic and harder-to-maintain code.
Code

jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[R58-79]

+        Optional<BibDatabaseContext> activeDatabaseOptional = stateManager.getActiveDatabase();
+        if (activeDatabaseOptional.isEmpty()) {
+            return;
+        }
+
+        BibDatabaseContext activeDatabase = activeDatabaseOptional.get();
+        Path initialDirectory = activeDatabase.getDatabasePath()
+                                              .map(Path::getParent)
+                                              .orElse(preferences.getImporterPreferences().getImportWorkingDirectory());
+
+        FileDialogConfiguration fileDialogConfiguration = new FileDialogConfiguration.Builder()
+                .addExtensionFilter(FileFilterConverter.toExtensionFilter(Localization.lang("BibTeX"), StandardFileType.BIBTEX_DB))
+                .withDefaultExtension(StandardFileType.BIBTEX_DB)
+                .withInitialDirectory(initialDirectory)
+                .build();
+
+        Optional<Path> selectedFile = dialogService.showFileOpenDialog(fileDialogConfiguration);
+        if (selectedFile.isEmpty()) {
+            return;
+        }
+
+        Path mergeFile = selectedFile.get();
Evidence
PR Compliance ID 11 requires using Optional APIs (e.g., ifPresent/ifPresentOrElse) instead of
manual presence checks and get(). The new execute() method uses isEmpty() + get() for both
activeDatabaseOptional and selectedFile.

AGENTS.md
jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[58-79]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MergeLibraryAction.execute()` uses `Optional.isEmpty()` followed by `Optional.get()`, which is discouraged; prefer `ifPresent(...)`, `ifPresentOrElse(...)`, `map(...)`, etc.
## Issue Context
This is newly added code and should follow JabRef's preferred Optional idioms.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[58-79]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


4. Duplicate change-scan logic📘 Rule violation ⚙ Maintainability
Description
scanForChanges(...) duplicates the existing change scanning pattern (load database via
OpenDatabase.loadDatabase(...) + compare via DatabaseChangeList.compareAndGetChanges(...)). This
increases maintenance burden and risks divergence if the scanning logic changes in one place but not
the other.
Code

jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[R98-105]

+    private List<DatabaseChange> scanForChanges(BibDatabaseContext activeDatabase, Path mergeFile) throws IOException {
+        ImportFormatPreferences importFormatPreferences = preferences.getImportFormatPreferences();
+        ParserResult result = OpenDatabase.loadDatabase(mergeFile, importFormatPreferences, new DummyFileUpdateMonitor());
+        BibDatabaseContext databaseOnDisk = result.getDatabaseContext();
+
+        DatabaseChangeResolverFactory databaseChangeResolverFactory = new DatabaseChangeResolverFactory(dialogService, activeDatabase, preferences, stateManager);
+        return DatabaseChangeList.compareAndGetChanges(activeDatabase, databaseOnDisk, databaseChangeResolverFactory);
+    }
Evidence
PR Compliance ID 7 requires avoiding duplicated logic. The new scanForChanges(...) method repeats
the same core steps already implemented in ChangeScanner.scanForChanges() (load DB with
preferences + DummyFileUpdateMonitor, then compare via DatabaseChangeList.compareAndGetChanges).

AGENTS.md
jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[98-105]
jabgui/src/main/java/org/jabref/gui/collab/ChangeScanner.java[35-51]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`MergeLibraryAction.scanForChanges(...)` duplicates the existing database-load-and-compare logic found in `ChangeScanner`.
## Issue Context
Duplicated logic makes future fixes/features (e.g., post-load actions, monitoring changes, preference handling) easy to miss in one of the code paths.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[98-105]
- jabgui/src/main/java/org/jabref/gui/collab/ChangeScanner.java[35-51]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. Wrong tab marked changed🐞 Bug ≡ Correctness
Description
MergeLibraryAction applies accepted DatabaseChange objects to the originally captured
BibDatabaseContext, but marks the currently active tab as changed after the dialog; if the user
switches tabs while the background scan runs, the wrong tab can be marked dirty.
Code

jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[R120-125]

+        if (compoundEdit.hasEdits()) {
+            undoManager.addEdit(compoundEdit);
+            stateManager.activeTabProperty()
+                        .get()
+                        .ifPresent(LibraryTab::markBaseChanged);
+        }
Evidence
DatabaseChangeList binds each DatabaseChange to the original database context passed to
compareAndGetChanges (here: the captured activeDatabase). However, MergeLibraryAction marks
stateManager.activeTabProperty() at apply time, which can differ from the tab corresponding to that
captured context in an async flow. Other async flows (e.g., ImportCommand) capture the intended
LibraryTab up front to avoid applying results to a different tab later.

jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[58-66]
jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[87-95]
jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[107-125]
jabgui/src/main/java/org/jabref/gui/collab/DatabaseChangeList.java[31-45]
jabgui/src/main/java/org/jabref/gui/importer/actions/ImportCommand.java[134-156]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The merge diff runs in a background task. Accepted changes are applied to the database context captured at action start, but the code marks whatever tab is active *at the end* as dirty. If the user changes the active tab while the background task is running, UI dirty state can be applied to the wrong tab.
### Issue Context
`DatabaseChangeList.compareAndGetChanges(originalDatabase, ...)` creates changes bound to `originalDatabase` (the captured context). The dirty flag and undo edit should be associated with the same initiating tab/context.
### Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[56-66]
- jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[87-95]
- jabgui/src/main/java/org/jabref/gui/collab/MergeLibraryAction.java[107-125]
### Suggested fix
- Capture the initiating `LibraryTab` (or otherwise a stable tab reference) before starting the background scan, similar to `ImportCommand` capturing `LibraryTab tab`.
- Use that captured tab to:
- determine the `BibDatabaseContext` to merge into,
- add the undo edit (prefer the tab’s undo manager if available),
- call `capturedTab.markBaseChanged()`.
- Optionally guard against the tab being closed while the task runs (if the captured tab is no longer present, abort applying changes).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@github-actions github-actions bot added the good first issue An issue intended for project-newcomers. Varies in difficulty. label Mar 26, 2026
@testlens-app

This comment has been minimized.

@github-actions github-actions bot added the status: changes-required Pull requests that are not yet complete label Mar 26, 2026
@github-actions github-actions bot mentioned this pull request Mar 26, 2026
@SaurabhSinghRajput-8777
Copy link
Copy Markdown

/assign-me

@faneeshh
Copy link
Copy Markdown
Contributor

/assign-me

This is a Pull Request.

@SaurabhSinghRajput-8777
Copy link
Copy Markdown

/assign-me

This is a Pull Request.

Yes i want to work on this

@calixtus
Copy link
Copy Markdown
Member

You have to assign yourself to an issue, not to a Pull Request.
The actual contributor, who made the Pull Request is already assigned to the Pull Request.
And he obviously solved the issue.
So you dont need to assign yourself to work on a Pull Request that is already done.
Unless you are a maintainer, marking a Pull Request for reviewing soon.
Are you a maintainer?

@testlens-app

This comment has been minimized.

@testlens-app

This comment has been minimized.

@github-actions github-actions bot added status: no-bot-comments and removed status: changes-required Pull requests that are not yet complete labels Mar 27, 2026
@testlens-app

This comment has been minimized.

Using the recommended method name “getDatabaseChanges” instead of “scanForChanges”
@testlens-app

This comment has been minimized.

Refactored method name has been updated
@testlens-app

This comment has been minimized.

@testlens-app

This comment has been minimized.

@testlens-app

This comment has been minimized.

@prasanthcp
Copy link
Copy Markdown
Author

Hi Team, Please let me know if anything more has to be done to resolve this issue.

@testlens-app
Copy link
Copy Markdown

testlens-app bot commented Mar 31, 2026

✅ All tests passed ✅

🏷️ Commit: 75ee850
▶️ Tests: 10214 executed
⚪️ Checks: 50/50 completed


Learn more about TestLens at testlens.app.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

first contrib good first issue An issue intended for project-newcomers. Varies in difficulty. status: no-bot-comments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

File >Merge...

5 participants