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

DDI import, fix for collectionMode #8491

Closed
wants to merge 2 commits into from
Closed

Conversation

landreev
Copy link
Contributor

@landreev landreev commented Mar 15, 2022

What this PR does / why we need it:

This is a followup PR for #8473 where collectionMode was made a multiples-allowed field. There was still a place in the code, in the DDI import, where the field was treated as a single-primitive.

Please note that the DDI import overall is still broken (#8210), and will still be broken after this PR! However, this PR will make it possible to fix it (as part of Victoiria's #8483).

The PR also includes an updated example json file (dataset-create-new-all-default-fields.json) reflecting the change in #8473.

Which issue(s) this PR closes:

Closes #8489

Special notes for your reviewer:

Please note that the DDI import overall is still broken (#8210), and will still be broken after this PR! However, this PR will make it possible to fix it (as part of Victoiria's #8483).

Suggestions on how to test this:

See the above, about ddi import still being broken, for reasons outside of this issue/this PR.
You can kinda test it, by attempting to import the example ddi (see attached, as a .txt file):

curl -H "X-Dataverse-key:xyz" -X POST http://localhost:8080/api/dataverses/root/datasets/:importddi --upload-file ddi_dataset.txt

before this PR, it'll be failing with

Error parsing data as Json: incorrect multiple for field collectionMode

after this PR, it'll be failing with the expected

Validation Failed: Subject is required. (Invalid value:edu.harvard.iq.dataverse.DatasetField...

ddi_dataset.txt

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

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

Additional documentation:

@coveralls
Copy link

coveralls commented Mar 15, 2022

Coverage Status

Coverage increased (+0.0005%) to 18.864% when pulling 0cfb6d8 on 8489-collmode-in-import into 0aaf203 on develop.

@landreev landreev marked this pull request as ready for review March 15, 2022 18:25
@qqmyers qqmyers mentioned this pull request Mar 15, 2022
@landreev landreev added this to the 5.10 milestone Mar 15, 2022
@pdurbin pdurbin mentioned this pull request Mar 15, 2022
@pdurbin
Copy link
Member

pdurbin commented Mar 15, 2022

Please note that the DDI import overall is still broken (#8210), and will still be broken after this PR!

I went ahead and merged this PR into #8483. That way, it's easier to test that DDI import works.

@pdurbin pdurbin self-assigned this Mar 15, 2022
@pdurbin
Copy link
Member

pdurbin commented Mar 16, 2022

As discussed at standup I'm closing this pull request because I've already merged it into pull request #8483 (where it's much easier to test). If the other pull request give us any trouble, we can always re-open this one and move it into QA independently.

@pdurbin pdurbin closed this Mar 16, 2022
@pdurbin pdurbin removed their assignment Mar 16, 2022
@pdurbin pdurbin removed this from the 5.10 milestone Mar 16, 2022
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.

DDI XML cannot be imported now that collectionMode has been made multiple (in PR #8473)
3 participants