G3-303 endpoints user genset access

TheJacksonLaboratory · Jul 30, 2024 · 4f8d045 · 4f8d045
1 parent 1d79fc2
commit 4f8d045
Show file tree

Hide file tree

Showing 5 changed files with 111 additions and 40 deletions.
diff --git a/src/geneweaver/api/controller/genesets.py b/src/geneweaver/api/controller/genesets.py
@@ -339,23 +339,34 @@ def get_geneset_ontology_terms(
         if terms_resp.get("message") == api_message.ACCESS_FORBIDDEN:
             raise HTTPException(status_code=403, detail=api_message.ACCESS_FORBIDDEN)
 
+        if terms_resp.get("message") == api_message.INACCESSIBLE_OR_FORBIDDEN:
+            raise HTTPException(
+                status_code=404, detail=api_message.INACCESSIBLE_OR_FORBIDDEN
+            )
+
     return terms_resp
 
 
 @router.put("/{geneset_id}/ontologies", status_code=204)
 def put_geneset_ontology_term(
     geneset_id: Annotated[
-        int, Path(format="int64", minimum=0, maxiumum=9223372036854775807)
+        int,
+        Path(
+            description=api_message.GENESET_ID,
+            format="int64",
+            minimum=0,
+            maxiumum=9223372036854775807,
+        ),
     ],
-    ontology_ref_term_id: str,
+    ontology_id: Annotated[str, Query(description=api_message.ONTOLOGY_ID)],
     user: UserInternal = Security(deps.full_user),
     cursor: Optional[deps.Cursor] = Depends(deps.cursor),
 ) -> None:
     """Set geneset threshold for geneset owner."""
     response = genset_service.add_geneset_ontology_term(
         cursor=cursor,
         geneset_id=geneset_id,
-        ref_term_id=ontology_ref_term_id,
+        term_ref_id=ontology_id,
         user=user,
     )
 
@@ -371,21 +382,32 @@ def put_geneset_ontology_term(
         if response.get("message") == api_message.RECORD_EXISTS:
             raise HTTPException(status_code=412, detail=api_message.RECORD_EXISTS)
 
+        if response.get("message") == api_message.INACCESSIBLE_OR_FORBIDDEN:
+            raise HTTPException(
+                status_code=404, detail=api_message.INACCESSIBLE_OR_FORBIDDEN
+            )
 
-@router.delete("/{geneset_id}/ontologies", status_code=204)
+
+@router.delete("/{geneset_id}/ontologies/{ontology_id}", status_code=204)
 def delete_geneset_ontology_term(
     geneset_id: Annotated[
-        int, Path(format="int64", minimum=0, maxiumum=9223372036854775807)
+        int,
+        Path(
+            description=api_message.GENESET_ID,
+            format="int64",
+            minimum=0,
+            maxiumum=9223372036854775807,
+        ),
     ],
-    ontology_ref_term_id: str,
+    ontology_id: Annotated[str, Path(description=api_message.ONTOLOGY_ID)],
     user: UserInternal = Security(deps.full_user),
     cursor: Optional[deps.Cursor] = Depends(deps.cursor),
 ) -> None:
     """Set geneset threshold for geneset owner."""
     response = genset_service.delete_geneset_ontology_term(
         cursor=cursor,
         geneset_id=geneset_id,
-        ref_term_id=ontology_ref_term_id,
+        term_ref_id=ontology_id,
         user=user,
     )
 
@@ -397,3 +419,8 @@ def delete_geneset_ontology_term(
             raise HTTPException(
                 status_code=404, detail=api_message.RECORD_NOT_FOUND_ERROR
             )
+
+        if response.get("message") == api_message.INACCESSIBLE_OR_FORBIDDEN:
+            raise HTTPException(
+                status_code=404, detail=api_message.INACCESSIBLE_OR_FORBIDDEN
+            )
diff --git a/src/geneweaver/api/controller/message.py b/src/geneweaver/api/controller/message.py
@@ -28,3 +28,4 @@
     "Gensets and/or publications search types ('genesets', 'publications') "
 )
 CHECK_DB_HEALTH = "Check DB health flag"
+ONTOLOGY_ID = "Ontology term reference ID"
diff --git a/src/geneweaver/api/services/geneset.py b/src/geneweaver/api/services/geneset.py
@@ -441,15 +441,15 @@ def get_geneset_ontology_terms(
 def add_geneset_ontology_term(
     cursor: Cursor,
     geneset_id: int,
-    ref_term_id: str,
+    term_ref_id: str,
     user: User,
     gso_ref_type: str = ONTO_GSO_REF_TYPE,
 ) -> dict:
     """Add ontology term to a geneset.
 
     :param cursor: DB cursor
     :param geneset_id: geneset identifier
-    :param ref_term_id ref term identifier
+    :param term_ref_id ref term identifier
     :param user: GW user
     :param limit: Limit the number of results.
     :param offset: Offset the results.
@@ -459,6 +459,12 @@ def add_geneset_ontology_term(
         if user is None or user.id is None:
             return {"error": True, "message": message.ACCESS_FORBIDDEN}
 
+        is_gs_readable = db_geneset.is_readable(
+            cursor=cursor, user_id=user.id, geneset_id=geneset_id
+        )
+        if is_gs_readable is False:
+            return {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN}
+
         owner = db_geneset.user_is_owner(
             cursor=cursor, user_id=user.id, geneset_id=geneset_id
         )
@@ -468,7 +474,7 @@ def add_geneset_ontology_term(
             return {"error": True, "message": message.ACCESS_FORBIDDEN}
 
         onto_term = db_ontology.by_ontology_term(
-            cursor=cursor, onto_ref_term_id=ref_term_id
+            cursor=cursor, onto_ref_term_id=term_ref_id
         )
 
         if onto_term is None:
@@ -493,15 +499,15 @@ def add_geneset_ontology_term(
 def delete_geneset_ontology_term(
     cursor: Cursor,
     geneset_id: int,
-    ref_term_id: str,
+    term_ref_id: str,
     user: User,
     gso_ref_type: str = ONTO_GSO_REF_TYPE,
 ) -> dict:
     """Delete ontology term from a geneset.
 
     :param cursor: DB cursor
     :param geneset_id: geneset identifier
-    :param ref_term_id ref term identifier
+    :param term_ref_id ref term identifier
     :param user: GW user
     :param limit: Limit the number of results.
     :param offset: Offset the results.
@@ -511,6 +517,12 @@ def delete_geneset_ontology_term(
         if user is None or user.id is None:
             return {"error": True, "message": message.ACCESS_FORBIDDEN}
 
+        is_gs_readable = db_geneset.is_readable(
+            cursor=cursor, user_id=user.id, geneset_id=geneset_id
+        )
+        if is_gs_readable is False:
+            return {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN}
+
         owner = db_geneset.user_is_owner(
             cursor=cursor, user_id=user.id, geneset_id=geneset_id
         )
@@ -520,7 +532,7 @@ def delete_geneset_ontology_term(
             return {"error": True, "message": message.ACCESS_FORBIDDEN}
 
         onto_term = db_ontology.by_ontology_term(
-            cursor=cursor, onto_ref_term_id=ref_term_id
+            cursor=cursor, onto_ref_term_id=term_ref_id
         )
 
         if onto_term is None:

diff --git a/tests/controllers/test_genesets.py b/tests/controllers/test_genesets.py
@@ -311,14 +311,21 @@ def test_get_geneset_ontology_terms_errors(mock_get_genenset_onto_terms, client)
     response = client.get("/api/genesets/1234/ontologies")
     assert response.status_code == 403
 
+    mock_get_genenset_onto_terms.return_value = {
+        "error": True,
+        "message": message.INACCESSIBLE_OR_FORBIDDEN,
+    }
+    response = client.get("/api/genesets/1234/ontologies")
+    assert response.status_code == 404
+
 
 @patch("geneweaver.api.services.geneset.add_geneset_ontology_term")
 def test_add_geneset_ontology_term_response(mock_add_genenset_onto_terms, client):
     """Test add geneset ontology_terms  response."""
     mock_resp = test_ontology_data.get("geneset_ontology_terms")
     mock_add_genenset_onto_terms.return_value = mock_resp
 
-    response = client.put("/api/genesets/1234/ontologies?ontology_ref_term_id=D001921")
+    response = client.put("/api/genesets/1234/ontologies?ontology_id=D001921")
     assert response.status_code == 204
 
 
@@ -331,33 +338,38 @@ def test_add_geneset_ontology_terms_errors(mock_add_genenset_onto_terms, client)
     }
     mock_add_genenset_onto_terms.return_value = mock_resp
 
-    response = client.put("/api/genesets/1234/ontologies?ontology_ref_term_id=D001921")
+    response = client.put("/api/genesets/1234/ontologies?ontology_id=D001921")
     assert response.status_code == 403
 
     mock_add_genenset_onto_terms.return_value = {
         "error": True,
         "message": message.RECORD_NOT_FOUND_ERROR,
     }
-    response = client.put("/api/genesets/1234/ontologies?ontology_ref_term_id=QEQWEWE")
+    response = client.put("/api/genesets/1234/ontologies?ontology_id=QEQWEWE")
     assert response.status_code == 404
 
     mock_add_genenset_onto_terms.return_value = {
         "error": True,
         "message": message.RECORD_EXISTS,
     }
-    response = client.put("/api/genesets/1234/ontologies?ontology_ref_term_id=D001921")
+    response = client.put("/api/genesets/1234/ontologies?ontology_id=D001921")
     assert response.status_code == 412
 
+    mock_add_genenset_onto_terms.return_value = {
+        "error": True,
+        "message": message.INACCESSIBLE_OR_FORBIDDEN,
+    }
+    response = client.put("/api/genesets/1234/ontologies?ontology_id=D001921")
+    assert response.status_code == 404
+
 
 @patch("geneweaver.api.services.geneset.delete_geneset_ontology_term")
 def test_delete_geneset_ontology_term_response(mock_delete_genenset_onto_terms, client):
     """Test delete geneset ontology_terms  response."""
     mock_resp = test_ontology_data.get("geneset_ontology_terms")
     mock_delete_genenset_onto_terms.return_value = mock_resp
 
-    response = client.delete(
-        "/api/genesets/1234/ontologies?ontology_ref_term_id=D001921"
-    )
+    response = client.delete("/api/genesets/1234/ontologies/D001921")
     assert response.status_code == 204
 
 
@@ -370,16 +382,19 @@ def test_delete_geneset_ontology_terms_errors(mock_delete_genenset_onto_terms, c
     }
     mock_delete_genenset_onto_terms.return_value = mock_resp
 
-    response = client.delete(
-        "/api/genesets/1234/ontologies?ontology_ref_term_id=D001921"
-    )
+    response = client.delete("/api/genesets/1234/ontologies/D001921")
     assert response.status_code == 403
 
     mock_delete_genenset_onto_terms.return_value = {
         "error": True,
         "message": message.RECORD_NOT_FOUND_ERROR,
     }
-    response = client.delete(
-        "/api/genesets/1234/ontologies?ontology_ref_term_id=QEQWEWE"
-    )
+    response = client.delete("/api/genesets/1234/ontologies/QEQWEWE")
+    assert response.status_code == 404
+
+    mock_delete_genenset_onto_terms.return_value = {
+        "error": True,
+        "message": message.INACCESSIBLE_OR_FORBIDDEN,
+    }
+    response = client.delete("/api/genesets/1234/ontologies/D001921")
     assert response.status_code == 404
diff --git a/tests/services/test_genset.py b/tests/services/test_genset.py
@@ -486,15 +486,15 @@ def test_add_geneset_ontology_term(mock_db_ontology, mock_db_geneset):
     )
 
     response = geneset.add_geneset_ontology_term(
-        cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
     )
     assert response == mock_reponse
 
     # user is not the geneset owner, but he is a curator
     mock_db_geneset.user_is_owner.return_value = False
     mock_user.role = AppRoles.curator
     response = geneset.add_geneset_ontology_term(
-        cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
     )
     assert response == mock_reponse
 
@@ -509,18 +509,26 @@ def test_add_geneset_ontology_term_errors(mock_db_ontology, mock_db_geneset):
         "data"
     )
 
-    # user is not the geneset owner and he is not a curator
+    # geneset is not found or not readable by user
+    mock_db_geneset.is_readable.return_value = False
+    response = geneset.delete_geneset_ontology_term(
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
+    )
+    assert response == {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN}
+
+    # user is not the geneset owner, and he is not a curator
+    mock_db_geneset.is_readable.return_value = True
     mock_db_geneset.user_is_owner.return_value = False
     mock_user.role = None
     response = geneset.add_geneset_ontology_term(
-        cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
     )
     assert response.get("error") is True
     assert response.get("message") == message.ACCESS_FORBIDDEN
 
     # user is not logged-in
     response = geneset.add_geneset_ontology_term(
-        cursor=None, user=None, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=None, geneset_id=1234, term_ref_id="D001921"
     )
     assert response.get("error") is True
     assert response.get("message") == message.ACCESS_FORBIDDEN
@@ -529,7 +537,7 @@ def test_add_geneset_ontology_term_errors(mock_db_ontology, mock_db_geneset):
     mock_db_ontology.by_ontology_term.return_value = None
     mock_db_geneset.user_is_owner.return_value = True
     response = geneset.add_geneset_ontology_term(
-        cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
     )
     assert response.get("error") is True
     assert response.get("message") == message.RECORD_NOT_FOUND_ERROR
@@ -540,7 +548,7 @@ def test_add_geneset_ontology_term_errors(mock_db_ontology, mock_db_geneset):
     mock_db_ontology.add_ontology_term_to_geneset.side_effect = Exception("ERROR")
     with pytest.raises(expected_exception=Exception):
         geneset.add_geneset_ontology_term(
-            cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+            cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
         )
 
 
@@ -556,15 +564,15 @@ def test_delete_geneset_ontology_term(mock_db_ontology, mock_db_geneset):
     )
 
     response = geneset.delete_geneset_ontology_term(
-        cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
     )
     assert response == mock_reponse
 
     # user is not the geneset owner, but he is a curator
     mock_db_geneset.user_is_owner.return_value = False
     mock_user.role = AppRoles.curator
     response = geneset.delete_geneset_ontology_term(
-        cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
     )
     assert response == mock_reponse
 
@@ -579,18 +587,26 @@ def test_delete_geneset_ontology_term_errors(mock_db_ontology, mock_db_geneset):
         "data"
     )
 
-    # user is not the geneset owner and he is not a curator
+    # geneset is not found or not readable by user
+    mock_db_geneset.is_readable.return_value = False
+    response = geneset.delete_geneset_ontology_term(
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
+    )
+    assert response == {"error": True, "message": message.INACCESSIBLE_OR_FORBIDDEN}
+
+    # user is not the geneset owner, and he is not a curator
+    mock_db_geneset.is_readable.return_value = True
     mock_db_geneset.user_is_owner.return_value = False
     mock_user.role = None
     response = geneset.delete_geneset_ontology_term(
-        cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
     )
     assert response.get("error") is True
     assert response.get("message") == message.ACCESS_FORBIDDEN
 
     # user is not logged-in
     response = geneset.delete_geneset_ontology_term(
-        cursor=None, user=None, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=None, geneset_id=1234, term_ref_id="D001921"
     )
     assert response.get("error") is True
     assert response.get("message") == message.ACCESS_FORBIDDEN
@@ -599,7 +615,7 @@ def test_delete_geneset_ontology_term_errors(mock_db_ontology, mock_db_geneset):
     mock_db_ontology.by_ontology_term.return_value = None
     mock_db_geneset.user_is_owner.return_value = True
     response = geneset.delete_geneset_ontology_term(
-        cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+        cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
     )
     assert response.get("error") is True
     assert response.get("message") == message.RECORD_NOT_FOUND_ERROR
@@ -610,5 +626,5 @@ def test_delete_geneset_ontology_term_errors(mock_db_ontology, mock_db_geneset):
     mock_db_ontology.delete_ontology_term_from_geneset.side_effect = Exception("ERROR")
     with pytest.raises(expected_exception=Exception):
         geneset.delete_geneset_ontology_term(
-            cursor=None, user=mock_user, geneset_id=1234, ref_term_id="D001921"
+            cursor=None, user=mock_user, geneset_id=1234, term_ref_id="D001921"
         )