Added error msg when isbn entry was not found #7036
Conversation
| // inspect the "no results" error message (if there is one) | ||
| Element potentialErrorMessageDiv = html.select("div#flash-notice.notice.add-bottom").first(); | ||
| if (potentialErrorMessageDiv.hasText() && potentialErrorMessageDiv.text().contains("No Results")) { | ||
| LOGGER.error("ISBN " + identifier + " not found at ottobib"); |
There was a problem hiding this comment.
| LOGGER.error("ISBN " + identifier + " not found at ottobib"); | |
| LOGGER.error("ISBN { } not found at ottobib", identifier); |
This is the recommended way
| @@ -59,6 +62,13 @@ public String getName() { | |||
| bibEntry = isbnViaOttoBibFetcher.performSearchById(identifier); | |||
| } | |||
|
|
|||
| // nothing found at ebook.de and ottobib | |||
| if (!bibEntry.isPresent()) { | |||
| FXDialog noEntryFoundDialog = new FXDialog(AlertType.ERROR); | |||
There was a problem hiding this comment.
This is not the right place. You cannot put GUI code into logic/model classes.
See https://devdocs.jabref.org/getting-into-the-code/high-level-documentation
The right place to check if the results are empty would be here: Than you have it generic as well for all kind of fetchers
There was a problem hiding this comment.
oops, my mistake. I'll fix it later today. I have another question about the ottobib fetcher though, should getting no results throw an exception? Ottobib seems like a special case since supplying an ISBN that leads to no entry doesn't give a 404 but instead leads to a special page with an error message.
There was a problem hiding this comment.
I thought about throwing a fetcher exception as well. But on the other hand we have optional for those cases.
So I would not throw an exception.
@tobiasdiez
There was a problem hiding this comment.
Thanks for your work so far.
Exceptions are only for exceptional program states. An unkown ISBN is a possible case in Ottobib. Thats probably why Ottobib does not show 404, but a page saying: unkown ISBN.
See: https://github.com/HugoMatilla/Effective-JAVA-Summary#57-use-exceptions-only-for-exceptional-conditions
| } else if (result.isEmpty()) { | ||
| String fetcher = selectedItemProperty().getValue().getName(); | ||
| String searchId = idText.getValue(); | ||
| dialogService.showErrorDialogAndWait(Localization.lang("Fetcher '%0' did not find an entry for id '%1'.", fetcher, searchId)); |
There was a problem hiding this comment.
Can I leave this error message or should it be unique?
| } else if (result.isEmpty()) { | ||
| String fetcher = selectedItemProperty().getValue().getName(); | ||
| String searchId = idText.getValue(); | ||
| dialogService.showErrorDialogAndWait(Localization.lang("Fetcher '%0' did not find an entry for id '%1'.", fetcher, searchId)); |
Co-authored-by: Christoph <[email protected]>
calixtus
changed the title
Fixes #7000
Added an error message that gets shown when the entered ISBN leads to no results on ebook.de and ottobib.
Screenshot of change:
