Skip to content

Show DOI lookup link in citation relations tab #13285

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We improved JabRef's internal document viewer. It now allows text section, searching and highlighting of search terms and page rotation [#13193](https://github.com/JabRef/jabref/pull/13193).
- When importing a PDF, there is no empty entry column shown in the multi merge dialog. [#13132](https://github.com/JabRef/jabref/issues/13132)
- We added a progress dialog to the "Check consistency" action and progress output to the corresponding cli command. [#12487](https://github.com/JabRef/jabref/issues/12487)
- We added Enhanced "Citation Relations" feature: "Look up a DOI and try again." is now a clickable hyperlink that triggers a DOI lookup. The link shows result states ("Looking up DOI...", "No DOI found", "List of citations") as appropriate.

### Fixed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import javafx.scene.control.Button;
import javafx.scene.control.ButtonType;
import javafx.scene.control.DialogPane;
import javafx.scene.control.Hyperlink;
import javafx.scene.control.Label;
import javafx.scene.control.ProgressIndicator;
import javafx.scene.control.ScrollPane;
Expand All @@ -30,6 +31,8 @@
import javafx.scene.layout.HBox;
import javafx.scene.layout.Priority;
import javafx.scene.layout.VBox;
import javafx.scene.text.Text;
import javafx.scene.text.TextFlow;

import org.jabref.gui.DialogService;
import org.jabref.gui.LibraryTab;
Expand All @@ -52,6 +55,9 @@
import org.jabref.logic.citation.SearchCitationsRelationsService;
import org.jabref.logic.database.DuplicateCheck;
import org.jabref.logic.exporter.BibWriter;
import org.jabref.logic.importer.FetcherClientException;
import org.jabref.logic.importer.FetcherServerException;
import org.jabref.logic.importer.fetcher.CrossRef;
import org.jabref.logic.importer.fetcher.citation.CitationFetcher;
import org.jabref.logic.l10n.Localization;
import org.jabref.logic.os.OS;
Expand Down Expand Up @@ -201,46 +207,50 @@ private SplitPane getPaneAndStartSearch(BibEntry entry) {

refreshCitingButton.setOnMouseClicked(_ -> {
searchForRelations(
entry,
citingListView,
entry,
citingListView,
abortCitingButton,
refreshCitingButton,
CitationFetcher.SearchType.CITES,
importCitingButton,
citingProgress);
refreshCitingButton,
CitationFetcher.SearchType.CITES,
importCitingButton,
citingProgress,
true);
});

refreshCitedByButton.setOnMouseClicked(_ -> searchForRelations(
entry,
citedByListView,
entry,
citedByListView,
abortCitedButton,
refreshCitedByButton,
CitationFetcher.SearchType.CITED_BY,
importCitedByButton,
citedByProgress));
refreshCitedByButton,
CitationFetcher.SearchType.CITED_BY,
importCitedByButton,
citedByProgress,
true));

// Create SplitPane to hold all nodes above
SplitPane container = new SplitPane(citingVBox, citedByVBox);
styleFetchedListView(citedByListView);
styleFetchedListView(citingListView);

searchForRelations(
entry,
citingListView,
abortCitingButton,
entry,
citingListView,
abortCitingButton,
refreshCitingButton,
CitationFetcher.SearchType.CITES,
importCitingButton,
citingProgress);
CitationFetcher.SearchType.CITES,
importCitingButton,
citingProgress,
false);

searchForRelations(
entry,
citedByListView,
abortCitedButton,
entry,
citedByListView,
abortCitedButton,
refreshCitedByButton,
CitationFetcher.SearchType.CITED_BY,
importCitedByButton,
citedByProgress);
CitationFetcher.SearchType.CITED_BY,
importCitedByButton,
citedByProgress,
false);

return container;
}
Expand Down Expand Up @@ -434,13 +444,29 @@ protected void bindToEntry(BibEntry entry) {
*/
private void searchForRelations(BibEntry entry, CheckListView<CitationRelationItem> listView, Button abortButton,
Button refreshButton, CitationFetcher.SearchType searchType, Button importButton,
ProgressIndicator progress) {
ProgressIndicator progress, boolean shouldRefresh) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid magic booleans. if someone calls searchForRelations(...,...,true), nobody knows what true means. this may be done for simple methods, but for a recursive method self-documenting code is most important.

if (entry.getDOI().isEmpty()) {
hideNodes(abortButton, progress);
showNodes(refreshButton);
listView.getItems().clear();
listView.setPlaceholder(
new Label(Localization.lang("The selected entry doesn't have a DOI linked to it. Lookup a DOI and try again.")));
Text doiLookUpText = new Text(Localization.lang("The selected entry doesn't have a DOI linked to it."));
Hyperlink doiLookUpHyperLink = new Hyperlink(Localization.lang("Lookup a DOI and try again."));
TextFlow doiLookUpTextFlow = new TextFlow(doiLookUpText, doiLookUpHyperLink);
Label placeHolder = new Label("", doiLookUpTextFlow);
doiLookUpHyperLink.setOnAction(e -> {
CrossRef doiFetcher = new CrossRef();
BackgroundTask.wrap(() -> doiFetcher.findIdentifier(entry))
.onRunning(() -> listView.setPlaceholder(new Label("Looking up DOI...")))
.onSuccess(doiIdentifier -> {
if (doiIdentifier.isPresent()) {
entry.setField(StandardField.DOI, doiIdentifier.get().asString());
searchForRelations(entry, listView, abortButton, refreshButton, searchType, importButton, progress, shouldRefresh);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are introducing a recursive cascade here with endless creation of UI elements.
This needs to be done differently.

Extract the background task. put the result in a property. update the view if the property is modified.

} else {
dialogService.notify("No DOI found");
}
}).onFailure((exception) -> handleIdentifierFetchingError(exception, doiFetcher)).executeWith(taskExecutor);
});
listView.setPlaceholder(placeHolder);
return;
}

Expand Down Expand Up @@ -481,6 +507,19 @@ private void searchForRelations(BibEntry entry, CheckListView<CitationRelationIt
.executeWith(taskExecutor);
}

private void handleIdentifierFetchingError(Exception exception, CrossRef fetcher) {
LOGGER.error("Error while fetching identifier", exception);
if (exception instanceof FetcherClientException) {
dialogService.showInformationDialogAndWait(Localization.lang("Look up %0", fetcher.getName()), Localization.lang("No data was found for the identifier"));
} else if (exception instanceof FetcherServerException) {
dialogService.showInformationDialogAndWait(Localization.lang("Look up %0", fetcher.getName()), Localization.lang("Server not available"));
} else if (exception.getCause() != null) {
dialogService.showWarningDialogAndWait(Localization.lang("Look up %0", fetcher.getName()), Localization.lang("Error occurred %0", exception.getCause().getMessage()));
} else {
dialogService.showWarningDialogAndWait(Localization.lang("Look up %0", fetcher.getName()), Localization.lang("Error occurred %0", exception.getCause().getMessage()));
}
}

/**
* TODO: Make the method return a callable and let the calling method create the background task.
*/
Expand Down
3 changes: 2 additions & 1 deletion jablib/src/main/resources/l10n/JabRef_en.properties
Original file line number Diff line number Diff line change
Expand Up @@ -2776,7 +2776,6 @@ Miscellaneous=Miscellaneous
File-related=File-related

Add\ selected\ entry(s)\ to\ library=Add selected entry(s) to library
The\ selected\ entry\ doesn't\ have\ a\ DOI\ linked\ to\ it.\ Lookup\ a\ DOI\ and\ try\ again.=The selected entry doesn't have a DOI linked to it. Lookup a DOI and try again.
Cited\ By=Cited By
Cites=Cites
No\ articles\ found=No articles found
Expand Down Expand Up @@ -2939,6 +2938,8 @@ You\ must\ select\ an\ identifier\ type.=You must select an identifier type.
You\ must\ specify\ a\ Bib(La)TeX\ source.=You must specify a Bib(La)TeX source.
You\ must\ specify\ an\ identifier.=You must specify an identifier.
You\ must\ specify\ one\ (or\ more)\ citations.=You must specify one (or more) citations.
The\ selected\ entry\ doesn't\ have\ a\ DOI\ linked\ to\ it.=The selected entry doesn't have a DOI linked to it.
Lookup\ a\ DOI\ and\ try\ again.=Lookup a DOI and try again.

# CommandLine
Available\ export\ formats\:=Available export formats:
Expand Down
Loading