fix: differences in forum v1 and v2 get responses (#83)

* fix: differences in forum v1 and v2 get responses
- fix the response of get thread API according to query params
when thread type is question
- fix the response of get thread API when answered question is marked
back as not answered when thread type is question.
- fix the response of user's subscribed thread API as it wasn't deleting
the subscriptions of the thread when thread was deleted, and was returning
more subscribed threads in the response
- add tests for the above cases
- close #47

* fix: match responses for abuse_count_flag
- return abuse_count_flag in get_threads
- pop historical_abuse_flaggers from comment and thread serializer

---------

Co-authored-by: Muhammad Faraz  Maqsood <faraz.maqsood@a006-01130.home>

forum/models/model_utils.py

@@ -875,6 +875,16 @@ def delete_comments_of_a_thread(thread_id: str) -> None:
         Comment().delete(comment["_id"])
 
 
+def delete_subscriptions_of_a_thread(thread_id: str) -> None:
+    """Delete subscriptions of a thread."""
+    for subscription in Subscriptions().get_list(
+        source_id=thread_id, source_type="CommentThread"
+    ):
+        Subscriptions().delete_subscription(
+            subscription["subscriber_id"], subscription["source_id"]
+        )
+
+
 def validate_params(
     params: dict[str, Any], user_id: Optional[str] = None
 ) -> Response | None:
@@ -935,9 +945,9 @@ def get_threads(
     user_id: str,
     serializer: Any,
     thread_ids: list[str],
-    include_context: Optional[bool] = False,
 ) -> dict[str, Any]:
     """get subscribed or all threads of a specific course for a specific user."""
+    count_flagged = bool(params.get("count_flagged", False))
     threads = handle_threads_query(
         thread_ids,
         user_id,
@@ -949,19 +959,18 @@ def get_threads(
         bool(params.get("unread", False)),
         bool(params.get("unanswered", False)),
         bool(params.get("unresponded", False)),
-        bool(params.get("count_flagged", False)),
+        count_flagged,
         params.get("sort_key", ""),
         int(params.get("page", 1)),
         int(params.get("per_page", 100)),
     )
-    context: dict[str, Any] = {}
-    if include_context:
-        context = {
-            "include_endorsed": True,
-            "include_read_state": True,
-        }
-        if user_id:
-            context["user_id"] = user_id
+    context: dict[str, Any] = {
+        "count_flagged": count_flagged,
+        "include_endorsed": True,
+        "include_read_state": True,
+    }
+    if user_id:
+        context["user_id"] = user_id
     serializer = serializer(threads.pop("collection"), many=True, context=context)
     threads["collection"] = serializer.data
     return threads

forum/serializers/comment.py

@@ -7,7 +7,7 @@
 from bson import ObjectId
 from rest_framework import serializers
 
-from forum.models import Comment
+from forum.models import Comment, CommentThread
 from forum.serializers.contents import ContentSerializer
 from forum.serializers.custom_datetime import CustomDateTimeField
 from forum.utils import prepare_comment_data_for_get_children
@@ -89,8 +89,20 @@ def get_children(self, obj: Any) -> list[dict[str, Any]]:
 
     def to_representation(self, instance: Any) -> dict[str, Any]:
         comment = super().to_representation(instance)
+        comment.pop("historical_abuse_flaggers")
         if comment["parent_id"] == "None":
             comment["parent_id"] = None
+
+        thread = CommentThread().get(comment["thread_id"])
+        comment_from_db = Comment().get(comment["id"])
+        if (
+            not comment["endorsed"]
+            and comment_from_db
+            and "endorsement" not in comment_from_db
+            and thread
+            and thread["thread_type"] == "question"
+        ):
+            comment.pop("endorsement", None)
         return comment
 
     def create(self, validated_data: dict[str, Any]) -> Any:

forum/serializers/thread.py

@@ -258,10 +258,30 @@ def to_representation(self, instance: dict[str, Any]) -> dict[str, Any]:
         """
         data = super().to_representation(instance)
         data.pop("closed_by_id")
+        data.pop("historical_abuse_flaggers")
         if not data.get("abuse_flagged_count", None):
             data.pop("abuse_flagged_count", None)
         if not data.get("closed_by", None):
             data.pop("close_reason_code", None)
+        if (
+            self.with_responses
+            and (
+                not ("recursive" in self.context_data)
+                or self.context_data.get("recursive") is True
+            )
+            and data.get("thread_type") == "question"
+        ):
+            children = data.pop("children")
+            data["non_endorsed_responses"] = []
+            data["endorsed_responses"] = []
+            for child in children:
+                if child["endorsed"]:
+                    data["endorsed_responses"].append(child)
+                else:
+                    data["non_endorsed_responses"].append(child)
+
+            data["non_endorsed_resp_total"] = len(data["non_endorsed_responses"])
+
         return data
 
     def create(self, validated_data: dict[str, Any]) -> Any:

forum/utils.py

@@ -50,7 +50,7 @@ def handle_proxy_requests(request: HttpRequest, suffix: str, method: str) -> Res
     )
 
 
-def str_to_bool(value: str | bool) -> bool:
+def str_to_bool(value: Any) -> bool:
     """Convert str to bool."""
     if isinstance(value, bool):
         return value

forum/views/threads.py

@@ -14,6 +14,7 @@
 from forum.models.comments import Comment
 from forum.models.model_utils import (
     delete_comments_of_a_thread,
+    delete_subscriptions_of_a_thread,
     get_threads,
     mark_as_read,
     update_stats_for_course,
@@ -68,21 +69,16 @@ def prepare_thread_api_response(
                 thread_data["resp_limit"] = get_int_value_from_collection(
                     data_or_params, "resp_limit", 100
                 )
-                context["recursive"] = str_to_bool(
-                    data_or_params.get("recursive", "False")
-                )
-                context["with_responses"] = str_to_bool(
-                    data_or_params.get("with_responses", "True")
-                )
-                context["mark_as_read"] = str_to_bool(
-                    data_or_params.get("mark_as_read", "False")
-                )
-                context["reverse_order"] = str_to_bool(
-                    data_or_params.get("reverse_order", "True")
-                )
-                context["merge_question_type_responses"] = str_to_bool(
-                    data_or_params.get("merge_question_type_responses", "False")
-                )
+                params = [
+                    "recursive",
+                    "with_responses",
+                    "mark_as_read",
+                    "reverse_order",
+                    "merge_question_type_responses",
+                ]
+                for param in params:
+                    if param in data_or_params:
+                        context[param] = str_to_bool(data_or_params.get(param))
                 if user_id and (user := Users().get(user_id)):
                     mark_as_read(user, thread)
 
@@ -168,6 +164,7 @@ def delete(self, request: Request, thread_id: str) -> Response:
                 status=status.HTTP_500_INTERNAL_SERVER_ERROR,
             )
         result = CommentThread().delete(thread_id)
+        delete_subscriptions_of_a_thread(thread_id)
         if result:
             update_stats_for_course(
                 thread["author_id"], thread["course_id"], threads=-1
@@ -358,5 +355,5 @@ def get(self, request: Request) -> Response:
         }
         filtered_threads = CommentThread().find(thread_filter)
         thread_ids = [thread["_id"] for thread in filtered_threads]
-        threads = get_threads(params, user_id, ThreadSerializer, thread_ids, True)
+        threads = get_threads(params, user_id, ThreadSerializer, thread_ids)
         return Response(data=threads, status=status.HTTP_200_OK)

tests/test_views/test_flags.py

@@ -162,8 +162,11 @@ def test_comment_flag_api_with_all_param(api_client: APIClient) -> None:
 
     assert response.status_code == 200
     flagged_comment = response.json()
-    assert flagged_comment["abuse_flaggers"] == [flag_user, flag_user_2]
-    assert flagged_comment["historical_abuse_flaggers"] == []
+    assert flagged_comment is not None
+    comment = Comment().get(flagged_comment["id"])
+    assert comment is not None
+    assert comment["abuse_flaggers"] == [flag_user, flag_user_2]
+    assert comment["historical_abuse_flaggers"] == []
 
     response = api_client.put_json(
         path=f"/api/v2/comments/{comment_id}/abuse_unflag",
@@ -173,10 +176,10 @@ def test_comment_flag_api_with_all_param(api_client: APIClient) -> None:
     assert response.status_code == 200
     unflagged_comment = response.json()
     assert unflagged_comment is not None
-    assert unflagged_comment["abuse_flaggers"] == []
-    assert set(unflagged_comment["historical_abuse_flaggers"]) == set(
-        [flag_user, flag_user_2]
-    )
+    comment = Comment().get(unflagged_comment["id"])
+    assert comment is not None
+    assert comment["abuse_flaggers"] == []
+    assert set(comment["historical_abuse_flaggers"]) == set([flag_user, flag_user_2])
 
     # Test thread abuse and unabuse.
     response = api_client.put_json(
@@ -186,8 +189,11 @@ def test_comment_flag_api_with_all_param(api_client: APIClient) -> None:
 
     assert response.status_code == 200
     flagged_thread = response.json()
-    assert flagged_thread["abuse_flaggers"] == [flag_user]
-    assert flagged_thread["historical_abuse_flaggers"] == []
+    assert flagged_thread is not None
+    thread = CommentThread().get(flagged_thread["id"])
+    assert thread is not None
+    assert thread["abuse_flaggers"] == [flag_user]
+    assert thread["historical_abuse_flaggers"] == []
 
     response = api_client.put_json(
         path=f"/api/v2/threads/{comment_thread_id}/abuse_unflag",
@@ -197,5 +203,7 @@ def test_comment_flag_api_with_all_param(api_client: APIClient) -> None:
     assert response.status_code == 200
     unflagged_thread = response.json()
     assert unflagged_thread is not None
-    assert unflagged_thread["abuse_flaggers"] == []
-    assert unflagged_thread["historical_abuse_flaggers"] == [flag_user]
+    thread = CommentThread().get(unflagged_thread["id"])
+    assert thread is not None
+    assert thread["abuse_flaggers"] == []
+    assert thread["historical_abuse_flaggers"] == [flag_user]