Skip to content

Report failed file downloads as notification instead of modal dialog - #774

Open
koppor wants to merge 4 commits into
mainfrom
fulltext-bulk-no-popup
Open

koppor wants to merge 4 commits into
mainfrom
fulltext-bulk-no-popup

Conversation

@koppor

@koppor koppor commented Sep 22, 2026

Copy link
Copy Markdown
Member

Summary

When a publisher refused a download during bulk full text download (e.g., a Cloudflare bot check answering HTTP 403), JabRef stopped with a modal error dialog per failing entry. Failed downloads are now reported as a notification with citation key, URL and HTTP status; the full response stays in the log.

Steps to test

  1. Select an entry, right click, "More file operations..." → "Attach file from URL".
  2. Enter a URL that answers 403 (e.g., a TechRxiv PDF link) and press OK.
  3. A notification appears, no dialog:

notification

Related issues and pull requests

Closes _____ (none; reported directly by a maintainer)

AI usage

Claude Code (claude-opus-5), AIL3 — root cause analysis, fix, test, manual GUI test.

AI CHECKLIST.md walkthrough

1. Code self-review

Nullability and control flow

  • [/] No == null / != null checks — JSpecify annotations (@NullMarked, @Nullable, @NonNull) used instead.
  • [/] No Objects.requireNonNull(...) — nullability expressed via JSpecify annotations.
  • [/] New classes annotated with @NullMarked (org.jspecify.annotations.NullMarked).
  • [/] New packages annotated with @NullMarked (org.jspecify.annotations.NullMarked).
  • Optional consumed with ifPresent / ifPresentOrElse / map / orElseThrow — never orElse(unusedValue) nor an isPresent() + get() block.
  • [/] StringUtil.isBlank(...) used instead of s == null || s.isBlank().

Exceptions

  • No catch (Exception e) — only specific exceptions are caught.
  • No throw new RuntimeException(...) / IllegalStateException(...) — these tear down the whole application.
  • Logged exceptions are passed as the last logger argument (LOGGER.info("...", e)), not concatenated into the message string.

Style and idioms

  • [/] New BibEntry objects built with withers (withField, not setField).
  • Modern Java used: List.of() / Map.of() / Set.of(), Path.of(), SequencedCollection / SequencedSet, text blocks.
  • [/] Regexes use a precompiled Pattern.compile(...) constant, not String.matches(...).
  • [/] Keep alphabetical ordering when adding variables or enum items.
  • Background work uses org.jabref.logic.util.BackgroundTask, not new Thread().
  • No commented-out code, no trivial comments restating the code, no AI-disclosure comments in source.
  • Markdown Javadoc (///) uses Markdown syntax, not JavaDoc inline tags: `code` instead of {@code}, [ClassName] instead of {@link}.

User-facing text

  • All user-facing text localized (Localization.lang in Java, % prefix in FXML).
  • Sentence case (not Title Case); no trailing !; labels do not end with :.
  • Variance expressed with placeholders ("...: %0"), not string concatenation.

Security

  • [/] User-controlled data (request params, entry fields, file contents) is HTML-escaped before being written into any text/html response — including exception/error messages, not just the success body (XSS).

Tests

  • [/] Behavior changes in org.jabref.model / org.jabref.logic have added or updated tests.
  • Tests assert object contents (assertEquals), use plain JUnit asserts (not AssertJ), have no @DisplayName, do not catch exceptions (let them propagate so JUnit reports setup/teardown failures directly), and use @TempDir instead of manual temp directories.
  • [/] Fetcher tests hit the live endpoints — the remote API is not mocked or stubbed (automated-review suggestions to mock it are rejected on purpose).

2. Verification commands

  • ./gradlew :jablib:check (or ./gradlew check for all modules).
  • ./gradlew checkstyleMain checkstyleTest checkstyleJmh.
  • ./gradlew modernizer.
  • ./gradlew --no-configuration-cache :rewriteDryRun reports no changes (run ./gradlew rewriteRun to fix).
  • [/] ./gradlew javadoc.
  • [/] npx markdownlint-cli2 "docs/**/*.md" "*.md" (only if Markdown changed).
  • [/] npm ci && npm run textlint reports no misspellings (only if Markdown changed).
  • [/] Only if formatting is still off after rewriteRun: docker run -v $(pwd):/github/workspace ghcr.io/leventebajczi/intellij-format:master "*.java" "" ".idea/codeStyles/Project.xml".

3. Documentation

  • CHANGELOG.md entry added if the change is visible to the user (end-user wording, no extra blank lines, sorted in next to existing entries about the same component/feature). Link the issue if one exists; link the PR only when no issue exists. Use TODO as the placeholder when neither is known yet — never a fake number. No entry for fixes to changes that were themselves introduced after the last release (feature only in ## [Unreleased]) — update the existing unreleased entry instead if needed.
  • Searched jabref/issues and jabref-koppor/issues for a related issue; linked only on a confident match, otherwise kept TODO (no closes/fixes for merely-similar issues).
  • [/] Requirement added to docs/requirements/<area>.md if the change is a new feature or significant bug fix (skip for refactors, minor fixes, and internal changes).
  • [/] Developer documentation under docs/ updated if behavior or architecture changed.

4. Pull request

  • PR body built from .github/PULL_REQUEST_TEMPLATE.md, every section filled.
  • All checklist items kept and marked [x], [ ], or [/].
  • "Steps to test" is a numbered list with a cropped screenshot of the result for every visible change; no video (only allowed when another program is involved, e.g. drag and drop or push to an external application).
  • All HTML comments removed from the PR body.
  • PR created with gh pr create --body-file <file> (not --body).
  • If CHANGELOG.md used a TODO placeholder (no issue confidently identified yet — an existing issue link always stays), the PR was opened as draft, the placeholder was replaced with the real PR-number link after PR creation, committed and pushed, and only then was the PR marked ready for review. If an issue is identified or created later, the link is switched to the issue.

Checklist

  • I own the copyright of the code submitted and I license it under the MIT license
  • If AI tools were used, I disclosed them in the "AI usage" section and reviewed, understood, and take full ownership of all AI-generated code
  • 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 one sentence (max 20 words) to CHANGELOG.md describing the change from the user's point of view (if the 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

🤖 Generated with Claude Code

A publisher blocking one download (e.g., a bot check answering 403) interrupted bulk full text downloads with a dialog per entry.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@koppor koppor added this to the 1 - soon-upstream milestone Sep 22, 2026
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@koppor
koppor marked this pull request as ready for review September 22, 2026 14:05
@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

PR Summary by Qodo

Report failed file downloads through notifications

🐞 Bug fix 🧪 Tests 📝 Documentation 🕐 10-20 Minutes

Grey Divider

AI Description

• Replace modal failures with notifications containing citation key, redacted URL, and HTTP status.
• Keep full publisher responses in logs while bulk downloads continue without per-entry
 interruptions.
• Add local HTTP failure coverage and document the user-visible fix.
Diagram

sequenceDiagram
    actor User
    participant Flow as Download flow
    participant Action as Download action
    participant Publisher
    participant Log as Application log
    participant Center as Notification center
    User->>Flow: Start downloads
    Flow->>Action: Download file
    Action->>Publisher: Request URL
    Publisher-->>Action: HTTP failure
    par Preserve details
        Action->>Log: Log full response
    and Inform user
        Action->>Center: Key, URL, status
    end
    Center-->>User: Non-modal failure
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Aggregate bulk failures into a completion summary
  • ➕ Avoids producing one notification for every failed entry
  • ➕ Presents all bulk-download failures in one reviewable result
  • ➖ Requires propagating batch context through shared download code
  • ➖ Delays feedback for standalone downloads and imports
  • ➖ Creates a substantially broader cross-cutting change

Recommendation: Use the PR's notification-based approach for this scoped fix because it removes modal interruption while preserving immediate feedback across standalone, import, and bulk workflows. Consider batch aggregation later only if real-world notification volume becomes disruptive.

Files changed (4) +40 / -9

Bug fix (1) +12 / -7
DownloadLinkedFileAction.javaReplace download failure dialogs with notifications +12/-7

Replace download failure dialogs with notifications

• Reports background download failures through the general notification group using the citation key, redacted URL, and concise HTTP status or exception reason. The complete exception and response body remain in the application log.

jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java

Tests (1) +27 / -1
LinkedFileViewModelTest.javaVerify HTTP failures produce only a notification +27/-1

Verify HTTP failures produce only a notification

• Extends the local HTTP test server to return configurable status codes. Adds a 403 test asserting the notification title and summary while verifying that no modal dialog interaction occurs.

jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java

Documentation (1) +1 / -0
CHANGELOG.mdDocument non-modal download failure reporting +1/-0

Document non-modal download failure reporting

• Adds an Unreleased changelog entry explaining that failed full-text downloads no longer open an error dialog for every entry.

CHANGELOG.md

Other (1) +0 / -1
JabRef_en.propertiesRemove obsolete modal error message +0/-1

Remove obsolete modal error message

• Removes the localized URL-and-details prompt that is no longer used after replacing the modal failure dialog.

jablib/src/main/resources/l10n/JabRef_en.properties

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Notification details remain untested ✓ Resolved 📘 Rule violation ≡ Correctness
Description
failedDownloadNotifiesInsteadOfOpeningDialog verifies notify(any(UiNotification.class)) without
asserting the notification title or description. The test still passes if the citation key, redacted
URL, or HTTP status is later omitted or rendered incorrectly.
Code

jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java[255]

+        verify(dialogService).notify(any(Notifications.UiNotification.class));
Evidence
Rules 17 and 19 require behavioral changes to have meaningful coverage and tests to compare object
contents directly. The implementation constructs specific notification content, but the new test
accepts any notification instance and therefore does not validate that content.

AGENTS.md: Update and Preserve High-Quality Tests for Behavioral Changes: AGENTS.md: Update and Preserve High-Quality Tests for Behavioral Changes: AGENTS.md: Update and Preserve High-Quality Tests for Behavioral Changes: AGENTS.md: Update and Preserve High-Quality Tests for Behavioral Changes
AGENTS.md: Assert Object Contents Directly with Plain JUnit: AGENTS.md: Assert Object Contents Directly with Plain JUnit: AGENTS.md: Assert Object Contents Directly with Plain JUnit: AGENTS.md: Assert Object Contents Directly with Plain JUnit
jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[236-238]
jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java[243-256]

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 failed-download test only verifies that some `UiNotification` is emitted, so it does not protect the newly added citation key, redacted URL, title, and HTTP status content.
## Fix Focus Areas
- jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java[243-256]
## Recommended Fix
Capture the `UiNotification` passed to `dialogService.notify`, then use plain JUnit `assertEquals` assertions on its title and description. Verify that the description contains the fixture citation key, redacted server URL, and expected `HTTP 403` reason while continuing to verify that no modal dialog interaction occurs.

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


2. Release note breaks the 20-word limit ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new CHANGELOG.md entry contains 22 words before its pull request link. This makes the added
release note inconsistent with the concise layout enforced for entries in the Unreleased section.
Code

CHANGELOG.md[22]

+- We fixed an issue where a failing download during full text document download opened a modal error dialog for each failing entry. [#774](https://github.com/JabRef/jabref-koppor/pull/774)
Evidence
Compliance rule 35 limits each new CHANGELOG entry to 20 words. The added entry has 22 words before
its reference link.

AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules
CHANGELOG.md[22-22]

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 new CHANGELOG entry contains 22 words, exceeding the maximum of 20 words required for release-note entries.
## Fix Focus Areas
- CHANGELOG.md[22-22]
## Recommended Fix
Shorten the user-facing sentence to no more than 20 words while retaining the `We fixed` prefix, the observable result, and the existing pull request link.

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



Remediation recommended

3. Bulk failures pile up indefinitely ✓ Resolved 🐞 Bug ☼ Reliability
Description
onFailure creates a UiNotification without calling withAutoClose, while that constructor only
removes the notification when it is clicked. Every failed background download adds another item to
an unbounded notification list, so bulk publisher failures remain for the session unless users
dismiss them individually.
Code

jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[R236-238]

+        dialogService.notify(new Notifications.UiNotification(
+                Localization.lang("Failed to download from URL"),
+                "%s\n%s\n%s".formatted(entry.getCitationKey().orElse(""), FetcherException.getRedactedUrl(urlDownload.getSource().toString()), reason).strip()));
Evidence
The two-argument notification constructor only installs click-to-remove behavior, whereas existing
transient notifications explicitly schedule automatic removal. JabRefDialogService appends each
new notification to its notification group, and this failure callback runs separately for every
failed download.

jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[158-163]
jabgui/src/main/java/org/jabref/gui/Notifications.java[61-70]
jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[462-478]

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

## Issue description
Failed download notifications do not expire, allowing bulk failures to accumulate indefinitely in the notification center.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[236-238]
- jabgui/src/main/java/org/jabref/gui/Notifications.java[61-70]
## Recommended Fix
Apply an appropriate automatic-close duration to failed download notifications, or introduce bounded aggregation that preserves failure details without retaining one notification per failed entry indefinitely.

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


4. Download errors appear under Preview ✓ Resolved 🐞 Bug ≡ Correctness
Description
onFailure submits a UiNotification, and JabRefDialogService routes that type into the
notification group titled Preview. Every failed attachment or bulk full-text download is therefore
presented under an unrelated category in the information center.
Code

jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[R236-238]

+        dialogService.notify(new Notifications.UiNotification(
+                Localization.lang("Failed to download from URL"),
+                "%s\n%s\n%s".formatted(entry.getCitationKey().orElse(""), FetcherException.getRedactedUrl(urlDownload.getSource().toString()), reason).strip()));
Evidence
The added code constructs a UiNotification; the dialog service routes every instance of that class
to uiNotifications, which is explicitly initialized with the localized title Preview. Those
groups are then installed directly in the application's visible information center.

jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[84-88]
jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[475-482]
jabgui/src/main/java/org/jabref/gui/JabRefGUI.java[370-374]

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

## Issue description
Failed download notifications are routed to the Preview notification group, which misclassifies download errors in the information center.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[236-238]
- jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[84-88]
- jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[475-482]
## Recommended Fix
Introduce or use a general/error notification type whose routing targets the Notifications group, and submit failed-download messages using that type rather than `UiNotification`.

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


Grey Divider

Context sources
Review mode: ⚖️ Balanced: This changes user-visible background download failure handling across GUI and bulk workflows, with exception parsing, notification behavior, and tests requiring a careful single-pass review.

Grey Divider

Tip of the day
💡 Did you know, you can route each action level your way: inline, summary, both, or drop

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit d432004 ⚖️ Balanced

Results up to commit 14a9445


🐞 Bugs (2) 📘 Rule violations (1) 📜 Skill insights (0)


Action required
1. Notification details remain untested 📘 Rule violation ≡ Correctness ⭐ New
Description
failedDownloadNotifiesInsteadOfOpeningDialog verifies notify(any(UiNotification.class)) without
asserting the notification title or description. The test still passes if the citation key, redacted
URL, or HTTP status is later omitted or rendered incorrectly.
Code

jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java[255]

+        verify(dialogService).notify(any(Notifications.UiNotification.class));
Evidence
Rules 17 and 19 require behavioral changes to have meaningful coverage and tests to compare object
contents directly. The implementation constructs specific notification content, but the new test
accepts any notification instance and therefore does not validate that content.

AGENTS.md: Update and Preserve High-Quality Tests for Behavioral Changes
AGENTS.md: Assert Object Contents Directly with Plain JUnit
jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[236-238]
jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java[243-256]

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 failed-download test only verifies that some `UiNotification` is emitted, so it does not protect the newly added citation key, redacted URL, title, and HTTP status content.

## Fix Focus Areas
- jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java[243-256]

## Recommended Fix
Capture the `UiNotification` passed to `dialogService.notify`, then use plain JUnit `assertEquals` assertions on its title and description. Verify that the description contains the fixture citation key, redacted server URL, and expected `HTTP 403` reason while continuing to verify that no modal dialog interaction occurs.

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


2. Release note breaks the 20-word limit ✓ Resolved 📘 Rule violation ⚙ Maintainability
Description
The new CHANGELOG.md entry contains 22 words before its pull request link. This makes the added
release note inconsistent with the concise layout enforced for entries in the Unreleased section.
Code

CHANGELOG.md[22]

+- We fixed an issue where a failing download during full text document download opened a modal error dialog for each failing entry. [#774](https://github.com/JabRef/jabref-koppor/pull/774)
Evidence
Compliance rule 35 limits each new CHANGELOG entry to 20 words. The added entry has 22 words before
its reference link.

AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules: AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules
CHANGELOG.md[22-22]

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 new CHANGELOG entry contains 22 words, exceeding the maximum of 20 words required for release-note entries.
## Fix Focus Areas
- CHANGELOG.md[22-22]
## Recommended Fix
Shorten the user-facing sentence to no more than 20 words while retaining the `We fixed` prefix, the observable result, and the existing pull request link.

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



Remediation recommended
3. Bulk failures pile up indefinitely 🐞 Bug ☼ Reliability ⭐ New
Description
onFailure creates a UiNotification without calling withAutoClose, while that constructor only
removes the notification when it is clicked. Every failed background download adds another item to
an unbounded notification list, so bulk publisher failures remain for the session unless users
dismiss them individually.
Code

jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[R236-238]

+        dialogService.notify(new Notifications.UiNotification(
+                Localization.lang("Failed to download from URL"),
+                "%s\n%s\n%s".formatted(entry.getCitationKey().orElse(""), FetcherException.getRedactedUrl(urlDownload.getSource().toString()), reason).strip()));
Evidence
The two-argument notification constructor only installs click-to-remove behavior, whereas existing
transient notifications explicitly schedule automatic removal. JabRefDialogService appends each
new notification to its notification group, and this failure callback runs separately for every
failed download.

jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[158-163]
jabgui/src/main/java/org/jabref/gui/Notifications.java[61-70]
jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[462-478]

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

## Issue description
Failed download notifications do not expire, allowing bulk failures to accumulate indefinitely in the notification center.

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[236-238]
- jabgui/src/main/java/org/jabref/gui/Notifications.java[61-70]

## Recommended Fix
Apply an appropriate automatic-close duration to failed download notifications, or introduce bounded aggregation that preserves failure details without retaining one notification per failed entry indefinitely.

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


4. Download errors appear under Preview 🐞 Bug ≡ Correctness ⭐ New
Description
onFailure submits a UiNotification, and JabRefDialogService routes that type into the
notification group titled Preview. Every failed attachment or bulk full-text download is therefore
presented under an unrelated category in the information center.
Code

jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[R236-238]

+        dialogService.notify(new Notifications.UiNotification(
+                Localization.lang("Failed to download from URL"),
+                "%s\n%s\n%s".formatted(entry.getCitationKey().orElse(""), FetcherException.getRedactedUrl(urlDownload.getSource().toString()), reason).strip()));
Evidence
The added code constructs a UiNotification; the dialog service routes every instance of that class
to uiNotifications, which is explicitly initialized with the localized title Preview. Those
groups are then installed directly in the application's visible information center.

jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[84-88]
jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[475-482]
jabgui/src/main/java/org/jabref/gui/JabRefGUI.java[370-374]

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

## Issue description
Failed download notifications are routed to the Preview notification group, which misclassifies download errors in the information center.

## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java[236-238]
- jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[84-88]
- jabgui/src/main/java/org/jabref/gui/JabRefDialogService.java[475-482]

## Recommended Fix
Introduce or use a general/error notification type whose routing targets the Notifications group, and submit failed-download messages using that type rather than `UiNotification`.

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


Context sources
Review mode: ⚖️ Balanced: This changes user-facing download failure handling across background and bulk workflows, with notification formatting and exception/HTTP-response logic requiring a careful review.
Results up to commit e2f2582


🐞 Bugs (0) 📘 Rule violations (1) 📜 Skill insights (0)


Action required
1. Release note breaks the 20-word limit 📘 Rule violation ⚙ Maintainability
Description
The new CHANGELOG.md entry contains 22 words before its pull request link. This makes the added
release note inconsistent with the concise layout enforced for entries in the Unreleased section.
Code

CHANGELOG.md[22]

+- We fixed an issue where a failing download during full text document download opened a modal error dialog for each failing entry. [#774](https://github.com/JabRef/jabref-koppor/pull/774)
Evidence
Compliance rule 35 limits each new CHANGELOG entry to 20 words. The added entry has 22 words before
its reference link.

AGENTS.md: Follow CHANGELOG Entry Wording and Layout Rules
CHANGELOG.md[22-22]

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 new CHANGELOG entry contains 22 words, exceeding the maximum of 20 words required for release-note entries.

## Fix Focus Areas
- CHANGELOG.md[22-22]

## Recommended Fix
Shorten the user-facing sentence to no more than 20 words while retaining the `We fixed` prefix, the observable result, and the existing pull request link.

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


Context sources
Review mode: ⚖️ Balanced: This changes runtime download-failure handling and user notifications across GUI and tests, with meaningful control-flow and error-reporting behavior that warrants a complete review.

Grey Divider

Qodo Logo

Comment thread CHANGELOG.md Outdated
@koppor
koppor marked this pull request as draft September 22, 2026 14:08
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@koppor
koppor marked this pull request as ready for review September 22, 2026 14:08
Comment thread jabgui/src/test/java/org/jabref/gui/fieldeditors/LinkedFileViewModelTest.java Outdated
Comment thread jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java Outdated
Comment thread jabgui/src/main/java/org/jabref/gui/linkedfile/DownloadLinkedFileAction.java Outdated
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 14a9445

…fication content

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@koppor
koppor marked this pull request as draft September 22, 2026 14:15
@koppor
koppor marked this pull request as ready for review September 22, 2026 14:16
@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit d432004

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant