MPT-18375 Add helpdesk chat answers api with unit and e2e test coverage#247
Merged
MPT-18375 Add helpdesk chat answers api with unit and e2e test coverage#247
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
jentyk
changed the title
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
tests/e2e/helpdesk/chats/answers/parameters/conftest.py (1)
14-16: Fixture naming does not match the identifier expected by.parameters(...).The accessor takes an
answer_id, but the fixture is named/value-shaped like a parameter id (PAR-*). Rename it to avoid confusion (or reuse the existinginvalid_chat_answer_idfixture).🔧 Suggested rename
`@pytest.fixture` -def invalid_chat_answer_parameter_id(): - return "PAR-0000-0000" +def invalid_chat_answer_id(): + return "ANS-0000-0000"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/helpdesk/chats/answers/parameters/conftest.py` around lines 14 - 16, The fixture invalid_chat_answer_parameter_id is misnamed for the accessor that expects an answer_id via .parameters(...); rename the fixture to invalid_chat_answer_id (or reuse the existing invalid_chat_answer_id fixture) so its name and returned value match the expected identifier used by the tests and the .parameters(...) accessor (update any references to invalid_chat_answer_parameter_id accordingly).tests/e2e/helpdesk/chats/answers/parameters/test_sync_parameters.py (1)
11-15: Strengthen the collection assertion to avoid a tautology.
assert len(result) >= 0always passes. Consider asserting element shape/identity when data exists.🔧 Suggested assertion improvement
def test_list_chat_answer_parameters(chat_answer_parameters_service): result = chat_answer_parameters_service.fetch_page(limit=20) - assert len(result) >= 0 + assert all(item.id is not None for item in result)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/helpdesk/chats/answers/parameters/test_sync_parameters.py` around lines 11 - 15, The test test_list_chat_answer_parameters currently asserts a tautology (len(result) >= 0); change it to assert the collection type and meaningful contents: call chat_answer_parameters_service.fetch_page(limit=20), assert isinstance(result, list) (or that result is iterable) and then if result: inspect the first item for expected keys/attributes (e.g., assert "id" in result[0] and "name" in result[0] or use the model's attributes) to verify element shape/identity; keep references to test_list_chat_answer_parameters and chat_answer_parameters_service.fetch_page(limit=20) when updating the assertions.tests/unit/resources/helpdesk/test_chat_answers.py (1)
103-146: Consider adding payload branch assertions for custom actions.Current tests only cover the no-body call path. Adding a payload case would lock in request serialization behavior for
submit/accept/query/validate.🔧 Example extension (sync pattern; mirror for async)
-@pytest.mark.parametrize("action", ["submit", "accept", "query", "validate"]) -def test_custom_resource_actions(chat_answers_service, action): +@pytest.mark.parametrize( + ("action", "payload", "expected_content"), + [ + ("submit", None, b""), + ("accept", {"reason": "ok"}, b'{"reason":"ok"}'), + ("query", {"reason": "need details"}, b'{"reason":"need details"}'), + ("validate", {"status": "validated"}, b'{"status":"validated"}'), + ], +) +def test_custom_resource_actions(chat_answers_service, action, payload, expected_content): response_expected_data = {"id": "ANS-1234-5678", "status": "Updated"} with respx.mock: mock_route = respx.post( f"https://api.example.com/public/v1/helpdesk/chats/CHT-0000-0000-0001/answers/ANS-1234-5678/{action}" ).mock( return_value=httpx.Response( status_code=httpx.codes.OK, headers={"content-type": "application/json"}, json=response_expected_data, ) ) - result = getattr(chat_answers_service, action)("ANS-1234-5678") + result = getattr(chat_answers_service, action)("ANS-1234-5678", payload) assert mock_route.call_count == 1 request = mock_route.calls[0].request - assert request.content == b"" + assert request.content == expected_content assert result.to_dict() == response_expected_data assert isinstance(result, ChatAnswer)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/unit/resources/helpdesk/test_chat_answers.py` around lines 103 - 146, Add a second parametrized branch to both test_custom_resource_actions and test_async_custom_resource_actions that sends a non-empty payload to the custom actions (e.g., payload = {"note": "foo", "meta": {"k": "v"}}) and asserts the request body is serialized correctly: ensure mock_route.call_count == 1, inspect mock_route.calls[0].request.content or .read() to match the JSON bytes of the payload and/or parse the request.json() to assert keys/values, and keep asserting the response maps to ChatAnswer; use getattr(chat_answers_service, action)(answer_id, payload) and await getattr(async_chat_answers_service, action)(answer_id, payload) for the async test.tests/e2e/helpdesk/chats/answers/parameters/test_async_parameters.py (1)
11-15: Improve the async list assertion signal.
assert len(result) >= 0is always true. Prefer validating returned item structure when present.🔧 Suggested assertion improvement
async def test_list_chat_answer_parameters(async_chat_answer_parameters_service): result = await async_chat_answer_parameters_service.fetch_page(limit=20) - assert len(result) >= 0 + assert all(item.id is not None for item in result)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/e2e/helpdesk/chats/answers/parameters/test_async_parameters.py` around lines 11 - 15, Replace the tautological assertion in test_list_chat_answer_parameters: ensure the return from async_chat_answer_parameters_service.fetch_page is a list and, when non-empty, validate the structure of at least one item (e.g., isinstance(result, list); if result: assert item is dict/object and contains expected fields such as id and name/value or match the ChatAnswerParameter schema). Locate the test_list_chat_answer_parameters function and update its assertions to check type and representative item keys/attributes instead of using assert len(result) >= 0.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/e2e/helpdesk/chats/answers/parameters/conftest.py`:
- Around line 14-16: The fixture invalid_chat_answer_parameter_id is misnamed
for the accessor that expects an answer_id via .parameters(...); rename the
fixture to invalid_chat_answer_id (or reuse the existing invalid_chat_answer_id
fixture) so its name and returned value match the expected identifier used by
the tests and the .parameters(...) accessor (update any references to
invalid_chat_answer_parameter_id accordingly).
In `@tests/e2e/helpdesk/chats/answers/parameters/test_async_parameters.py`:
- Around line 11-15: Replace the tautological assertion in
test_list_chat_answer_parameters: ensure the return from
async_chat_answer_parameters_service.fetch_page is a list and, when non-empty,
validate the structure of at least one item (e.g., isinstance(result, list); if
result: assert item is dict/object and contains expected fields such as id and
name/value or match the ChatAnswerParameter schema). Locate the
test_list_chat_answer_parameters function and update its assertions to check
type and representative item keys/attributes instead of using assert len(result)
>= 0.
In `@tests/e2e/helpdesk/chats/answers/parameters/test_sync_parameters.py`:
- Around line 11-15: The test test_list_chat_answer_parameters currently asserts
a tautology (len(result) >= 0); change it to assert the collection type and
meaningful contents: call chat_answer_parameters_service.fetch_page(limit=20),
assert isinstance(result, list) (or that result is iterable) and then if result:
inspect the first item for expected keys/attributes (e.g., assert "id" in
result[0] and "name" in result[0] or use the model's attributes) to verify
element shape/identity; keep references to test_list_chat_answer_parameters and
chat_answer_parameters_service.fetch_page(limit=20) when updating the
assertions.
In `@tests/unit/resources/helpdesk/test_chat_answers.py`:
- Around line 103-146: Add a second parametrized branch to both
test_custom_resource_actions and test_async_custom_resource_actions that sends a
non-empty payload to the custom actions (e.g., payload = {"note": "foo", "meta":
{"k": "v"}}) and asserts the request body is serialized correctly: ensure
mock_route.call_count == 1, inspect mock_route.calls[0].request.content or
.read() to match the JSON bytes of the payload and/or parse the request.json()
to assert keys/values, and keep asserting the response maps to ChatAnswer; use
getattr(chat_answers_service, action)(answer_id, payload) and await
getattr(async_chat_answers_service, action)(answer_id, payload) for the async
test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 11417695-9238-4321-af84-0743975d7dd5
📒 Files selected for processing (14)
mpt_api_client/resources/helpdesk/chat_answer_parameters.pympt_api_client/resources/helpdesk/chat_answers.pympt_api_client/resources/helpdesk/chats.pytests/e2e/helpdesk/chats/answers/__init__.pytests/e2e/helpdesk/chats/answers/conftest.pytests/e2e/helpdesk/chats/answers/parameters/__init__.pytests/e2e/helpdesk/chats/answers/parameters/conftest.pytests/e2e/helpdesk/chats/answers/parameters/test_async_parameters.pytests/e2e/helpdesk/chats/answers/parameters/test_sync_parameters.pytests/e2e/helpdesk/chats/answers/test_async_answers.pytests/e2e/helpdesk/chats/answers/test_sync_answers.pytests/unit/resources/helpdesk/test_chat_answer_parameters.pytests/unit/resources/helpdesk/test_chat_answers.pytests/unit/resources/helpdesk/test_chats.py
robcsegal
approved these changes
Mar 24, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/public/v1/helpdesk/chats/{chat_id}/answerssubmit,accept,query,validate/public/v1/helpdesk/chats/{chat_id}/answers/{answer_id}/parameterswith list-only accessanswers(chat_id)into chat services and addparameters(answer_id)under answersTesting
uv run pytest tests/unit/resources/helpdesk/test_chats.py tests/unit/resources/helpdesk/test_chat_answers.py tests/unit/resources/helpdesk/test_chat_answer_parameters.pymake check-allCloses MPT-18375
Release Notes
ChatAnswerresource model with synchronous and asynchronous services for the/public/v1/helpdesk/chats/{chat_id}/answersendpointsubmit(),accept(),query(), andvalidate()with optional payload supportChatAnswerParameterresource model with synchronous and asynchronous services for the/public/v1/helpdesk/chats/{chat_id}/answers/{answer_id}/parametersendpoint (collection operations only)parameters(answer_id)accessor method on chat answers services to access answer-specific parametersanswers(chat_id)factory method on chats services to create chat answers service instances