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

356 publish dataset #423

Merged
merged 106 commits into from
Aug 6, 2024
Merged

356 publish dataset #423

merged 106 commits into from
Aug 6, 2024

Conversation

ekraffmiller
Copy link
Contributor

@ekraffmiller ekraffmiller commented Jun 29, 2024

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:

  1. Create a new dataset, which will be a Draft version.
  2. Go to the Dataset Page, Click Publish Dataset Menu, and Publish dropdown
  3. Click continue
  4. The Publish Dataset should close and the "Publish in Progress" alert will appear on the Dataset Page.
  5. When the publish is completed, the "Publish in Progress" will disappear and the Dataset will be version 1.0
  6. Edit the Dataset Metadata and choose publish again.
  7. Now the Publish Dataset Model will give an option of Major or Minor release.
  8. Choose the version update type, and click continue.
  9. The Dataset will be published again with the new version chosen (either + 1.0 for Major update, or + .1 for Minor update

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:

GermanSaracca
GermanSaracca previously approved these changes Jul 18, 2024
Copy link
Contributor

@GermanSaracca GermanSaracca left a 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 🚀

@GermanSaracca GermanSaracca removed their assignment Jul 18, 2024
@GPortas GPortas self-assigned this Jul 31, 2024
@GPortas
Copy link
Contributor

GPortas commented Jul 31, 2024

@ekraffmiller Can you please resolve merge conflicts? Thanks!

@GPortas
Copy link
Contributor

GPortas commented Aug 1, 2024

@ekraffmiller

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!

@ekraffmiller
Copy link
Contributor Author

I fixed the merge conflicts and updated the notes for testing 👍

@ekraffmiller ekraffmiller removed their assignment Aug 1, 2024
Copy link
Contributor

@GPortas GPortas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekraffmiller

I am encountering the following error in the files tab when publishing the created dataset for the first time.

Screenshot 2024-08-02 at 10 51 58

I attach also a recording:

Untitled.mov

Could this be related to the change from draft to latest-published as default?

@ekraffmiller
Copy link
Contributor Author

@GPortas I encountered that error before, the fix for it may have been lost in the merge, I will look into that.

Copy link
Contributor

@GPortas GPortas left a 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

@ekraffmiller
Copy link
Contributor Author

@GPortas thanks for catching that! I made the fix and updated the e2e tests to check that.

Copy link
Contributor

@GPortas GPortas left a 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.

@GPortas GPortas merged commit 393e926 into develop Aug 6, 2024
10 of 14 checks passed
@GPortas GPortas deleted the 356-publish-dataset branch August 6, 2024 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GREI Re-arch GREI re-architecture-related internal Changes only affect the internal API Size: 10 A percentage of a sprint. 7 hours. SPA: Dataset page (View) UI Tasks related to the user interface (UI) or frontend development
Projects
Status: Done 🧹
Development

Successfully merging this pull request may close these issues.

Add Publish Dataset Version feature
4 participants