Quick import should work if DOI is included

[koppor]

1. Open "Import new entry from Id"
   
2. Paste https://www.scitepress.org/Link.aspx?doi=10.5220/0010404301780189
3. Click on the arrow on the right to trigger the fetch.

Expected result: Entry found

Actual result: No entry found

One can find the entry when searching for 10.5220/0010404301780189

There needs to be implemented a heuristic at the CompositeIdFetcher. Before passing the DOI to the DOI parser, a) existence of ?doi= needs to be checked, b) if yes, that part removed. Then passed to DOI.parse

/src/main/java/org/jabref/logic/importer/CompositeIdFetcher.java#L23

        Optional<DOI> doi = DOI.parse(identifier);

Test-enabling: Heuristics as own method. This method needs to be tested. Method can be "package private".

With that being implemented, #7575 can also be fixed (howto at #7575 (comment)).

[sean-leichtle]

I would like to work on this, though I'm not certain what form the heuristic should take. Wouldn't it be easier to insert
cleanedDOI = cleanedDOI.replaceAll("\\?doi=", ""); in the try-block of the parse-method of org/jabref/model/entry/identifier/DOI.java or have I completely misunderstood the issue?

[github-actions]

As a general advice for newcomers: check out Contributing for a start. Also, guidelines for setting up a local workspace is worth having a look at.

Feel free to ask here at GitHub, if you have any issue related questions. If you have questions about how to setup your workspace use JabRef's Gitter chat. Try to open a (draft) pull-request early on, so that people can see you are working on the issue and so that they can see the direction the pull request is heading towards. This way, you will likely receive valuable feedback.

[koppor]

TL;DR: The code in CompositeIdFetcher does not use DOI.findInText, which it should.

@sean-leichtle At first, this might look as the solution. When we introduced the code, we had much discussions. Please check the code also of the other identifiers:

And so on.

OK, there is lines 89 (EXACT_DOI_PATT) and lines 90 (FIND_DOI_PATT) are not that strict. Nevertheless, being strict is intentional. Thus, the DOI parser is very scrict. There is no relaxed version. You will introduce a first version - embedded "somehwere".

However, there is public static Optional findInText(String text) {, which should be used. Thus, to change the functionality, instead of DOI.parse, DOI.findInText should be used. Can you try? And add test cases? -- Assumption: The DOI pattern matcher does not match other identifers (such as eprint, mathscinet, issn, ...)

Side note: Exceptions from the rule:

• ARK does not do that
• MathSciNetId was implemetned differently. this should not have happend. Maybe, it is hard to come up with a Grammar for MAthSciNet

[sean-leichtle]

Can you try? And add test cases?

Sure, though I may have some questions, especially about testing.

[koppor]

General hints on UI testing are that a) the ViewModel can be tested, b) if UI tests are really needed (I don't think in these tests), then hints can be read in the changes at #9794.

In the concrete case, a test addition to org.jabref.logic.importer.fetcher.CompositeIdFetcherTest seems to be enough.

Side note: I also updated the other issue for code and test hints: #7575 (comment)

[sean-leichtle]

@koppor
Just a point of clarification - your original description of the problem indicates that input https://www.scitepress.org/Link.aspx?doi=10.5220/0010404301780189 results in "no entry found" (= a toaster report). With the same input, I receive "Input error. Could not find a suitable import format." (= pop-up titled "Exception Details"). Did you actually mean you get an input error as I do or do you really get a "no entry found"?

[koppor]

@sean-leichtle Thank you for working to refine the issue description. I used today's version from the main branch and recorded following video: https://www.loom.com/share/61c33deca44b4ec3951f69e7ddc1f846

I see at minute 00:07 "No entry found".

Do you use some different button?

[koppor]

My event log also does not show any exception:

Sid enote: The log could be refined in adding some details on the search.

[sean-leichtle]

@koppor
Unfortunately not. I get the error I described just as soon as I paste https://www.scitepress.org/Link.aspx?doi=10.5220/0010404301780189into the field, that is without pressing the arrow button.

[sean-leichtle]

@koppor
I'm on macOS Monterey 12.6.x if that makes a difference.

[koppor]

I am on Windows. Interesting cc @Siedlerchr.

However, IntelliJ should also work on macOS. There, one can set breakpoints and step through the code and see where code changes are needed. I think, we already discussed, that the change should go into CompositeIdFetcher?

[sean-leichtle]

A bit more context: inserting a doi-number, such as 10.5220/0010404301780189, results in a successful import, though, again, without pressing the arrow button.

[koppor]

A bit more context: inserting a doi-number, such as 10.5220/0010404301780189, results in a successful import, though, again, without pressing the arrow button.

How can you trigger Quick Import without pressing the button for Quick Import. That button is the arrow button - do you refer to some other button?

Maybe it would be good if you share a video. Loom was recommended me by Microsoft.

As said, working on a fix can happen in parallel.

[sean-leichtle]

Sorry for the delay. Here is a video of the behavior I described:

bildschirmaufnahme.mp4

[sean-leichtle]

Ok, if I enter https://www.scitepress.org/Link.aspx?doi=10.5220/0010404301780189 manually, ie. not copy-paste, then I get a "no entry found" toaster report.

[koppor]

Can you paste the exception here?

[sean-leichtle]

org.jabref.logic.importer.ImportException: Could not find a suitable import format.
at [email protected]/org.jabref.logic.importer.ImportFormatReader.importUnknownFormat(ImportFormatReader.java:247)
at [email protected]/org.jabref.logic.importer.ImportFormatReader.importUnknownFormat(ImportFormatReader.java:267)
at [email protected]/org.jabref.gui.externalfiles.ImportHandler.tryImportFormats(ImportHandler.java:306)
at [email protected]/org.jabref.gui.externalfiles.ImportHandler.handleStringData(ImportHandler.java:301)
at [email protected]/org.jabref.gui.maintable.MainTable.handleNonBibTeXStringData(MainTable.java:344)
at java.base/java.util.Optional.orElseGet(Optional.java:364)
at [email protected]/org.jabref.gui.maintable.MainTable.paste(MainTable.java:330)
at [email protected]/org.jabref.gui.LibraryTab.paste(LibraryTab.java:783)
at [email protected]/org.jabref.gui.edit.EditAction.lambda$execute$0(EditAction.java:76)
at java.base/java.util.Optional.ifPresent(Optional.java:178)
at [email protected]/org.jabref.gui.edit.EditAction.execute(EditAction.java:49)
at [email protected]/org.jabref.gui.actions.JabRefAction.lambda$new$3(JabRefAction.java:40)
at [email protected]/org.controlsfx.control.action.Action.handle(Action.java:423)
at [email protected]/org.controlsfx.control.action.Action.handle(Action.java:64)
at javafx.base@20/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
at javafx.base@20/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
at javafx.base@20/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
at javafx.base@20/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
at javafx.base@20/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
at javafx.base@20/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
at javafx.base@20/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:49)
at javafx.base@20/javafx.event.Event.fireEvent(Event.java:198)
at javafx.controls@20/javafx.scene.control.MenuItem.fire(MenuItem.java:459)
at javafx.controls@20/com.sun.javafx.scene.control.GlobalMenuAdapter.lambda$bindMenuItemProperties$2(GlobalMenuAdapter.java:150)
at javafx.base@20/com.sun.javafx.event.CompositeEventHandler.dispatchBubblingEvent(CompositeEventHandler.java:86)
at javafx.base@20/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:232)
at javafx.base@20/com.sun.javafx.event.EventHandlerManager.dispatchBubblingEvent(EventHandlerManager.java:189)
at javafx.base@20/com.sun.javafx.event.BasicEventDispatcher.dispatchEvent(BasicEventDispatcher.java:58)
at javafx.base@20/com.sun.javafx.event.EventDispatchChainImpl.dispatchEvent(EventDispatchChainImpl.java:114)
at javafx.base@20/com.sun.javafx.event.EventUtil.fireEventImpl(EventUtil.java:74)
at javafx.base@20/com.sun.javafx.event.EventUtil.fireEvent(EventUtil.java:54)
at javafx.base@20/javafx.event.Event.fireEvent(Event.java:198)
at javafx.controls@20/javafx.scene.control.MenuItem.fire(MenuItem.java:459)
at javafx.graphics@20/com.sun.javafx.tk.quantum.GlassSystemMenu$1.action(GlassSystemMenu.java:234)

[koppor]

You pasted into the main table? Nice. You can work on that this works for those kinds of strings too

[sean-leichtle]

I'm sorry, I don't mean to be slow on the uptake, but could you clarify?

[Siedlerchr]

@sean-leichtle @koppor This is the buggy behavior of javafx under mac that copy paste is triggered twice when you paste in the text field and the maintable also still gets focus
refs #9367
We probaly need to add some more workarounds

[sean-leichtle]

@koppor I've lodged a pull request, though I declared the method public (and not, as you requested, package private) for testing purposes. Is there a way to declare the method as package private and still test? I also want to add that my testing experience is preliminary, so I'd appreciate feedback on this point.

[Siedlerchr]

package private is just a method without any modifier. The test class has to be in the same package then you can use it