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

Feature/Handle nested dataverses[PLAT-1328] #3

Closed

Conversation

pattisdr
Copy link

@pattisdr pattisdr commented Feb 8, 2019

Purpose

We're getting a Key Error if someone has a nested dataverse inside of a dataverse when connecting a dataset. thanks @felliott for pointing me towards the fix.

Changes

On the Dataset.id property, when looping through the contents of a dataverse, verify that the resource type is a "dataset" before attempting to access the "protocol" key.

To Duplicate

Create a test dataverse on https://demo.dataverse.org/, and then add both a dataverse and a dataset to that top-level dataverse. (Create the nested dataverse first).

So I had dataverse Grapes Dataverse, with a Raisins Dataverse and a strawberries dataset.
screen shot 2019-02-08 at 12 17 59 pm

On the OSF connect your Grapes Dataverse, select your strawberries dataset and hit save. Before, you would get a key error when the _id property would loop through all the content in Grapes Dataverse (Raisins Dataverse and strawberries dataset). If it hit a dataverse before finding the matching dataset, it would get an error when looking for the protocol key which doesn't exist on a dataverse.

Ticket

https://openscience.atlassian.net/browse/PLAT-1328

…rses.

- Dataverse dicts don't have protocol key.
…n on the Dataset.id property if a dataverse is hit before a dataset.
@felliott
Copy link
Member

LGTM, but, alas, I don't have a commit bit for this repo!

@pattisdr, do you mind also submitting this to upstream, as well? I filed an issue at IQSS#50 Thank you!

Cheers,
@felliott

@pattisdr
Copy link
Author

Submitted PR here! IQSS#51

@pdurbin
Copy link

pdurbin commented Mar 4, 2019

@pattisdr @felliott thanks! Upstream we just merged IQSS#51

@pattisdr
Copy link
Author

pattisdr commented Mar 4, 2019

hooray @pdurbin!

@pattisdr
Copy link
Author

Closing as this work was merged upstream.

@pattisdr pattisdr closed this Mar 13, 2019
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.

3 participants