fix(spp_api_v2_change_request): fix URL encoding, silent failures, and 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

Author: jeremi

spp_api_v2_change_request/__manifest__.py

@@ -25,4 +25,32 @@
     "summary": """
         REST API endpoints for Change Request V2.
     """,
+    "description": """
+OpenSPP API V2 - Change Request
+================================
+
+Extends OpenSPP API V2 with Change Request endpoints.
+
+Endpoints
+---------
+- ``POST /ChangeRequest`` - Create a new change request
+- ``GET /ChangeRequest/{identifier}`` - Read a change request by reference
+- ``GET /ChangeRequest`` - Search change requests
+- ``PUT /ChangeRequest/{identifier}`` - Update change request detail data
+- ``POST /ChangeRequest/{identifier}/$submit`` - Submit for approval
+- ``POST /ChangeRequest/{identifier}/$approve`` - Approve (requires permission)
+- ``POST /ChangeRequest/{identifier}/$reject`` - Reject (requires permission)
+- ``POST /ChangeRequest/{identifier}/$request-revision`` - Request revision (requires permission)
+- ``POST /ChangeRequest/{identifier}/$apply`` - Apply changes to registrant
+- ``POST /ChangeRequest/{identifier}/$reset`` - Reset rejected/revision CR to draft
+- ``GET /ChangeRequest/$types`` - List all active CR types
+- ``GET /ChangeRequest/$types/{code}`` - Get field schema for a CR type
+
+Design Principles
+-----------------
+- Uses CR reference (CR/2024/00001), NOT database IDs
+- Returns appropriate HTTP status codes
+- Follows OpenSPP API V2 patterns
+- Requires authentication via OAuth 2.0
+    """,
 }

spp_api_v2_change_request/routers/change_request.py

@@ -3,6 +3,7 @@
 
 import logging
 from typing import Annotated
+from urllib.parse import urlencode
 
 from odoo.api import Environment
 from odoo.exceptions import UserError, ValidationError
@@ -29,6 +30,8 @@
     ApproveActionData,
     ChangeRequestCreate,
     ChangeRequestResponse,
+    ChangeRequestTypeInfo,
+    ChangeRequestTypeSchema,
     ChangeRequestUpdate,
     RejectActionData,
     RequestRevisionActionData,
@@ -92,6 +95,66 @@ async def create_change_request(
     return service.to_api_schema(cr)
 
 
+# Type schema endpoints are registered before /{p1}/{p2}/{p3} so that FastAPI
+# matches "$types" as a literal path segment rather than capturing it as
+# path parameters.
+
+
+@change_request_router.get(
+    "/$types",
+    response_model=list[ChangeRequestTypeInfo],
+)
+async def list_change_request_types(
+    env: Annotated[Environment, Depends(odoo_env)],
+    api_client: Annotated[dict, Depends(get_authenticated_client)],
+):
+    """
+    List all active Change Request types.
+
+    Returns type code, name, target type, and whether an applicant is required.
+    """
+    if not api_client.has_scope("change_request", "read"):
+        raise HTTPException(
+            status_code=status.HTTP_403_FORBIDDEN,
+            detail="Client does not have permission to read change request types",
+        )
+
+    service = ChangeRequestService(env)
+    return service.get_type_list()
+
+
+@change_request_router.get(
+    "/$types/{code}",
+    response_model=ChangeRequestTypeSchema,
+)
+async def get_change_request_type_schema(
+    code: Annotated[str, Path(description="CR type code (e.g., edit_individual)")],
+    env: Annotated[Environment, Depends(odoo_env)],
+    api_client: Annotated[dict, Depends(get_authenticated_client)],
+):
+    """
+    Get the full field schema for a Change Request type.
+
+    Returns type info, field definitions, and document requirements.
+    """
+    if not api_client.has_scope("change_request", "read"):
+        raise HTTPException(
+            status_code=status.HTTP_403_FORBIDDEN,
+            detail="Client does not have permission to read change request types",
+        )
+
+    service = ChangeRequestService(env)
+    result = service.get_type_schema(code)
+
+    if result is None:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail=f"Change request type not found: {code}",
+        )
+
+    return result
+
+
 @change_request_router.get("/{p1}/{p2}/{p3}", response_model=ChangeRequestResponse)
 async def read_change_request(
     p1: Annotated[str, Path(description="Reference part 1 (e.g., CR)")],
@@ -182,45 +245,26 @@ async def search_change_requests(
 
     # Build pagination URLs
     base_url = "/api/v2/spp/ChangeRequest"
-    query_parts = []
-    if registrant:
-        query_parts.append(f"registrant={registrant}")
-    if request_type:
-        query_parts.append(f"requestType={request_type}")
-    if cr_status:
-        query_parts.append(f"status={cr_status}")
-    if created_after:
-        query_parts.append(f"createdAfter={created_after}")
-    if created_before:
-        query_parts.append(f"createdBefore={created_before}")
-    query_params = "&".join(query_parts)
-
-    # Build self URL
-    self_url = (
-        f"{base_url}?{query_params}&_count={count}&_offset={offset}"
-        if query_params
-        else f"{base_url}?_count={count}&_offset={offset}"
-    )
+    base_params = {
+        k: v
+        for k, v in {
+            "registrant": registrant,
+            "requestType": request_type,
+            "status": cr_status,
+            "createdAfter": created_after,
+            "createdBefore": created_before,
+        }.items()
+        if v is not None
+    }
 
-    # Build next URL
-    next_url = None
-    if offset + count < total:
-        next_offset = offset + count
-        next_url = (
-            f"{base_url}?{query_params}&_count={count}&_offset={next_offset}"
-            if query_params
-            else f"{base_url}?_count={count}&_offset={next_offset}"
-        )
+    def build_url(offset_val: int) -> str:
+        """Build properly URL-encoded pagination URL."""
+        url_params = {**base_params, "_count": count, "_offset": offset_val}
+        return f"{base_url}?{urlencode(url_params)}"
 
-    # Build prev URL
-    prev_url = None
-    if offset > 0:
-        prev_offset = max(0, offset - count)
-        prev_url = (
-            f"{base_url}?{query_params}&_count={count}&_offset={prev_offset}"
-            if query_params
-            else f"{base_url}?_count={count}&_offset={prev_offset}"
-        )
+    self_url = build_url(offset)
+    next_url = build_url(offset + count) if offset + count < total else None
+    prev_url = build_url(max(0, offset - count)) if offset > 0 else None
 
     return create_search_result(
         data=data,

spp_api_v2_change_request/schemas/change_request.py

@@ -1,7 +1,7 @@
 # Part of OpenSPP. See LICENSE file for full copyright and licensing details.
 """Pydantic schemas for Change Request API."""
 
-from datetime import date, datetime
+from datetime import datetime
 from typing import Any, Literal
 
 from pydantic import BaseModel, Field
@@ -161,19 +161,6 @@ class Config:
         }
 
 
-class ChangeRequestAction(BaseModel):
-    """Schema for CR action requests (submit, approve, reject)."""
-
-    reason: str | None = Field(
-        None,
-        description="Reason for rejection or revision notes",
-    )
-    comment: str | None = Field(
-        None,
-        description="Optional comment for approval",
-    )
-
-
 class ApproveActionData(BaseModel):
     """Data for approve action."""
 
@@ -203,31 +190,61 @@ class RequestRevisionActionData(BaseModel):
     )
 
 
-class ChangeRequestSearchParams(BaseModel):
-    """Search parameters for change requests."""
+class FieldChoice(BaseModel):
+    """Value/label pair for selection choices, vocabulary codes, and documents."""
 
-    registrant: str | None = Field(
-        None,
-        description="Registrant identifier (system|value)",
-    )
-    request_type: str | None = Field(
-        None,
-        alias="requestType",
-        description="Type code to filter by",
-    )
-    status: str | None = Field(
-        None,
-        description="Status to filter by",
+    value: str = Field(..., description="Machine-readable value or code")
+    label: str = Field(..., description="Human-readable display label")
+
+
+class VocabularyInfo(BaseModel):
+    """Vocabulary namespace and available codes for a vocabulary field."""
+
+    namespace_uri: str = Field(..., alias="namespaceUri", description="Vocabulary namespace URI")
+    codes: list[FieldChoice] = Field(default_factory=list, description="Available codes")
+
+    class Config:
+        populate_by_name = True
+
+
+class FieldDefinition(BaseModel):
+    """Schema definition for a single field on a CR detail model."""
+
+    name: str = Field(..., description="Field name")
+    label: str = Field(..., description="Human-readable field label")
+    type: str = Field(
+        ...,
+        description="Field type (string, text, integer, float, boolean, date, datetime, selection, code, reference)",
     )
-    created_after: date | None = Field(
-        None,
-        alias="createdAfter",
-        description="Created on or after this date",
+    required: bool = Field(False, description="Whether the field is required")
+    readonly: bool = Field(False, description="Whether the field is read-only or computed")
+    help: str | None = Field(None, description="Help text for the field")
+    choices: list[FieldChoice] | None = Field(None, description="Available choices for selection fields")
+    vocabulary: VocabularyInfo | None = Field(None, description="Vocabulary info for code fields")
+
+
+class ChangeRequestTypeInfo(BaseModel):
+    """Summary info for a CR type."""
+
+    code: str = Field(..., description="Type code (e.g., add_member)")
+    name: str = Field(..., description="Human-readable type name")
+    target_type: str = Field(..., alias="targetType", description="Target registrant type (individual, group, both)")
+    requires_applicant: bool = Field(False, alias="requiresApplicant", description="Whether an applicant is required")
+
+    class Config:
+        populate_by_name = True
+
+
+class ChangeRequestTypeSchema(BaseModel):
+    """Full schema for a CR type including field definitions."""
+
+    type_info: ChangeRequestTypeInfo = Field(..., alias="typeInfo", description="Type summary")
+    fields: list[FieldDefinition] = Field(default_factory=list, description="Available detail fields")
+    available_documents: list[FieldChoice] = Field(
+        default_factory=list, alias="availableDocuments", description="Documents that can be attached"
     )
-    created_before: date | None = Field(
-        None,
-        alias="createdBefore",
-        description="Created on or before this date",
+    required_documents: list[FieldChoice] = Field(
+        default_factory=list, alias="requiredDocuments", description="Documents that must be attached"
     )
 
     class Config: