Skip to content

Fix DOI import in RIS Importer#6557

Closed
braincident wants to merge 10 commits into
JabRef:masterfrom
braincident:master
Closed

Fix DOI import in RIS Importer#6557
braincident wants to merge 10 commits into
JabRef:masterfrom
braincident:master

Conversation

@braincident

@braincident braincident commented May 30, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor

Fixes #6530
As mentioned in #6530, for a RIS file with DOI field, it should be loaded with or without starting with "doi:", so I modified the else case to solve this issue.

  • Change in CHANGELOG.md described (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 documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Siedlerchr
Siedlerchr requested changes May 30, 2020
View reviewed changes

@Siedlerchr Siedlerchr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix. Could you please add some tests for this?

@Siedlerchr Siedlerchr changed the title Fix #6530 Fix DOI import in RIS Importer May 30, 2020
@Siedlerchr Siedlerchr added the status: changes-required Pull requests that are not yet complete label May 30, 2020
@braincident
Create RisImporterTest.java
879f1ce 
Add tests for addDoi() in RisImporter.java
@braincident
Update RisImporterTest.java
7c9b53b
@braincident

braincident commented May 31, 2020

Copy link
Copy Markdown
Contributor Author

I've added 2 simple tests, please take a look.

@braincident braincident requested a review from Siedlerchr May 31, 2020 03:39
@braincident
Update RisImporterTest.java
953e028 
Make changes following checkstyle rules.
@Siedlerchr

Siedlerchr commented May 31, 2020 •
edited
Loading

Copy link
Copy Markdown
Member

While your test approach is a way, the easier and recommended way is to put a RIS file and the corresponding bib file in the test folder:
jabref\src\test\resources\org\jabref\logic\importer\fileformat
Just import the RIS manually in a new library, then check if the DOI is correct imported and copy that bib file over to the folder together with the RIS file and give it a name.
It will then be automatically picked up by the FileImporter Tests.

So please remove your test class and follow the above way.

@Siedlerchr Siedlerchr closed this Jun 6, 2020
Siedlerchr added a commit that referenced this pull request Jun 6, 2020
@Siedlerchr
Fix DOI import in RIS Importer
835e778 
Use DOI parse and only add valid DOIs


Follow up from #6557

Fixes #6530
@Siedlerchr Siedlerchr mentioned this pull request Jun 6, 2020
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Awaiting requested review from Siedlerchr

Assignees

No one assigned

Labels

status: changes-required Pull requests that are not yet complete

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

DOI not imported from RIS file

2 participants

@braincident @Siedlerchr