Skip to content

fix(spp_api_v2_change_request): fix URL encoding, type schema endpoints, and validation#37

Open
jeremi wants to merge 3 commits into19.0from
fix/change-request-api
Open

fix(spp_api_v2_change_request): fix URL encoding, type schema endpoints, and validation#37
jeremi wants to merge 3 commits into19.0from
fix/change-request-api

Conversation

@jeremi
Copy link
Member

@jeremi jeremi commented Feb 17, 2026

Summary

  • Split CR reference into path segments (/{p1}/{p2}/{p3}) to fix URL-encoded slashes being rejected by FastAPI
  • Add $types and $types/{code} endpoints for listing CR types and their field schemas
  • Extract DETAIL_SKIP_FIELDS constant and add ODOO_TYPE_MAP / SKIP_FIELD_TYPES for field schema generation
  • Add test helpers (common.py) and type schema tests
  • Fix silent failures in service layer with explicit error handling

Test plan

  • Run test_single_module.sh spp_api_v2_change_request
  • Verify CR CRUD via API with references containing slashes (e.g., CR/2026/00001)
  • Verify GET /ChangeRequest/$types returns type list
  • Verify GET /ChangeRequest/$types/{code} returns field schema

…d validation gaps

- Fix pagination URLs not being URL-encoded (broken links with pipe/colon chars)
- Raise ValidationError on unresolved vocabulary/partner lookups instead of silently skipping
- Add detail input validation against type schema (unknown, internal, readonly fields)
- Fix manifest: PATCH→PUT, add missing $request-revision and $reset endpoints
- Remove unused ChangeRequestAction and ChangeRequestSearchParams schemas
- Remove unnecessary hasattr check for revision_notes (always on mixin)
- Add comments explaining why _do_reject/_do_request_revision are called directly
- Extract shared test fixtures into tests/common.py ChangeRequestTestCase
- Add tests for reject/approve wrong state, unknown fields, unresolved vocab, readonly fields
@gemini-code-assist
Copy link

Summary of Changes

Hello @jeremi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the Change Request API by improving its robustness, discoverability, and usability. It addresses issues with URL encoding for references, introduces new endpoints for dynamic schema discovery, and strengthens data validation in the service layer. These changes provide a more reliable and self-documenting API, making it easier for clients to interact with Change Requests and their associated data structures.

Highlights

  • URL Encoding Fix: Fixed URL encoding issues by splitting Change Request references into path segments (/{p1}/{p2}/{p3}) to correctly handle slashes in identifiers, which were previously rejected by FastAPI.
  • New Type Schema Endpoints: Introduced new API endpoints: GET /ChangeRequest/$types to list all active Change Request types and GET /ChangeRequest/$types/{code} to retrieve the detailed field schema for a specific CR type.
  • Standardized Schema Generation: Extracted constants such as DETAIL_SKIP_FIELDS, ODOO_TYPE_MAP, and SKIP_FIELD_TYPES to standardize and improve the consistency of field schema generation and detail serialization.
  • Enhanced Validation and Error Handling: Implemented explicit error handling and robust validation in the service layer, including checks for unknown fields, unresolved vocabulary codes, and attempts to modify read-only fields during Change Request detail updates.
  • API Documentation and Method Update: Updated the API documentation in __manifest__.py to reflect the new type schema endpoints, new actions ($request-revision, $reset), and changed the update method for Change Request detail data from PATCH to PUT.
  • Improved Testing Infrastructure: Added a new common test helper (common.py) and dedicated test files (test_change_request_type_schema.py) to provide a more robust and maintainable testing framework for the Change Request API.
Changelog
  • spp_api_v2_change_request/manifest.py
    • Updated API endpoint documentation to reflect new functionalities.
    • Changed the update method for Change Requests from 'PATCH' to 'PUT'.
    • Added documentation for new endpoints: 'request-revision', 'reset', '$types', and '$types/{code}'.
  • spp_api_v2_change_request/routers/change_request.py
    • Imported 'urlencode' for proper URL parameter handling.
    • Added 'ChangeRequestTypeInfo' and 'ChangeRequestTypeSchema' imports.
    • Implemented new GET endpoints for '/ChangeRequest/$types' and '/ChangeRequest/$types/{code}' to retrieve CR type information and field schemas.
    • Refactored pagination URL construction to use 'urllib.parse.urlencode' for robust query parameter encoding.
  • spp_api_v2_change_request/schemas/change_request.py
    • Removed unused 'date' import.
    • Deleted the generic 'ChangeRequestAction' schema.
    • Introduced new Pydantic schemas: 'FieldChoice', 'VocabularyInfo', 'FieldDefinition', 'ChangeRequestTypeInfo', and 'ChangeRequestTypeSchema' to support dynamic type schema generation.
  • spp_api_v2_change_request/services/change_request_service.py
    • Imported 'ast' for safe evaluation of domain strings.
    • Defined global constants 'DETAIL_SKIP_FIELDS', 'ODOO_TYPE_MAP', and 'SKIP_FIELD_TYPES' for consistent field handling.
    • Modified 'to_api_schema' to check 'cr.revision_notes' directly.
    • Refactored '_serialize_detail' to use the new 'DETAIL_SKIP_FIELDS' constant.
    • Introduced '_validate_detail_input' method to perform validation against the type's field schema, checking for unknown, unavailable, or read-only fields.
    • Enhanced '_deserialize_detail' to explicitly raise 'ValidationError' for unresolved vocabulary codes or registrants.
    • Added 'get_type_list' and 'get_type_schema' methods to retrieve CR type summaries and full field schemas.
    • Implemented helper methods '_build_field_definitions', '_extract_selection_choices', and '_extract_vocabulary_info_from_domain' for dynamic schema generation.
    • Added comments to 'reject' and 'request_revision' methods clarifying direct calls to underlying Odoo implementations.
  • spp_api_v2_change_request/tests/init.py
    • Added imports for new common test utilities and type schema tests.
  • spp_api_v2_change_request/tests/common.py
    • Added a new file defining 'ChangeRequestTestCase', a base class for CR API V2 tests, providing shared setup for vocabularies, registrants, and CR types.
  • spp_api_v2_change_request/tests/test_change_request_api.py
    • Refactored the test class to use the new common test base class.
  • spp_api_v2_change_request/tests/test_change_request_service.py
    • Refactored the test class to use the new common test base class.
    • Added new tests for state validation and detail validation.
  • spp_api_v2_change_request/tests/test_change_request_type_schema.py
    • Added a new test file for Change Request type list and schema service methods.
Activity
  • The pull request introduces significant new API endpoints and refactors existing logic.
  • New test files and a common test base class were added to support the new functionalities and improve test maintainability.
  • Error handling and data validation have been explicitly improved in the service layer.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request significantly improves the Change Request API by fixing URL encoding issues and introducing $types and $types/{code} endpoints for schema discovery. A comprehensive security audit confirmed no critical vulnerabilities were identified, and the changes adhere to secure practices including input validation and enhanced error handling. While the implementation is robust, consider improving the handling of malformed date strings and refining the API schema for greater precision.

Comment on lines 215 to 218
type: str = Field(
...,
description="Field type (string, text, integer, float, boolean, date, datetime, selection, code, reference)",
)

Choose a reason for hiding this comment

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

medium

The type field in the FieldDefinition schema has a finite set of possible string values. To make the API schema more precise and enable stricter validation on the client and server side, it's better to use typing.Literal instead of str. This also serves as clear documentation for the allowed values.

Suggested change
type: str = Field(
...,
description="Field type (string, text, integer, float, boolean, date, datetime, selection, code, reference)",
)
type: Literal[
"string",
"text",
"integer",
"float",
"boolean",
"date",
"datetime",
"selection",
"code",
"reference",
] = Field(
...,
description="Field data type",
)

…pe schema with JSON Schema 2020-12

Add generic OdooModelSchemaBuilder that converts any Odoo model's fields
to a standard JSON Schema 2020-12 document. Refactor CR type schema
endpoint to return detailSchema (JSON Schema) instead of proprietary
FieldDefinition objects, enabling third-party tooling (ajv, jsonschema,
react-jsonschema-form) to work out of the box.

Key changes:
- Add spp_api_v2/services/schema_builder.py with field type mapping,
  vocabulary extraction, and selection choice handling
- Replace FieldDefinition/VocabularyInfo pydantic models with a plain
  dict[str, Any] detailSchema field on ChangeRequestTypeSchema
- Optimize _validate_detail_input to use direct field introspection
  instead of building full schema (avoids unnecessary DB queries)
- Use anyOf for many2one reference types (2020-12 conformance)
…r fields

Replace plain str with Literal for fields that have a known, fixed set of
valid values: ChangeRequestResponse.status and ChangeRequestTypeInfo.target_type.
This provides stricter validation and clearer API documentation.
@codecov
Copy link

codecov bot commented Feb 18, 2026

Codecov Report

❌ Patch coverage is 90.26403% with 59 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (19.0@3258a93). Learn more about missing BASE report.

Files with missing lines Patch % Lines
...pp_api_v2_change_request/routers/change_request.py 21.73% 18 Missing ⚠️
spp_api_v2/tests/test_schema_builder.py 93.92% 11 Missing ⚠️
spp_api_v2/services/schema_builder.py 93.33% 8 Missing ⚠️
..._change_request/services/change_request_service.py 85.96% 8 Missing ⚠️
...e_request/tests/test_change_request_type_schema.py 93.60% 8 Missing ⚠️
spp_api_v2_change_request/tests/common.py 91.66% 3 Missing ⚠️
...hange_request/tests/test_change_request_service.py 92.68% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             19.0      #37   +/-   ##
=======================================
  Coverage        ?   68.82%           
=======================================
  Files           ?      388           
  Lines           ?    31615           
  Branches        ?        0           
=======================================
  Hits            ?    21760           
  Misses          ?     9855           
  Partials        ?        0           
Flag Coverage Δ
spp_api_v2 89.27% <93.72%> (?)
spp_api_v2_change_request 73.97% <86.79%> (?)
spp_api_v2_cycles 65.45% <ø> (?)
spp_api_v2_data 48.59% <ø> (?)
spp_api_v2_entitlements 68.43% <ø> (?)
spp_api_v2_products 64.39% <ø> (?)
spp_api_v2_service_points 63.20% <ø> (?)
spp_api_v2_vocabulary 43.70% <ø> (?)
spp_base_common 92.81% <ø> (?)
spp_dci_client_dr 75.33% <ø> (?)
spp_dci_client_ibr 78.05% <ø> (?)
spp_dci_server 51.27% <ø> (?)
spp_mis_demo_v2 75.34% <ø> (?)
spp_programs 49.57% <ø> (?)
spp_security 51.08% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant