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

5886 link harvested datasets via api #6935

Merged
merged 4 commits into from May 27, 2020

Conversation

sekmiller
Copy link
Contributor

@sekmiller sekmiller commented May 26, 2020

What this PR does / why we need it:
Allows a user to link a harvested dataset via the api. Previously there was a test that required that the dataset be published before it could be linked. This expands the test to allow datasets that haven't been published but are harvested to be linked

Which issue(s) this PR closes:

Closes #5886

Special notes for your reviewer:
I wasn't able to come up with a way to test this in the integration test. Any suggestions here would be welcome. (I tested by manually giving an unpublished dataset a dummy harvesting client id)

Suggestions on how to test this:

Does this PR introduce a user interface change?:
no

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

Additional documentation:
None

@coveralls
Copy link

Coverage Status

Coverage remained the same at 19.632% when pulling 988572b on 5886-link-harvested-datasets-via-api into 91ddb11 on develop.

IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from Code Review 🦁 to QA 🔎✅ May 26, 2020
@kcondon kcondon self-assigned this May 26, 2020
@kcondon
Copy link
Contributor

kcondon commented May 26, 2020

@sekmiller I have some questions when you have a minute:

  1. how does it fix the issue it closes since it appears to be ui related and create dataset related.
  2. how can you harvest unpublished datasets for them to be linked in the first place?

@scolapasta
Copy link
Contributor

Right, the issue wasn't unpublished, harvested (which can't exist), but (some) harvested datasets being identified as unpublished, possibly because they were missing a publication date in the db.

@kcondon
Copy link
Contributor

kcondon commented May 26, 2020

@scolapasta Do you have a known test case? Also, what about the issue it purports to close with the JSF errors?

@scolapasta
Copy link
Contributor

JSF issue was just a type - 5586 instead of 5886. (I just fixed it for @sekmiller in the description)

@scolapasta
Copy link
Contributor

as for test cases, here is a search results of four harvested datasets in production that could not be linked:

https://dataverse.harvard.edu/dataverse/harvard?q=covid+coronavirus&fq1=metadataSource%3A%22Harvested%22&fq0=dvObjectType%3A%28dataverses+OR+datasets+OR+files%29&types=dataverses%3Adatasets%3Afiles&sort=score&order=

(can't link to dataset pages of course, because they don't have any. - here are there dbids for the API: 3822842, 3724321, 3767398, 3767393 )

@kcondon
Copy link
Contributor

kcondon commented May 26, 2020

@scolapasta Thanks, just found that out from Julian as well.

@kcondon
Copy link
Contributor

kcondon commented May 27, 2020

@sekmiller This works now except for the case where the dataset is unpublished, it shows an incomplete error: {"status":"ERROR"} rather than {"status":"ERROR","message":"Can't link a dataset that has not been published"}

Update: this was due to languages being enabled and the lang bundle not yet having the new key. When we use the default bundle, it works as expected.

@kcondon kcondon assigned sekmiller and unassigned kcondon May 27, 2020
@kcondon kcondon moved this from QA 🔎✅ to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) May 27, 2020
@kcondon kcondon moved this from IQSS Team - In Progress 💻 to Code Review 🦁 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) May 27, 2020
@kcondon kcondon moved this from Code Review 🦁 to IQSS Team - In Progress 💻 in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) May 27, 2020
@kcondon kcondon assigned kcondon and unassigned sekmiller May 27, 2020
@kcondon kcondon moved this from IQSS Team - In Progress 💻 to QA 🔎✅ in IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) May 27, 2020
@kcondon kcondon merged commit 0cc916a into develop May 27, 2020
IQSS/dataverse (TO BE RETIRED / DELETED in favor of project 34) automation moved this from QA 🔎✅ to Done 🚀 May 27, 2020
@kcondon kcondon deleted the 5886-link-harvested-datasets-via-api branch May 27, 2020 14:42
@djbrooke djbrooke added this to the Dataverse 5 milestone May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Dataset linking API doesn't work for harvested datasets
6 participants