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

feat(get): get more infos from user (DEV-641) #181

Merged
merged 9 commits into from Apr 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions docs/dsp-tools-create.md
Expand Up @@ -394,8 +394,8 @@ This object contains user definitions. A user has the following elements:
- _familyName_: surname of the user
- _password_: password of the user
- _lang_: the default language of the user: "en", "de", "fr", "it" (optional, default: "en")
- _groups_: List of groups the user belongs to. The name of the group has to be provided with the ontology's namespace,
p.ex. "onto:editors". The given ontology defined in the same ontology file has no name, so only ":editors" is required
- _groups_: List of groups the user belongs to. The name of the group has to be provided with the project's shortname,
p.ex. "shortname:editors". The project defined in the same ontology file has no name, so only ":editors" is required
if the user belongs to the group "editors". (optional)
- _projects_: List of projects the user belongs to. The project name has to be followed by a ":" and either "member"
or "admin". This indicates if the new user has admin rights in the given project or is an ordinary
Expand Down
2 changes: 1 addition & 1 deletion knora/dsplib/models/project.py
Expand Up @@ -516,7 +516,7 @@ def getAllProjects(con: Connection) -> list[Project]:
result = con.get(Project.ROUTE)
if 'projects' not in result:
raise BaseError("Request got no projects!")
return list(map(lambda a: Project.fromJsonObj(con, a), result['projects']))
return [Project.fromJsonObj(con, a) for a in result['projects']]

def print(self) -> None:
"""
Expand Down
78 changes: 45 additions & 33 deletions knora/dsplib/models/user.py
@@ -1,3 +1,4 @@
from __future__ import annotations
import json
import os
import sys
Expand Down Expand Up @@ -197,7 +198,7 @@ def __init__(self,
if isinstance(lang, Languages):
self._lang = lang
else:
lmap = dict(map(lambda a: (a.value, a), Languages))
lmap = {a.value: a for a in Languages}
if lmap.get(lang) is None:
raise BaseError('Invalid language string "' + lang + '"!')
self._lang = lmap[lang]
Expand Down Expand Up @@ -297,7 +298,7 @@ def lang(self, value: Optional[Union[str, Languages]]):
self._lang = value
self._changed.add('lang')
else:
lmap = dict(map(lambda a: (a.value, a), Languages))
lmap = {a.value: a for a in Languages}
if lmap.get(value) is None:
raise BaseError('Invalid language string "' + value + '"!')
self._lang = lmap[value]
Expand Down Expand Up @@ -368,7 +369,7 @@ def in_projects(self) -> dict[str, bool]:
return self._in_projects

@in_projects.setter
def in_project(self, value: Any):
def in_projects(self, value: Any):
raise BaseError(
'Project membership cannot be modified directly! Use methods "addToProject" and "rmFromProject"')

Expand Down Expand Up @@ -444,7 +445,7 @@ def has_changed(self, name: str):
return name in self._changed

@classmethod
def fromJsonObj(cls, con: Connection, json_obj: Any):
def fromJsonObj(cls, con: Connection, json_obj: Any) -> User:
"""
Internal method! Should not be used directly!

Expand Down Expand Up @@ -475,18 +476,17 @@ def fromJsonObj(cls, con: Connection, json_obj: Any):
in_groups: set[str] = set()
if json_obj.get('permissions') is not None and json_obj['permissions'].get('groupsPerProject') is not None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

generally, if x is not None can normally be rewritten as if not x (but that doesn't need to be done in this PR... I know it's all over the place in dsp-tools)

sysadmin = False
project_groups = json_obj['permissions']['groupsPerProject']
for project in project_groups:
if project == Project.SYSTEM_PROJECT:
if Group.PROJECT_SYSTEMADMIN_GROUP in project_groups[project]:
for project_iri, group_memberships in json_obj['permissions']['groupsPerProject'].items():
if project_iri == Project.SYSTEM_PROJECT:
if Group.PROJECT_SYSTEMADMIN_GROUP in group_memberships:
sysadmin = True
Comment on lines +481 to 482
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could be simplified even further (I think): sysadmin = (Group.PROJECT_SYSTEMADMIN_GROUP in group_memberships)

else:
for group in project_groups[project]:
for group in group_memberships:
if group == Group.PROJECT_MEMBER_GROUP:
if in_projects.get(project) is None:
in_projects[project] = False
if in_projects.get(project_iri) is None:
in_projects[project_iri] = False
elif group == Group.PROJECT_ADMIN_GROUP:
in_projects[project] = True
in_projects[project_iri] = True
Comment on lines -478 to +489
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is just a refactoring, without semantic modification.

Copy link
Collaborator

Choose a reason for hiding this comment

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

nice!

else:
in_groups.add(group)
return cls(con=con,
Expand Down Expand Up @@ -675,41 +675,53 @@ def getAllUsers(con: Connection) -> list[Any]:
result = con.get(User.ROUTE)
if 'users' not in result:
raise BaseError("Request got no users!")
return list(map(lambda a: User.fromJsonObj(con, a), result['users']))
return [User.fromJsonObj(con, a) for a in result['users']]

@staticmethod
def getAllUsersForProject(con: Connection, proj_shortcode: str) -> list[Any]:
def getAllUsersForProject(con: Connection, proj_shortcode: str) -> Optional[list[User]]:
"""
Get a list of all users that belong to a project(static method)
Get a list of all users that belong to a project (static method)

:param con: Connection instance
:project_shortcode: Shortcode of the project
:return: List of users belonging to that project
"""

result = con.get(User.ROUTE)
if 'users' not in result:
raise BaseError("Request got no users!")
all_users = result["users"]
project_users = []
for user in all_users:
project_list = con.get(
User.IRI + urllib.parse.quote_plus(user["id"], safe='') + '/project-memberships')
project = project_list["projects"]
for proj in project:
if proj["id"] == "http://rdfh.ch/projects/" + proj_shortcode:
project_users.append(user)
return list(map(lambda a: User.fromJsonObj(con, a), project_users))

def createDefinitionFileObj(self):
user = {
members = con.get(f'/admin/projects/shortcode/{proj_shortcode}/members')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

With this route, the same task can be achieved with less code

if members is None or len(members) < 1:
Copy link
Collaborator

Choose a reason for hiding this comment

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

this can be simplified to if not members: (both None and empty lists/dicts/sets evaluate as false in an if-clause)

return None
res: list[User] = [User.fromJsonObj(con, a) for a in members['members']]
res.reverse()
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure the order of the objects you get from the API is deterministic (at least if the cache is rebuilt), so this may not reliably reproduce the initial order. But if it works in the cases you tested, feel free to leave it in.

Choose a reason for hiding this comment

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

In order to find out if it works as expected (also in the future), you could add a test for this method.

return res

def createDefinitionFileObj(
self,
con: Connection,
proj_shortname: str,
proj_shortcode: str
) -> dict[str, Union[str, list[str], None]]:
user: dict[str, Union[str, list[str], None]] = {
"username": self.username,
"email": self.email,
"givenName": self.givenName,
"familyName": self.familyName,
"password": "",
"lang": self.lang.value
}
if self.lang:
user["lang"] = self.lang.value
groups = list()
for group_iri in self._in_groups:
group_info = con.get(f'/admin/groups/{urllib.parse.quote_plus(group_iri)}')
if 'group' in group_info and 'name' in group_info['group']:
groupname = group_info['group']['name']
groups.append(f'{proj_shortname}:{groupname}')
user["groups"] = groups
user["projects"] = list()
for proj, is_admin in self._in_projects.items():
if proj_shortcode in proj:
if is_admin:
user["projects"].append(f"{proj_shortname}:admin")
else:
user["projects"].append(f"{proj_shortname}:member")
return user

def print(self) -> None:
Expand Down
13 changes: 8 additions & 5 deletions knora/dsplib/utils/onto_get.py
Expand Up @@ -62,11 +62,14 @@ def get_ontology(project_identifier: str, outfile: str, server: str, user: str,
print("Getting users...")
users_obj = []
users = User.getAllUsersForProject(con=con, proj_shortcode=project.shortcode)
for user in users:
users_obj.append(user.createDefinitionFileObj())
if verbose:
print(f"\tGot user '{user.username}'")
project_obj["users"] = users_obj
if users:
for usr in users:
users_obj.append(usr.createDefinitionFileObj(
con=con, proj_shortname=project.shortname, proj_shortcode=project.shortcode
))
if verbose:
print(f"\tGot user '{usr.username}'")
Comment on lines +66 to +71
Copy link
Collaborator

Choose a reason for hiding this comment

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

is usr intentional?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

no, rather random

project_obj["users"] = users_obj

# get the lists
if verbose:
Expand Down