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

Allow users with write permission to see hidden metadata #9126

Merged
merged 3 commits into from Nov 2, 2023

Conversation

toniprieto
Copy link
Contributor

@toniprieto toniprieto commented Oct 13, 2023

References

Description

This PR shows the hidden metadata to users with item write permissions in order to edit this metadata correctly during submission/revision and edition of archived items.

A test have been modified to allow this new behavior.

Instructions for Reviewers

Define a hidden metadata and try to reproduce the issue with a normal submitter or local admin. After applying this change, the submitters and local admins should be able to see this hidden metadada during submission/revision.

Also, logged in as a local admin, try adding new values to hidden metadata. This should work properly.

NOTE: this PR changes the behavior of hidden metadata a bit compared to previous versions. In previous versions, a local administrator was not shown the hidden metadata in the item view, although he could see it when editing it. With this change the hidden metadata is also shown in the item view.

Checklist

  • My PR is small in size (e.g. less than 1,000 lines of code, not including comments & integration tests). Exceptions may be made if previously agreed upon.
  • My PR passes Checkstyle validation based on the Code Style Guide.
  • My PR includes Javadoc for all new (or modified) public methods and classes. It also includes Javadoc for large or complex private methods.
  • My PR passes all tests and includes new/updated Unit or Integration Tests based on the Code Testing Guide.
  • If my PR includes new libraries/dependencies (in any pom.xml), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.
  • If my PR modifies REST API endpoints, I've opened a separate REST Contract PR related to this change.
  • If my PR includes new configurations, I've provided basic technical documentation in the PR itself.
  • If my PR fixes an issue ticket, I've linked them together.

@tdonohue tdonohue added bug component: submission Related to configurable submission system authorization Related to user authorization / permissions labels Oct 13, 2023
@tdonohue
Copy link
Member

Thanks @toniprieto ! I'm going to pull this over to the 7.6.x maintenance board. However, I think this requires some discussion because it does slightly change the meaning of metadata.hide.*. I'll bring this up in the next developers meeting.

I do see why that is necessary though, so I don't object to the change of behavior. But, it does require some discussion as to whether to allow it in a bug-fix-only release (if so, we need to thoroughly document the behavior change).

@tdonohue tdonohue added the needs discussion Ticket or PR needs discussion before it can be moved forward. label Oct 13, 2023
@toniprieto
Copy link
Contributor Author

toniprieto commented Oct 16, 2023

I have added the missing test.

@tdonohue Thanks for initiating this discussion. I submitted this change because it seemed the closest behavior to previous versions, but as I think more closely about the problem, I see that there may be different use cases for this hidden metadata. I've been thinking lately if being able to configure different levels/scopes for each hidden metadata might be a better solution, although I don't know if it would be complicated to implement.

@tdonohue tdonohue added port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release and removed needs discussion Ticket or PR needs discussion before it can be moved forward. labels Oct 19, 2023
Copy link
Contributor

@corrad82-4s corrad82-4s left a comment

Choose a reason for hiding this comment

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

Code changes look good to me and reflect the expected behavior goal of this PR and of the issue addressed by it.
Thank you for addressing @tdonohue 's request by adding a test that verifies users with only READ permissions cannot see hidden metadata.

Copy link
Member

@tdonohue tdonohue left a comment

Choose a reason for hiding this comment

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

👍 Thanks @toniprieto ! Code looks good, as do the tests. I forgot to mention we discussed this approach a week or two ago in a DevMtg and there were no objections. Several Committers liked this idea of redefining hidden metadata to allow users with WRITE permissions to see it. Merging this for 7.6.1 and I'll update our docs around "hidden" metadata.

@tdonohue tdonohue merged commit 33cc216 into DSpace:main Nov 2, 2023
13 checks passed
@dspace-bot
Copy link

Successfully created backport PR for dspace-7_x:

@tdonohue tdonohue removed the port to dspace-7_x This PR needs to be ported to `dspace-7_x` branch for next bug-fix release label Nov 2, 2023
@tdonohue tdonohue added this to the 8.0 milestone Nov 2, 2023
@tdonohue
Copy link
Member

tdonohue commented Nov 2, 2023

Updated the documentation for "metadata.hide" settings at https://wiki.lyrasis.org/display/DSDOC7x/Configuration+Reference#ConfigurationReference-HidingMetadata

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
authorization Related to user authorization / permissions bug component: submission Related to configurable submission system
Projects
Status: ✅ Done
4 participants