dataverse icon indicating copy to clipboard operation
dataverse copied to clipboard

8715 importddi termofuse

Open lubitchv opened this issue 2 years ago • 9 comments

What this PR does / why we need it: importddi api does not set Terms of Use although there is a treatment for it in ImportDDIServiceBean. This PR fixes it. Which issue(s) this PR closes:

  • Closes #8715

Suggestions on how to test this: Create import xml with <dataAccs> <notes type="DVN:TOU" level="dv">Test</notes> </dataAccs> in stdyDscr section.

curl -H X-Dataverse-key:$API_TOKEN -X POST http://localhost:8080/api/dataverses/:root/datasets/:importddi --upload-file test_term_of_use.xml

Does this PR introduce a user interface change? If mockups are available, please link/include them here: No

Is there a release notes update needed for this change?: No

lubitchv avatar May 25 '22 21:05 lubitchv

Coverage Status

Coverage increased (+0.5%) to 19.71% when pulling de4eee3351a4661ba6d40002b0a81eec661d30d1 on lubitchv:8715-importddi-termofuse into 96feda16f292faa23b727dfa8deb006d69bcd98a on IQSS:develop.

coveralls avatar May 25 '22 21:05 coveralls

Create import xml with <dataAccs> <notes type="DVN:TOU" level="dv">Test</notes> </dataAccs> in stdyDscr section.

I assume it's safe to use doc/sphinx-guides/source/_static/api/ddi_dataset.xml as a starting point.

pdurbin avatar May 26 '22 15:05 pdurbin

@pdurbin I see that doc/sphinx-guides/source/_static/api/ddi_dataset.xml does not have "Terms of Use" section it only has "Terms of Access". Should "Terms of Use" also be added there?

lubitchv avatar May 26 '22 16:05 lubitchv

@lubitchv I'm fine with adding it there as long as it doesn't make any tests fail. 😄

I guess you could try it and we could see what Jenkins reports back. 😄

Or you could test it locally: https://guides.dataverse.org/en/5.10.1/developers/testing.html#integration-tests

When I wrote that comment I was thinking, "What DDI file would a reviewer or QA use to test?" Perhaps you could simply upload one to this issue.

pdurbin avatar Jun 08 '22 19:06 pdurbin

I added terms of use into doc/sphinx-guides/source/_static/api/ddi_dataset.xml. It seems to pass all the test. there is also an example test_term_of_use.txt hat attached to the issue #8715 Although it has extension txt, it is xml file. github did not let me to attach xml.

lubitchv avatar Jun 09 '22 16:06 lubitchv

@lubitchv yes, I know you can't see https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-8743/3/testReport/ but all the tests passed. 😄

To attach an xml file, I always just add ".txt" to it. Kind of annoying but it works. 😄

Sounds like we have two files to test with. Thanks!

Do you think it's worth writing a release note snippet?

pdurbin avatar Jun 09 '22 17:06 pdurbin

I am not sure. It does not seem to affect anything, even cached xml files since it is import. The only think to add is that now users can import xml with Terms of Use that was skipped before because of the bug. I can add it if you think it is needed in the release notes.

lubitchv avatar Jun 09 '22 17:06 lubitchv

Sure, please go ahead. I imagine it would be something like this? Please correct any misunderstandings I have!

  • Terms of Use is now imported when using DDI format through harvesting or the native API. (Issue #8715 PR #8743)

Maybe harvesting isn't affected? I'm not sure.

pdurbin avatar Jun 09 '22 17:06 pdurbin

I haven't thought about harvesting, but you are right. Harvester client in HarvesterServiceBean uses ImportDDIServiceBean. So harvesting would be affected. I will add the note that you have suggested.

lubitchv avatar Jun 09 '22 18:06 lubitchv

Looks good, merging.

landreev avatar Sep 08 '22 22:09 landreev