Dataset Page: Guest User can't get file info for a deaccessioned dataset

[ChengShi-1]

What this PR does / why we need it:

download/count and downloadsize will return 404 for user: guests, the getByPersistentId should be refactored to adapt the change

If user:guest and the dataset is deaccessioned:

dataset is ok to show basic info, versions and deaccessioned reason
NO File info could be shown, cannot direct to file page as well
Hide share button
Hide metric(prevent fetching download/count

Which issue(s) this PR closes:

• Closes Prevent guests of a deaccessioned dataset from getting file's info #773

Special notes for your reviewer:

⚠️ waiting for IQSS/dataverse#11702

Suggestions on how to test this:

As a guest user, in a dataset, check if the dataset metric component and the Share button is hidden. As a dataset owner, check if the dataset metric component and the Share button is shown.

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]

coverage: 97.473% (-0.6%) from 98.052%
when pulling 8a3ef41 on Refactor-api-calls-for-deaccessioned-dataset
into fc16e44 on develop.

[g-saracca]

Hi @ChengShi-1 , some comments here 👍🏼

[g-saracca]

Is this logic replicating JSF condition?

[g-saracca]

Why are we deleting the test 'gets the dataset by persistentId when is locked'?

[ChengShi-1]

I wrongly overwrite this test while merging conflict :( I'll recover this

[g-saracca]

I dont understand this, why this checking for 404? Is the description of this PR outdated?
In steves PR I see this comment "guest user should see deaccessioned file count and download size"

[g-saracca]

Also you could use JSDataverseReadErrorHandler and don't cast the error error: ReadError, as you could have other many types of errors. Try something like this:

} catch (err: ReadError | unknown) {
        if (err instanceof ReadError) {
          const error = new JSDataverseReadErrorHandler(err)
          const statusCode = error.getStatusCode()
          if (statusCode === 404) return datasetDetails
        } else if (err instanceof Error && err.message.includes('404')) {
          return datasetDetails
        } else {
          throw error
        }
}

[ChengShi-1]

@g-saracca Sorry, the description said it should return 403, but it actually it returns 404.

• For guest users with includeDeaccessioned=false: Returns 404 Not Found
• For guest users with includeDeaccessioned=true: Returns 200 OK with download size.

Also, alternatively, if the dataset is deaccessioned and canEditDataset is false, skip calling the downloadSize endpoints and keep the defaults, avoiding the error path entirely. Do you think it will be better?

[g-saracca]

Sorry I still don't understand.
If you are a guest user and do NOT request to include deaccessioned version you will receive a 404, but you will receive a 200 ok being a guest user requesting a deaccessioned version? how can be that way?
I'm lost with this thing sorry 🙏🏼

[ChengShi-1]

Yes, guest user could still access files' downloadSize info for deaccessioned dataset, which will work for a deaccessioned tombstone

Important Note: A tombstone landing page with the basic citation metadata will always be accessible to the public if they use the persistent URL (Handle or DOI) provided in the citation for that dataset. Users will not be able to see any of the files or additional metadata that were previously available prior to deaccession.

[ChengShi-1]

For the backend, the issue was includeDeaccessioned=true, user: guest returns 404, so they fixed, now it returns 200.

In the UI, if we don't skip the 404, the UI will be 404 error page.
The downloadSize is a part of getByPersistentId in datasetJSDataverseRepository, and fileDownloadSizes is a field of the Dataset model. In order to prevent the showing of 404 error page, I catches 404 here.

You should not be sorry, this is truly confusing, I am not sure if I did it right. Thanks for helping!

[g-saracca]

Ok I think I understand now, try to use the suggestion I made for catching the error in a more robust way.

[ofahimIQSS]

Looks good to me, merging.