FE-1566: Create avatar links in the web of the image they point at - #9499
FE-1566: Create avatar links in the web of the image they point at#9499claude[bot] wants to merge 7 commits into
Conversation
The web the link is created in decides who can see it. Deriving it from the uploaded file entity keeps a link in the same web as the image it points at.
Each link outside the web of the image it points at is archived and replaced by a link in that web, carrying the same endpoints, properties and public visibility. A link which already has a replacement is only archived, so the migration is safe to re-run.
|
The latest updates on your projects. Learn more about Vercel for GitHub. 4 Skipped Deployments
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9499 +/- ##
==========================================
- Coverage 61.03% 61.01% -0.02%
==========================================
Files 1449 1450 +1
Lines 144693 144820 +127
Branches 6692 6707 +15
==========================================
+ Hits 88307 88357 +50
- Misses 55282 55355 +73
- Partials 1104 1108 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
PR SummaryMedium Risk Overview Going forward, avatar link creation in In the workspace switcher, avatars with an image get a white background so transparent logos don’t show grey/blue row or header highlights on hover/focus/selection. Reviewed by Cursor Bugbot for commit 600c40f. Bugbot is set up for automated code reviews on this repo. Configure here. |
Collect every `Has Avatar` link across all webs, group them by the user or organization they belong to, keep the one with the latest `createdAtDecisionTime` in the web of its image, and archive the rest. An image left with no link pointing at it in any web is archived too.
There was a problem hiding this comment.
🟡 Changes recommended
Unpaginated migration queries can miss records and archive images that remain referenced.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes organization avatar visibility by colocating avatar links with their image entities and repairing existing data.
Changes:
- Creates file links in the file entity’s web.
- Migrates existing avatar links and archives orphaned images.
- Preserves white avatar backgrounds in the workspace switcher.
File summaries
| File | Description |
|---|---|
workspace-switcher.tsx |
Adds white image-avatar backgrounds. |
file-upload-context.tsx |
Creates links in the file’s web. |
031-move-avatar-links-to-image-web.migration.ts |
Repairs avatar links and orphaned images. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
CiaranMn
left a comment
There was a problem hiding this comment.
I confirmed this works locally by:
- Adding an avatar locally on
main - Switching to the branch. Migration ran and link moved to the org web.
- Blue focus bug has gone
@TimDiekmann for a second pair of eyes on the migration
| const policies: CreateEntityParameters["policies"] = publiclyViewable | ||
| ? [ | ||
| { | ||
| name: publicViewPolicyName(entityUuid), |
There was a problem hiding this comment.
I'm not sure if we want to create a policy for each avatar, this creates policies for every avatar and does not scale. Shouldn't avatars not always be public? Then we can determine that through the entity type itself.
There was a problem hiding this comment.
This block does not create a policy per avatar. It carries over a per-link public-view-entity-<uuid> policy only when the original link already had one (queryPolicies by name at :205-211, policies is undefined otherwise). Those per-link policies come from today's upload path: the frontend passes makePublic for the Has Avatar link as well as the file (file-upload-context.tsx:486-500), and createEntityResolver turns that into a per-entity policy (entity.ts:88-98), the same way file.ts:247-256 does for the image. So the migration mirrors the existing convention rather than introducing it.
Agreed that a type-level rule is the better shape. It is one more entity_is_of_base_type() entry for Has Avatar in the filters of the seeded public-view-entity policy (seed_policies.rs:386-401), and update_seeded_policies pushes the changed resource constraint to existing databases on the next hash-graph migrate. With that in place this carry-over goes away and the frontend can stop passing makePublic for the link.
One caveat: the image still needs a view grant for non-members. default-web-view-entity is role-scoped, so the per-file public-view-entity-<uuid> from file.ts stays load-bearing unless Image File is also made public by type, which would expose every uploaded image, not only avatars.
Happy to do either: add the Has Avatar type policy in this PR and drop this block, or keep this PR data-only and open a follow-up for the type-level policy. Which do you prefer?
Generated by Claude Code
Requested by Ciaran Morinan · Slack thread
Before: an organization's avatar is visible only to the person who uploaded it. Every other member of the organization sees the empty "Click to upload" dropzone on the organization's settings page, and no image next to the organization's name elsewhere in the app. Because each member saw the organization as having no avatar, re-uploading created a fresh image and a fresh
Has Avatarlink each time, so one organization can have several links to several images spread across the personal webs of whoever uploaded them.After: every member of the organization sees the avatar. When the API next starts, each user and organization is left with a single
Has Avatarlink, the most recently created one, in the same web as the image it points at; every other avatar link is archived, and an image nothing links to any more is archived with it. In the workspace switcher at the top of the sidebar, an avatar tile keeps a white background when its row or the header button is hovered, focused or selected, so a logo with transparent pixels no longer shows the grey or blue highlight through it.How: the avatar's Image File entity is created in the organization's web, but the
Has Avatarlink to it was created with nowebId.$webIdis optional in thecreateEntitymutation, soapps/hash-api/src/graphql/resolvers/knowledge/entity/entity.ts:115fell back towebId ?? user.accountIdand the link landed in the uploading user's own web, where no other member of the organization holds a role.apps/hash-frontend/src/shared/file-upload-context.tsx:497now passes the web of the file entity the link points at, so the link is created in the same web as the image.The links already stored are repaired by
031-move-avatar-links-to-image-web.migration.ts. It visits every user and organization web as that web's machine actor, since no single actor can see the entities of every web, collects the non-archivedHas Avatarlinks in each, and groups them by their left entity, the user or organization they belong to. Within a group the link with the latestmetadata.provenance.createdAtDecisionTimewins; a tie goes to a link already in its image's web. If the winner sits outside its image's web, a replacement with the same left entity, right entity, properties, draft state and public-view policy is created in the image's web and the original is archived; a winner that already has such a replacement is archived without a second one being created. Every other link in the group is archived as the machine actor of the web it is in. The images those archived links pointed at are then checked across every web for any remaining non-archived link of any type pointing at them, and an image with none is archived by its own web's machine actor. A second run finds one link per user and organization, in the image's web, and changes nothing.The switcher tile showed the row highlight because
Avatarin@hashintel/design-systemsets no background when it has asrc, so the image sat directly on the row.apps/hash-frontend/src/shared/layout/layout-with-sidebar/sidebar/workspace-switcher.tsxnow gives the header button'sAvatarand each row'sAvatarapalette.common.whitebackground whenever it has an image. An avatar without an image keeps the component's own blue fill behind the initial.🔗 Related links
🔍 What does this change?
apps/hash-frontend/src/shared/layout/layout-with-sidebar/sidebar/workspace-switcher.tsxsets a white background on the header button'sAvatarand each workspace row'sAvatarwhen they have an image.apps/hash-frontend/src/shared/file-upload-context.tsx:497passeswebIdto thecreateEntitymutation for the link entity, taken from the file entity the link points at.apps/hash-api/src/graph/ensure-system-graph-is-initialized/migrate-ontology-types/migrations/031-move-avatar-links-to-image-web.migration.tsholds the whole repair: collectingHas Avatarlinks per web, grouping them by user or organization, keeping the newest in the image's web, archiving the rest, and archiving images left without a link.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
The resolver default is unchanged:
entity.ts:115still assigns a link entity touser.accountIdwhen the caller omitswebId. Only the avatar call site passes awebId, so any other caller that omits one still creates its link in the acting user's web.If a migration run is interrupted after the superseded links are archived but before their images are, a later run has no link left to find those images by, and they stay unarchived. No duplicate link results from an interruption at any point.
🐾 Next steps
Suggested follow-ups, none of them in this PR:
webIdto the left entity's web inentity.ts:115, which would cover every caller that omits it.Has Avatarlink's public-view policy from the UI.org-form.tsxanduse-update-profile-avatar.tsxomitlinkedEntityDatawhen the profile already has an avatar, sofile-upload-context.tsx:412returns before any link work and replacing an avatar cannot add a policy the link is missing.makePublicis true and an existing file entity is updated.apps/hash-api/src/graph/knowledge/system-types/file.ts:247and:435attach it only on the create branch. PassingmakePublic: truefromuse-update-profile-avatar.tsx:42, asorg-form.tsx:152does, would match the two call sites.apps/hash-frontend/src/lib/user-and-org.ts:194and:204instead of skipping an unreadable avatar link silently.🛡 What tests cover this?
No automated tests. ESLint and
oxfmtwere run against the changed files and are clean.tsc --noEmitand the unit tests could not be run in the environment this branch was written in: both need the generated types from@blockprotocol/type-system, which come from awasm-packandcargo-instabuild driven bymise, and the downloads for those tools are blocked there. CI covers both, and the backend integration job runs the migration on API startup.❓ How to test this?
yarn workspace @apps/hash-api start:migratetwice against a database holding an organization with two or moreHas Avatarlinks created before this branch, by different members. After the first run the organization has one non-archivedHas Avatarlink, the most recently created one, in the organization's web; the older links and the images only they pointed at are archived. The second run makes no further change.