Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add search to return article and books for biodiversity (#8539) #8680

Merged
merged 21 commits into from
May 9, 2022
Merged

Add search to return article and books for biodiversity (#8539) #8680

merged 21 commits into from
May 9, 2022

Conversation

Jonathan-Oliveira
Copy link
Contributor

@Jonathan-Oliveira Jonathan-Oliveira commented Apr 15, 2022 •
edited
Loading

The present pull request partially solves the #8539, where any search for an author works.

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

@Siedlerchr Siedlerchr changed the title Add search by author to return article and books (#8539) Add search by author to return article and books for biodiversity (#8539) Apr 15, 2022
@Siedlerchr
Copy link
Member

Thanks for your contribution, on a quick look, the code already looks good. Will take a detailed look tomorrow.

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 18, 2022
@koppor
Copy link
Member

koppor commented Apr 18, 2022

Seems that we need an API key. Can you please add a short note to fetchers.md?

koppor
koppor requested changes Apr 18, 2022
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Thank you for the PR. Basing on the IEEE fetcher was risky, since that fetcher is a complicated one. I have some code remarks while reading it.

src/test/java/org/jabref/logic/importer/fetcher/BiodiversityLibraryTest.java Show resolved Hide resolved
src/test/java/org/jabref/logic/importer/fetcher/BiodiversityLibraryTest.java Outdated Show resolved Hide resolved
...main/java/org/jabref/logic/importer/fetcher/transformers/BiodiversityLibraryTransformer.java Outdated Show resolved Hide resolved
...main/java/org/jabref/logic/importer/fetcher/transformers/BiodiversityLibraryTransformer.java Outdated Show resolved Hide resolved
...main/java/org/jabref/logic/importer/fetcher/transformers/BiodiversityLibraryTransformer.java Outdated Show resolved Hide resolved
...main/java/org/jabref/logic/importer/fetcher/transformers/BiodiversityLibraryTransformer.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/importer/fetcher/BiodiversityLibrary.java Show resolved Hide resolved
...main/java/org/jabref/logic/importer/fetcher/transformers/BiodiversityLibraryTransformer.java Outdated Show resolved Hide resolved
@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 18, 2022
@koppor
Copy link
Member

koppor commented Apr 18, 2022

zbMath and jstor look simpler to be usable as basis.

@Jonathan-Oliveira
Copy link
Contributor Author

Thanks for reviewing, I'll make the changes

koppor
koppor reviewed Apr 20, 2022
View reviewed changes
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Some more code comments.

src/main/java/org/jabref/logic/importer/fetcher/BiodiversityLibrary.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/importer/fetcher/BiodiversityLibrary.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/importer/fetcher/BiodiversityLibrary.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/importer/fetcher/BiodiversityLibrary.java Outdated Show resolved Hide resolved
src/main/java/org/jabref/logic/importer/fetcher/BiodiversityLibrary.java Outdated Show resolved Hide resolved
@koppor
Copy link
Member

koppor commented Apr 25, 2022

The API key should also be customizable as implemented at #7720.

@Siedlerchr
Copy link
Member

@Jonathan-Oliveira What's the status here? It would be nice if you could finish the PR

@Jonathan-Oliveira Jonathan-Oliveira force-pushed the add-new-websearch-options branch 3 times, most recently from 51d7f75 to 4eee171 Compare May 7, 2022 20:49
@Jonathan-Oliveira
Copy link
Contributor Author

Jonathan-Oliveira commented May 7, 2022 •
edited
Loading

@Siedlerchr I was able to fix most of the comments, it's just missing a better documentation in fetchres.md.

Siedlerchr and others added 9 commits May 9, 2022 21:42
@Siedlerchr
fix publisher place
1fa6d57
@calixtus
parameterized test, normalized var names
e78d234
@calixtus
Merge remote-tracking branch 'Jonathan-Oliveira/add-new-websearch-opt…
6956560 
…ions' into add-new-websearch-options
@Siedlerchr
remove checkstyle suppress
ad99880
@Siedlerchr
Merge branch 'add-new-websearch-options' of github.com:Jonathan-Olive…
9f9db66 
…ira/jabref into add-new-websearch-options

* 'add-new-websearch-options' of github.com:Jonathan-Oliveira/jabref:
  parameterized test, normalized var names
@calixtus
fixed actually using api key
5f23945
@calixtus @Siedlerchr @koppor
Fixed search query
3a0fccf 
Co-authored-by: Christoph <siedlerkiller@gmail.com>
Co-authored-by: Oliver Kopp <kopp.dev@gmail.com>
@Siedlerchr
Fix fields and test bib entry
81730c3
@Siedlerchr
Add changelog
c234375 
fix fetchers.md
@koppor
Copy link
Member

koppor commented May 9, 2022

We took the liberty to work on this PR in our developers call. We switched from plain-author searching to "search for everything". We used PublicationSearch from https://www.biodiversitylibrary.org/docs/api3.html.

We also converted (some) test cases to ParamterizedTests.

@Siedlerchr
Copy link
Member

Siedlerchr commented May 9, 2022 •
edited
Loading

@Jonathan-Oliveira Thanks for your work on this! We also implemented the option to support the new Customize Api Key interface, so the Key can be put into the preferences.

@koppor koppor changed the title Add search by author to return article and books for biodiversity (#8539) Add search to return article and books for biodiversity (#8539) May 9, 2022
@koppor koppor merged commit 68278c5 into JabRef:main May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siedlerchr Siedlerchr Siedlerchr left review comments

@koppor koppor koppor requested changes

Assignees
No one assigned
Labels
fetcher
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Jonathan-Oliveira @Siedlerchr @koppor @ThiloteE @calixtus