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-1436] Implementation of Advanced Role Management #85

Closed
wants to merge 16 commits into from

Conversation

grahamtriggs
Copy link
Member

Reopening as old pull request was incorrect

See vivo-project/Vitro#80 for details

@j2blake j2blake self-requested a review October 23, 2018 15:13
Copy link
Member

@j2blake j2blake left a comment

Choose a reason for hiding this comment

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

home/src/main/resources/rdf/display/firsttime/PropertyConfig.n3 and
home/src/main/resources/rdf/tbox/firsttime/vitroAnnotations.n3
still contain hundreds of references to
http://vitro.mannlib.cornell.edu/ns/vitro/0.7#hiddenFromDisplayBelowRoleLevelAnnot,
http://vitro.mannlib.cornell.edu/ns/vitro/0.7#prohibitedFromUpdateBelowRoleLevelAnnot, and
http://vitro.mannlib.cornell.edu/ns/vitro/0.7#hiddenFromDisplayBelowRoleLevelAnnot

Copy link
Member

@j2blake j2blake left a comment

Choose a reason for hiding this comment

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

This is my full list of review points, including comments on unused code, commented code, undocumented methods, and others.

Since the project doesn't have a style guide, these are simply observations. Act on them or not, as you see fit.

VIVO - main

edu.cornell.mannlib.vivo.auth.policy.InfoContentEntityChecker

lines 7, 9 Logic imports of PolicyDecision and AbstractPropertyStatementAction are not used.

edu.cornell.mannlib.vivo.auth.policy.SelfEditorRelationship

line 43 Style Why use a nested class?
line 46 Logic This line has no function.

edu.cornell.mannlib.vivo.auth.policy.AdvisingRelationshipChecker

lines 7, 9 Logic These import statements are not used.

edu.cornell.mannlib.vivo.auth.policy.CourseChecker

lines 7, 9 Logic These import statements are not used.

edu.cornell.mannlib.vivo.auth.policy.GrantChecker

lines 7, 9 Logic These import statements are not used.

edu.cornell.mannlib.vivo.auth.policy.PresentationChecker

lines 7,9 Logic These import statements are not used.

edu.cornell.mannlib.vivo.auth.policy.ProjectOrServiceChecker

lines 7, 9 Logic These import statements are not used.

VIVO - home

src/main/resources/rdf/auth/firsttime/permission_entities.n3

OK

VIVO - webapp

src/main/webapp/WEB-INF/resources/startup_listeners.txt

OK

public static class Setup implements ServletContextListener {
@Override
public void contextInitialized(ServletContextEvent sce) {
ServletContext ctx = sce.getServletContext();
Copy link
Member

Choose a reason for hiding this comment

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

The ctx variable does not appear to be used... and can likely be removed.

@@ -33,6 +33,9 @@ edu.cornell.mannlib.vitro.webapp.servlet.setup.FileGraphSetup
# edu.cornell.mannlib.vitro.webapp.migration.rel17.Release17Migrator
edu.cornell.mannlib.vitro.webapp.migration.rel18.Release18Migrator

edu.cornell.mannlib.vitro.webapp.migration.auth.AuthMigrator
Copy link
Member

Choose a reason for hiding this comment

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

Please add a comment describing this migrator.

Copy link
Member

Choose a reason for hiding this comment

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

resolved

@grahamtriggs
Copy link
Member Author

@j2blake The issues with leftover roles in the ontology files was a deliberate choice - it is relatively easy (albeit slightly time consuming) to delete lines that are no longer required. However, not knowing how long it will take to review the role management, or what might happen in ontology development in that time, I was hoping to reduce potential merge problems by delaying removing the old permissions.

@grahamtriggs
Copy link
Member Author

@j2blake About the nested class in SelfEditorRelationship - it was done for consistency with other setup code that is called from startup_listeners.txt. I've added a comment to the class.

@grahamtriggs grahamtriggs changed the base branch from develop to dev-ctsaconnect June 27, 2019 21:45
@grahamtriggs grahamtriggs changed the base branch from dev-ctsaconnect to develop June 27, 2019 21:45
@awoods awoods closed this Jan 28, 2020
@gneissone
Copy link
Member

Reopening this pull request. In an effort to follow current best practices, the VIVO project will now be developed against the master branch. As a result of removing the develop branch, this pull request was automatically closed by GitHub, apologies for any confusion this may have caused.

@gneissone gneissone reopened this Feb 5, 2020
@gneissone gneissone changed the base branch from develop to master February 5, 2020 16:22
Base automatically changed from master to main January 20, 2021 17:28
awoods and others added 4 commits February 5, 2021 10:56
* Add 'home' files into build war artifact

Related to: https://jira.lyrasis.org/browse/VIVO-1443

* Disable copying exploded war to Tomcat dir

- hardcode vivo.all.log file name
- must now set system property: -Dvivo-dir=/opt/vivo/home/

Related to: https://jira.lyrasis.org/browse/VIVO-1443

* Ensure build does not remove and re-add VIVO_HOME/rdf

Related to: https://jira.lyrasis.org/browse/VIVO-1443

* Remove unnecessary profile from installer/pom.xml

Related to: https://jira.lyrasis.org/browse/VIVO-1443

* Rename example config files to have 'default' prefix

Related to: https://jira.lyrasis.org/browse/VIVO-1443

* Require common properties to be in JNDI

Properties include:
- vitro/home
- vitro/appName
- vitro/rootUserAddress
- vitro/defaultNamespace

Related to: https://jira.lyrasis.org/browse/VIVO-1443

* VIVO-1443: app name (#2)

* Non-functional change to comment in example.applicationSetup.n3

Related to: https://jira.lyrasis.org/browse/VIVO-1741

* Update orcidConfirm.ftl (vivo-project#199)

* remove example-settings.xml
* simplify war name and afford override during build
* default app name to vivo and pass into context.xml

Co-authored-by: Andrew Woods <[email protected]>
Co-authored-by: L.O <[email protected]>

Co-authored-by: Andrew Woods <[email protected]>
Co-authored-by: William Welling <[email protected]>
Co-authored-by: L.O <[email protected]>
@litvinovg
Copy link
Collaborator

Superseded by #3887

@litvinovg litvinovg closed this Jun 15, 2023
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.

5 participants