Skip to content

feat: public URL sharing for Canvas dashboards#8731

Merged
djbarnwal merged 9 commits intomainfrom
feat/canvas-public-url
Mar 12, 2026
Merged

feat: public URL sharing for Canvas dashboards#8731
djbarnwal merged 9 commits intomainfrom
feat/canvas-public-url

Conversation

@djbarnwal
Copy link
Copy Markdown
Member

@djbarnwal djbarnwal commented Jan 30, 2026

https://linear.app/rilldata/issue/APP-200/sharing-public-urls-for-canvas-dashboards

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

@djbarnwal djbarnwal requested a review from ericokuma January 30, 2026 14:08
Copy link
Copy Markdown
Contributor

Everything looks good except I don't see the Dashboard Title listed on /settings/public-urls for the public URL created

Screenshot 2026-02-03 at 9.31.30 AM.png

Copy link
Copy Markdown
Contributor

@ericokuma ericokuma left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See last comment

@djbarnwal djbarnwal requested a review from ericokuma March 6, 2026 20:53
@djbarnwal
Copy link
Copy Markdown
Member Author

Fixed titles
image

@ericokuma
Copy link
Copy Markdown
Contributor

From Claude Code:

Product Functional Testing: Canvas Public URL Feature

I've reviewed all 10 changed files across the feat/canvas-public-url branch. Here's the analysis:


Bugs & Correctness Issues

1. dashboardDataProvider null at submission time In CreatePublicURLForm.svelte, the provider is set via a callback from child components. If the user submits before the child mounts or the callback fires, dashboardDataProvider will be null and throw "Dashboard data provider not initialized". This is a race condition — the form's submit button should be disabled until the provider is registered, but there's no such guard.

2. Code duplication in canvas-form-utils.ts hasCanvasFilters() duplicates the core logic of getCanvasFilters() — both iterate the filter map and check for non-empty expressions. If the filter emptiness check changes, one could drift from the other. hasCanvasFilters should call getCanvasFilters and check if the result is non-empty.

3. Canvas state serialization asymmetry in +page.ts (routing)

if (canvasResource) {
  redirectUrl.search = tokenResp.token.state; // raw params
} else {
  redirectUrl.search = new URLSearchParams({ state: tokenResp.token.state }).toString();
}

If tokenResp.token.state already includes a leading ?, setting redirectUrl.search directly could produce ??... or unexpected behavior. Need to verify the API contract — does state come back with or without the ? prefix?

4. Token resource lookup migration (resourceNameresources?.[0]?.name) In the public URLs settings page, the lookup changed from token.resourceName to token.resources?.[0]?.name. If any existing tokens still use the old resourceName field and don't have resources populated, they'll fail to resolve a dashboard title. This is a backwards-compatibility concern depending on the API migration status.


Security Concerns

5. Filter stripping in getSanitizedCanvasStateUrl() The function strips f and f.* params from the URL to prevent leaking hidden filter info. However, it only checks for params starting with f. If canvas state encodes filter data under other param names (now or in the future), those could leak. Worth verifying this covers all filter-related params comprehensively.

6. Canvas fields set to undefined (full access) In CanvasFiltersSection.svelte, the data provider sends fields: undefined, which means no field restrictions. For explore dashboards, fields limits visible measures/dimensions. Canvas public URLs appear to grant full access to all fields — is this intentional? If canvas dashboards can have field-level restrictions, this is a security gap.


Edge Cases

7. {#if !url} wrapper removal The old code wrapped form inputs in {#if !url} — hiding them once a URL was generated. This wrapper was removed, so form inputs (title, expiration) are now always visible. After creating a public URL, users can see the form fields but presumably can't change them. Could be confusing UX — users might think they can edit the URL's settings.

8. Canvas query enablement in TopNavigationBar.svelte

enabled: !!instanceId && !!dashboard && !!onCanvasDashboardPage && !!onPublicURLPage

The query only fires when BOTH onCanvasDashboardPage AND onPublicURLPage are true. This means the canvas display name won't resolve on the regular (non-public) canvas dashboard page's nav bar. Is that correct? If the nav bar also shows a title on the regular canvas page, this would be a gap.

9. Banner cleanup on navigation The canvas share page adds a banner via eventBus.emit("add-banner", ...) and removes it on beforeNavigate. If the user navigates to another share page (canvas→canvas), the old banner removal and new banner addition could race depending on event ordering.

10. Null safety fix in selector.ts client.instanceIdclient?.instanceId — this is a good defensive fix, but it raises the question: when is client null? If it's null during the canvas share page flow (before runtime connects), does the selector return a sensible default or does it cascade into downstream errors?


Dead Code & Cleanup

11. Commented-out useValidDashboardTitle in settings page There's a commented-out function. Should be removed before merge — dead code in comments decays quickly and adds noise.


Summary

Severity Issue File
Medium dashboardDataProvider null race at submit CreatePublicURLForm.svelte
Medium State param ? prefix ambiguity +page.ts (routing)
Medium fields: undefined grants full access CanvasFiltersSection.svelte
Low hasCanvasFilters / getCanvasFilters duplication canvas-form-utils.ts
Low resourceName → resources[0].name backwards compat settings page
Low Form inputs visible after URL creation CreatePublicURLForm.svelte
Low Canvas query only enabled on public URL pages TopNavigationBar.svelte
Cleanup Commented-out dead code settings page

The overall architecture — splitting into CanvasFiltersSection/ExploreFiltersSection with a callback-based provider pattern — is sound. The main risks are around the provider initialization race, state serialization edge cases, and the fields: undefined full-access behavior for canvas.

@djbarnwal djbarnwal merged commit 3323b05 into main Mar 12, 2026
13 of 14 checks passed
@djbarnwal djbarnwal deleted the feat/canvas-public-url branch March 12, 2026 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants