JabRef / jabref

Graphical Java application for managing BibTeX and biblatex (.bib) databases
https://devdocs.jabref.org
MIT License
3.54k stars 2.47k forks source link

UI: Paging support for fetchers #5507

Open JoHaHu opened 4 years ago

JoHaHu commented 4 years ago

Update 2023-04-24: UI is the focus; the migration is a separate task

So far only a predetermined number of search results are shown. I would like support for pagination. For this we have to do the following:

If there were no concerns, I would take a look.

matthiasgeiger commented 4 years ago

:+1:

A lot of users would appreciate that!

However, it should be configurable by fetcher as not all fetchers allow it technically, or we have other constraints like banning if to much requests are created in short time.

Siedlerchr commented 4 years ago

Refs https://github.com/koppor/jabref/issues/347

koppor commented 4 years ago

The epic behind is "JabRef for Literature Surveys". I described another step at https://github.com/koppor/jabref/issues/369.

Finally, it would be great if JabRef offered "repeatable literature survey". Meaning: JabRef stores the search properties (queries, sort order, what was excluded, ...) in a JSON file. A reviewer can open the JSON file in JabRef an click on "repeat". Then, the search is repeated from the machine of the reviewer. The reviewer kann then check for differences.

koppor commented 4 years ago

First implementation is crafted at https://github.com/JabRef/jabref/pull/6236. However, the UI is missing. We close the pull request until someone steps in and adds a UI.

koppor commented 3 years ago

Still some fetchers left to implement pagination. UI has to be developed.

prashant1712 commented 2 years ago

Hi, we are a group of students studying Master of Computer Science in University of Adelaide and we wanted to check if this issue is available for us to work on for our assignment. And if it is available, we would like to get some valuable inputs based on the previous contributions to this issue.

koppor commented 2 years ago

Dear @prashant1712. Thank you for your interest.

One can find the context of the feature at the "Web Search":

grafik See https://docs.jabref.org/collect/import-using-online-bibliographic-database for the full documentation.

I searched for "test" at "ArXiV":

grafik

Total items found are 20.

grafik

I think, there are many, many more hits.

As one can see in the code, the ArXiv fetcher already implements an interface for PagedSearchBasedFetcher:

grafik

The next step is (this ticket!) to provide two buttons "Previous" and "Next" to the search modal dialog, which navigates 20 entries back/forth:

grafik

ThiloteE commented 2 years ago

Welcome and thank you! Adding to koppor, also check out the guidelines for contributing to Jabref. This graphic about the contributing process might also be handy. In general, it is advised to open a (draft) pull request early on so that reviewers have time to comment and the general direction of the request becomes clear. This will allow you to receive valuable feedback!

If you have any questions, feel free to ask! Either here at GitHub, or you also can join our gitter chat.

JunyuLuo123 commented 11 months ago

Hi @JoHaHu could I please have a go on investing this issue? (u7110253, W-10)

pal-anish commented 7 months ago

Hi @koppor As you have mentioned, we need to do two things

  1. Adding new buttons
  2. Adding the pagination functionality to it

I have successfully reproduced the bug/issue and figured it out with the help of the above thread comments. I need some brainstorming about this issue on how to solve it. I am requesting some suggestions for it.

pal-anish commented 7 months ago

Hi @calixtus @koppor @ThiloteE I have successfully reproduced the bug/issue and figured it out with the help of the above thread comments. I need some brainstorming about this issue. I got some idea to fix it but not sure about it:

  1. Found that in ImportEntriesDialog.fxml buttons are getting created
  2. For this perticular example in ArXivFetcher.java performSearchPaged() is responsible for this search.
  3. Which calls the PagedSearchBasedFetcher.java class for this paged searching.
koppor commented 7 months ago

@pal-anish Please try it out. Maybe drawing some UML diagrams helps to think about it. The state of the fetching should be contained in org.jabref.gui.importer.ImportEntriesViewModel (located next to ImportEntriesDialog.*).

Things to respect:

By Ctrl and click on 'org.jabref.gui.importer.ImportEntriesDialog#ImportEntriesDialog`, I found out that the dialog is called four times:

image

Only the org.jabref.gui.importer.fetcher.WebSearchPaneViewModel#search is the one calling from a search.

Two ways to proceed:

a) try to think about the handling of the background task for page-based fetching b) copy the existing dialog to Web* (meaning: WebImortEntriesDialog, ... existing) and letting WebSearchPaneViewModel call that new dialog.

I would propose to work on b). Then, you can move the fetching logic into WebImortEntriesDialogViewModel and add the paging logic there. Remember to use test-driven development. You can add tests for WebImortEntriesDialogViewModel to do some fetching. You can pass a mocked fetcher to check the behavior of your 'WebImortEntriesDialogViewModel`.

Ajitaganguly3 commented 6 months ago

Hi @koppor , @ThiloteE , can you please tell me where the WebImportEntriesDialog and WebImortEntriesDialogViewModel is located? I'm unable to find these two

koppor commented 6 months ago

Hi @koppor , @ThiloteE , can you please tell me where the WebImportEntriesDialog and WebImortEntriesDialogViewModel is located? I'm unable to find these two

I think, I was unclear in the term "copy" at

b) copy the existing dialog to Web* (meaning: WebImortEntriesDialog, ... existing) and letting WebSearchPaneViewModel call that new dialog.

Do you know this term? "Copy". It is to duplicate at class. Create a new class and copy the contents to it. A code clone.

I am not sure how to explain. Click on the source class. Press Ctrl C. Then press Ctrl V. Enter the new name.

GuChad369 commented 6 months ago

Hi @koppor, I'm currently working on this issue, and I've identified a problem with the hard-coded line return new ArrayList<>(performSearchPaged(luceneQuery, 0).getContent()); in PagedSearchBasedFetcher class. It only retrieves the first page of results, and I'm unsure how to implement fetching the next page without causing unintended issues. Could you provide some guidance on how to proceed? Thank you.

calixtus commented 6 months ago

You could define and write some tests cases to ensure the expected behaviour in other methods stays the same.

koppor commented 6 months ago

Hi @koppor, I'm currently working on this issue, and I've identified a problem with the hard-coded line return new ArrayList<>(performSearchPaged(luceneQuery, 0).getContent()); in PagedSearchBasedFetcher class. It only retrieves the first page of results, and I'm unsure how to implement fetching the next page without causing unintended issues. Could you provide some guidance on how to proceed? Thank you.

Well, this is the first call. Did you see the other method @GuChad369

org.jabref.logic.importer.PagedSearchBasedFetcher#performSearchPaged(java.lang.String, int)

int is the page number. starting form 0

See org.jabref.logic.importer.fetcher.PagedSearchFetcherTest#pageSearchReturnsUniqueResultsPerPage for a usage of that method.