Conversation
Review Summary by QodoImprove citation refresh button and fix DOI case sensitivity
WalkthroughsDescription• Improved citation refresh button with smart caching logic - Retries immediately on error state - Requests confirmation if data fetched recently - Refetches normally if data is old enough • Fixed case-insensitive DOI comparison in duplicate detection • Added public methods to expose cache state checking • Enhanced test coverage with new duplicate detection scenarios Diagramflowchart LR
A["Refresh Button Clicked"] --> B{"Check Cache State"}
B -->|Error Found| C["Retry Immediately"]
B -->|Recently Fetched| D["Ask User Confirmation"]
D -->|User Confirms| E["Refetch Data"]
D -->|User Declines| F["Cancel"]
B -->|Data Old| E
C --> E
E --> G["Update Citations"]
G["Update Citations"] --> H["Display Results"]
I["DOI Comparison"] --> J{"Case Insensitive?"}
J -->|Yes| K["Match Found"]
J -->|No| L["No Match"]
File Changes1. jabgui/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java
|
Code Review by Qodo
1. setField() used in DOI tests
|
| simpleArticle.setField(StandardField.DOI, "10.1016/j.is.2004.02.002"); | ||
| unrelatedArticle.setField(StandardField.DOI, "10.1016/J.IS.2004.02.002"); | ||
|
|
||
| assertTrue(duplicateChecker.isDuplicate(simpleArticle, unrelatedArticle, BibDatabaseMode.BIBTEX)); | ||
| } | ||
|
|
||
| @Test | ||
| void twoEntriesWithSameDoiButDifferentCasingAndDifferentTypesAreDuplicates() { | ||
| simpleArticle.setField(StandardField.DOI, "10.1016/j.is.2004.02.002"); | ||
| unrelatedArticle.setField(StandardField.DOI, "10.1016/J.IS.2004.02.002"); | ||
| unrelatedArticle.setType(StandardEntryType.Misc); | ||
|
|
There was a problem hiding this comment.
1. setfield() used in doi tests 📘 Rule violation ✓ Correctness
New tests mutate BibEntry using setField, contrary to the preferred withField style. This reduces consistency with JabRef’s functional/withers pattern.
Agent Prompt
## Issue description
New tests use `setField` to modify `BibEntry` instances, but JabRef prefers `withField`.
## Issue Context
The project encourages immutable/functional patterns and consistent test/code style.
## Fix Focus Areas
- jablib/src/test/java/org/jabref/logic/database/DuplicateCheckTest.java[405-418]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| boolean hasError = citationComponents.listView().getPlaceholder() instanceof Label label | ||
| && label.getText().startsWith(Localization.lang("Error")); | ||
|
|
||
| if (hasError) { | ||
| searchForRelations(citationComponents, otherCitationComponents, true); | ||
| return; | ||
| } |
There was a problem hiding this comment.
2. Localized error retry broken 🐞 Bug ✓ Correctness
CitationRelationsTab.handleRefresh detects an error state by checking whether the placeholder label
text starts with the localized token "Error", but the actual localized error sentences may not start
with that token (e.g., zh_CN), so errors are not recognized and refresh won’t reliably retry
immediately after a failure. Additionally, the DOI lookup failure path sets placeholders with
hardcoded "Error ", which will never match non-English Localization.lang("Error") prefixes.
Agent Prompt
## Issue description
`CitationRelationsTab.handleRefresh` decides whether to “retry immediately on error” by checking if the list placeholder is a `Label` whose text starts with `Localization.lang("Error")`. This is not reliable across locales (some translations of the full error sentence don’t start with the localized “Error” token) and also fails for the DOI lookup failure path, which sets placeholders to a hardcoded English prefix (`"Error " + ex.getMessage()`).
## Issue Context
The UI already sets error placeholders in `executeSearch(...).onFailure(...)` using fully localized sentences. The refresh logic should not depend on the *rendered* (localized) text to detect error state.
## Fix approach (one of these)
1) **Explicit state**: add a boolean/enum fetch state to `CitationComponents` or maintain a `Map<CitationComponents, FetchState>` in the tab; set it to ERROR on failures and OK on success. `handleRefresh` should consult that state.
2) **Stable marker on placeholder node**: when creating the error placeholder `Label`, set a stable marker such as `placeholder.setId("citation-relations-error")` or a style class. Then `handleRefresh` checks for that marker instead of string prefixes.
Also ensure the DOI lookup failure uses `Localization.lang(...)` and the same error marker.
## Fix Focus Areas
- jabgui/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java[787-811]
- jabgui/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java[847-858]
- jabgui/src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java[995-1005]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
6ec4b39 to
be4c490
Compare
- Ask user before refetching if data was fetched recently - Retry immediately if last fetch resulted in an error - Refetch normally if data is old enough (TTL expired) Fixes JabRef#12247
be4c490 to
df2834a
Compare
This comment has been minimized.
This comment has been minimized.
| - We enabled drag and drop of Windows shortcut (`.lnk`) files to open libraries. [#15036](https://github.com/JabRef/jabref/issues/15036) | ||
| - We refined the "Select files to import" page in "Search for unlinked local files" dialog to give the users the choice of linking the file to a related entry or import it to a new entry. [#13689](https://github.com/JabRef/jabref/issues/13689) | ||
| - The "Make/Sync bibliography" button in OO/LO panel now refreshes citations before generating bibliographies. [#14387](https://github.com/JabRef/jabref/issues/14387) | ||
| - Improved refresh button in Citation Relations tab: asks for confirmation if data was fetched recently, retries immediately on error, and refetches normally if data is old. [#12247](https://github.com/JabRef/jabref/issues/12247) |
There was a problem hiding this comment.
Too much information.
| - Improved refresh button in Citation Relations tab: asks for confirmation if data was fetched recently, retries immediately on error, and refetches normally if data is old. [#12247](https://github.com/JabRef/jabref/issues/12247) | |
| - Improved responsiveness and user interface of refresh button in Citation Relations tab. [#12247](https://github.com/JabRef/jabref/issues/12247) |
|
|
||
| private void handleRefresh(CitationComponents citationComponents, CitationComponents otherCitationComponents) { | ||
| BibEntry entry = citationComponents.entry(); | ||
| boolean isCites = citationComponents.searchType() == CitationFetcher.SearchType.CITES; |
There was a problem hiding this comment.
Wouldnt it be easier to just put this into the arguments of the method?

eg.
refreshCitingButton.setOnMouseClicked(_ -> handleRefresh(citingComponents, citedByComponents, refreshCitedByButton.setOnMouseClicked(_ -> handleRefresh(citedByComponents, citingComponents, CitationFetcher.SearchType.CITED_BY));There was a problem hiding this comment.
Can you expand? The current code reads: if mouse is clicked on the left, a new handler is installed on the right?
Maybe, you mean putting CitationFetcher.SearchType.CITED_BY as parameter and keep the parameter order? However, this is not the style of this component ^^
There was a problem hiding this comment.
Maybe i misread the code. Was a quick interpretation by looking at CitationFetcher.SearchTypes enum providing only two alternatives CITES and CITED_BY and the method being called by the mentioned event triggers.
✅ All tests passed ✅🏷️ Commit: a0d40d3 Learn more about TestLens at testlens.app. |
|
@calixtus This is ready for review when you have a moment! |
* Improve refresh button behavior in Citation Relations tab - Ask user before refetching if data was fetched recently - Retry immediately if last fetch resulted in an error - Refetch normally if data is old enough (TTL expired) Fixes JabRef#12247 * Update CHANGELOG for JabRef#12247 * Add missing localization keys and shorten CHANGELOG entry
Related issues and pull requests
Closes #12247
PR Description
Improved the refresh button behavior in the Citation Relations tab.
Before, clicking refresh always refetched from the remote unconditionally.
Now it checks the cache state first:
if there was an error it retries immediately,
if the data was fetched recently it asks the user for confirmation,
if the data is old enough it refetches normally.
Steps to test
Checklist
CHANGELOG.mdin a way that can be understood by the average user (if change is visible to the user)