feat(graphql): add mutations and make GraphQL the authoritative write layer#990
feat(graphql): add mutations and make GraphQL the authoritative write layer#990shaoster wants to merge 9 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bea0c7f812
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| currentState { state } | ||
| tags { id name color isPublic } | ||
| thumbnail { url imageId crop { x y width height } croppedUrl } |
There was a problem hiding this comment.
When MCP callers use get_piece_details, the tool contract in potterdoc_mcp/server.py says it returns all custom fields and the complete state history, and the previous REST call to /api/pieces/{id}/ did so. This new GraphQL selection only asks for summary fields (currentState { state }, tags, thumbnail), so agents lose the history/custom-field data they need to inspect an existing piece before deciding transitions or edits.
Useful? React with 👍 / 👎.
| if tags is not None: | ||
| payload["tags"] = tags | ||
| if not payload: | ||
| input_payload["tags"] = tags |
There was a problem hiding this comment.
For MCP calls that pass tag IDs as strings, this now forwards tags unchanged even though the GraphQL input type is UpdatePieceInput.tags: [Int]; the MCP tool is still typed as tags: list[str], and the old REST serializer accepted string IDs by casting them. In that scenario GraphQL rejects the variables before the resolver runs, so updating tags through MCP fails unless these values are coerced to integers or the tool/input types are aligned.
Useful? React with 👍 / 👎.
| try: | ||
| data = resolve_piece_detail(str(id), info.context.request) |
There was a problem hiding this comment.
For bearer-token callers such as MCP, request.user remains anonymous until get_request_user() is invoked. This resolver skips that bridge and passes the raw request into resolve_piece_detail, so the queryset only sees anonymous-visible shared terminal pieces; get_piece_details over an API token therefore reports the owner’s private pieces as not found.
Useful? React with 👍 / 👎.
| uploadImage(pieceId: $pieceId, input: $input) { | ||
| id | ||
| currentState { state } |
There was a problem hiding this comment.
When MCP callers upload an image, the tool contract says they receive the created PieceStateImage plus background task IDs, and the old REST endpoint returned those values. This GraphQL mutation only requests the updated piece ID/current state, so callers lose the new image_id needed by crop_piece_image and the conversion task ID needed to track async processing.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f13f6c0d9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| current_state=CurrentStateType(state=current_state.get("state", "")), | ||
| thumbnail=ThumbnailType.from_dict(data.get("thumbnail")), | ||
| tags=[TagType.from_dict(t) for t in (data.get("tags") or [])], | ||
| states=data.get("states") or [], |
There was a problem hiding this comment.
When MCP get_piece_details or any GraphQL client calls piece(id), resolve_piece_detail returns PieceDetailSerializer data whose state-history key is history (api/serializers.py:650-655), but this constructor reads states, so the GraphQL states field is always []. Fresh evidence: the current code now selects states, but the backing serializer still emits history, so agents lose all custom fields, images, and state history for existing pieces.
Useful? React with 👍 / 👎.
| if input.notes is not None: | ||
| payload["notes"] = input.notes |
There was a problem hiding this comment.
When a GraphQL caller passes notes to updatePiece, this payload is handed to PieceUpdateSerializer, which has no notes field (api/serializers.py:1053-1062), so the mutation can return success while the requested notes change is not applied. Either remove this input field or route it to the current-state notes update path so clients do not get a silent no-op from the authoritative GraphQL write API.
Useful? React with 👍 / 👎.
| raise StrawberryGraphQLError("Authentication required.") | ||
| from api.global_entries.logic import global_entries_impl | ||
|
|
||
| response = global_entries_impl(info.context.request, global_name) |
There was a problem hiding this comment.
When MCP calls list_global_entries, this resolver passes the GraphQL POST HttpRequest into global_entries_impl, whose list path only runs for request.method == "GET" and whose create path expects DRF-only request.data (api/global_entries/logic.py:99-107, 158-179). As a result, the new GraphQL-backed globals query either returns a create-style error or raises instead of listing entries; build the list directly or call the shared logic with request semantics that actually match a REST GET.
Useful? React with 👍 / 👎.
| return PieceType.from_summary(data) | ||
|
|
There was a problem hiding this comment.
When a GraphQL caller supplies location to updateCurrentState, the resolver forwards it to PieceStateUpdateSerializer, but that serializer only declares notes, images, custom_fields, and created (api/serializers.py:973-980). The mutation can therefore accept the SDL-advertised field while leaving the location unchanged, which is especially misleading for LLM/MCP clients discovering writes from the schema; either remove this input or map it to a real persisted field.
Useful? React with 👍 / 👎.
| data = await self._graphql(query, variables) | ||
| return data["pieces"] # type: ignore[no-any-return] |
There was a problem hiding this comment.
After this switch to _graphql, the existing potterdoc_mcp/tests still mock REST methods and response shapes (for example test_get_piece patches _http.get and expects /api/pieces/{id}/, while test_transition_piece_no_custom_fields expects POST /api/pieces/{id}/states/). I checked .github/workflows/ci.yml:163-167, env-agent.sh:800-810, and potterdoc_mcp/BUILD.bazel:35-40: PR coverage runs all non-lint Bazel tests, including //potterdoc_mcp:potterdoc_mcp_test, so these stale tests will fail until they are updated to mock /api/graphql/ and GraphQL data payloads.
Useful? React with 👍 / 👎.
… layer (#986) Extracts piece operation logic into resolver functions, adds a full GraphQL mutation surface (create/update/transition/image ops), and switches the MCP client from REST to GraphQL for all write operations. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…hQL, REST shims Gap 1: Add PieceDetailType with full state history (states JSON field) and update piece(id) query to return it instead of PieceSummarySerializer shape. MCP get_piece query now requests states, notes, created, lastModified, photoCount. Gap 2: Add workflowSchema and globals(globalName) queries to GraphQL schema. MCP client now calls GraphQL instead of REST for get_workflow_schema and list_global_entries. Gap 3: Convert piece_detail (GET/PATCH) and piece_current_state (PATCH) and piece_past_state (PATCH/DELETE) REST views to delegate to resolver functions. Adds _map_exc helper for Http404 / PermissionDenied / ValidationError mapping. Pieces POST and piece_states POST kept inline (serializers accept richer data than the resolver signatures handle). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…-op fields, MCP tests - Bug 1: call get_request_user() and set request.user in piece(), workflow_schema(), and globals() resolvers so Bearer-token auth is reflected in queryset lookups - Bug 2: read 'history' key (not 'states') from PieceDetailSerializer output in PieceDetailType.from_detail() - Bug 4: force request.method = 'GET' in globals() resolver so global_entries_impl activates the list branch (GraphQL uses POST) - Bug 5: remove notes from UpdatePieceInput — PieceUpdateSerializer has no notes field - Bug 6: remove location from UpdateStateInput — PieceStateUpdateSerializer has no location field; remove from both update_current_state and update_past_state payloads - Bug 7: coerce tags to int in update_piece_metadata client method and server tool - Bug 8: upload_image mutation now returns PieceDetailType with states field so callers can find the new image ID in the state history - MCP tests: updated all tests to mock _graphql() instead of REST HTTP methods Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
shaoster
force-pushed
the
issue/986-graphql-authoritative
branch
from
8f13f6c to
f9863bf
Compare
…ge resolver - Wrap JSON scalar definition in TYPE_CHECKING guard so mypy sees a bare NewType (valid as a type annotation) while runtime gets the full strawberry.scalar registration - Add type: ignore[arg-type] on CsrfViewMiddleware constructor and process_view call where Django's internal callable types don't match mypy's expected signatures - Replace hasattr(result, "status_code") with isinstance(result, Response) in resolve_upload_image to narrow the union type and satisfy mypy's union-attr check; add missing Response import Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
NewType("JSON", object) created a nominal type that mypy rejected for
dict/list assignments and cross-function calls. TypeAlias = Any is the
correct pattern: mypy treats JSON fields as Any (fully assignable), while
at runtime the strawberry.scalar registration runs as before.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ed133005cf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| original_method = request.method | ||
| request.method = "GET" | ||
| try: | ||
| response = global_entries_impl(request, global_name) |
There was a problem hiding this comment.
When globalName is a filterable global such as glaze_combination, this still passes Strawberry's plain Django request into global_entries_impl. The GET path calls apply_global_filters, which reads request.query_params, but a GraphQL WSGIRequest only has GET, so globals(globalName: "glaze_combination") raises instead of listing the entries agents need for glaze-combination fields. Wrap/adapt the request with DRF request semantics or provide a query_params fallback before reusing the REST helper.
Useful? React with 👍 / 👎.
| @strawberry.mutation(description="Update crop bounds for an image.") | ||
| def crop_image( | ||
| self, info: strawberry.Info, image_id: strawberry.ID, crop: ImageCropInput | ||
| ) -> PieceType: |
There was a problem hiding this comment.
When cropImage is used on a state image that is not the piece thumbnail, the resolver computes full piece detail but this mutation declares and serializes the result as PieceType, which has no states/images field. The MCP client can therefore only receive thumbnail, so the updated crop is absent from the response for non-thumbnail images, unlike the old REST crop endpoint that returned the piece detail with history. Return PieceDetailType or an image payload so callers can verify the crop they just changed.
Useful? React with 👍 / 👎.
| name: str | None = strawberry.field(default=None, description="New name.") | ||
| shared: bool | None = strawberry.field(default=None, description="Shared flag.") | ||
| tags: list[int] | None = strawberry.field( |
There was a problem hiding this comment.
For GraphQL-only callers that want to use the new updatePastState or deletePastState mutations, those resolvers reject unless piece.is_editable is already true (api/piece/resolvers.py:88-103). The existing REST flow enables that by PATCHing is_editable, but this new input only allows name/shared/tags, so there is no GraphQL path to enter or leave editable mode before calling the past-state mutations.
Useful? React with 👍 / 👎.
| notes: str | None = strawberry.field(default=None) | ||
| custom_fields: JSON | None = strawberry.field(default=None) | ||
| images: JSON | None = strawberry.field(default=None) |
There was a problem hiding this comment.
When a user needs to backdate a state, the REST current-state PATCH supports created and PieceStateUpdateSerializer persists it (api/serializers.py:970-1014; covered by api/tests/test_patch_current_state.py). This new GraphQL input is shared by updateCurrentState and updatePastState but omits created, so GraphQL-only clients cannot perform an existing state edit that remains supported by the resolver/serializer.
Useful? React with 👍 / 👎.
| @strawberry.mutation(description="Transition piece to a new workflow state.") | ||
| def transition_piece( | ||
| self, info: strawberry.Info, id: strawberry.ID, input: TransitionPieceInput | ||
| ) -> PieceType: |
There was a problem hiding this comment.
When a GraphQL caller transitions a piece with transitionPiece, the resolver creates a new PieceState but this mutation exposes only PieceType, so callers cannot select the new state's id, notes, custom fields, or images from the mutation response. This is especially limiting for MCP/agent flows because updatePastState and deletePastState require a stateId, but the transition response no longer provides one without an extra detail query.
Useful? React with 👍 / 👎.
…dateStateInput All write mutations now return PieceDetailType (full history + states JSON) instead of PieceType (summary only). Resolvers already compute the full detail dict via serialize_piece_detail — only the wrapper was truncating it. This lets callers see the new state after transitionPiece, the updated crop after cropImage, and the full history after any write. Also exposes the `created` field on UpdateStateInput so GraphQL callers can backdate a state's creation timestamp, matching what PieceStateUpdateSerializer already supports via the REST PATCH endpoint. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds 3 new Query fields: stateSchema, glazeCombinationImages, and filters arg on globals. Adds 6 new Mutation fields: moveImage, uploadImageToPastState, uploadImageFromRefs, uploadImageFromRefsToPastState, createGlobal, addFavorite, removeFavorite. Creates api/global_entries/resolvers.py with the global entry and favorite resolver functions. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e0439be41e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ) -> JSON: | ||
| user = _require_auth(info) | ||
| info.context.request.user = user | ||
| from api.global_entries.resolvers import resolve_create_global |
There was a problem hiding this comment.
When callers use the new createGlobal/favorite mutations under Bazel or the packaged runtime, this lazy import targets api/global_entries/resolvers.py, but that new file is not listed in either api_schema_lib or api_runtime_files in api/BUILD.bazel (I checked the src lists around the added GraphQL files). The import can therefore work from a checkout while failing with ModuleNotFoundError in Bazel tests/deployments; add the resolver file to the same Bazel/package lists as the other new modules.
Useful? React with 👍 / 👎.
| # Wrap with DRF Request so .data, .user, etc. are available. | ||
| drf_request = DRFRequest(raw) | ||
| drf_request._full_data = data # type: ignore[attr-defined] |
There was a problem hiding this comment.
For GraphQL global mutations, wrapping the RequestFactory request in DRF's Request does not carry raw.user into drf_request.user; accessing .user re-runs DRF authentication and falls back to anonymous unless the wrapper's user property is set. In that case createGlobal (and the identical favorite wrappers below) call the shared global-entry logic as AnonymousUser, so private entries/favorites fail or attach to the wrong principal; set drf_request.user = request.user before passing it on.
Useful? React with 👍 / 👎.
| response = _view(request) | ||
| return response.data |
There was a problem hiding this comment.
When glazeCombinationImages is queried through the normal GraphQL POST endpoint, this passes that POST request directly into the DRF view that is declared @api_view(["GET"]) in api/analysis_views.py, so DRF returns a 405 response and the GraphQL field returns the error payload instead of the grouped images. Use the underlying query logic directly or adapt the request method as the globals resolver does.
Useful? React with 👍 / 👎.
| link = ( | ||
| PieceStateImage.objects.select_related("piece_state__piece") | ||
| .filter(image=image, piece_state__piece__user=request.user) | ||
| .order_by("-id") | ||
| .first() |
There was a problem hiding this comment.
When the same Image is linked from multiple piece states, moveImage has no source state/link id and this resolver silently picks the newest PieceStateImage; the REST route this replaces includes piece_state_id in the URL precisely to identify which link is being moved. In that duplicate-image scenario, moving an older state's image through GraphQL moves or deletes the wrong state image, so include a source link/state identifier instead of selecting by image_id alone.
Useful? React with 👍 / 👎.
| for i, key in enumerate(r2_keys): | ||
| caption = captions[i] if i < len(captions) else "" | ||
| # Derive the public URL from the key. | ||
| public_url = r2_module.public_url_for_key(key) | ||
| _attach_image_to_piece_state_plain(piece_state, public_url, key, caption, user) |
There was a problem hiding this comment.
When a GraphQL caller supplies r2Keys, this accepts each string and turns it into a public URL without checking that it is a server-generated image key for the current user. The presigned upload endpoint scopes keys as images/<user.id>/..., but this path would let a caller attach any known R2 object key (including another user's image key or a non-image prefix) to their piece and enqueue processing for it; validate the owner-scoped image prefix/extension or record upload grants before creating the image link.
Useful? React with 👍 / 👎.
| piece_state = piece.current_state | ||
| if piece_state is None: | ||
| raise Http404("Piece has no current state.") | ||
| result = _fetch_url_to_r2(url, request.user.id) |
There was a problem hiding this comment.
Because uploadImage accepts an arbitrary caller-provided URL and passes it directly to _fetch_url_to_r2, any authenticated GraphQL caller can make the backend issue a server-side GET to an HTTPS endpoint of their choice; the helper only checks the URL scheme and validates content type after the fetch. This newly exposes the existing fetch helper beyond trusted OpenAI file refs, so internal HTTPS services or very large image responses can be probed/downloaded from the PotterDoc server context unless the target host/IP is constrained or this flow is limited to vetted refs/presigned uploads.
Useful? React with 👍 / 👎.
…-combination method, DRF user, BUILD.bazel - globals query: monkey-patch request.query_params = request.GET so apply_global_filters works with a raw Django HttpRequest (no DRF wrapper) - glazeCombinationImages: set request.method="GET" around the @api_view(["GET"]) call to avoid a 405 from DRF's method dispatch - global_entries/resolvers.py: set drf_request._user = request.user explicitly so DRF's Request.user property skips re-authentication and uses the correct principal instead of falling back to AnonymousUser - BUILD.bazel: add global_entries/resolvers.py to api_lib and lint srcs so the lazy imports work in Bazel tests and deployments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b7b0308694
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if request.method == "POST" and not has_bearer: | ||
| reason = _csrf_middleware.process_view(request, None, (), {}) # type: ignore[arg-type] |
There was a problem hiding this comment.
For browser users who are authenticated by the session but do not yet have a bearer access token, read-only GraphQL calls such as the piece list still arrive as POSTs and are rejected here unless a CSRF cookie/header already exists. The frontend GraphQL client only adds X-CSRFToken when the cookie is present and fetchPieces does not first call ensureCsrfCookie, so an initial session-backed piece-list request can 403 before reaching the resolver even though it does not mutate state. Parse the operation and enforce this only for mutations, or ensure read-query callers bootstrap CSRF first.
Useful? React with 👍 / 👎.
| for k, v in filters.items(): | ||
| qd[k] = str(v) |
There was a problem hiding this comment.
When a GraphQL caller filters globals with a multi-value filter such as globals(globalName: "glaze_combination", filters: {"glaze_type_ids": [id1, id2]}), this writes the Python list representation into the synthetic QueryDict. The reused apply_global_filters path handles m2m_id filters by splitting comma-separated REST query strings, so it looks for malformed PKs like ['1' and returns no matches. Encode list values as comma-joined/query-list values before passing them to the REST helper.
Useful? React with 👍 / 👎.
| piece = get_object_or_404(_piece_detail_queryset(request), pk=piece_id) | ||
| return Response(_serialize_piece_detail(piece, request)) | ||
| try: | ||
| return Response(resolve_update_piece(piece_id, dict(request.data), request)) |
There was a problem hiding this comment.
When a REST client PATCHes with form-encoded or multipart data, request.data is a QueryDict; wrapping it with dict() turns scalar fields into one-element lists and bypasses DRF's HTML list handling, so valid requests such as name=New Mug are handed to the serializer as {'name': ['New Mug']} and fail validation. The previous view passed request.data directly, so keep the original request data object for these resolver calls.
Useful? React with 👍 / 👎.
| if not hasattr(request, "query_params"): | ||
| request.query_params = request.GET # type: ignore[attr-defined] |
There was a problem hiding this comment.
When one GraphQL operation resolves globals more than once, the first call creates request.query_params; a later call with filters replaces request.GET but skips updating query_params because the attribute already exists. Since apply_global_filters reads request.query_params, those later filters are ignored or use stale values from an earlier field. Reassign query_params from the new QueryDict each time filters are installed, ideally restoring the original request state afterward.
Useful? React with 👍 / 👎.
Adds api/graphql/rest_bridge.py: a declarative RestRoute table maps each REST endpoint to a GraphQL operation. make_rest_view() / make_multi_route_view() generate DRF views from the table at startup via schema.execute_sync(). Key design: - RestRoute dataclass: method, graphql_op, data_key, extract_vars, success_status, unauthenticated_status, key_renames, post_process, success_status_key, extend_schema_kwargs - _execute_route: handles bearer auth, per-route unauthenticated_status, DRFValidationError from extract_vars, structured dict errors via extensions, 204 No Content, key renames, post-processing - Dynamic global/favorite routes generated from workflow.yml at startup - File upload routes (multipart) remain as original views - OpenAPI schema helpers register drf-spectacular annotations per-route Also fixes: - mutations.py: is_editable, thumbnail, notes/images in transitions, showcase_video_url/owner_alias on PieceDetailType, 401 vs 403 auth codes - schema.py: allow anonymous piece query (shared pieces), globals list filter encoding (list values comma-joined for m2m_id filters), always refresh query_params after updating request.GET - types.py: PieceDetailType reads history key, UpdateStateInput has created - resolvers.py: transition_piece passes notes/images through - BUILD.bazel: adds rest_bridge.py and global_entries/resolvers.py All 13 test suites pass (449 tests). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73be1a31b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| vars["limit"] = max(1, min(100, int(qp.get("limit", 20)))) | ||
| except (ValueError, TypeError): | ||
| vars["limit"] = 20 |
There was a problem hiding this comment.
When a REST caller omits limit or sends an invalid limit, this bridge now sends 20 to GraphQL, but the previous REST view and _DEFAULT_PAGE_SIZE use 16. Users with more than 16 pieces will see different first-page contents from /api/pieces/ after this change; reuse _DEFAULT_PAGE_SIZE here to preserve the REST contract.
Useful? React with 👍 / 👎.
| id name shared isEditable canEdit notes | ||
| created lastModified photoCount currentLocation | ||
| showcaseVideoUrl ownerAlias |
There was a problem hiding this comment.
When /api/pieces/{id}/ or a piece mutation response is served through this detail fragment, showcase_story and showcase_fields are dropped even though PieceDetailSerializer includes them via PieceSummarySerializer. Clients such as mapPieceDetail then default existing showcase content to empty values after a detail fetch; expose/select showcaseStory and showcaseFields on PieceDetailType as the summary query does.
Useful? React with 👍 / 👎.
| id name shared isEditable canEdit notes | ||
| created lastModified photoCount currentLocation | ||
| showcaseVideoUrl ownerAlias | ||
| currentState { state } |
There was a problem hiding this comment.
When PieceDetailPage calls /api/pieces/{id}/?exclude_history=true, PieceDetailSerializer still returns an empty history but a full current_state; this fragment only requests currentState { state }, and _enrich_piece_detail can only rebuild the full object from history. In that mode the REST response loses current-state notes, custom_fields, and images, so the initial piece detail load has incomplete state data.
Useful? React with 👍 / 👎.
| mutation UpdatePiece($id: ID!, $name: String, $shared: Boolean, $isEditable: Boolean, $thumbnail: JSON, $tags: [Int!], $currentLocation: String) { | ||
| updatePiece(id: $id, input: { name: $name, shared: $shared, isEditable: $isEditable, thumbnail: $thumbnail, tags: $tags, currentLocation: $currentLocation }) { |
There was a problem hiding this comment.
When existing REST clients PATCH /api/pieces/{id}/ with showcase_story or showcase_fields, this GraphQL operation/input has no variables for those fields and _extract_piece_patch_vars drops them, while PieceUpdateSerializer still accepts and persists them. Those PATCHes now return success without changing the showcase configuration; add both fields to the GraphQL input and bridge variables.
Useful? React with 👍 / 👎.
| method="PATCH", | ||
| graphql_op=_PIECE_DETAIL_FRAGMENT + """ | ||
| mutation UpdatePiece($id: ID!, $name: String, $shared: Boolean, $isEditable: Boolean, $thumbnail: JSON, $tags: [Int!], $currentLocation: String) { | ||
| updatePiece(id: $id, input: { name: $name, shared: $shared, isEditable: $isEditable, thumbnail: $thumbnail, tags: $tags, currentLocation: $currentLocation }) { |
There was a problem hiding this comment.
When a REST PATCH updates only metadata such as name, _extract_piece_patch_vars omits currentLocation, but this inline input still sends currentLocation: null; update_piece treats any value other than UNSET as an update, so unrelated PATCHes clear the piece's location. The same omitted-vs-null collapse also prevents thumbnail: null from clearing a thumbnail, so build the input only from fields present in the REST payload.
Useful? React with 👍 / 👎.
| extract_vars=lambda request, kwargs: { | ||
| "name": request.data.get("name", ""), | ||
| "notes": request.data.get("notes", ""), | ||
| "currentLocation": request.data.get("current_location"), |
There was a problem hiding this comment.
When REST clients create a piece with the existing thumbnail field, for example the new-piece dialog's curated thumbnail, this bridge only forwards name, notes, and currentLocation into createPiece. PieceCreateSerializer still accepts and persists thumbnail, so new pieces created through /api/pieces/ silently lose the selected thumbnail after this change.
Useful? React with 👍 / 👎.
| # GraphQL returns null for queries that return Optional (e.g. piece by id | ||
| # when not found). Map to 404 so the REST contract is preserved. | ||
| return Response({"detail": "Not found."}, status=404) | ||
| body = _snake_keys(body) |
There was a problem hiding this comment.
When REST clients call /api/workflow/schema/... through this bridge, the generic snake-case pass rewrites JSON Schema keywords emitted by build_ui_schema, e.g. additionalProperties becomes additional_properties (and anyOf would become any_of). The frontend protocol type and any schema consumer expect standard JSON Schema casing, so these bridged workflow responses are no longer valid schemas; skip recursive key conversion for JSON-scalar routes or only convert GraphQL object fields.
Useful? React with 👍 / 👎.
| ext_detail: Any = extensions.get("detail") | ||
| if ext_detail is not None and isinstance(ext_detail, dict): | ||
| err_body = {k: [str(v) for v in (vs if isinstance(vs, list) else [vs])] for k, vs in ext_detail.items()} | ||
| return Response(err_body, status=http_status) |
There was a problem hiding this comment.
When a bridged mutation raises a serializer ValidationError with nested detail, such as PATCH /api/pieces/{id}/ with a malformed thumbnail object, this conversion stringifies the nested dict/list into one message. The previous DRF response preserved nested field errors, so clients can no longer attach errors to the right subfield; normalize ErrorDetail leaves but return the nested structure unchanged.
Useful? React with 👍 / 👎.
1 participant
Summary
api/graphql/rest_bridge.py) that generates DRF views from a declarativeRestRoutetable — each route maps a REST endpoint to a GraphQL operation viaschema.execute_sync()upload-image/,upload-image-refs/) remain as original views since they handle multipart/binary data incompatible with GraphQLKey design decisions
RestRoutedataclass:method,graphql_op,data_key,extract_vars,success_status,unauthenticated_status,key_renames,post_process,success_status_key,extend_schema_kwargsmake_rest_view()/make_multi_route_view(): factory functions that register drf-spectacular OpenAPI schemas and dispatch to_execute_route()_require_authin mutations returns 403 for anonymous (no header) and 401 for invalid Bearer, matching original per-endpoint behavior;unauthenticated_statusoverrides this per routeValidationErrordetail dicts are passed throughextensionsto preserve field-keyed format;MethodNotAllowed→ 405;Http404message preservedworkflow.ymlusing the sameRestRoutefactoryTest plan
gz_test //api/tests:api_piece_test— 309 passedgz_test //api/tests:api_glaze_test— 140 passedgz_test //api/tests:api_model_test— passedgz_test //api/tests:api_mypy— passed🤖 Generated with Claude Code