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

rename Private URL to Preview URL and other changes #10961

Open
wants to merge 44 commits into
base: develop
Choose a base branch
from

Conversation

sekmiller
Copy link
Contributor

@sekmiller sekmiller commented Oct 23, 2024

What this PR does / why we need it: Updates the Private URL to Preview URL in Edit Dataset Popup and native api calls. The request was due to confusion over the use of Private URLs. Also hides Collection info when there is an Anonymous Preview

Which issue(s) this PR closes:

Special notes for your reviewer: the native api references to privateurl remain and have been marked as deprecated. The classes and service beans, etc. remain with the private url nomenclature.
With respect to 10950 I ran into some jsf weirdness for hiding the breadcrumbs. there may be a better way to handle it.

Suggestions on how to test this:
This is the doc I used to guide the update to the Edit Dataset Popup: https://user-images.githubusercontent.com/18374574/151025261-ed70e998-9341-4013-9e0f-3c1d4d2c83e4.jpg

I did make one change to the popup to show the resulting url on create. Julian signed off on showing the URL in the popup. He is also reviewing the UI/UX for acceptance testing.

Creation of the "general" preview url will always be available to those that have manage Dataset permission permissions via the "Preview URL" option under the Edit Dataset menu. "Anonymous Preview" is only available if the AnonymizedFieldTypeNames setting has been added to the installation. (Use curl -X PUT -d 'author, datasetContact, contributor, depositor, grantNumber, publication' http://localhost:8080/api/admin/settings/:AnonymizedFieldTypeNames to add the setting)

Verify that the newly created preview url works and indicates to the user that they are viewing an unpublished dataset. Also if the "Anonymous Preview" is used that the Anonymized metadata are displayed as "withheld". Also in the anonymous preview you should not be able to determine the Dataverse Collection that hosts the dataset. (That is, the "breadcrumbs" that indicate the Collections in the ownership tree should not be visible, nor should the header showing the immediate owner of the dataset.)

In the native api verify that previewUrl and privateUrl may be used interchangeably.(curl -H "X-Dataverse-key:35f79221-0fc4-433b-956c-cf20f7f13d87" -X POST "http://localhost:8080/api/datasets/:persistentId/privateUrl?anonymizedAccess=true&persistentId=doi:10.5072/FK2/GNJY26")

  • if you have anonymizedAccess=true and AnonymizedFieldTypeNames are not set you should get an error.

Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Yes. See doc above.

Is there a release notes update needed for this change?:
Included.

Additional documentation:
The app documentation has been updated for this change

@coveralls
Copy link

coveralls commented Oct 23, 2024

Coverage Status

coverage: 21.849% (-0.005%) from 21.854%
when pulling 1cd7496 on 8184-preview-url-changes
into befc5a9 on develop.

This comment has been minimized.

@sekmiller sekmiller added this to the 6.5 milestone Oct 23, 2024
@sekmiller sekmiller added the Size: 3 A percentage of a sprint. 2.1 hours. label Oct 23, 2024
@jggautier
Copy link
Contributor

Hey @sekmiller. I haven't seen what you've done but from your comments in this PR, shouldn't this PR close the GitHub issue Some researchers unsure of difference between "Private URL" and "Anonymous Private URL" #8185, instead of the GitHub issue it's set to close now, Reviewers using anonymous private URL might learn dataset author's identity from information about the Dataverse installation or collection #8184?

@sekmiller
Copy link
Contributor Author

Hey @jggautier, the reason that I marked it with 8184 was because that is the one that was in the queue- and it has the references to changing the name to "Preview URL". 8185 has "no status", but I can add it to the "Closes" above.

This comment has been minimized.

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

Overall, looks good. I found one place where I think we can remove some minor code duplication. I also have some doc suggestions.

p.s. Tests are passing. 🎉 https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-10961/15/testReport/

doc/release-notes/8184-rename-private-url.md Outdated Show resolved Hide resolved
@@ -2171,6 +2171,7 @@ public Response getAssignments(@Context ContainerRequestContext crc, @PathParam(

@GET
@AuthRequired
@Deprecated(forRemoval = true, since = "2024-10-17")
@Path("{id}/privateUrl")
public Response getPrivateUrlData(@Context ContainerRequestContext crc, @PathParam("id") String idSupplied) {
return response( req -> {
Copy link
Member

Choose a reason for hiding this comment

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

Untested but you should be able to replace the whole body with this...

return getPreviewUrlData(crc, idSupplied);

... and the same pattern should work for the other two methods below. That way we don't have to maintain code in two places.

doc/sphinx-guides/source/api/native-api.rst Show resolved Hide resolved

This comment has been minimized.

This comment has been minimized.

1 similar comment

This comment has been minimized.

This comment has been minimized.

@sekmiller sekmiller removed their assignment Nov 7, 2024

This comment has been minimized.

@cmbz cmbz added the FY25 Sprint 10 FY25 Sprint 10 (2024-11-06 - 2024-11-20) label Nov 7, 2024

This comment has been minimized.

@pdurbin
Copy link
Member

pdurbin commented Nov 12, 2024

I'm ready to approve this but API tests didn't run. I'm kicking of another attempt manually: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-10961/22/

Copy link
Member

@pdurbin pdurbin left a comment

Choose a reason for hiding this comment

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

API tests are passing: https://jenkins.dataverse.org/job/IQSS-Dataverse-Develop-PR/job/PR-10961/22/testReport/

Approved.

I didn't run the code myself but it looks good.

@pdurbin pdurbin changed the title 8184 preview url changes rename Private URL to Preview URL and other changes Nov 12, 2024

This comment has been minimized.

Copy link

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:8184-preview-url-changes
ghcr.io/gdcc/configbaker:8184-preview-url-changes

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

@ofahimIQSS ofahimIQSS self-assigned this Nov 14, 2024
Copy link

📦 Pushed preview images as

ghcr.io/gdcc/dataverse:8184-preview-url-changes
ghcr.io/gdcc/configbaker:8184-preview-url-changes

🚢 See on GHCR. Use by referencing with full name as printed above, mind the registry name.

@ofahimIQSS
Copy link
Contributor

Hey @sekmiller: Found a typo: published ot if the URL
image

@ofahimIQSS
Copy link
Contributor

ofahimIQSS commented Nov 15, 2024

@sekmiller Another observation I had during testing: "Disable General Preview URL" button isn't working as expected when I modify an existing unpublished dataset file.

I compared the 2 behaviors in Internal with the PR and Demo with previous code.

To test this, open up an existing dataset thats unpublished and go to Preview URL and click on disable general preview url button. Then from dataset page, go back to Preview URL screen. Here is where the issue lies: "Preview URL has not been created". should be displayed instead of showing a URL as it is doing now.

Also noticed that the green success disabling message wasn't displayed either:
image

Screen.Recording.2024-11-15.at.12.17.56.PM.mov

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment