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

VIVO-4030 #482

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

VIVO-4030 #482

wants to merge 4 commits into from

Conversation

litvinovg
Copy link
Member

@litvinovg litvinovg commented Dec 12, 2024

VIVO GitHub issue
Related VIVO PR

What does this pull request do?

Created policy data sets for publish operations on properties by users with public role.
Created policy data sets for publish operations on classes by users with public role.
Removed exceptional case related to publish operation and public role in edit form.
Adjusted authorization migration script to retain publish and update permissions related to public role.
Created migration for instances which have already updated to access control configuration version 1, which is part of VIVO 1.15.0

How should this be tested?

Try migration for VIVO instances earlier than 1.15.0 release. Verify that publish permissions related to public role are preserved and could be modified.
For VIVO 1.15.0 this fix should allow to assign publish property permissions to public role.

Interested parties

@VIVO-project/vivo-committers

Reviewers' expertise

Candidates for reviewing this PR should have some of the following expertises:

  1. Java
  2. N3
  3. Vitro access ontology

Reviewers' report template

Please update the following template which should be used by reviewers.

General comment

A reviewer should provide here comments and suggestions for requested changes if any.

Testing

A reviewer should briefly describe here how it was tested

Code reviewing

A reviewer should briefly describe here which part was code reviewed

@brianjlowe
Copy link
Member

brianjlowe commented Jan 28, 2025

The migration from 1.14 doesn't seem to be working for me. Here are the steps I did:

  1. Clean install of 1.14.3 with empty tdbContentModels.
  2. Edit object property "access provided by (scires)".
  3. Set publish permission to "curator and above"; save property.
  4. Create new object property.
  5. Set publish permission to "editor and above"; save property.
  6. Clean install of PR.
  7. Open "access provided by".
  8. Publish permission is set to "public" (should be curator and above).
  9. Open new property I created.
  10. No permissions checkboxes are checked (should be publish to editor and above; public display/edit).

@litvinovg
Copy link
Member Author

First step should be to clean tdbConfigurationModels, not tdbContentModels

@brianjlowe
Copy link
Member

brianjlowe commented Feb 3, 2025

Retract comment below; caused by operator error. Above steps do work.

Repeating the above steps with both empty tdbModels and tdbContentModels at start improves one aspect. Now the new property I added in step 4 has permissions checked, but the publish permissions for both this and the "access provided by" properties are still wrong. After the upgrade using the PR, I have all the publish checkboxes checked for both properties, when one should be only curator and above and the other should be only editor and above.

brianjlowe
brianjlowe previously approved these changes Feb 10, 2025
Copy link
Member

@brianjlowe brianjlowe left a comment

Choose a reason for hiding this comment

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

Tested upgrade from 1.14.3 and editing of publish flags in 1.15. The object and data properties now appear in the public linked data where enabled. The only thing I noticed in testing is that we lack rdf:type and rdfs:label in the public linked data, but they appear when the linked data is requested by a root user. This makes some sense, since there is no place to edit permissions for these properties in the backend editor, but these are probably the two most useful things to include in linked data. Perhaps they were always published before regardless of role level? If that's the case and it's easy to replicate that behavior here, maybe it would make sense to include in this PR. Otherwise, we can open a separate issue.

@litvinovg
Copy link
Member Author

Rebased PR on top of main branch. Added publish permissions for rdfs:label and rdf:type properties.
Created linked VIVO PR vivo-project/VIVO#4054

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

No public linked data is published in 1.15.0 because "publish" permission is disabled for all properties
2 participants