fix: based on reviews

ChengShi-1 · ChengShi-1 · commit 26469a97030d · 2025-11-14T13:14:37.000-05:00
diff --git a/src/sections/edit-dataset-terms/dataset-terms-tab/DatasetTermsTab.module.scss b/src/sections/edit-dataset-terms/dataset-terms-tab/DatasetTermsTab.module.scss
@@ -16,3 +16,9 @@
   background-color: var(--bs-light);
   border-radius: 4px;
 }
+
+.license-icon {
+  margin-top: 0.5rem;
+  display: flex;
+  gap: 0.5rem;
+}
diff --git a/src/sections/edit-dataset-terms/dataset-terms-tab/DatasetTermsTab.tsx b/src/sections/edit-dataset-terms/dataset-terms-tab/DatasetTermsTab.tsx
@@ -1,5 +1,6 @@
 import { useEffect, useMemo, useRef } from 'react'
 import { useTranslation } from 'react-i18next'
+import { useNavigate } from 'react-router-dom'
 import { useForm, Controller, FormProvider } from 'react-hook-form'
 import { toast } from 'react-toastify'
 import { Form, Row, Col, Button, Alert } from '@iqss/dataverse-design-system'
@@ -9,6 +10,11 @@ import { useGetLicenses } from './useGetLicenses'
 import { DatasetRepository } from '@/dataset/domain/repositories/DatasetRepository'
 import { useDataset } from '../../dataset/DatasetContext'
 import { useUpdateDatasetLicense } from './useUpdateDatasetLicense'
+import { Route, QueryParamKey } from '../../Route.enum'
+import {
+  DatasetNonNumericVersionSearchParam,
+  DatasetPublishingStatus
+} from '../../../dataset/domain/models/Dataset'
 import styles from './DatasetTermsTab.module.scss'
 
 interface DatasetTermsFormData {
@@ -25,6 +31,7 @@ export function DatasetTermsTab({ licenseRepository, datasetRepository }: Datase
   const { t } = useTranslation('dataset')
   const { t: tShared } = useTranslation('shared')
   const { dataset, refreshDataset } = useDataset()
+  const navigate = useNavigate()
 
   const formContainerRef = useRef<HTMLDivElement>(null)
 
@@ -150,6 +157,21 @@ export function DatasetTermsTab({ licenseRepository, datasetRepository }: Datase
     }
   }
 
+  const handleCancel = () => {
+    if (!dataset) return
+
+    const searchParams = new URLSearchParams()
+    searchParams.set(QueryParamKey.PERSISTENT_ID, dataset.persistentId)
+
+    if (dataset.version.publishingStatus === DatasetPublishingStatus.DRAFT) {
+      searchParams.set(QueryParamKey.VERSION, DatasetNonNumericVersionSearchParam.DRAFT)
+    } else {
+      searchParams.set(QueryParamKey.VERSION, dataset.version.number.toString())
+    }
+
+    navigate(`${Route.DATASETS}?${searchParams.toString()}`)
+  }
+
   return (
     <div ref={formContainerRef} className={styles['dataset-terms-tab']}>
       <FormProvider {...form}>
@@ -190,7 +212,7 @@ export function DatasetTermsTab({ licenseRepository, datasetRepository }: Datase
               {errorLicenses && <Alert variant="danger">{errorLicenses}</Alert>}
 
               {currentLicenseOption && !isCustomTerms && (
-                <>
+                <div className={styles['license-icon']}>
                   {currentLicenseOption.iconUri && (
                     <img
                       src={currentLicenseOption.iconUri}
@@ -203,7 +225,7 @@ export function DatasetTermsTab({ licenseRepository, datasetRepository }: Datase
                       {currentLicenseOption.label}
                     </a>
                   )}
-                </>
+                </div>
               )}
             </Col>
           </Row>
@@ -261,16 +283,7 @@ export function DatasetTermsTab({ licenseRepository, datasetRepository }: Datase
             <Button type="submit" disabled={!isValid || isLoading}>
               {isLoading ? tShared('saving') : tShared('saveChanges')}
             </Button>
-            <Button
-              variant="secondary"
-              type="button"
-              disabled={isLoading}
-              onClick={() =>
-                reset({
-                  license: defaultLicenseValue,
-                  customTerms: initialCustomTerms
-                })
-              }>
+            <Button variant="secondary" type="button" disabled={isLoading} onClick={handleCancel}>
               {tShared('cancel')}
             </Button>
           </div>
diff --git a/src/sections/edit-dataset-terms/restricted-files-tab/RestrictedFilesTab.tsx b/src/sections/edit-dataset-terms/restricted-files-tab/RestrictedFilesTab.tsx
@@ -4,21 +4,28 @@ import { useForm, Controller, FormProvider, useWatch } from 'react-hook-form'
 import { toast } from 'react-toastify'
 import { Form, Row, Col, Button, Alert } from '@iqss/dataverse-design-system'
 import styles from '../dataset-terms-tab/DatasetTermsTab.module.scss'
-import { TermsOfAccess } from '@/dataset/domain/models/Dataset'
+import {
+  DatasetNonNumericVersionSearchParam,
+  DatasetPublishingStatus,
+  TermsOfAccess
+} from '@/dataset/domain/models/Dataset'
 import { DatasetRepository } from '@/dataset/domain/repositories/DatasetRepository'
 import { useDataset } from '../../dataset/DatasetContext'
 import { useUpdateTermsOfAccess } from './useUpdateTermsOfAccess'
+import { QueryParamKey, Route } from '@/sections/Route.enum'
+import { useNavigate } from 'react-router-dom'
 
 interface RestrictedFilesTabProps {
   datasetRepository: DatasetRepository
 }
 
 export function RestrictedFilesTab({
-  datasetRepository: _datasetRepository
+  datasetRepository: datasetRepository
 }: RestrictedFilesTabProps) {
   const { t } = useTranslation('dataset')
   const { t: tShared } = useTranslation('shared')
   const { dataset, refreshDataset } = useDataset()
+  const navigate = useNavigate()
 
   const defaultTermsOfAccess: TermsOfAccess = {
     fileAccessRequest: false,
@@ -36,7 +43,7 @@ export function RestrictedFilesTab({
   const formContainerRef = useRef<HTMLDivElement>(null)
 
   const { handleUpdateTermsOfAccess, isLoading, error } = useUpdateTermsOfAccess({
-    datasetRepository: _datasetRepository,
+    datasetRepository: datasetRepository,
     onSuccessfulUpdateTermsOfAccess: () => {
       toast.success(t('alerts.termsUpdated.alertText'))
       refreshDataset()
@@ -101,6 +108,20 @@ export function RestrictedFilesTab({
       })
   }, [initialTermsOfAccess, isRequestAccessEnabled, t])
 
+  const handleCancel = () => {
+    if (!dataset) return
+
+    const searchParams = new URLSearchParams()
+    searchParams.set(QueryParamKey.PERSISTENT_ID, dataset.persistentId)
+
+    if (dataset.version.publishingStatus === DatasetPublishingStatus.DRAFT) {
+      searchParams.set(QueryParamKey.VERSION, DatasetNonNumericVersionSearchParam.DRAFT)
+    } else {
+      searchParams.set(QueryParamKey.VERSION, dataset.version.number.toString())
+    }
+
+    navigate(`${Route.DATASETS}?${searchParams.toString()}`)
+  }
   return (
     <div ref={formContainerRef}>
       {
@@ -199,14 +220,7 @@ export function RestrictedFilesTab({
               disabled={isLoading || (!isRequestAccessEnabled && !isTermsOfAccessProvided)}>
               {isLoading ? tShared('saving') : tShared('saveChanges')}
             </Button>
-            <Button
-              variant="secondary"
-              type="button"
-              onClick={() => {
-                const resetValues =
-                  (dataset?.termsOfUse.termsOfAccess as TermsOfAccess) ?? initialTermsOfAccess
-                reset(resetValues)
-              }}>
+            <Button variant="secondary" type="button" onClick={handleCancel}>
               {tShared('cancel')}
             </Button>
           </div>
diff --git a/tests/component/sections/edit-dataset-terms/DatasetTermsTab.spec.tsx b/tests/component/sections/edit-dataset-terms/DatasetTermsTab.spec.tsx
@@ -180,26 +180,6 @@ describe('DatasetTermsTab', () => {
 
       cy.findByRole('button', { name: 'Save Changes' }).should('be.enabled')
     })
-
-    it('resets form when cancel is clicked', () => {
-      cy.customMount(
-        withProviders(
-          <DatasetTermsTab
-            licenseRepository={licenseRepository}
-            datasetRepository={datasetRepository}
-          />,
-          mockDataset
-        )
-      )
-
-      cy.findByRole('option', { name: 'Custom Dataset Terms' }).should('be.selected')
-      cy.findByTestId('customTerms.confidentialityDeclaration')
-        .clear()
-        .type('test confidentiality declaration')
-
-      cy.findByRole('button', { name: 'Cancel' }).click()
-      cy.findByText('test confidentiality declaration').should('not.exist')
-    })
   })
 
   describe('Loading States', () => {
diff --git a/tests/component/sections/edit-dataset-terms/RestrictedFilesTab.spec.tsx b/tests/component/sections/edit-dataset-terms/RestrictedFilesTab.spec.tsx
@@ -129,33 +129,6 @@ describe('RestrictedFilesTab', () => {
       cy.findByRole('button', { name: 'Save Changes' }).should('be.enabled')
     })
 
-    it('resets form to initial values when cancel is clicked', () => {
-      const termsOfAccess = TermsOfAccessMother.create({
-        termsOfAccessForRestrictedFiles: 'Original terms',
-        fileAccessRequest: true
-      })
-
-      const datasetWithTerms = DatasetMother.create({
-        id: 123,
-        termsOfUse: {
-          termsOfAccess: termsOfAccess
-        }
-      })
-
-      cy.customMount(
-        withProviders(
-          <RestrictedFilesTab datasetRepository={datasetRepository} />,
-          datasetWithTerms
-        )
-      )
-
-      cy.findByLabelText('Terms of Access for Restricted Files').clear().type('Modified terms')
-      cy.findByDisplayValue('Modified terms').should('exist')
-
-      cy.findByRole('button', { name: 'Cancel' }).click()
-      cy.findByDisplayValue('Original terms').should('exist')
-    })
-
     it('submits form data when save is clicked', () => {
       cy.customMount(
         withProviders(<RestrictedFilesTab datasetRepository={datasetRepository} />, mockDataset)
diff --git a/tests/e2e-integration/e2e/sections/edit-dataset-terms/EditDatasetTerms.spec.tsx b/tests/e2e-integration/e2e/sections/edit-dataset-terms/EditDatasetTerms.spec.tsx
@@ -4,7 +4,7 @@ import { DatasetNonNumericVersionSearchParam } from '../../../../../src/dataset/
 import { DatasetHelper } from '../../../shared/datasets/DatasetHelper'
 import { QueryParamKey, Route } from '../../../../../src/sections/Route.enum'
 
-describe('Edit Dataset Terms End-to-End', () => {
+describe('Edit Dataset Terms', () => {
   beforeEach(() => {
     TestsUtils.login().then((token) => {
       cy.wrap(TestsUtils.setup(token))
@@ -48,17 +48,16 @@ describe('Edit Dataset Terms End-to-End', () => {
 
         cy.findByTestId('customTerms.termsOfUse').type('These are custom terms for this dataset')
 
-        cy.findByRole('button', { name: 'Save Changes' }).should('be.enabled')
+        cy.findByRole('button', { name: 'Save Changes' }).click()
 
+        cy.findByText(/The license for this dataset has been updated./i).should('exist')
         cy.findByRole('button', { name: 'Cancel' }).click()
-        cy.get('select').should('not.have.value', 'CUSTOM')
+        cy.findByText('Custom Dataset Terms').should('exist')
       })
     })
-
-    // TODO: Add tests for custom terms requirement and toast messages
   })
 
-  describe.only('Restricted Files Tab', () => {
+  describe('Restricted Files Tab', () => {
     it('visits the Restricted Files + Terms of Access tab and interacts with form', () => {
       const datasetTitle = faker.lorem.sentence()
 
@@ -106,8 +105,13 @@ describe('Edit Dataset Terms End-to-End', () => {
 
         cy.findByLabelText('Size of Collection').type('500 MB')
 
-        cy.findByRole('button', { name: 'Save Changes' }).should('be.enabled')
-        cy.findByRole('button', { name: 'Cancel' }).should('exist')
+        cy.findByRole('button', { name: 'Save Changes' }).click()
+        cy.findByRole('button', { name: 'Cancel' }).click()
+
+        cy.findByRole('tab', { name: 'Terms and Guestbook' }).click()
+        cy.findByText(/Data can be accessed through the university data center/i).should('exist')
+        cy.findByText(/dataowner@university.edu/i).should('exist')
+        cy.findByText(/500 MB/i).should('exist')
       })
     })
 
@@ -158,7 +162,6 @@ describe('Edit Dataset Terms End-to-End', () => {
 
         cy.visit(editDatasetTermsUrl)
 
-        // Should start with restricted files tab active
         cy.findByRole('tab', { name: 'Restricted Files + Terms of Access' }).should(
           'have.attr',
           'aria-selected',
@@ -179,30 +182,11 @@ describe('Edit Dataset Terms End-to-End', () => {
         const editDatasetTermsUrl = `/spa${Route.EDIT_DATASET_TERMS}?${searchParams.toString()}`
 
         cy.visit(editDatasetTermsUrl)
-
-        // Click on dataset title in breadcrumbs to go back
         cy.findByRole('link', { name: datasetTitle }).click()
 
-        // Should navigate to dataset page
         cy.url().should('include', '/datasets')
         cy.findByRole('heading', { name: datasetTitle }).should('exist')
       })
     })
-
-    it('handles non-existent dataset gracefully', () => {
-      const searchParams = new URLSearchParams()
-      searchParams.set(QueryParamKey.PERSISTENT_ID, 'non-existent-dataset')
-      searchParams.set(QueryParamKey.VERSION, DatasetNonNumericVersionSearchParam.DRAFT)
-
-      const editDatasetTermsUrl = `/spa${Route.EDIT_DATASET_TERMS}?${searchParams.toString()}`
-
-      cy.visit(editDatasetTermsUrl)
-
-      cy.findByText('The dataset you are looking for was not found').should('exist')
-    })
-
-    it('handles unauthorized access gracefully', () => {
-      cy.log('Test for unauthorized access would go here')
-    })
   })
 })
