Refactor OpenExternalFileAction #13508

Muskan244 · 2025-07-08T08:16:53Z

This PR refactors the existing OpenExternalFileAction logic by introducing two separate files:

OpenSingleExternalFileAction: Handles the case where exactly one entry is selected and it contains exactly one linked file.
OpenSelectedEntriesFilesAction: Handles all other cases. Also includes a check for the single-entry-single-file scenario and delegates to OpenSingleExternalFileAction when appropriate.

Steps to test

Open JabRef and select one entry with one linked file.
If there is no attached file, Right-click and choose "Attach File".
Right-click and choose "Open File". It should invoke OpenSingleExternalFileAction and open that file.
Select multiple entries or an entry with multiple files.
Right-click and choose "Open File". It should invoke OpenSelectedEntriesFilesAction and open all files (with confirmation if more than 10).

Mandatory checks

I own the copyright of the code submitted and I license it under the MIT license
[/] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
[/] Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
[/] Screenshots added in PR description (if change is visible to the user)
[/] 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.

…ion and OpenSelectedEntriesFilesAction - Introduced OpenSingleExternalFileAction to handle single entry with one linked file - Introduced OpenSelectedEntriesFilesAction to handle multiple selection cases Fixes JabRef#13431

jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java

Muskan244 · 2025-07-08T10:12:58Z

Hi @koppor,
I noticed that the CI reports some test failures in CitationStyleGeneratorTest and CSLFormatUtilsTest, mainly due to month formatting differences like "July" vs. "Jul." and some submodules modification detection when i haven't done any.
These tests seem unrelated to my changes, which are limited to splitting OpenExternalFileAction into two separate classes for better handling of file opening based on selection. Just wanted to flag this for clarity. Please let me know if there is some issue from my side and i am missing it or if anything else is needed from my side!

koppor · 2025-07-08T10:15:43Z

@Muskan244 look at submodules FAQ at https://devdocs.jabref.org/code-howtos/faq.html

With the right sub module checkout, the tests will pass.

Muskan244 · 2025-07-10T06:42:24Z

Hi! I’ve pushed the requested changes, and it looks like all checks have passed. Just checking in to see if there's anything more I should do. Thanks again for guiding me through this!

jabgui/src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FulltextSearchResultsTab.java

jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java

koppor · 2025-07-10T09:30:12Z

jabgui/src/main/java/org/jabref/gui/maintable/OpenSelectedEntriesFilesAction.java

+            if (selectedEntries.size() == 1) {
+                BibEntry entry = selectedEntries.getFirst();
+                List<LinkedFile> files = entry.getFiles();
+
+                if (files.size() == 1) {
+                    new OpenSingleExternalFileAction(
+                            dialogService,
+                            preferences,
+                            entry,
+                            files.getFirst(),
+                            taskExecutor,
+                            databaseContext
+                    ).execute();
+                    return;
+                }
+            }


No need for this IMHO. If you have a special reason, add Java comment.

I added this check because single-entry single-file cases were not opening correctly via right-click in the main table. This fallback ensures the expected behaviour. I’ve now also added a Java comment explaining this. If there's something i am missing, I would appreciate the clarification.

What about two entries having a file attached each? What about an entry having two files attached? Does that work?

(I don't know what "not correctly" means on your text. Nothing percieved while trying out, wrong file opened, ...)

Yes, I think it works correctly for two entries having a file attached each and also for an entry having two files attached, both of these open with OpenSelectedEntriesFileAction.java.
When I said single-entry single-file cases were not opening correctly, I meant that we have created two files, one for single entry and other for multiple entries but when i apply the condition to check for that it always was using execute function of OpenSelectedEntriesFileAction.java and shows error when i apply the check in RightClickMenu file. I hope it clarifies my logic. If i am missing something, I would appreciate the guidance.

shows error

Please outline what is shown.

It is very strange that it works for >= 2 files, but not for one.

…laining code

…13505) Bumps [org.junit.platform:junit-platform-launcher](https://github.com/junit-team/junit-framework) from 1.13.1 to 1.13.3. - [Release notes](https://github.com/junit-team/junit-framework/releases) - [Commits](https://github.com/junit-team/junit-framework/commits) --- updated-dependencies: - dependency-name: org.junit.platform:junit-platform-launcher dependency-version: 1.13.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <[email protected]> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Oliver Kopp <[email protected]>

…ional fallback

koppor · 2025-07-11T08:22:21Z

I removed the check for the single file. Works here.

I just ask for a second review.

trag-bot · 2025-07-11T08:24:16Z

@trag-bot didn't find any issues in the code! ✅✨

resolved

JabRef/jabref#13508 Move Method public execute() : void from class org.jabref.gui.maintable.OpenExternalFileAction to public execute() : void from class org.jabref.gui.maintable.OpenSelectedEntriesFilesAction Move Method public execute() : void from class org.jabref.gui.maintable.OpenExternalFileAction to public execute() : void from class org.jabref.gui.maintable.OpenSingleExternalFileAction

JabRef/jabref#13508

* upstream/main: Bump com.squareup.okhttp3:okhttp from 5.0.0 to 5.1.0 in /versions (#13541) Fixed Issue 13418: "Search ShortSience" should do a latex-to-unicode conversion (#13532) New Crowdin updates (#13548) Add focus to field Link in the "Add file link" dialog. (#13520) Bump org.ow2.asm:asm from 9.6 to 9.8 in /versions (#13547) Make validation optional for saving library properties preferences (#13488) Bump org.apache.commons:commons-lang3 from 3.17.0 to 3.18.0 in /versions (#13546) --porcelain does not output any info or warn errors on the console (#13469) Bump jablib/src/main/resources/csl-locales from `7e137db` to `3bad433` (#13545) Refine "File exists" warning (#13491) Bump org.openrewrite.recipe:rewrite-recipe-bom from 3.8.0 to 3.11.1 (#13544) Bump org.openrewrite.rewrite from 7.8.0 to 7.11.0 (#13542) Bump jablib/src/main/resources/csl-styles from `704ff9f` to `59f124d` (#13540) Add architecture test for logging infrastructure (#13534) More harsh test on title of PR New Crowdin updates (#13533) Update extra-java-module-info plugin (#13527) [Junie]: fix: display help output for --help argument (#13446) Refactor OpenExternalFileAction (#13508)

Muskan244 and others added 3 commits July 8, 2025 13:23

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

91da0cf

Cleaned up constant name, dialog text, and removed extra comment

bcf3b76

trag-bot bot reviewed Jul 8, 2025

View reviewed changes

jabgui/src/main/java/org/jabref/gui/maintable/OpenSingleExternalFileAction.java Outdated Show resolved Hide resolved

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

a8a1235

Muskan244 changed the title ~~Fix for issue 13431~~ Refactor OpenExternalFileAction Jul 8, 2025

Muskan244 and others added 3 commits July 8, 2025 14:32

Add null checks to OpenSingleExternalFileAction constructor

6ccf927

Add @nonnull annotations and fix localization key warnings

4dd6694

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

90233e4

fix submodules

dc0d67f

koppor previously requested changes Jul 10, 2025

View reviewed changes

Muskan244 and others added 2 commits July 10, 2025 17:17

Use database latest context, update button text, and add comments exp…

dd9ca53

…laining code

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

87b7ca9

koppor mentioned this pull request Jul 11, 2025

[CAYW] Add application=xyz parameter palukku/jabref#10

Closed

koppor added project: gsoc and removed project: gsoc labels Jul 11, 2025

dependabot bot and others added 3 commits July 11, 2025 13:30

Fix: clarify fallback logic, update dialog labels, and avoid null Opt…

77fd7a1

…ional fallback

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

e419d1b

Muskan244 requested a review from koppor July 11, 2025 08:12

koppor mentioned this pull request Jul 11, 2025

Same variable highlighted as modifed tsantalis/RefactoringMiner#958

Closed

koppor added 2 commits July 11, 2025 10:22

Remove exceptional case (and streamline code)

82d7c64

Streamline code

5f2d697

koppor added the status: awaiting-second-review For non-trivial changes label Jul 11, 2025

Siedlerchr approved these changes Jul 12, 2025

View reviewed changes

Siedlerchr added this pull request to the merge queue Jul 12, 2025

Merged via the queue into JabRef:main with commit dd823a4 Jul 12, 2025
1 check passed

github-actions bot mentioned this pull request Jul 12, 2025

Refactor OpenExternalFileAction #13431

Closed

Muskan244 deleted the fix-for-issue-13431 branch July 18, 2025 09:51

tsantalis added a commit to tsantalis/RefactoringMiner that referenced this pull request Aug 2, 2025

Enable detection of Inline Variable on top of Replace Loop with Pipeline

e085868

JabRef/jabref#13508

tsantalis mentioned this pull request Aug 2, 2025

Overlapping Extract/Inline Variable refactorings are ignored within Moved methods tsantalis/RefactoringMiner#967

Closed

Uh oh!

Refactor OpenExternalFileAction #13508

Refactor OpenExternalFileAction #13508

Uh oh!

Conversation

Muskan244 commented Jul 8, 2025

Steps to test

Mandatory checks

Uh oh!

Uh oh!

Muskan244 commented Jul 8, 2025

Uh oh!

koppor commented Jul 8, 2025

Uh oh!

Muskan244 commented Jul 10, 2025

Uh oh!

Uh oh!

Uh oh!

koppor Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

Muskan244 Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

koppor Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

Muskan244 Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

koppor Jul 11, 2025

Choose a reason for hiding this comment

Uh oh!

koppor commented Jul 11, 2025

Uh oh!

trag-bot bot commented Jul 11, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants