Skip to content

refactor(clients)!: retire the multipart upload path - #2405

Draft
vecchp wants to merge 1 commit into
mainfrom
DEV-warnings/remove-update-client-profile-photo
Draft

vecchp wants to merge 1 commit into
mainfrom
DEV-warnings/remove-update-client-profile-photo

Conversation

@vecchp

@vecchp vecchp commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Stacked on #2402 — review that first. Base retargets to main once it merges.

This is the follow-up #2402 named but deliberately left undone:

The backend updateClientProfilePhoto mutation intentionally stays. Unlike createClientDocument, it is in the shipped bundle, so it has to outlive this build reaching users. Removing it is a later PR once the rollout is confirmed — not an oversight.

⚠️ Merge gate — this is a draft on purpose

Do not take it out of draft until both hold:

  1. refactor(common)!: remove the dead multipart client-document upload #2402 has merged to main.
  2. The production EAS update carrying the deleteClientProfilePhoto wiring has published, and no active runtime version is still serving a pre-refactor(common)!: remove the dead multipart client-document upload #2402 bundle.

An installed binary still running the old bundle sends updateClientProfilePhoto(photo: null) as its delete path. Merging this before it has taken the update breaks photo deletion for those users.

What goes

updateClientProfilePhoto was deprecated in favour of generateClientProfilePhotoUpload/resolveClientProfilePhotoUpload for uploads and deleteClientProfilePhoto for removal. #2402 moved ProfilePhotoModal off the last remaining caller, so nothing in the tree references it any more.

Removed: the mutation, ClientProfilePhotoInput, and the now-unused strawberry.file_uploads.Upload import in clients/types.py.

scalar Upload stays in the schema: CreateClientProfileInput.profilePhoto and CreateHmisClientProfileInput.profilePhoto are auto-derived by strawberry_django from the model's ImageField. Retiring those is a separate question about model-derived inputs, not part of this change.

Test changes

_update_client_profile_photo_fixture had four call sites. One tested the deleted mutation; the other three only needed a photo to exist:

  • test_update_client_profile_photo is deleted. The presigned path it was standing in for is already covered by test_generate_client_profile_photo_upload and test_resolve_client_profile_photo_upload.
  • The fixture is replaced by _set_client_profile_photo, which writes the photo onto the model. It returns build_img_url(...) rather than profile_photo.url — the GraphQL type renders profile photos through imgproxy, so the seeded value has to be produced the same way the assertions read it.

Verification

  • pytest — 1519 passed, 31 skipped
  • mypy clean (cache cleared first), ruff clean
  • expo-betterangels tsc --noEmit clean
  • schema.graphql regenerated; frontend types regenerated across all four codegen consumers. The generated diff is confined to ClientProfilePhotoInput, MutationUpdateClientProfilePhotoArgs, UpdateClientProfilePhotoPayload and the Mutation field.

🤖 Generated with Claude Code

Summary by Sourcery

Remove the deprecated multipart client profile photo upload path now that the presigned upload and deletion flows are established.

Enhancements:

  • Retire the legacy client profile photo mutation and its associated input and generated API types.
  • Preserve profile photo fields on client and HMIS profile outputs while removing obsolete upload fields from generated schema inputs.
  • Simplify profile photo test setup by seeding model images directly and generating URLs consistently with production rendering.

Tests:

  • Remove coverage for the retired mutation and update related query and deletion tests to use direct photo setup.

Chores:

  • Regenerate the GraphQL schema and frontend client types after removing the deprecated photo mutation.

@sourcery-ai

sourcery-ai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor
Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

This PR completes the client profile photo mutation deprecation by removing its resolver, input and generated schema artifacts, while preserving the Upload scalar required by model-derived inputs. Tests now seed photos directly through the model and continue validating the supported presigned upload, resolution, deletion, and imgproxy-rendered query paths; review the draft merge gate carefully before approval.

File-Level Changes

Change Details Files
Remove the deprecated client profile photo mutation and its schema/input types.
  • Delete the resolver and ClientProfilePhotoInput definition.
  • Remove the mutation, argument input, and payload union from the generated GraphQL schema and regenerate frontend types.
  • Retain the Upload scalar used by model-derived profile creation inputs.
apps/betterangels-backend/clients/schema.py
apps/betterangels-backend/clients/types.py
apps/betterangels-backend/schema.graphql
libs/ba-platform/src/lib/apollo/graphql/__generated__/types.ts
libs/expo/shared/clients/src/lib/apollo/graphql/__generated__/schema.ts
Update backend tests to seed profile photos directly instead of invoking the removed mutation.
  • Replace the GraphQL upload fixture with a model-level helper that saves a ContentFile and returns the imgproxy URL.
  • Remove the obsolete mutation test and update query and deletion tests to use the new helper.
apps/betterangels-backend/clients/tests/utils.py
apps/betterangels-backend/clients/tests/test_mutations.py
apps/betterangels-backend/clients/tests/test_queries.py

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

vecchp pushed a commit that referenced this pull request Sep 1, 2026
`createClientDocument` was the last multipart upload mutation. The presigned
pipeline replaced it in #2018, which deleted the components that called it, and
nothing has called it since: no `.graphql` document, no generated operation, no
component, and zero occurrences in the compiled mobile bundle — against one
occurrence of `updateClientProfilePhoto` in the same bundle, so the search is
sound rather than a false negative. Its only remaining consumers were backend
test fixtures.

Reviewing the previous commit surfaced that this path was also entirely
unvalidated — no size limit and no content-type allowlist, because
`CLIENT_DOCUMENT_CONFIG` is only ever applied on the presigned side. Rather than
add a validator and an exception class to code nobody reaches, remove the path:

- the `create_client_document` mutation and `CreateClientDocumentInput`
- `create_multipart_attachment`, added in the previous commit and now orphaned
- `python-magic`, whose only two call sites were the deleted `Attachment.save()`
  override and this service

`_create_client_document_fixture` now builds its document through the presigned
mutations, so the ~28 test call sites keep working. Two tests went with the
mutation: `test_create_client_document`, and
`test_create_client_document_permission`, which duplicated
`test_resolve_client_document_uploads_permission` parameter for parameter.

`updateClientProfilePhoto` stays. It is present in the shipped bundle, so it has
to outlive the app taking #2406, which moves photo deletion onto
`deleteClientProfilePhoto`. #2405 removes it once that rollout is confirmed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vecchp
vecchp force-pushed the DEV-warnings/attachment-save-deprecation branch from 2b78694 to ca8f224 Compare September 1, 2026 00:13
@vecchp
vecchp force-pushed the DEV-warnings/remove-update-client-profile-photo branch from 5c5c626 to ebd427c Compare September 1, 2026 00:24
@vecchp vecchp changed the title refactor(clients)!: remove the deprecated updateClientProfilePhoto mutation refactor(clients)!: retire the multipart upload path Sep 1, 2026
vecchp pushed a commit that referenced this pull request Sep 1, 2026
`createClientDocument` was the last multipart upload mutation. The presigned
pipeline replaced it in #2018, which deleted the components that called it, and
nothing has called it since: no `.graphql` document, no generated operation, no
component, and zero occurrences in the compiled mobile bundle — against one
occurrence of `updateClientProfilePhoto` in the same bundle, so the search is
sound rather than a false negative. Its only remaining consumers were backend
test fixtures.

Reviewing the previous commit surfaced that this path was also entirely
unvalidated — no size limit and no content-type allowlist, because
`CLIENT_DOCUMENT_CONFIG` is only ever applied on the presigned side. Rather than
add a validator and an exception class to code nobody reaches, remove the path:

- the `create_client_document` mutation and `CreateClientDocumentInput`
- `create_multipart_attachment`, added in the previous commit and now orphaned
- `python-magic`, whose only two call sites were the deleted `Attachment.save()`
  override and this service

`_create_client_document_fixture` now builds its document through the presigned
mutations, so the ~28 test call sites keep working. Two tests went with the
mutation: `test_create_client_document`, and
`test_create_client_document_permission`, which duplicated
`test_resolve_client_document_uploads_permission` parameter for parameter.

`updateClientProfilePhoto` stays. It is present in the shipped bundle, so it has
to outlive the app taking #2406, which moves photo deletion onto
`deleteClientProfilePhoto`. #2405 removes it once that rollout is confirmed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vecchp
vecchp force-pushed the DEV-warnings/attachment-save-deprecation branch from ca8f224 to 4f0643e Compare September 1, 2026 15:47
Base automatically changed from DEV-warnings/attachment-save-deprecation to main September 8, 2026 14:46
@vecchp
vecchp force-pushed the DEV-warnings/remove-update-client-profile-photo branch from ebd427c to a57017e Compare September 8, 2026 15:21
@vecchp vecchp added the graphql-inspector:approved-breaking-change Auto approve breaking changes to graphql schema label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

🚀 Expo continuous deployment is ready for betterangels!

  • Project → betterangels
  • Environment → Preview
  • Platforms → android, ios
  • Scheme → betterangels
  🤖 Android 🍎 iOS
Runtime Version 4d84f3f87051ebb9c1713e7e2d4522e4b23135d9 4d84f3f87051ebb9c1713e7e2d4522e4b23135d9
Build Details Build Permalink
DetailsDistribution: INTERNAL
Build profile: preview
Runtime version: 4d84f3f87051ebb9c1713e7e2d4522e4b23135d9
App version: 1.2.11
Git commit: a1900145e6c2315da5a85930acd4fcf2f8619aa2
Build Permalink
DetailsDistribution: INTERNAL
Build profile: preview
Runtime version: 4d84f3f87051ebb9c1713e7e2d4522e4b23135d9
App version: 1.2.11
Git commit: a1900145e6c2315da5a85930acd4fcf2f8619aa2
Update Details Update Permalink
DetailsBranch: DEV-warnings-remove-update-client-profile-photo
Runtime version: 4d84f3f87051ebb9c1713e7e2d4522e4b23135d9
Git commit: 89f8b949f4e0203813b2fba3077cd714916f4534
Update Permalink
DetailsBranch: DEV-warnings-remove-update-client-profile-photo
Runtime version: 4d84f3f87051ebb9c1713e7e2d4522e4b23135d9
Git commit: 89f8b949f4e0203813b2fba3077cd714916f4534
Update QR

iOS Simulator Build: Simulator Build Link

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

🔍 [betterangels-admin] Preview available at: https://admin.dev.betterangels.la/branches/DEV-warnings-remove-update-client-profile-photo

Last updated: 2026-09-10T21:52:40.835Z

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown

🔍 [shelter-web] Preview available at: https://shelter.dev.betterangels.la/branches/DEV-warnings-remove-update-client-profile-photo

Last updated: 2026-09-10T21:52:40.835Z

Uploads moved to generateClientProfilePhotoUpload/resolveClientProfilePhotoUpload
and removal to deleteClientProfilePhoto, leaving updateClientProfilePhoto with no
caller in the tree.

`profilePhoto` also came off the four client-profile inputs. It was derived from
`profile_photo: auto` on bases shared by the output types and the inputs, so
`createClientProfile`, `updateClientProfile` and their HMIS counterparts all
accepted a multipart file with no size limit and no content-type allowlist —
the same unvalidated surface `createClientDocument` was deleted for. Declaring
the field on the output types only keeps it readable and takes it off the inputs.

Nothing sent it. Both call sites stripped it first: `toUpdateClientProfileInputHmis`
destructured it away as a "disallowed field", which the type now does for it, and
`ClientProfileForm` still deletes it because its own form values carry a photo.

`scalar Upload` leaves the schema with it.

The test fixture that drove the mutation is replaced by a helper that writes the
photo onto the model, since the three remaining call sites only needed a photo to
exist. It returns the URL through build_img_url so the seeded value matches what
the GraphQL type renders.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vecchp
vecchp force-pushed the DEV-warnings/remove-update-client-profile-photo branch from f4f7c4b to c92dc55 Compare September 10, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

graphql-inspector:approved-breaking-change Auto approve breaking changes to graphql schema

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants