RHIDP-14000: update feedback unit and e2e tests

Signed-off-by: Jordan Dubrick <jdubrick@redhat.com>

Author: Jdubrick

tests/e2e/features/feedback.feature

@@ -310,4 +310,59 @@ Feature: feedback endpoint API tests
                         "cause": "Failed to store feedback at directory: /invalid"
                     }
         }
-        """
+        """
+
+  Scenario: Check if sequential feedback status toggling maintains consistency
+    When The feedback is enabled
+     Then The status code of the response is 200
+    When The feedback is disabled
+     Then The status code of the response is 200
+    When The feedback is enabled
+     Then The status code of the response is 200
+    When I retreive the current feedback status
+     Then The status code of the response is 200
+     And The body of the response is the following
+        """
+        {
+            "functionality": "feedback",
+            "status": {
+                        "enabled": true
+                        }
+        }
+        """
+
+  Scenario: Check if submitting duplicate feedback succeeds
+    And A new conversation is initialized
+    And The feedback is enabled
+     When I submit the following feedback for the conversation created before
+        """
+        {
+            "llm_response": "bar",
+            "sentiment": -1,
+            "user_feedback": "Not satisfied",
+            "user_question": "Sample Question"
+        }
+        """
+     Then The status code of the response is 200
+     And The body of the response is the following
+        """
+        {
+            "response": "feedback received"
+        }
+        """
+     When I submit the following feedback for the conversation created before
+        """
+        {
+            "llm_response": "bar",
+            "sentiment": -1,
+            "user_feedback": "Not satisfied",
+            "user_question": "Sample Question"
+        }
+        """
+     Then The status code of the response is 200
+     And The body of the response is the following
+        """
+        {
+            "response": "feedback received"
+        }
+        """

tests/unit/app/endpoints/test_feedback.py

@@ -2,6 +2,10 @@
 
 """Unit tests for the /feedback REST API endpoint."""
 
+import asyncio
+import json
+import threading
+from pathlib import Path
 from typing import Any
 
 import pytest
@@ -82,6 +86,34 @@ async def test_assert_feedback_enabled(mocker: MockerFixture) -> None:
     await assert_feedback_enabled(mocker.Mock())
 
 
+@pytest.mark.asyncio
+async def test_assert_feedback_enabled_disabled_full_config_chain(
+    mocker: MockerFixture,
+) -> None:
+    """Test the full config-to-exception chain when feedback is disabled.
+
+    Unlike test_assert_feedback_enabled_disabled (which mocks is_feedback_enabled),
+    this test sets up real UserDataCollection config and verifies the complete path:
+    configuration -> is_feedback_enabled() -> assert_feedback_enabled() -> HTTPException.
+    """
+    mock_config = AppConfig()
+    mock_config._configuration = mocker.Mock()
+    mock_config._configuration.user_data_collection = UserDataCollection(
+        feedback_enabled=False,
+        feedback_storage=None,
+        transcripts_enabled=False,
+        transcripts_storage=None,
+    )
+    mocker.patch("app.endpoints.feedback.configuration", mock_config)
+
+    with pytest.raises(HTTPException) as exc_info:
+        await assert_feedback_enabled(mocker.Mock())
+
+    assert exc_info.value.status_code == status.HTTP_403_FORBIDDEN
+    assert exc_info.value.detail["response"] == "Storing feedback is disabled"  # type: ignore
+    assert exc_info.value.detail["cause"] == "Storing feedback is disabled."  # type: ignore
+
+
 @pytest.mark.parametrize(
     "feedback_request_data",
     [
@@ -263,9 +295,48 @@ def test_store_feedback_on_io_error(
     assert "Failed to store feedback at directory" in detail["cause"]  # type: ignore
 
 
+def test_store_feedback_real_filesystem(tmp_path: Path, mocker: MockerFixture) -> None:
+    """Test that store_feedback writes valid JSON to the real filesystem.
+
+    Unlike test_store_feedback (which mocks builtins.open, Path, and json.dump),
+    this test exercises real filesystem I/O via tmp_path to verify actual file
+    creation and valid JSON content.
+    """
+    configuration.user_data_collection_configuration.feedback_storage = str(tmp_path)
+
+    fake_uuid = "test-feedback-uuid"
+    mocker.patch("app.endpoints.feedback.get_suid", return_value=fake_uuid)
+
+    feedback_data = {
+        "conversation_id": "12345678-abcd-0000-0123-456789abcdef",
+        "user_question": "What is Kubernetes?",
+        "llm_response": "Kubernetes is a container orchestration platform.",
+        "sentiment": 1,
+    }
+
+    store_feedback("test_user_id", feedback_data)
+
+    feedback_file = tmp_path / f"{fake_uuid}.json"
+    assert feedback_file.exists(), f"Expected file {feedback_file} to exist"
+
+    with open(feedback_file, encoding="utf-8") as f:
+        stored_data = json.load(f)
+
+    assert stored_data["user_id"] == "test_user_id"
+    assert "timestamp" in stored_data
+    assert stored_data["conversation_id"] == "12345678-abcd-0000-0123-456789abcdef"
+    assert stored_data["user_question"] == "What is Kubernetes?"
+    assert (
+        stored_data["llm_response"]
+        == "Kubernetes is a container orchestration platform."
+    )
+    assert stored_data["sentiment"] == 1
+
+
 @pytest.mark.asyncio
 async def test_update_feedback_status_different(mocker: MockerFixture) -> None:
     """Test that update_feedback_status returns the correct status with an update."""
+    mock_authorization_resolvers(mocker)
     configuration.user_data_collection_configuration.feedback_enabled = True
 
     # Authorization tuple required by URL endpoint handler
@@ -287,6 +358,7 @@ async def test_update_feedback_status_different(mocker: MockerFixture) -> None:
 @pytest.mark.asyncio
 async def test_update_feedback_status_no_change(mocker: MockerFixture) -> None:
     """Test that update_feedback_status returns the correct status with no update."""
+    mock_authorization_resolvers(mocker)
     configuration.user_data_collection_configuration.feedback_enabled = True
 
     # Authorization tuple required by URL endpoint handler
@@ -305,6 +377,41 @@ async def test_update_feedback_status_no_change(mocker: MockerFixture) -> None:
     }
 
 
+def test_update_feedback_status_concurrent(mocker: MockerFixture) -> None:
+    """Test that concurrent calls to update_feedback_status do not raise errors."""
+    mock_authorization_resolvers(mocker)
+    configuration.user_data_collection_configuration.feedback_enabled = True
+
+    auth: AuthTuple = ("test_user_id", "test_user", True, "test_token")
+    results: list[Any] = [None, None, None]
+    errors: list[Exception | None] = [None, None, None]
+
+    def worker(index: int, desired_status: bool) -> None:
+        """Thread worker that calls update_feedback_status."""
+        req = FeedbackStatusUpdateRequest(status=desired_status)
+        try:
+            results[index] = asyncio.run(update_feedback_status(req, auth=auth))
+        except Exception as exc:  # pylint: disable=broad-exception-caught
+            errors[index] = exc
+
+    threads = [
+        threading.Thread(target=worker, args=(0, False)),
+        threading.Thread(target=worker, args=(1, True)),
+        threading.Thread(target=worker, args=(2, False)),
+    ]
+    for t in threads:
+        t.start()
+    for t in threads:
+        t.join()
+
+    for i, err in enumerate(errors):
+        assert err is None, f"Thread {i} raised: {err}"
+    for i, result in enumerate(results):
+        assert result is not None, f"Thread {i} returned None"
+        assert "previous_status" in result.status
+        assert "updated_status" in result.status
+
+
 @pytest.mark.parametrize(
     "payload",
     [