[Backend] Remove and edit post route and methods to update and delete a post from the discussion section

backend/src/models/analysis.py

@@ -106,3 +106,29 @@ def units_to_annotate(self):
                     })
 
         return units
+
+    def find_discussion_post(self, discussion_post_id):
+        """
+        Finds a specific discussion post in an analysis by the discussion post id otherwise returns none
+        if no discussion with that post id in the analysis.
+        """
+        for discussion in self.discussions:
+            if discussion['post_id'] == discussion_post_id:
+                return discussion
+
+        return None
+
+    def find_authored_discussion_post(self, discussion_post_id, client_id):
+        """
+        Finds a discussion post from a user that authored the post in an analysis otherwise returns none if the post
+        was found, but the user did not author the post
+        """
+        discussion_post = self.find_discussion_post(discussion_post_id)
+
+        if discussion_post is None:
+            raise ValueError(f"Post '{discussion_post_id}' does not exist.")
+
+        if discussion_post['author_id'] == client_id:
+            return discussion_post
+
+        return None

backend/src/repository/analysis_collection.py

@@ -497,3 +497,28 @@ def add_discussion_post(self, analysis_name: str, discussion_post: object):
         updated_document.pop("_id", None)
 
         return updated_document['discussions']
+
+    def updated_discussion_post(self, discussion_post_id: str, discussion_content: str, analysis_name: str):
+        """ Edits a discussion post from an analysis to update the discussion post's content """
+
+        updated_document = self.collection.find_one_and_update({"name": analysis_name}, {
+            "$set": {"discussions.$[item].content": discussion_content}
+        },
+                                                               array_filters=[{"item.post_id": discussion_post_id}],
+                                                               return_document=ReturnDocument.AFTER)
+
+        updated_document.pop("_id", None)
+
+        return updated_document['discussions']
+
+    def delete_discussion_post(self, discussion_post_id: str, analysis_name: str):
+        """ Removes a discussion post from an analysis """
+
+        updated_document = self.collection.find_one_and_update({"name": analysis_name}, {
+            "$pull": {"discussions": {"post_id": discussion_post_id}}
+        },
+                                                               return_document=ReturnDocument.AFTER)
+
+        updated_document.pop("_id", None)
+
+        return updated_document['discussions']

backend/src/routers/analysis_discussion_router.py

@@ -6,43 +6,37 @@
 import logging
 from uuid import uuid4
 
-from fastapi import (APIRouter, Depends, Form, Security)
+from fastapi import (APIRouter, Depends, Form, Security, HTTPException, status)
 
 from ..dependencies import database
 from ..models.user import VerifyUser
+from ..models.analysis import Analysis
 from ..security.security import get_current_user
 
 logger = logging.getLogger(__name__)
 
 router = APIRouter(tags=["analysis"], dependencies=[Depends(database)])
 
 
-def mock_discussion_fixture():
-    """Mock dicussion fixture to fake the discussions being returned"""
-    return [{
-        "post_id": "90jkflsd8d-6298-4afb-add5-6ef710eb5e98", "author_id": "3bghhsmnyqi6uxovazy07ryn9q1tqbnt",
-        "author_fullname": "Hello Person", "publish_timestamp": "2023-10-09T21:13:22.687000",
-        "content": "Nulla diam mi, semper vitae.", "attachments": [], "thread": []
-    }, {
-        "post_id": "a677fdsfsacf8-4ff9-a406-b113a7952f7e", "author_id": "kw0g790fdx715xsr1ead2jk0pqubtlyz",
-        "author_fullname": "Developer Person", "publish_timestamp": "2023-10-10T21:13:22.687000",
-        "content": "Morbi laoreet justo.", "attachments": [], "thread": []
-    }, {
-        "post_id": "e602ffdsfa-fdsfdsa-9f42-862c826255ef", "author_id": "exqkhvidr7uh2ndslsdymbzfbmqjlunk",
-        "author_fullname": "Variant Review Report Preparer Person", "publish_timestamp": "2023-10-13T21:13:22.687000",
-        "content": "Mauris pretium sem at nunc sollicitudin also.", "attachments": [], "thread": []
-    }]
-
-
 @router.get("/{analysis_name}/discussions")
-def get_analysis_discussions(analysis_name: str):
-    """ Returns a list of genomic units for a given analysis """
+def get_analysis_discussions(analysis_name: str, repositories=Depends(database)):
+    """ Returns a list of discussion posts for a given analysis """
     logger.info("Retrieving the analysis '%s' discussions ", analysis_name)
-    return mock_discussion_fixture()
+
+    found_analysis = repositories['analysis'].find_by_name(analysis_name)
+
+    if not found_analysis:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND, detail=f"Analysis '{analysis_name}' does not exist.'"
+        )
+
+    analysis = Analysis(**found_analysis)
+
+    return analysis.discussions
 
 
 @router.post("/{analysis_name}/discussions")
-def add_analysis_discussions(
+def add_analysis_discussion(
     analysis_name: str,
     discussion_content: str = Form(...),
     repositories=Depends(database),
@@ -52,6 +46,14 @@ def add_analysis_discussions(
     logger.info("Adding the analysis '%s' from user '%s'", analysis_name, client_id)
     logger.info("The message: %s", discussion_content)
 
+    found_analysis = repositories['analysis'].find_by_name(analysis_name)
+
+    if not found_analysis:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND, detail=f"Analysis '{analysis_name}' does not exist.'"
+        )
+
+    analysis = Analysis(**found_analysis)
     current_user = repositories["user"].find_by_client_id(client_id)
 
     new_discussion_post = {
@@ -64,4 +66,74 @@ def add_analysis_discussions(
         "thread": [],
     }
 
-    return repositories['analysis'].add_discussion_post(analysis_name, new_discussion_post)
+    return repositories['analysis'].add_discussion_post(analysis.name, new_discussion_post)
+
+
+@router.put("/{analysis_name}/discussions/{discussion_post_id}")
+def update_analysis_discussion_post(
+    analysis_name: str,
+    discussion_post_id: str,
+    discussion_content: str = Form(...),
+    repositories=Depends(database),
+    client_id: VerifyUser = Security(get_current_user)
+):
+    """ Updates a discussion post's content in an analysis by the discussion post id """
+    logger.info(
+        "Editing post '%s' by user '%s' from the analysis '%s' with new content: '%s'", discussion_post_id, client_id,
+        analysis_name, discussion_content
+    )
+
+    found_analysis = repositories['analysis'].find_by_name(analysis_name)
+
+    if not found_analysis:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=f"Analysis '{analysis_name}' does not exist. Unable to update discussion post.'"
+        )
+
+    analysis = Analysis(**found_analysis)
+
+    try:
+        valid_post = analysis.find_authored_discussion_post(discussion_post_id, client_id)
+    except ValueError as e:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) from e
+
+    if not valid_post:
+        raise HTTPException(
+            status_code=status.HTTP_401_UNAUTHORIZED, detail="User cannot update post they did not author."
+        )
+
+    return repositories['analysis'].updated_discussion_post(valid_post['post_id'], discussion_content, analysis.name)
+
+
+@router.delete("/{analysis_name}/discussions/{discussion_post_id}")
+def delete_analysis_discussion(
+    analysis_name: str,
+    discussion_post_id: str,
+    repositories=Depends(database),
+    client_id: VerifyUser = Security(get_current_user)
+):
+    """ Deletes a discussion post in an analysis by the discussion post id """
+    logger.info("Deleting post %s by user '%s' from the analysis '%s'", discussion_post_id, client_id, analysis_name)
+
+    found_analysis = repositories['analysis'].find_by_name(analysis_name)
+
+    if not found_analysis:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=f"Analysis '{analysis_name}' does not exist. Unable to delete discussion post.'"
+        )
+
+    analysis = Analysis(**found_analysis)
+
+    try:
+        valid_post = analysis.find_authored_discussion_post(discussion_post_id, client_id)
+    except ValueError as e:
+        raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=str(e)) from e
+
+    if not valid_post:
+        raise HTTPException(
+            status_code=status.HTTP_401_UNAUTHORIZED, detail="User cannot delete post they did not author."
+        )
+
+    return repositories['analysis'].delete_discussion_post(valid_post['post_id'], analysis.name)

backend/tests/integration/conftest.py

@@ -74,7 +74,7 @@ def mock_current_user():
 @pytest.fixture(name="mock_user")
 def test_auth_user():
     """A mocked user that can be used to generate an OAuth2 access token"""
-    return {"sub": "johndoe", "scopes": ["read", "write"]}
+    return {"sub": "johndoe-client-id", "scopes": ["read", "write"]}
 
 
 @pytest.fixture(name="mock_access_token")

backend/tests/integration/test_analysis_routers.py

@@ -80,7 +80,7 @@ def test_import_analysis_with_phenotips_json(
     assert response.status_code == 200
     response_data = json.loads(response.text)
     assert response_data['latest_status'] == "Preparation"
-    assert response_data['timeline'][0]['username'] == 'johndoe'
+    assert response_data['timeline'][0]['username'] == 'johndoe-client-id'
 
 
 def test_update_analysis_section(client, mock_access_token, mock_repositories, update_analysis_section_response_json):
@@ -380,6 +380,7 @@ def valid_query_side_effect(*args, **kwargs):  # pylint: disable=unused-argument
         return analysis
 
     mock_repositories["user"].collection.find_one.return_value = {"full_name": new_post_user}
+    mock_repositories['analysis'].collection.find_one.return_value = cpam0002_analysis_json
     mock_repositories["analysis"].collection.find_one_and_update.side_effect = valid_query_side_effect
 
     response = client.post(
@@ -398,6 +399,141 @@ def valid_query_side_effect(*args, **kwargs):  # pylint: disable=unused-argument
     assert actual_most_recent_post['content'] == new_post_content
 
 
+def test_update_discussion_post_in_analysis(client, mock_access_token, mock_repositories, cpam0002_analysis_json):
+    """ Tests successfully updating an existing post in the discussions with the user being the author """
+    cpam_analysis = "CPAM0002"
+    discussion_post_id = "fake-post-id"
+    discussion_content = "I am an integration test post. Look at me!"
+
+    # Inject a new discussion post by John Doe
+    def valid_query_side_effect_one(*args, **kwargs):  # pylint: disable=unused-argument
+        analysis = cpam0002_analysis_json
+
+        new_discussion_post = {
+            "post_id": "fake-post-id", "author_id": "johndoe-client-id", "author_fullname": 'johndoe',
+            "content": "Hello, I am a discussion post."
+        }
+
+        analysis['discussions'].append(new_discussion_post)
+        analysis['_id'] = 'fake-mongo-object-id'
+        return analysis
+
+    def valid_query_side_effect_two(*args, **kwargs):  # pylint: disable=unused-argument
+        find, query = args  # pylint: disable=unused-variable
+        query_filter = kwargs
+
+        analysis = cpam0002_analysis_json
+        fake_post_content = query['$set']['discussions.$[item].content']
+        fake_post_id = query_filter['array_filters'][0]['item.post_id']
+
+        for d in analysis['discussions']:
+            if d['post_id'] == fake_post_id:
+                d['content'] = fake_post_content
+
+        analysis['_id'] = 'fake-mongo-object-id'
+
+        return analysis
+
+    mock_repositories['analysis'].collection.find_one.side_effect = valid_query_side_effect_one
+    mock_repositories["analysis"].collection.find_one_and_update.side_effect = valid_query_side_effect_two
+
+    response = client.put(
+        "/analysis/" + cpam_analysis + "/discussions/" + discussion_post_id,
+        headers={"Authorization": "Bearer " + mock_access_token},
+        data={"discussion_content": discussion_content}
+    )
+
+    actual_post = None
+
+    for d in response.json():
+        if d['post_id'] == discussion_post_id:
+            actual_post = d
+
+    assert len(response.json()) == 4
+    assert actual_post['content'] == discussion_content
+
+
+def test_update_post_in_analysis_author_mismatch(client, mock_access_token, mock_repositories, cpam0002_analysis_json):
+    """ Tests updating a post that the author did not post and results in an unauthorized failure """
+    cpam_analysis = "CPAM0002"
+    discussion_post_id = "9027ec8d-6298-4afb-add5-6ef710eb5e98"
+    discussion_content = "I am an integration test post. Look at me!"
+
+    mock_repositories['analysis'].collection.find_one.return_value = cpam0002_analysis_json
+
+    response = client.put(
+        "/analysis/" + cpam_analysis + "/discussions/" + discussion_post_id,
+        headers={"Authorization": "Bearer " + mock_access_token},
+        data={"discussion_content": discussion_content}
+    )
+
+    expected_failure_detail = {'detail': 'User cannot update post they did not author.'}
+
+    assert response.status_code == 401
+    assert response.json() == expected_failure_detail
+
+
+def test_delete_discussion_post_in_analysis(client, mock_access_token, mock_repositories, cpam0002_analysis_json):
+    """ Tests successfully deleting an existing post in the discussions with the user being the author """
+    cpam_analysis = "CPAM0002"
+    discussion_post_id = "fake-post-id"
+
+    # Inject a new discussion post by John Doe
+    def valid_query_side_effect_one(*args, **kwargs):  # pylint: disable=unused-argument
+        analysis = cpam0002_analysis_json
+
+        new_discussion_post = {
+            "post_id": "fake-post-id",
+            "author_id": "johndoe-client-id",
+            "author_fullname": 'johndoe',
+        }
+
+        analysis['discussions'].append(new_discussion_post)
+        analysis['_id'] = 'fake-mongo-object-id'
+        return analysis
+
+    def valid_query_side_effect_two(*args, **kwargs):  # pylint: disable=unused-argument
+        find, query = args  # pylint: disable=unused-variable
+
+        analysis = cpam0002_analysis_json
+        fake_post_id = query['$pull']['discussions']['post_id']
+
+        analysis['discussions'] = [x for x in analysis['discussions'] if fake_post_id not in x['post_id']]
+        analysis['_id'] = 'fake-mongo-object-id'
+
+        return analysis
+
+    mock_repositories['analysis'].collection.find_one.side_effect = valid_query_side_effect_one
+    mock_repositories["analysis"].collection.find_one_and_update.side_effect = valid_query_side_effect_two
+
+    response = client.delete(
+        "/analysis/" + cpam_analysis + "/discussions/" + discussion_post_id,
+        headers={"Authorization": "Bearer " + mock_access_token}
+    )
+
+    assert len(response.json()) == 3
+
+
+def test_handle_delete_post_not_existing_in_analysis(
+    client, mock_access_token, mock_repositories, cpam0002_analysis_json
+):
+    """ Tests failure of deleting a discussion post but does not exist in the analysis  """
+    cpam_analysis = "CPAM0002"
+    discussion_post_id = "fake-post-id"
+
+    mock_repositories['analysis'].collection.find_one.return_value = cpam0002_analysis_json
+
+    response = client.delete(
+        "/analysis/" + cpam_analysis + "/discussions/" + discussion_post_id,
+        headers={"Authorization": "Bearer " + mock_access_token}
+    )
+
+    expected_failure_detail = {'detail': f"Post '{discussion_post_id}' does not exist."}
+
+    assert response.status_code == 404
+    assert response.json() == expected_failure_detail
+
+
 @pytest.fixture(name="analysis_updates_json")
 def fixture_analysis_updates_json():
     """The JSON that is being sent from a client to the endpoint with updates in it"""