-
Notifications
You must be signed in to change notification settings - Fork 490
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
Add API endpoint for comparing Dataset Versions #10945
Add API endpoint for comparing Dataset Versions #10945
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
HI @stevenwinship I tried the latest version of the API, it looks good, just needs a few small changes:
|
This comment has been minimized.
This comment has been minimized.
Fixed |
There's potential for merge conflicts with this PR: Right? 🤔 |
I think it's conflict-free. Maybe in IT tests. I will help if needed. |
DataverseRequest req = createDataverseRequest(getRequestUser(crc)); | ||
DatasetVersion dsv1 = getDatasetVersionOrDie(req, versionId1, findDatasetOrDie(id), uriInfo, headers); | ||
DatasetVersion dsv2 = getDatasetVersionOrDie(req, versionId2, findDatasetOrDie(id), uriInfo, headers); | ||
return ok(DatasetVersion.compareVersions(dsv1, dsv2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a test to make sure that the versions are considered in the correct order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added check. return 400 if not in proper order
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks very good. Just a couple of minor nits and a reminder to bring the branch up to date with Dev. Thanks
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1 similar comment
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the updates. approved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
IT and unit test passed - merging PR |
@stevenwinship can you please resolve the branch conflicts. Thanks! |
📦 Pushed preview images as
🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name. |
IT and Unit tests passing - Merging PR monitoring and Jenkins. |
What this PR does / why we need it: Need an API endpoint that will compare two dataset versions and return a list of differences between the versions. This is needed to support the SPA Dataset Page
Which issue(s) this PR closes: #10888
Special notes for your reviewer:
Suggestions on how to test this: See IT and Unit tests
Does this PR introduce a user interface change? If mockups are available, please link/include them here: New API
Is there a release notes update needed for this change?: Included
Additional documentation:
https://dataverse-guide--10945.org.readthedocs.build/en/10945/