-
Notifications
You must be signed in to change notification settings - Fork 16
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/492 publish dataset with compare versions #551
base: develop
Are you sure you want to change the base?
Feature/492 publish dataset with compare versions #551
Conversation
…ajorVersionUpdate param to PublishDatasetModal.tsx
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.
Looks good, just some questions
src/dataset/infrastructure/repositories/DatasetJSDataverseRepository.ts
Outdated
Show resolved
Hide resolved
tests/component/dataset/infrastructure/mappers/JSDatasetMapper.spec.ts
Outdated
Show resolved
Hide resolved
thanks @g-saracca, ready to review now |
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.
Great job @ekraffmiller , approving!
Just one comment, you are mapping the js-dataverse versionDiff in toDatasetVersionDiff
but not changing any property, maybe we don't need the mapper at all?
Thanks! Yes, I thought maybe we didn't need the mapper, but wanted to follow the convention used in the mapping logic. Feel free to remove it if you want. |
@ekraffmiller , we have some merge conflicts and I think js-dataverse could now be update to use latest alpha 👍🏼 |
7a0d850
to
e2e050c
Compare
Ok, I resolved the conflicts and updated to alpha.8 |
need to resolve merge conflicts on this one |
What this PR does / why we need it:
This PR adds a use case for comparing dataset versions, and uses it to add a property to the Dataset object, a boolean that indicates whether the next version should be a major or minor version update.
The boolean requiresMajorVersionUpdate will be true if the draft contains added, removed or replaced files. If this boolean is true, then the PublishDatasetModal will include help text describing the major version update.
Which issue(s) this PR closes:
Special notes for your reviewer:
Suggestions on how to test this:
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: