428 Show New Version Numbers in Publish Dataset Modal #473

ekraffmiller · 2024-08-27T13:15:42Z

What this PR does / why we need it:

For Drafts of previously released versions, the Modal will show the new version numbers for Minor and Major Version updates.

This PR also enables "Update Current Version" option, because it the js-dataverse use case has been updated to allow this option

Which issue(s) this PR closes:

Closes Publish Dataset: Show new version numbers in Publish Popup #428

Special notes for your reviewer:

Suggestions on how to test this:

Create a test user account in JSF

Create a new account that is not a super user
Login as dataverseAdmin
From the Root Collection, choose Edit
Give the newly created user "Data Curator" role in the root collection

Test Publishing from "Data Curator Role"

From the Root Collection, Create a dataset and publish it using this account
Edit the dataset and choose the publish action again
In the publish popup, "Update Current Version" should not be visible. There should be two options: Minor (1.1) and Major (2.0)
Choose the Minor Option. After the page is published, the new version should be 1.1
Edit the Metadata again, and choose the Publish action.
In the Publish Popup, there should be options: Minor (1.2) and Major (2.0)
Choose the Major option. The new version of the Dataset should be 2.0

Test Update Current Version:
Only if you are logged in as a super user, there should be a third Publish Option, "Update Current Version". To test this:

Login as dataverseAdmin
Create a dataset, and publish it
Edit the dataset metadata and choose the Publish Action
Choose the "Update Current Version" option
The new version of the Dataset should have the same version number (1.0)

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:

… version

coveralls · 2024-08-27T13:28:29Z

coverage: 97.429% (-0.01%) from 97.44%
when pulling 342d7ec on feat/428-publish-version-numbers
into dc857c3 on develop.

src/dataset/infrastructure/repositories/DatasetJSDataverseRepository.ts

...ata-fields-section/metadata-input-level-fields-block/input-levels-table/InputLevelsTable.tsx

src/sections/dataset/publish-dataset/PublishDatasetModal.tsx

g-saracca · 2024-09-02T15:07:31Z

src/sections/dataset/publish-dataset/PublishDatasetModal.tsx

+  if (releasedVersionExists && (!nextMajorVersion || !nextMinorVersion)) {
+    console.log('Error: nextMajorVersion or nextMinorVersion is missing')
+    return null
+  }


I think we don't need this, if everything works well, this case should never happen.
and in any case we should not get to this point, if there is no released version and there is no major or minor version we should directly hide the button to publish dataset, but again I think we don't need that also.
What do you think?

I agree, this case should never happen, but since the two version number params are optional, I thought I should have a check just for completeness. I was trying to make the params required only if releasedVersionExists==true, but I couldn't find a clean way to do that. But I don't mind removing it

g-saracca · 2024-09-02T15:14:55Z

src/sections/dataset/publish-dataset/PublishDatasetModal.tsx

+  if (releasedVersionExists && (!nextMajorVersion || !nextMinorVersion)) {
+    console.log('Error: nextMajorVersion or nextMinorVersion is missing')
+    return null
+  }
  return (
    <Modal show={show} onHide={handleClose} size="lg">


I think Modal could be size="xl" everything fits better.

g-saracca · 2024-09-02T15:22:56Z

src/sections/dataset/publish-dataset/useGetVersionLabels.tsx

@@ -0,0 +1,39 @@
+import { useEffect, useState } from 'react'
+import { DatasetRepository } from '../../../dataset/domain/repositories/DatasetRepository'


We don't need this hook anymore right? If that is the case I think we can delete it.

yes, will remove it

tests/e2e-integration/e2e/sections/dataset/Dataset.spec.tsx

g-saracca · 2024-09-02T15:30:45Z

tests/component/sections/dataset/dataset-publish/PublishDatasetModal.spec.tsx

          handleClose={handleClose}
        />
      )
-      cy.findByText('Update Current Version').should('exist')
+      cy.contains('Update Current Version').should('exist')
    })
  })
  describe('PublishDatasetModal', () => {


In this file we have three describe('PublishDatasetModal', we should only have one same describe with different cases inside.

g-saracca · 2024-09-02T15:39:09Z

tests/component/sections/dataset/dataset-publish/PublishDatasetModal.spec.tsx

          handleClose={handleClose}
        />
      )
-      cy.findByText('Update Current Version').should('exist')
+      cy.contains('Update Current Version').should('exist')


you can change all the cy.contains for:
cy.findByLabelText(/Update Current Version/).should('exist')

I think is more suitable for finding an input by its label.

ekraffmiller · 2024-09-03T21:49:00Z

@g-saracca I made the requested changes, thanks! FileJSDataverseRepository.spec.ts is failing in Github, but I can't reproduce it locally, can you try it in your local environment?

g-saracca · 2024-09-03T21:52:51Z

@g-saracca I made the requested changes, thanks! FileJSDataverseRepository.spec.ts is failing in Github, but I can't reproduce it locally, can you try it in your local environment?

Hi @ekraffmiller , yes, that happen to me too. Try applying this change in the cypress config👇
https://github.com/IQSS/dataverse-frontend/pull/477/files#diff-3f62e885350a48939f240b9daf40f6b06d41da7751f2b613238fc7669ac0a1b2

ekraffmiller · 2024-09-03T22:22:45Z

@g-saracca I made the requested changes, thanks! FileJSDataverseRepository.spec.ts is failing in Github, but I can't reproduce it locally, can you try it in your local environment?

Hi @ekraffmiller , yes, that happen to me too. Try applying this change in the cypress config👇 https://github.com/IQSS/dataverse-frontend/pull/477/files#diff-3f62e885350a48939f240b9daf40f6b06d41da7751f2b613238fc7669ac0a1b2

That worked, thanks! I just realized I missed some requested changes, will do that now 👍

g-saracca

@ekraffmiller Thanks for addressing all the changes!

I'm sorry I missed some observations in the first review.

I think we need to set margin-bottom: 0; to PublishDatasetHelpText component in .warningText class to avoid too much spacing.
Also the CC0 link should have a target="_blank" attribute so it opens the link in a new tab.
And could we enable the update current version checkbox now that is allowed by the use case? Or perhaps we should do it in a separate issue?

ekraffmiller · 2024-09-05T02:31:53Z

thanks @g-saracca, requested changes made 👍

g-saracca

Thanks @ekraffmiller , approving!
Will be nice to add in the How To test this, a note that update current version is enabled now.

ekraffmiller · 2024-09-05T11:40:51Z

Thanks @ekraffmiller , approving! Will be nice to add in the How To test this, a note that update current version is enabled now.

Thanks! I added testing instructions.

ekraffmiller · 2024-09-06T20:27:31Z

I added an issue for the error that occurred in the UploadDatasetFiles unit test: #481

ChengShi-1

approve merging commit

ekraffmiller added 2 commits August 27, 2024 09:05

feat: add custom hook to get version labels based on latest-published…

ee72460

… version

fix: lint errors

d7c99cb

ekraffmiller added 4 commits August 27, 2024 15:32

refactor: move API call for versionLabels to getByPersistentId use case

74e6f25

refactor: simplify logic for conditionally defined properties

380c524

fix tsc errors

ced374e

fix Dataset mapping logic, PublishDatasetModal popup text, update tests

272d359

ekraffmiller marked this pull request as ready for review August 29, 2024 19:54

ekraffmiller added GREI Re-arch GREI re-architecture-related SPA: Dataset page (View) Size: 3 A percentage of a sprint. 2.1 hours. labels Aug 29, 2024

g-saracca self-assigned this Sep 2, 2024

g-saracca requested changes Sep 2, 2024

View reviewed changes

g-saracca assigned ekraffmiller and unassigned g-saracca Sep 2, 2024

ekraffmiller added 2 commits September 3, 2024 16:37

updates based on review suggestions

89bf3e3

fix: initialization of DatasetVersionNumber

d87f165

fix: set Cypress command timeout

aa1d58c

ekraffmiller assigned g-saracca Sep 3, 2024

fix: requested changes for PublishDatasetModal.spec.tsx

9001f35

ekraffmiller removed their assignment Sep 4, 2024

g-saracca requested changes Sep 4, 2024

View reviewed changes

g-saracca assigned ekraffmiller Sep 4, 2024

ekraffmiller added 3 commits September 4, 2024 18:24

Enable 'Update Current Version' Publish option

a2ddc95

fix: PublishDatasetHelpText style

13cf58b

fix: Licence link

4d15b6a

g-saracca previously approved these changes Sep 5, 2024

View reviewed changes

g-saracca unassigned ekraffmiller and g-saracca Sep 5, 2024

ekraffmiller assigned ChengShi-1 Sep 6, 2024

resolve merge conflicts from develop

342d7ec

ekraffmiller dismissed g-saracca’s stale review via 342d7ec September 6, 2024 19:14

ChengShi-1 approved these changes Sep 6, 2024

View reviewed changes

ChengShi-1 merged commit 1603ea2 into develop Sep 6, 2024
11 of 14 checks passed

g-saracca deleted the feat/428-publish-version-numbers branch September 9, 2024 17:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

428 Show New Version Numbers in Publish Dataset Modal #473

428 Show New Version Numbers in Publish Dataset Modal #473

ekraffmiller commented Aug 27, 2024 •

edited

Loading

coveralls commented Aug 27, 2024 •

edited

Loading

g-saracca Sep 2, 2024

ekraffmiller Sep 3, 2024

g-saracca Sep 2, 2024

g-saracca Sep 2, 2024

ekraffmiller Sep 3, 2024

g-saracca Sep 2, 2024

g-saracca Sep 2, 2024

ekraffmiller commented Sep 3, 2024

g-saracca commented Sep 3, 2024 •

edited

Loading

ekraffmiller commented Sep 3, 2024

g-saracca left a comment

ekraffmiller commented Sep 5, 2024

g-saracca left a comment •

edited

Loading

ekraffmiller commented Sep 5, 2024

ekraffmiller commented Sep 6, 2024

ChengShi-1 left a comment

		@@ -0,0 +1,39 @@
		import { useEffect, useState } from 'react'
		import { DatasetRepository } from '../../../dataset/domain/repositories/DatasetRepository'

428 Show New Version Numbers in Publish Dataset Modal #473

428 Show New Version Numbers in Publish Dataset Modal #473

Conversation

ekraffmiller commented Aug 27, 2024 • edited Loading

What this PR does / why we need it:

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:

coveralls commented Aug 27, 2024 • edited Loading

g-saracca Sep 2, 2024

Choose a reason for hiding this comment

ekraffmiller Sep 3, 2024

Choose a reason for hiding this comment

g-saracca Sep 2, 2024

Choose a reason for hiding this comment

g-saracca Sep 2, 2024

Choose a reason for hiding this comment

ekraffmiller Sep 3, 2024

Choose a reason for hiding this comment

g-saracca Sep 2, 2024

Choose a reason for hiding this comment

g-saracca Sep 2, 2024

Choose a reason for hiding this comment

ekraffmiller commented Sep 3, 2024

g-saracca commented Sep 3, 2024 • edited Loading

ekraffmiller commented Sep 3, 2024

g-saracca left a comment

Choose a reason for hiding this comment

ekraffmiller commented Sep 5, 2024

g-saracca left a comment • edited Loading

Choose a reason for hiding this comment

ekraffmiller commented Sep 5, 2024

ekraffmiller commented Sep 6, 2024

ChengShi-1 left a comment

Choose a reason for hiding this comment

ekraffmiller commented Aug 27, 2024 •

edited

Loading

coveralls commented Aug 27, 2024 •

edited

Loading

g-saracca commented Sep 3, 2024 •

edited

Loading

g-saracca left a comment •

edited

Loading