move to PUT endpoint to set feedback status

Signed-off-by: Jordan Dubrick <[email protected]>

Author: Jdubrick

docs/openapi.json

@@ -386,21 +386,19 @@
                         }
                     }
                 }
-            }
-        },
-        "/v1/feedback/toggle": {
-            "post": {
+            },
+            "put": {
                 "tags": [
                     "feedback"
                 ],
-                "summary": "Feedback Toggle",
-                "description": "Handle feedback toggle requests.\n\nReturns the current enbaled status of the feedback functionality after it has been\nupdated to the desired value from the request.\n\nReturns:\n    StatusResponse: Indicates whether feedback is enabled.",
-                "operationId": "feedback_toggle_v1_feedback_toggle_post",
+                "summary": "Update Feedback Status",
+                "description": "Handle feedback status update requests.\n\nTakes a request with the desired state of the feedback status.\nReturns the updated state of the feedback status based on the request's value.\n\nReturns:\n    StatusResponse: Indicates whether feedback is enabled.",
+                "operationId": "update_feedback_status_v1_feedback_status_put",
                 "requestBody": {
                     "content": {
                         "application/json": {
                             "schema": {
-                                "$ref": "#/components/schemas/FeedbackToggleRequest"
+                                "$ref": "#/components/schemas/FeedbackStatusUpdateRequest"
                             }
                         }
                     },
@@ -412,7 +410,7 @@
                         "content": {
                             "application/json": {
                                 "schema": {
-                                    "$ref": "#/components/schemas/StatusResponse"
+                                    "$ref": "#/components/schemas/FeedbackStatusUpdateResponse"
                                 }
                             }
                         }
@@ -1488,7 +1486,7 @@
                     }
                 ]
             },
-            "FeedbackToggleRequest": {
+            "FeedbackStatusUpdateRequest": {
                 "properties": {
                     "status": {
                         "type": "boolean",
@@ -1502,8 +1500,32 @@
                     }
                 },
                 "type": "object",
-                "title": "FeedbackToggleRequest",
-                "description": "Model representing a feedback toggle request.\n\nAttributes:\n    status: Boolean controlling what the Feedback status should be.\n\nExample:\n    ```python\n    feedback_request = FeedbackRequest(\n        status=false\n    )\n    ```"
+                "title": "FeedbackStatusUpdateRequest",
+                "description": "Model representing a feedback status update request.\n\nAttributes:\n    status: Value of the desired feedback enabled state.\n\nExample:\n    ```python\n    feedback_request = FeedbackRequest(\n        status=false\n    )\n    ```"
+            },
+            "FeedbackStatusUpdateResponse": {
+                "properties": {
+                    "status": {
+                        "additionalProperties": true,
+                        "type": "object",
+                        "title": "Status"
+                    }
+                },
+                "type": "object",
+                "required": [
+                    "status"
+                ],
+                "title": "FeedbackStatusUpdateResponse",
+                "description": "Model representing a response to a feedback status update request.\n\nAttributes:\n    status: The previous and current status of the service and who updated it.\n\nExample:\n    ```python\n    status_response = StatusResponse(\n        status={\n            \"previous_status\": true,\n            \"updated_status\": false,\n            \"updated_by\": \"user/test\"\n        },\n    )\n    ```",
+                "examples": [
+                    {
+                        "status": {
+                            "previous_status": true,
+                            "updated_by": "user/test",
+                            "updated_status": false
+                        }
+                    }
+                ]
             },
             "ForbiddenResponse": {
                 "properties": {

src/app/endpoints/feedback.py

@@ -1,6 +1,7 @@
 """Handler for REST API endpoint for user feedback."""
 
 import logging
+import threading
 from typing import Annotated, Any
 from pathlib import Path
 import json
@@ -12,20 +13,21 @@
 from authorization.middleware import authorize
 from configuration import configuration
 from models.config import Action
-from models.requests import FeedbackRequest
+from models.requests import FeedbackRequest, FeedbackStatusUpdateRequest
 from models.responses import (
     ErrorResponse,
     FeedbackResponse,
+    FeedbackStatusUpdateResponse,
     StatusResponse,
     UnauthorizedResponse,
     ForbiddenResponse,
 )
-from models.requests import FeedbackToggleRequest
 from utils.suid import get_suid
 
 logger = logging.getLogger(__name__)
 router = APIRouter(prefix="/feedback", tags=["feedback"])
 auth_dependency = get_auth_dependency()
+feedback_status_lock = threading.Lock()
 
 # Response for the feedback endpoint
 feedback_response: dict[int | str, dict[str, Any]] = {
@@ -177,33 +179,39 @@ def feedback_status() -> StatusResponse:
     )
 
 
-@router.post("/toggle")
-def feedback_toggle(feedback_toggle_request: FeedbackToggleRequest) -> StatusResponse:
+@router.put("/status")
+@authorize(Action.ADMIN)
+async def update_feedback_status(
+    feedback_update_request: FeedbackStatusUpdateRequest,
+    auth: Annotated[AuthTuple, Depends(auth_dependency)],
+) -> FeedbackStatusUpdateResponse:
     """
-    Handle feedback toggle requests.
+    Handle feedback status update requests.
 
-    Returns the current enbaled status of the feedback functionality after it has been
-    updated to the desired value from the request.
+    Takes a request with the desired state of the feedback status.
+    Returns the updated state of the feedback status based on the request's value.
 
     Returns:
         StatusResponse: Indicates whether feedback is enabled.
     """
-    fetched_status = toggle_feedback_status(feedback_toggle_request)
-    return StatusResponse(functionality="feedback", status={"enabled": fetched_status})
-
-
-def toggle_feedback_status(req: FeedbackToggleRequest) -> bool:
-    """
-    Toggles the feedback boolean stored in the configuration.
-
-    This toggling is temporary until the service is interrupted as it will not edit
-    any configuration files.
+    user_id, _, _ = auth
+    requested_status = feedback_update_request.get_value()
 
-    Parameters:
-        req (FeedbackToggleRequest): The request received with a boolean for the new value.
+    with feedback_status_lock:
+        previous_status = (
+            configuration.user_data_collection_configuration.feedback_enabled
+        )
+        configuration.user_data_collection_configuration.feedback_enabled = (
+            requested_status
+        )
+        updated_status = (
+            configuration.user_data_collection_configuration.feedback_enabled
+        )
 
-    Returns:
-        bool: The current enabled status of feedback.
-    """
-    configuration.user_data_collection_configuration.feedback_enabled = req.get_value()
-    return is_feedback_enabled()
+    return FeedbackStatusUpdateResponse(
+        status={
+            "previous_status": previous_status,
+            "updated_status": updated_status,
+            "updated_by": user_id,
+        }
+    )

src/models/requests.py

@@ -382,11 +382,11 @@ def check_feedback_provided(self) -> Self:
         return self
 
 
-class FeedbackToggleRequest(BaseModel):
-    """Model representing a feedback toggle request.
+class FeedbackStatusUpdateRequest(BaseModel):
+    """Model representing a feedback status update request.
 
     Attributes:
-        status: Boolean controlling what the Feedback status should be.
+        status: Value of the desired feedback enabled state.
 
     Example:
         ```python

src/models/responses.py

@@ -570,3 +570,39 @@ class ErrorResponse(BaseModel):
             ]
         }
     }
+
+
+class FeedbackStatusUpdateResponse(BaseModel):
+    """Model representing a response to a feedback status update request.
+
+    Attributes:
+        status: The previous and current status of the service and who updated it.
+
+    Example:
+        ```python
+        status_response = StatusResponse(
+            status={
+                "previous_status": true,
+                "updated_status": false,
+                "updated_by": "user/test"
+            },
+        )
+        ```
+    """
+
+    status: dict
+
+    # provides examples for /docs endpoint
+    model_config = {
+        "json_schema_extra": {
+            "examples": [
+                {
+                    "status": {
+                        "previous_status": True,
+                        "updated_status": False,
+                        "updated_by": "user/test",
+                    },
+                }
+            ]
+        }
+    }

tests/unit/app/endpoints/test_feedback.py

@@ -9,10 +9,9 @@
     assert_feedback_enabled,
     feedback_endpoint_handler,
     store_feedback,
-    feedback_status,
-    feedback_toggle,
+    update_feedback_status,
 )
-from models.requests import FeedbackToggleRequest
+from models.requests import FeedbackStatusUpdateRequest
 from tests.unit.utils.auth_helpers import mock_authorization_resolvers
 
 
@@ -198,52 +197,33 @@ def test_store_feedback_on_io_error(mocker, feedback_request_data):
         store_feedback(user_id, feedback_request_data)
 
 
-def test_feedback_status():
-    """Test that feedback_status returns the correct status response."""
+async def test_update_feedback_status_different():
+    """Test that update_feedback_status returns the correct status with an update."""
     configuration.user_data_collection_configuration.feedback_enabled = True
 
-    response = feedback_status()
-    assert response.functionality == "feedback"
-    assert response.status == {"enabled": True}
-
-
-def test_feedback_toggle_enabled():
-    """Test that feedback_toggle processes feedback toggle for disabled status payloads."""
-
-    configuration.user_data_collection_configuration.feedback_enabled = True
-
-    feedback_toggle_request = FeedbackToggleRequest(status=False)
-
-    response = feedback_toggle(feedback_toggle_request=feedback_toggle_request)
-
-    assert response.functionality == "feedback"
-    assert response.status == {"enabled": False}
-
-
-def test_feedback_toggle_disabled():
-    """Test that feedback_toggle processes feedback toggle for enabled status payloads."""
-
-    configuration.user_data_collection_configuration.feedback_enabled = False
-
-    feedback_toggle_request = FeedbackToggleRequest(status=True)
-
-    response = feedback_toggle(feedback_toggle_request=feedback_toggle_request)
-
-    assert response.functionality == "feedback"
-    assert response.status == {"enabled": True}
-
+    req = FeedbackStatusUpdateRequest(status=False)
+    resp = await update_feedback_status(
+        req,
+        auth=("test_user_id", "test_username", "test_token"),
+    )
+    assert resp.status == {
+        "previous_status": True,
+        "updated_status": False,
+        "updated_by": "test_user_id",
+    }
 
-def test_feedback_toggle_equivalent():
-    """
-    Test that feedback_toggle processes feedback toggle for status payloads with the same value
-    as what is presently set.
-    """
 
+async def test_update_feedback_status_no_change():
+    """Test that update_feedback_status returns the correct status with no update."""
     configuration.user_data_collection_configuration.feedback_enabled = True
 
-    feedback_toggle_request = FeedbackToggleRequest(status=True)
-
-    response = feedback_toggle(feedback_toggle_request=feedback_toggle_request)
-
-    assert response.functionality == "feedback"
-    assert response.status == {"enabled": True}
+    req = FeedbackStatusUpdateRequest(status=True)
+    resp = await update_feedback_status(
+        req,
+        auth=("test_user_id", "test_username", "test_token"),
+    )
+    assert resp.status == {
+        "previous_status": True,
+        "updated_status": True,
+        "updated_by": "test_user_id",
+    }