Fixes for MyPy and preexisting unit tests

Author: jstone-dev

src/mavedb/routers/collections.py

@@ -10,7 +10,7 @@
 
 from mavedb import deps
 from mavedb.lib.authentication import UserData, get_current_user
-from mavedb.lib.authorization import require_current_user_with_email
+from mavedb.lib.authorization import require_current_user, require_current_user_with_email
 from mavedb.lib.logging import LoggedRoute
 from mavedb.lib.logging.context import (
     format_raised_exception_info_as_dict,
@@ -46,7 +46,7 @@
 def list_my_collections(
     *,
     db: Session = Depends(deps.get_db),
-    user_data: Optional[UserData] = Depends(get_current_user),
+    user_data: UserData = Depends(require_current_user),
 ) -> Dict[str, Sequence[Collection]]:
     """
     List my collections.
@@ -685,7 +685,7 @@ async def add_user_to_collection_role(
         )
     # A user can only be in one role per collection, so remove from any other roles
     elif collection_user_association:
-        item.users.remove(User)
+        item.users.remove(user)
 
     setattr(user, "role", role)
     item.users.append(user)
@@ -762,7 +762,7 @@ async def remove_user_from_collection_role(
     assert_permission(user_data, item, Action.ADD_ROLE)
 
     # Since this is a post request, user should not already be in this role
-    if collection_user_association.contribution_role != role:
+    if collection_user_association is not None and collection_user_association.contribution_role != role:
         logger.info(
             msg="Failed to remove user from collection role; the requested user does not currently hold the requested role for this collection.",
             extra=logging_context(),

src/mavedb/routers/permissions.py

@@ -50,7 +50,7 @@ async def check_permission(
     """
     save_to_logging_context({"requested_resource": urn})
 
-    item: Optional[Union[ExperimentSet, Experiment, ScoreSet]] = None
+    item: Optional[Union[Collection, ExperimentSet, Experiment, ScoreSet]] = None
 
     if model_name == ModelName.experiment_set:
         item = db.query(ExperimentSet).filter(ExperimentSet.urn == urn).one_or_none()

src/mavedb/view_models/collection.py

@@ -33,12 +33,16 @@ class CollectionBase(BaseModel):
     )
 
 
-class CollectionModify(CollectionBase):
+class CollectionModify(BaseModel):
     # all fields should be optional, because the client should specify only the fields they want to update
     private: Optional[bool] = Field(
         description="Whether the collection is visible to all MaveDB users. If set during collection update, input ignored unless requesting user is collection admin."
     )
     name: Optional[str]
+    description: Optional[str]
+    badge_name: Optional[str] = Field(
+        description="Badge name. Input ignored unless requesting user has MaveDB admin privileges."
+    )
 
 
 class CollectionCreate(CollectionBase):

tests/helpers/constants.py

@@ -193,6 +193,7 @@
     # keys to be set after receiving response
     "urn": None,
     "experimentSetUrn": None,
+    "officialCollections": [],
 }
 
 TEST_EXPERIMENT_WITH_KEYWORD_RESPONSE = {
@@ -227,6 +228,7 @@
     # keys to be set after receiving response
     "urn": None,
     "experimentSetUrn": None,
+    "officialCollections": [],
 }
 
 TEST_EXPERIMENT_WITH_KEYWORD_HAS_DUPLICATE_OTHERS_RESPONSE = {
@@ -270,6 +272,7 @@
     # keys to be set after receiving response
     "urn": None,
     "experimentSetUrn": None,
+    "officialCollections": [],
 }
 
 TEST_TAXONOMY = {
@@ -402,6 +405,7 @@
     # keys to be set after receiving response
     "urn": None,
     "processingState": ProcessingState.incomplete.name,
+    "officialCollections": [],
 }
 
 TEST_MINIMAL_ACC_SCORESET = {
@@ -473,6 +477,7 @@
     # keys to be set after receiving response
     "urn": None,
     "processingState": ProcessingState.incomplete.name,
+    "officialCollections": [],
 }
 
 TEST_CDOT_TRANSCRIPT = {

tests/routers/test_permissions.py

@@ -65,9 +65,8 @@ def test_cannot_get_permission_with_wrong_action_in_experiment_set(client, setup
     assert response.status_code == 422
     response_data = response.json()
     assert (
-        response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'read', "
-        "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores',"
-        " 'add_role', 'publish'"
+        response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'lookup', 'read', "
+        "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores', 'add_role', 'publish', 'add_badge'"
     )
 
 
@@ -210,9 +209,8 @@ def test_cannot_get_permission_with_wrong_action_in_experiment(client, setup_rou
     assert response.status_code == 422
     response_data = response.json()
     assert (
-        response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'read', "
-        "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores',"
-        " 'add_role', 'publish'"
+        response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'lookup', 'read', "
+        "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores', 'add_role', 'publish', 'add_badge'"
     )
 
 
@@ -347,9 +345,8 @@ def test_cannot_get_permission_with_wrong_action_in_score_set(client, setup_rout
     assert response.status_code == 422
     response_data = response.json()
     assert (
-        response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'read', "
-        "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores',"
-        " 'add_role', 'publish'"
+        response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: 'lookup', 'read', "
+        "'update', 'delete', 'add_experiment', 'add_score_set', 'set_scores', 'add_role', 'publish', 'add_badge'"
     )
 
 
@@ -369,5 +366,5 @@ def test_cannot_get_permission_with_non_existing_item(client, setup_router_db):
     response_data = response.json()
     assert (
         response_data["detail"][0]["msg"] == "value is not a valid enumeration member; permitted: "
-        "'experiment', 'experiment-set', 'score-set'"
+        "'collection', 'experiment', 'experiment-set', 'score-set'"
     )