task/DES-2381: fix created project's response to include deletable (#111

)

* Fix created project's response to include deletable

Fixes DES-2381

* Clean some code

* Fix queries

* Fix tests

* Fix pep8 errors

* Fix model's relationships

* Fix getting custom system usr retrieval

* Remove unused logging

geoapi/custom/designsafe/project_users.py

@@ -1,7 +1,4 @@
 from urllib.parse import quote
-from geoapi.log import logging
-
-logger = logging.getLogger(__name__)
 
 
 def get_system_users(tenant_id, jwt, system_id: str):

geoapi/models/project.py

@@ -17,7 +17,7 @@ class ProjectUser(Base):
     creator = Column(Boolean, nullable=False, default=False)
     admin = Column(Boolean, nullable=False, default=False)
     project = relationship('Project', backref=backref('project_users', cascade="all, delete-orphan"))
-    user = relationship('User')
+    user = relationship('User', viewonly=True)
 
     def __repr__(self):
         return f'<ProjectUser(user_id={self.user_id}, project_id={self.project_id}, admin={self.admin}, creator={self.creator})>'
@@ -41,7 +41,8 @@ class Project(Base):
 
     users = relationship('User',
                          secondary='projects_users',
-                         back_populates='projects')
+                         back_populates='projects',
+                         overlaps="project,project_users")
     point_clouds = relationship('PointCloud', cascade="all, delete-orphan")
 
     def __repr__(self):

geoapi/models/users.py

@@ -15,7 +15,8 @@ class User(Base):
     streetviews = relationship('Streetview', cascade="all, delete-orphan")
     projects = relationship('Project',
                             secondary='projects_users',
-                            back_populates='users', lazy="joined")
+                            back_populates='users', lazy="joined",
+                            overlaps="project,project_users")
 
     def __repr__(self):
         return '<User(uname={}, id={})>'.format(self.username, self.id)

geoapi/services/projects.py

@@ -48,7 +48,7 @@ def create(data: dict, user: User) -> Project:
         project.project_users[0].creator = True
         db_session.add(project)
         db_session.commit()
-
+        setattr(project, 'deletable', True)
         return project
 
     @staticmethod
@@ -100,11 +100,8 @@ def list(user: User) -> List[Project]:
         :param user: User
         :return: List[Project]
         """
-
         projects_and_project_user = db_session.query(Project, ProjectUser) \
             .join(ProjectUser) \
-            .filter(User.username == user.username) \
-            .filter(User.tenant_id == user.tenant_id) \
             .filter(ProjectUser.user_id == user.id) \
             .order_by(desc(Project.created)) \
             .all()
@@ -130,8 +127,11 @@ def get(project_id: Optional[int] = None, uuid: Optional[str] = None, user: User
             raise ValueError("project_id or uid is required")
 
         if project and user and not is_anonymous(user):
-            project_user = db_session.query(ProjectUser).filter(Project.id == project.id).filter(User.id == user.id).first()
-            setattr(project, 'deletable', project_user.admin or project_user.creator)
+            project_user = db_session.query(ProjectUser)\
+                .filter(ProjectUser.project_id == project.id)\
+                .filter(ProjectUser.user_id == user.id).one_or_none()
+            if project_user:
+                setattr(project, 'deletable', project_user.admin or project_user.creator)
         return project
 
     @staticmethod
@@ -292,8 +292,9 @@ def addUserToProject(projectId: int, username: str, admin: bool) -> None:
         proj.users.append(user)
         db_session.commit()
 
-        project_user = db_session.query(ProjectUser).filter(Project.id == projectId)\
-            .filter(User.id == user.id).first()
+        project_user = db_session.query(ProjectUser)\
+            .filter(ProjectUser.project_id == projectId)\
+            .filter(ProjectUser.user_id == user.id).one()
         project_user.admin = admin
         db_session.commit()
 

geoapi/services/users.py

@@ -41,7 +41,7 @@ def canAccess(user: User, projectId: int) -> bool:
             .join(Project)\
             .filter(ProjectUser.user_id == user.id)\
             .filter(Project.tenant_id == user.tenant_id)\
-            .filter(ProjectUser.project_id == projectId).first()
+            .filter(ProjectUser.project_id == projectId).one_or_none()
         if up:
             return True
         return False
@@ -52,7 +52,7 @@ def is_admin_or_creator(user: User, projectId: int) -> bool:
             .join(Project) \
             .filter(ProjectUser.user_id == user.id) \
             .filter(Project.tenant_id == user.tenant_id) \
-            .filter(ProjectUser.project_id == projectId).first()
+            .filter(ProjectUser.project_id == projectId).one_or_none()
         if up:
             return up.admin or up.creator
         return False

geoapi/tasks/external_data.py

@@ -10,7 +10,7 @@
 
 from geoapi.celery_app import app
 from geoapi.exceptions import InvalidCoordinateReferenceSystem, MissingServiceAccount
-from geoapi.models import User, Project, ProjectUser, ObservableDataProject, Task
+from geoapi.models import User, ProjectUser, ObservableDataProject, Task
 from geoapi.utils.agave import (AgaveUtils, SystemUser, get_system_users, get_metadata_using_service_account,
                                 AgaveFileGetError, AgaveListingError)
 from geoapi.log import logger
@@ -344,7 +344,9 @@ def refresh_observable_projects():
             current_users = set([SystemUser(username=u.user.username, admin=u.admin) for u in o.project.project_users])
             updated_users = set(get_system_users(o.project.tenant_id, importing_user.jwt, o.system_id))
 
-            current_creator = db_session.query(ProjectUser).filter(Project.id == o.id).filter(ProjectUser.creator is True).one_or_none()
+            current_creator = db_session.query(ProjectUser)\
+                .filter(ProjectUser.project_id == o.id)\
+                .filter(ProjectUser.creator is True).one_or_none()
 
             if current_users != updated_users:
                 logger.info("Updating users from:{} to:{}".format(current_users, updated_users))
@@ -362,9 +364,9 @@ def refresh_observable_projects():
                 db_session.commit()
 
                 if current_creator:
-                    # reset the creator
+                    # reset the creator by finding that updated user again and updating it.
                     current_creator = db_session.query(ProjectUser)\
-                        .filter(Project.id == o.id)\
+                        .filter(ProjectUser.project_id == o.id)\
                         .filter(ProjectUser.user_id == current_creator.user_id)\
                         .one_or_none()
                     if current_creator:

geoapi/tests/api_tests/test_projects_routes.py

@@ -24,6 +24,17 @@ def test_get_projects_but_not_admin_or_creator(test_client, user2, projects_fixt
     assert data[0]["deletable"] is False
 
 
+def test_get_projects_with_multiple(test_client, user1, projects_fixture2, projects_fixture):
+    resp = test_client.get('/projects/', headers={'x-jwt-assertion-test': user1.jwt})
+    data = resp.get_json()
+    assert resp.status_code == 200
+    assert len(data) == 2
+    assert data[0]["deletable"] is True
+    assert data[0]["id"] == projects_fixture.id
+    assert data[1]["id"] == projects_fixture2.id
+    assert data[1]["deletable"] is True
+
+
 def test_get_projects_not_allowed(test_client):
     resp = test_client.get('/projects/')
     assert resp.status_code == 403

geoapi/tests/api_tests/test_projects_service.py

@@ -21,6 +21,7 @@ def test_create_project():
     assert len(proj.users) == 1
     assert proj.name == "test name"
     assert proj.description == "test description"
+    assert proj.deletable
 
 
 def test_delete_project(projects_fixture, remove_project_assets_mock, user1):

geoapi/tests/api_tests/test_users_service.py

@@ -1,8 +1,7 @@
 from geoapi.services.users import UserService
 from geoapi.services.projects import ProjectsService
 from geoapi.exceptions import ApiException
-from geoapi.models.project import ProjectUser, Project
-from geoapi.models.users import User
+from geoapi.models.project import ProjectUser
 from geoapi.db import db_session
 import pytest
 
@@ -54,8 +53,8 @@ def test_add_existing_user_to_project(user1, user2, projects_fixture):
 
     ProjectsService.addUserToProject(projectId=projects_fixture.id, username=user2.username, admin=False)
     assert UserService.canAccess(user2, projects_fixture.id)
-    project_user = db_session.query(ProjectUser).filter(Project.id == projects_fixture.id) \
-        .filter(User.id == user2.id).first()
+    project_user = db_session.query(ProjectUser).filter(ProjectUser.project_id == projects_fixture.id) \
+        .filter(ProjectUser.user_id == user2.id).one_or_none()
     assert project_user.admin is False
     assert UserService.canAccess(user1, projects_fixture.id)
 
@@ -65,8 +64,8 @@ def test_add_existing_user_to_project_as_admin(user1, user2, projects_fixture):
 
     ProjectsService.addUserToProject(projectId=projects_fixture.id, username=user2.username, admin=True)
     assert UserService.canAccess(user2, projects_fixture.id)
-    project_user = db_session.query(ProjectUser).filter(Project.id == projects_fixture.id) \
-        .filter(User.id == user2.id).first()
+    project_user = db_session.query(ProjectUser).filter(ProjectUser.project_id == projects_fixture.id) \
+        .filter(ProjectUser.user_id == user2.id).one_or_none()
     assert project_user.admin is True
     assert UserService.canAccess(user1, projects_fixture.id)
 

geoapi/tests/custom/test_designsafe.py

@@ -18,3 +18,7 @@ def test_get_system_users(requests_mock, project_response):
     users = get_system_users(tenant_id="DESIGNSAFE", jwt="dummy", system_id=f"project-{uuid}")
     users_as_list_of_dict = [{u.username: u.admin} for u in users]
     assert users_as_list_of_dict == [{'user_pi': True}, {'user_copi': True}, {'user3': False}, {'user4': False}]
+
+    users = get_system_users(tenant_id="designsafe", jwt="dummy", system_id=f"project-{uuid}")
+    users_as_list_of_dict = [{u.username: u.admin} for u in users]
+    assert users_as_list_of_dict == [{'user_pi': True}, {'user_copi': True}, {'user3': False}, {'user4': False}]

geoapi/utils/agave.py

@@ -308,9 +308,8 @@ def get_system_users(tenant_id, jwt, system_id: str) -> List[SystemUser]:
     :return: list of users with admin status
     """
 
-    if tenant_id in custom_system_user_retrieval:
-        return custom_system_user_retrieval[tenant_id](tenant_id, jwt, system_id)
-
+    if tenant_id.upper() in custom_system_user_retrieval:
+        return custom_system_user_retrieval[tenant_id.upper()](tenant_id, jwt, system_id)
     return get_default_system_users(tenant_id, jwt, system_id)