Skip to content

make sure OpenAPI contract says there is a 422 return exception and i…#666

Open
ramonavic wants to merge 1 commit intomainfrom
task/small-fixes-answer-flow
Open

make sure OpenAPI contract says there is a 422 return exception and i…#666
ramonavic wants to merge 1 commit intomainfrom
task/small-fixes-answer-flow

Conversation

@ramonavic
Copy link
Contributor

…mprove tests

Meldingen

Ticket: SIG-1234

Before opening a pull request, please ensure your branch is based on main and targets main.

Check requirements when they are met (strikethrough when not applicable):

  • Linters and Static Analysis tools have passed
  • Has tests with 100% coverage on new code (if feasible) and all test should pass
  • Scenario tests have been added/updated (if applicable)
  • Test coverage percentage threshold still passes
  • Documentation and Swagger have been updated
  • All other Required PR checks have passed

Be kind to code reviewers, please try to keep pull requests as small and focused as possible :)

Copilot AI review requested due to automatic review settings March 9, 2026 10:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the “answer additional question” API endpoint contract/behavior so request validation errors are represented as standard FastAPI/Pydantic 422s in the OpenAPI spec, and adjusts tests around invalid “time” answers.

Changes:

  • Removes the dynamic request-body dependency that injected type based on question_id, letting FastAPI validate AnswerInputUnion directly.
  • Updates the OpenAPI responses for the answer-question endpoint (removing the default_response entry).
  • Expands and tightens the time-answer validation tests (adds None as an invalid input and asserts additional error details).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
tests/api/v1/endpoints/test_melding.py Extends invalid time input cases and adds assertions on the returned validation error shape.
meldingen/api/v1/endpoints/melding.py Simplifies the answer-question endpoint input handling to rely on direct AnswerInputUnion body parsing and adjusts documented responses.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines 2323 to 2326
(1000, "Input should be a valid string"),
(10.00, "Input should be a valid string"),
(None, "Input should be a valid string"),
],
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This parametrization now includes None, but the test function annotates time_value as str | int a few lines below. With mypy --strict enabled in pre-commit, this will likely introduce a new type error; please widen the annotation to include None (or use a broader type) to keep the test type-checking clean.

Copilot uses AI. Check for mistakes.
Comment on lines 2361 to +2363
assert len(detail) == 1
assert detail[0].get("msg") == error_message
assert detail[0].get("type") == "string_type"
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assertion detail[0].get("type") == "string_type" is not correct for the first parametrized cases where time_value is a string but fails the regex pattern; Pydantic/FastAPI typically reports those as a pattern-mismatch error type (not a string-type error). Consider parametrizing the expected type alongside msg, or only asserting type == "string_type" for the non-string inputs.

Copilot uses AI. Check for mistakes.
Comment on lines 667 to 669
responses={
**not_found_response,
**unauthorized_response,
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The openapi_extra requestBody example just above this responses={...} block omits the required type discriminator for AnswerInputUnion (it only shows { "text": ... }). Since this endpoint now accepts AnswerInputUnion directly, update the example to include type so the OpenAPI documentation matches the actual request schema.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants