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.

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

Add 'RFC' entry generator ID-type option #3988

Merged
merged 4 commits into from
Apr 29, 2018
Merged

Add 'RFC' entry generator ID-type option #3988

merged 4 commits into from
Apr 29, 2018

Conversation

jacksonrya
Copy link
Contributor

@jacksonrya jacksonrya commented Apr 27, 2018

fixes #3971

UI Image of entry type in dropdown

RFC, an additional ID-based entry generator ID type, has been added in order to fetch more complete BibTeX data compared to DOI fetches of RFC type.

Following the DiVA ID type classes as template, an RFC fetcher class and fetcher test has been added.
I added RFC to the ID-based entry generator list within WebFetchers so it is an option in the dialog's dropdown.

Currently, the RFC fetcher class accepts search requests with or without the "rfc" prefix--is this an okay feature?

Additionally, help in finding the best RFC BibTeX API. I am using the API referenced in #3971.

P.S. This is my first ever contribution to open source, so thanks for your patience... 😄


  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

Added RFC to BibTeX option to the drop-down menu of the id-based entry generator section
of the new entry dialog. The RFC fetcher accepts entrys that include or exclude the
rfc prefix. BibTex data is retreived from 'https://datatracker.ietf.org'
(e.g. https://datatracker.ietf.org/doc/rfc1945/bibtex/'). A test for the fetcher is included;
.
@jacksonrya jacksonrya changed the title [WIP] New ID-based entry generator ID type, RFC #3971 [WIP] New ID-based entry generator ID type, RFC Apr 27, 2018
@jacksonrya jacksonrya changed the title [WIP] New ID-based entry generator ID type, RFC [WIP] RFC, New ID-based entry generator ID type Apr 27, 2018
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Overall looks good!
Please have a look at the Travis output which seems to fail

@Override
public URL getURLForID(String identifier) throws URISyntaxException, MalformedURLException, FetcherException {
// Add "rfc" prefix if user's search entry was only digits
identifier = (!identifier.startsWith("rfc")) ? "rfc" + identifier : identifier;
Copy link
Member

Choose a reason for hiding this comment

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

Even better if you check for case insensitive with equals ignore case

Copy link
Member

@stefan-kolb stefan-kolb left a comment

Choose a reason for hiding this comment

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

I added a few comments, other than that the code looks fine 😄 👍

entry.setField("number", "1945");
entry.setField("howpublished", "{RfcFetcher 1945}");
entry.setField("publisher", "{RfcFetcher Editor}");
entry.setField("doi", "{10.17487/RFC1945}");
Copy link
Member

Choose a reason for hiding this comment

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

Can you extract that BibEntry in the @beforeeach block and add a few more tests, e.g.:

  • no id
  • invalid
  • id not found but valid
  • id with rfc
  • id without rfc
  • ...

Test for when 'RfcFetcher to Bibtex' help page is added

@Test
public void testGetHelpPage() {
Copy link
Member

Choose a reason for hiding this comment

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

If possible, we should add this help file now.

Copy link
Member

Choose a reason for hiding this comment

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

This is also the problem that the tests/compiling fails:

/home/travis/build/JabRef/jabref/src/main/java/org/jabref/logic/importer/fetcher/RfcFetcher.java:36: error: cannot find symbol
        return HelpFile.FETCHER_RFC;
                       ^
  symbol:   variable FETCHER_RFC
  location: class HelpFile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just submitted pull request for updated help files. I'll include change to the HelpFile enum and add helppage test after successful merge into the Help Repo. Thanks! :)

@Siedlerchr
Copy link
Member

Siedlerchr commented Apr 27, 2018

I've merged your help file change and restarted the travis build. so the help pages related error should be gone now!
Thanks again for your contribution 🥇

Add RFC fetcher that obtains more accurate BibTeX data
than the DOI fetcher, includes test cases.
@jacksonrya jacksonrya changed the title [WIP] RFC, New ID-based entry generator ID type Add 'RFC' entry generator ID-type option Apr 28, 2018
Add RFC fetcher that obtains more accurate BibTeX data
than the DOI fetcher, includes test cases.
@jacksonrya
Copy link
Contributor Author

All set, thanks for all of your help! I've learned a lot about the process, and hopefully won't be as needy next time :)

@stefan-kolb
Copy link
Member

Thank you for your contribution 😄 ! Hope to see more from you in the future!

}

@Test
public void performSearchByIdThrowsExceptionWithInvalidIdentifier() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

If this throws an Exception we should asert for the Exception!

Copy link
Member

Choose a reason for hiding this comment

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

You cann use assertThrows() for this

@stefan-kolb stefan-kolb merged commit 5d23970 into JabRef:master Apr 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add RFC Standard Fetcher
3 participants