fix: cast avatar_asset to CharField to resolve mixed type errors in URL concatenation - #9512
Conversation
|
Linked to Plane Work Item(s) References This comment was auto-generated by Plane |
📝 WalkthroughWalkthroughAvatar asset identifiers are explicitly cast to character fields before URL concatenation across analytics, distribution, search, issue actor, export, and cycle-transfer queries. New contract tests cover affected endpoints and verify static asset URL output. ChangesAvatar URL casting
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/api/plane/space/views/issue.py (1)
713-722: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse the reaction actor fields for reaction URLs.
Line 716 still casts
votes__actor__avatar_asset, so reaction payloads can return the wrong actor avatar (ornullwhen there is no vote). Useissue_reactions__actor__*throughout thisCase.Proposed fix
- votes__actor__avatar_asset__isnull=False, + issue_reactions__actor__avatar_asset__isnull=False, then=Concat( Value("/api/assets/v2/static/"), - Cast("votes__actor__avatar_asset", CharField()), + Cast("issue_reactions__actor__avatar_asset", CharField()), Value("/"), ), ), When( - votes__actor__avatar_asset__isnull=True, - then=F("votes__actor__avatar"), + issue_reactions__actor__avatar_asset__isnull=True, + then=F("issue_reactions__actor__avatar"), ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/plane/space/views/issue.py` around lines 713 - 722, Update the reaction URL Case expression to use issue_reactions__actor__* fields consistently instead of votes__actor__* fields, including the avatar asset null check, Cast, and fallback avatar reference. Preserve the existing URL construction and fallback behavior while applying these changes throughout the Case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@apps/api/plane/space/views/issue.py`:
- Around line 713-722: Update the reaction URL Case expression to use
issue_reactions__actor__* fields consistently instead of votes__actor__* fields,
including the avatar asset null check, Cast, and fallback avatar reference.
Preserve the existing URL construction and fallback behavior while applying
these changes throughout the Case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c5d1e1d-7bc9-4c7c-b643-a3f1df07f31e
📒 Files selected for processing (12)
apps/api/plane/app/views/analytic/base.pyapps/api/plane/app/views/analytic/project_analytics.pyapps/api/plane/app/views/cycle/archive.pyapps/api/plane/app/views/cycle/base.pyapps/api/plane/app/views/module/archive.pyapps/api/plane/app/views/module/base.pyapps/api/plane/app/views/search/base.pyapps/api/plane/bgtasks/analytic_plot_export.pyapps/api/plane/space/utils/grouper.pyapps/api/plane/space/views/issue.pyapps/api/plane/tests/contract/app/test_avatar_url_annotation.pyapps/api/plane/utils/cycle_transfer_issues.py
Description
Module detail pages return a 500 and the UI falls back to its "no modules exist" empty state. Creating a work item with a module attached shows "work item creation failed" — even though the work item is created and is linked to the module.
Both are the same bug: a Django 4.2 → 5.2 regression in every queryset that builds a user's avatar URL in SQL.
On Django 5.x,
ConcatPair.as_postgresqlresolves the output field of each argument to decide whether it needs a text cast.avatar_assetis a UUID FK column, so pairing it with aCharFieldraises:The affected querysets are lazy, so this fires during response rendering — after DRF's exception handler is out of scope — which is why it surfaces as a bare 500 with no useful error body.
Why the module list works but module detail doesn't:
ModuleViewSet.listuses.values()and never touches the assignee distribution.retrievebuilds it, and dies.Why work-item creation reports failure:
addIssueToModuleinapps/web/core/components/issues/issue-modal/base.tsxawaitsfetchModuleDetailsGETs the module detail endpoint. That 500s,Promise.allrejects, and thecatchinhandleCreateIssueraises the error toast — after both writes have already committed. So the reported failure is cosmetic, but it leaves users re-creating work items that already exist.Fix: cast the UUID column to text at all 24 call sites (11 files), matching each site's existing
output_field=style:No behaviour change —
avatar_urlrenders identically (/api/assets/v2/static/<uuid>/), asserted in the new tests.Setting
output_fieldon the outerConcatdoes not work here:Concat.__init__wraps its arguments in nestedConcatPairs that each resolve their own output field, so the inner pair still raises.Type of Change
Screenshots and Media (if applicable)
N/A — server-side 500 with no response body. Reproduced and verified at the API layer instead; see below.
Test Scenarios
Added
apps/api/plane/tests/contract/app/test_avatar_url_annotation.py— 9 contract tests covering the affected endpoints, with a user whose avatar comes from aFileAssetso the previously-brokenCasebranch is actually exercised. All 9 fail on the unpatched tree and pass with the fix.Endpoints confirmed returning 500 /
FieldErrorbefore this change, and 200 after:GET .../modules/<id>/GET .../archived-modules/<id>/GET .../archived-cycles/<id>/GET .../cycles/<id>/analytics/?type=issuesGET .../analytics/?x_axis=assignees__id&y_axis=issue_countGET .../default-analytics/GET .../advance-analytics-stats/?type=work-items(also with&cycle_id=)GET .../entity-search/?query_type=user_mentionPOST .../cycles/<id>/transfer-issues/GET /api/public/anchor/<anchor>/issues/Also patched, same pattern, not covered by the new tests:
plane/bgtasks/analytic_plot_export.py— the analytics CSV export. The endpoint returns 200 because the work is deferred, so theFieldErrorlands in the Celery worker rather than the response.plane/space/utils/grouper.py— vote/reaction actor avatars in the public space grouper.plane/app/views/search/base.py,plane/app/views/module/archive.py,plane/app/views/cycle/base.py,plane/app/views/cycle/archive.py— remaining sites of the identical expression, latent under other query parameters.Verified there are no remaining unpatched instances: all 24
Concat(call sites underapps/api/planeare this avatar pattern, and all now cast.Full suite: 492 passed, 12 failed. All 12 failures are pre-existing
test_authentication.pymagic-link tests that need instance configuration — confirmed failing identically on a clean checkout of the parent commit.ruff check --fix apps/api(what CI runs) exits 0. The 3F401s it auto-fixes are pre-existing inplane/app/views/issue/sub_issue.pyandplane/app/views/project/invite.py, both untouched here.References
WEB-8477 — Bug 1 ("Unable to create work item after selecting module").
Introduced by #9325 (
chore: upgrade Django 4.2 → 5.2).This also repairs the public-space work items endpoint and several analytics
endpoints that fail the same way, so WEB-8477 Bug 2 (empty Cycles chart) and
Bug 3 (published project URL) are worth re-testing against this branch — both
hit affected endpoints, but neither was confirmed as this root cause.
Summary by CodeRabbit
Bug Fixes
Tests