collection patch && tests

Author: bea777515

py/API_operations/CRUD/Collections.py

@@ -3,7 +3,7 @@
 # Copyright (C) 2015-2020  Picheral, Colin, Irisson (UPMC-CNRS)
 #
 from typing import List, Union, Optional, Tuple, Dict, Any
-
+from fastapi import HTTPException
 from API_models.crud import CreateCollectionReq, CollectionAggregatedRsp
 from API_models.exports import TaxonomyRecast
 from BO.Collection import CollectionBO, CollectionIDT
@@ -38,20 +38,23 @@ def create(
         coll_id = CollectionBO.create(self.session, req.title, req.project_ids)
         return coll_id
 
-    def _check_access(
-        self, a_coll: CollectionBO, user_id: UserIDT
-    ) -> Optional[CollectionBO]:
+    def update(
+        self,
+        current_user_id: UserIDT,
+        collection_id: CollectionIDT,
+        req: Dict[str, Any],
+    ):
         """
-        Quick & dirty access check by catching the exception.
+        Update a collection.
         """
-        try:
+        if "project_ids" in req:
             PermissionConsistentProjectSet(
-                self.ro_session,
-                a_coll.project_ids,  # Need the R/W session here, as the projects MRU is written to. TODO
-            ).can_be_administered_by(user_id, update_preference=False)
-        except AssertionError:
-            return None
-        return a_coll
+                self.session, req["project_ids"]
+            ).can_be_administered_by(current_user_id)
+        present_collection = self.query(current_user_id, collection_id, for_update=True)
+        if present_collection is None:
+            raise HTTPException(status_code=404, detail="Collection not found")
+        present_collection.update(session=self.session, collection_update=req)
 
     def list(
         self, current_user_id: UserIDT, collection_ids: Optional[str] = None
@@ -65,11 +68,11 @@ def list(
         for a_rec in qry:
             coll_bo = CollectionBO(a_rec)
             coll_bo._read_composing_projects()
-            checked = self._check_access(coll_bo, current_user_id)
+            checked = self._check_access(current_user_id, coll_bo.project_ids)
             if checked is None:
                 continue
-            checked.enrich()
-            ret.append(checked)
+            coll_bo.enrich()
+            ret.append(coll_bo)
         return ret
 
     def search(self, current_user_id: UserIDT, title: str) -> List[CollectionBO]:
@@ -78,11 +81,11 @@ def search(self, current_user_id: UserIDT, title: str) -> List[CollectionBO]:
         for a_rec in qry:
             coll_bo = CollectionBO(a_rec)
             coll_bo._read_composing_projects()
-            checked = self._check_access(coll_bo, current_user_id)
+            checked = self._check_access(current_user_id, coll_bo.project_ids)
             if checked is None:
                 continue
-            checked.enrich()
-            ret.append(checked)
+            coll_bo.enrich()
+            ret.append(coll_bo)
         return ret
 
     def query(
@@ -93,7 +96,10 @@ def query(
         )
         if ret is None:
             return ret
-        return self._check_access(ret, current_user_id)
+        check = self._check_access(current_user_id, ret.project_ids)
+        if check is None:
+            return None
+        return ret
 
     def query_by_title(self, title: str) -> CollectionBO:
         # Return a unique collection from its title
@@ -151,6 +157,21 @@ def _query_recast_for_coll(self, coll_id: CollectionIDT):
         )
         return qry
 
+    def _check_access(
+        self, user_id: UserIDT, project_ids: ProjectIDListT
+    ) -> Optional[ProjectIDListT]:
+        """
+        Quick & dirty access check by catching the exception.
+        """
+        try:
+            PermissionConsistentProjectSet(
+                self.ro_session,
+                project_ids,  # Need the R/W session here, as the projects MRU is written to. TODO
+            ).can_be_administered_by(user_id, update_preference=False)
+        except AssertionError:
+            return None
+        return project_ids
+
     def aggregated_from_projects(
         self,
         current_user_id: UserIDT,

py/BO/Collection.py

@@ -121,7 +121,7 @@ def _add_composing_projects(
     @staticmethod
     def _get_annotators_from_histo(
         session, project_ids: ProjectIDListT, status: Optional[int] = None
-    ) -> List[User]:
+    ) -> List[UserIDT]:
         pqry = session.query(User.id, User.organisation)
         pqry.join(User, User.id == ObjectHeader.classif_who)
         pqry.filter(Project.projid == any_(project_ids))
@@ -130,8 +130,7 @@ def _get_annotators_from_histo(
             pqry.filter(User.status == status)
         users: List[Any] = pqry.all()
         creator_user = [u.id for u in users]
-        creator_org = [u.organisation for u in users]
-        return creator_user, creator_org
+        return creator_user
 
     def update(
         self,
@@ -148,7 +147,7 @@ def update(
                 COLLECTION_ROLE_ASSOCIATED_PERSON: "associate_organisations",
             },
         }
-        by_role = {}
+        by_role: Dict[str, Any] = {}
         print("____coll_udpate", collection_update)
         for key, value in collection_update.items():
             if key == "project_ids":
@@ -157,7 +156,14 @@ def update(
                 # Redo sanity check & aggregation as underlying projects might have changed (or not as stated just above. lol)
                 self.set_composing_projects(session, value)
             # Simple fields update
-            if key in ["title", "short_title", "citation", "abstract", "description"]:
+            if key in [
+                "title",
+                "short_title",
+                "citation",
+                "abstract",
+                "description",
+                "license",
+            ]:
                 setattr(self._collection, key, value)
             if key in ["provider_user", "contact_user"] and value is not None:
                 # Copy provider user id contact user id
@@ -333,6 +339,6 @@ class MinimalCollectionBO(BaseModel):
     external_id: Union[str, None] = None
     title: str
     short_title: Union[str, None] = None
-    provider_user_id: UserIDT
-    contact_user: Union[ContactUserBO, None] = None
+    provider_user: UserIDT
+    contact_user: Union[UserIDT, None] = None
     project_ids: ProjectIDListT

py/BO/Project.py

@@ -719,15 +719,16 @@ def in_collections(session: Session, projid: int) -> List[MinimalCollectionBO]:
                 CollectionProject.collection_id == r.id
             )
             project_ids = [p.project_id for p in qry_proj]
+
             ret.append(
                 MinimalCollectionBO(
-                    r.id,
-                    r.external_id,
-                    r.title,
-                    r.short_title,
-                    r.provider_user_id,
-                    contact,
-                    project_ids,
+                    id=r.id,
+                    external_id=r.external_id or None,
+                    title=r.title,
+                    short_title=r.short_title or None,
+                    provider_user=r.provider_user_id,
+                    contact_user=r.contact_user_id or None,
+                    project_ids=project_ids,
                 )
             )
         return ret

py/DB/Collection.py

@@ -69,6 +69,19 @@ def __str__(self) -> str:
 COLLECTION_ROLE_INSTITUTION_CODE_PROVIDER = "P"
 
 
+class CollectionRole(Model):
+    __tablename__ = "collection_role"
+    """ n<->n valued (with role) relationship b/w collection and users """
+    collection_id: int = Column(INTEGER, ForeignKey("collection.id"), primary_key=True)
+    identity: str = Column(VARCHAR(1000), nullable=False)
+    role: str = Column(VARCHAR(1), nullable=False, primary_key=True)
+    # The relationships are created in Relations.py but the typing here helps IDE
+    collection: relationship
+
+    def __str__(self) -> str:
+        return "{0},{1}:{2}".format(self.collection_id, self.identity, self.role)
+
+
 class CollectionUserRole(Model):
     __tablename__ = "collection_user_role"
     """ n<->n valued (with role) relationship b/w collection and users """

py/main.py

@@ -851,7 +851,7 @@ def get_collection(
     responses={200: {"content": {"application/json": {"example": null}}}},
 )
 def update_collection(
-    collection: CollectionReq = Body(...),
+    collection: CollectionModel = Body(...),
     collection_id: int = Path(
         ...,
         description="Internal, the unique numeric id of this collection.",
@@ -867,15 +867,39 @@ def update_collection(
 
      Note: The collection is updated only if manageable.
     """
+    collection_update = collection.dict()
     with CollectionsService() as sce:
         with RightsThrower():
-            present_collection = sce.query(current_user, collection_id, for_update=True)
-        if present_collection is None:
-            raise HTTPException(status_code=404, detail="Collection not found")
-        collection_update = collection.dict(exclude_unset=True)
-        present_collection.update(
-            session=sce.session, collection_update=collection_update
-        )
+            sce.update(current_user, collection_id, collection_update)
+
+
+@app.patch(
+    "/collections/{collection_id}",
+    operation_id="patch_collection",
+    tags=["collections"],
+    responses={200: {"content": {"application/json": {"example": null}}}},
+)
+def patch_collection(
+    collection: CollectionReq = Body(...),
+    collection_id: int = Path(
+        ...,
+        description="Internal, the unique numeric id of this collection.",
+        example=1,
+    ),
+    current_user: int = Depends(get_current_user),
+) -> None:
+    """
+    **Partial Update of the collection**. Note that some updates are silently failing when not compatible
+     with the composing projects.
+
+     **Returns NULL upon success.**
+
+     Note: The collection is partiallly updated only if manageable.
+    """
+    collection_update = collection.dict(exclude_unset=True)
+    with CollectionsService() as sce:
+        with RightsThrower():
+            sce.update(current_user, collection_id, collection_update)
 
 
 @app.put(