-
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
356 publish dataset #423
356 publish dataset #423
Conversation
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.
All the requested changes were implemented, approving 🚀
@ekraffmiller Can you please resolve merge conflicts? Thanks! |
New conflicts have emerged, please can you resolve them? On the other hand, can you describe the steps to test this UI change? For example, specific storybooks, maybe a short description in Suggestions on how to test this would be very helpful. Thanks in advance! |
I fixed the merge conflicts and updated the notes for testing 👍 |
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.
I am encountering the following error in the files tab when publishing the created dataset for the first time.
I attach also a recording:
Untitled.mov
Could this be related to the change from draft to latest-published as default?
@GPortas I encountered that error before, the fix for it may have been lost in the merge, I will look into that. |
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 addressing the changes! I haven't encountered that error again.
The major version update seems to work correctly. But the minor version update does not update the minor version, instead it updates the major version. For example, if I am on version 1.0 and select minor, it publishes 2.0 instead of 1.1. I have checked that in JSF it publishes 1.1.
I attach a recording:
minorerr.mov
@GPortas thanks for catching that! I made the fix and updated the e2e tests to check that. |
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.
@ekraffmiller Thank you for addressing the changes!
Merging.
What this PR does / why we need it:
Which issue(s) this PR closes:
Closes #356
Special notes for your reviewer:
I wasn't able to add the major and minor version numbers to the popup, because they aren't currently part of the Dataset model. I added an issue for this: #428
Suggestions on how to test this:
Review the PublishDatasetModal Stories: http://localhost:6006/?path=/story/sections-dataset-page-publishdatasetmodal--default
Steps to test in the SPA:
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: