feat: personal canvas dashboard#9507
Conversation
0daacba to
3fc2cd5
Compare
3fc2cd5 to
d1fc6ca
Compare
nishantmonu51
left a comment
There was a problem hiding this comment.
A few correctness bugs (two nil-pointer panics, a crash in the dashboard listing) and leftover debug logging, plus a layering and an access-control concern. Details inline.
Developed in collaboration with Claude Code
| } | ||
| ownedByUser := userID != "" && userID == spec.Annotations["admin_owner_user_id"] | ||
|
|
||
| shared, ok := spec.Annotations["admin_shared"] |
There was a problem hiding this comment.
When admin_shared is "true" this allows any claims, including public-URL/magic-token contexts, not just project members. The share UI describes this as sharing "with all users in the project"; if that is the intent, the rule should require an authenticated project user rather than allowing all callers.
There was a problem hiding this comment.
These apply on top of underlying access rules. So it wont be accessible by users outside of the project. I can double check once to make sure
There was a problem hiding this comment.
Confirmed it is not accessible by users outside of the project.
|
@begelundmuller : Can you also do a quick review here ? |
There was a problem hiding this comment.
Pull request overview
Adds a personal canvas dashboard feature behind the personalCanvases / personal_canvases feature flag. Personal dashboards are stored as owner-scoped virtual files (via new admin APIs) and are treated as admin-managed canvases through canvas annotations, with UI affordances to distinguish them in listings/status.
Changes:
- Introduces a
FileIOabstraction and wiresfileArtifactsto use either runtime-backed IO or admin “virtual personal file” IO. - Adds admin server + web-admin client support for listing/creating/getting/editing/deleting personal files, backed by
virtual_files.owner_id. - Extends canvas spec with
annotationsand updates UI to list/edit personal dashboards and mark them in status.
Reviewed changes
Copilot reviewed 51 out of 56 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| web-local/src/routes/+layout.ts | Passes a RuntimeFileIO into fileArtifacts setup. |
| web-common/src/features/entity-management/FileAndResourceWatcher.svelte | Sets fileArtifacts client + runtime IO early for descendants. |
| web-common/src/features/entity-management/file-artifacts.ts | Updates fileArtifacts to depend on a FileIO implementation. |
| web-common/src/features/entity-management/file-io.ts | Adds FileIO interface + runtime-backed implementation. |
| web-common/src/features/feature-flags.ts | Adds personalCanvases feature flag. |
| web-common/src/layout/workspace/WorkspaceHeader.svelte | Adds showBreadcrumbs option for isolated editor layouts. |
| web-common/src/features/projects/status/NameCell.svelte | Adds personal-owner icon + tooltip in status table cells. |
| web-common/src/runtime-client/gen/index.schemas.ts | Adds annotations to V1CanvasSpec (type currently too loose). |
| web-common/src/proto/gen/rill/runtime/v1/resources_pb.ts | Adds CanvasSpec.annotations map field in generated TS proto. |
| web-common/src/proto/gen/rill/admin/v1/api_pb.ts | Adds generated admin API messages for personal files. |
| runtime/security.go | Uses canvas annotations to treat admin-managed canvases as owner/admin-only. |
| admin/server/virtual_file.go | Implements personal file CRUD and injects admin-managed annotations into YAML. |
| admin/database/postgres/migrations/0094.sql | Adds owner_id column to virtual_files. |
| admin/database/database.go | Plumbs owner_id through DB types/options. |
| admin/database/postgres/postgres.go | Adds queries/upserts for owner-scoped virtual files. |
| proto/gen/rill/admin/v1/api.pb.gw.go | Adds gateway handlers for personal file endpoints. |
| proto/gen/rill/admin/v1/api_grpc.pb.go | Adds gRPC stubs for personal file endpoints. |
| web-admin/src/client/gen/default/default.ts | Adds web-admin HTTP client + query helpers for personal files. |
| web-admin/src/client/gen/index.schemas.ts | Adds web-admin schema types for personal file endpoints. |
| web-admin/src/features/personal-files/selectors.ts | Combines personal-file list + runtime resources into filtered selectors. |
| web-admin/src/features/personal-files/virtual-file-io.ts | Implements admin-backed FileIO for personal virtual files. |
| web-admin/src/features/personal-files/SharePersonalFile.svelte | Adds UI to toggle sharing via admin_shared annotation. |
| web-admin/src/features/personal-files/canvas/CreatePersonalCanvasDialog.svelte | Create/copy flow for personal dashboards. |
| web-admin/src/features/personal-files/canvas/PersonalCanvasesList.svelte | Renders “My dashboards” section. |
| web-admin/src/features/personal-files/canvas/PersonalCanvasCompositeCell.svelte | Cell renderer for personal dashboard list entries. |
| web-admin/src/features/personal-files/canvas/VirtualCanvasEditor.svelte | Isolated canvas editor for personal dashboards. |
| web-admin/src/features/personal-files/canvas/CanvasPersonalFile.svelte | Personal dashboard view/edit wrapper + delete flow. |
| web-admin/src/features/personal-files/canvas/mode-utils.ts | Stores per-dashboard view/edit mode. |
| web-admin/src/features/dashboards/listing/selectors.ts | Filters dashboards list to include/exclude personal dashboards based on annotations. |
| web-admin/src/features/projects/status/selectors.ts | Determines whether a resource is “personal” based on annotations. |
| web-admin/src/features/projects/ProjectHeader.svelte | Shows share controls for personal dashboards, hides project share on personal page. |
| web-admin/src/routes/[organization]/[project]/+page.svelte | Adds personal dashboards section + create button integration. |
| web-admin/src/routes/[organization]/[project]/-/personal/+layout.ts | Provides VirtualFileIo to the personal routes. |
| web-admin/src/routes/[organization]/[project]/-/personal/+layout.svelte | Injects admin VirtualFileIo into fileArtifacts for personal pages. |
| web-admin/src/routes/[organization]/[project]/-/personal/[name]/+page.ts | Loads the personal file YAML from admin service. |
| web-admin/src/routes/[organization]/[project]/-/personal/[name]/+page.svelte | Binds the loaded YAML into a FileArtifact and renders the personal canvas page. |
Files not reviewed (4)
- proto/gen/rill/admin/v1/api.pb.gw.go: Generated file
- proto/gen/rill/admin/v1/api_grpc.pb.go: Generated file
- proto/gen/rill/runtime/v1/resources.pb.go: Generated file
- proto/gen/rill/runtime/v1/resources.pb.validate.go: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
59ed587 to
13ed8b0
Compare
| return nil, err | ||
| } | ||
|
|
||
| return res.Resource.Resource.(*runtimev1.Resource_Canvas).Canvas.Spec, nil |
There was a problem hiding this comment.
Could it be a problem that it returns Spec and not ValidSpec?
There was a problem hiding this comment.
Yes this is a copy paste error. Updated to return ValidSpec
| var doc map[string]any | ||
| if err := yaml.Unmarshal([]byte(data), &doc); err != nil { | ||
| return nil, status.Errorf(codes.InvalidArgument, "invalid YAML: %s", err.Error()) | ||
| } | ||
| if doc == nil { | ||
| doc = map[string]any{} | ||
| } | ||
|
|
||
| if displayName != "" { | ||
| doc["display_name"] = displayName | ||
| } | ||
|
|
||
| annotations, _ := doc["annotations"].(map[string]any) | ||
| if annotations == nil { | ||
| annotations = map[string]any{} | ||
| } | ||
| annotations["admin_owner_user_id"] = ownerID | ||
| annotations["admin_managed"] = "true" | ||
| annotations["admin_nonce"] = time.Now().Format(time.RFC3339Nano) | ||
| doc["annotations"] = annotations | ||
|
|
||
| out, err := yaml.Marshal(doc) |
There was a problem hiding this comment.
Maybe I'm missing something, but I don't see a check that doc["type"] == kind?
Generally would be good to carefully audit this code and ensure it only allows creation of canvas resources, not any other resource types (which could be a significant security leak).
There was a problem hiding this comment.
The initial yaml came from UI in the current path so explicit setting was not needed. But I updated to have an explit set in the backend to prevent clients creating non-canvas dashbaords.
* feat: personal canvas dashboard * Update APIs * Add the UI for viewing personal files * Fix security policies * fix edge cases with save/load * Add edit/preview modes * Fix first component stuck on loading spinner * Add sharing support * Add delete * Fix canvas not in list * fix lint * PR comments * Use switch for canvas source selection * PR comments * PR comments and bug fixes --------- Co-authored-by: Nishant Bangarwa <nishant.monu51@gmail.com>
This reverts commit 5dbdec6.
Adds a personal canvas dashboard feature under
personal_canvasesfeature flag. Saved as virtual files with user id as owner. These wont show up in primary dashboard lists but will show up in status with a user icon beside it.Closes ENG-1123
Checklist: