Skip to content

Point at Gaffer and Navigator from the embedded UI - #5695

Open
George-Payne wants to merge 4 commits into
masterfrom
embedded-ui-tool-links
Open

Point at Gaffer and Navigator from the embedded UI#5695
George-Payne wants to merge 4 commits into
masterfrom
embedded-ui-tool-links

Conversation

@George-Payne

@George-Payne George-Payne commented Aug 3, 2026

Copy link
Copy Markdown
Member

Surfaces Gaffer from the projections pages, and turns the Tools sidebar's Navigator entry into a deep link at the node you are viewing.

Sidebar

image

Projections Overview

image

Projections detail

image
  • Components/Projections/Projections.razor — a dense 38px ribbon under the page header, the whole band linking out. It sits inside the available-and-leader branch so it never stacks under the "not enabled" or "open projections on the leader" notices, which say something more urgent.
  • Components/Projections/ProjectionDetail.razor — a link beside the Source heading, for user projections only. System projections ship with the server, so there is nothing to author locally and no link to offer.
  • Components/Layout/NavMenu.razor — a Gaffer entry in Tools, and Navigator now hands the browser a kurrentdb:// connection string built from the address the browser actually reached this node on, rather than the node's advertised address, which can be cluster-internal and unreachable from the client. +discover only when gossip reports more than one member.
  • Components/Pages/Navigator.razor — deleted, with wwwroot/navigator.png (232K out of the binary). Its content was a sentence, a download link and a screenshot; the sidebar entry does that job directly.
  • Components/Shared/GafferLink.cs, NavigatorLink.cs — outbound URL construction. Gaffer links are UTM-tagged with the same source/medium/campaign Navigator sends, so the two admin surfaces aggregate as one referral channel rather than two. The Navigator connection string deliberately carries no credentials — this UI never sees the signed-in user's password — and adds tls=false for an http node, without which Navigator attempts TLS against an insecure server and fails to connect.
  • UI/Theme/KurrentIcons.cs — the Gaffer logomark, ported from Navigator with its 327 34 52 52 source viewBox mapped onto MudIcon's 24x24.
  • KurrentDB.csprojInternalsVisibleTo for KurrentDB.Components.Tests, so the URL building is testable without making the helpers public. Core, Api.V2 and others already declare the same.
  • Tests — 12 new in KurrentDB.Components.Tests: ribbon presence across the page's available, populated and subsystem-disabled states, the detail link's system/user gating, and the Navigator connection string's scheme selection, tls flag and explicit port.

Projections authored in this UI are click-ops: untracked, unreviewable, and
impossible to test before they run in production. Surface Gaffer at the three
places someone is most likely to be about to write one by hand.

- Projections list: a dense ribbon under the page header, the whole band
  linking out. Unconditional on the working page, but inside the
  available-and-leader branch so it never stacks under the "not enabled" or
  "open projections on the leader" notices, which say something more urgent.
- Projection detail: a link beside the Source heading, for user projections
  only. System projections ship with the server, so there is nothing to
  author locally and no link to offer.
- Tools sidebar: a Gaffer entry below Navigator, opening in a new tab.
- KurrentIcons.Gaffer: the logomark ported from Navigator, its "327 34 52 52"
  source viewBox mapped onto MudIcon's 24x24.
- GafferLink: one home for the UTM contract. source/medium and the projections
  campaign match what Navigator sends for the same links, so both admin
  surfaces aggregate as one referral channel rather than two.

Copy is sentence case per the handbook, and the ribbon reads as a label rather
than a sentence so all four verbs fit on one 38px line.
The Tools entry led to a page whose whole content was a sentence, a download
link and a screenshot. Replace it with the thing someone wanted from it: hand
the browser a connection string for the node they are looking at, so an
installed Navigator opens on it, and offer the download only when nothing
claims the scheme.

Mirrors the Cloud console's connect modal (bespin
ui/src/components/modals/connect-modal/utils/openNavigator.ts), with three
differences:

- No credentials in the connection string. Cloud can assume its own default
  admin password; this UI never sees the signed-in user's, and a connection
  string is not the place for one. Navigator prompts instead.
- tls=false when the node serves http, or Navigator attempts TLS against an
  insecure node and fails to connect, which reads as the deep link being
  broken.
- Loss of page focus, not the return value of window.open, as the signal that
  something claimed the scheme. No browser API reports whether a protocol
  handler ran, so both are heuristics, but the returned window is truthy either
  way and never triggers the fallback.

The connection string is built from the address the browser reached this node
on rather than the node's advertised address, which can be cluster-internal and
unreachable from the client. +discover only when gossip reports more than one
member.

InternalsVisibleTo on the host project so the string building can be tested
without making the helpers public; every other project that needs this already
declares it.
@George-Payne
George-Payne requested a review from a team as a code owner August 3, 2026 11:15
@George-Payne George-Payne self-assigned this Aug 3, 2026
The embedded UI guide listed Navigator under "Other pages" and described a
feature-comparison table, neither of which is true now that the page is gone
and the entry opens the app. Split the sidebar's Tools section out from the
pages, since these two links leave the UI rather than navigating it, and cover
Gaffer alongside.
@George-Payne
George-Payne requested a review from a team as a code owner August 3, 2026 11:17
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Aug 3, 2026

Copy link
Copy Markdown

Deploying eventstore with  Cloudflare Pages  Cloudflare Pages

Latest commit: c2cbbeb
Status: ✅  Deploy successful!
Preview URL: https://3a100354.eventstore.pages.dev
Branch Preview URL: https://embedded-ui-tool-links.eventstore.pages.dev

View logs

@qodo-code-review

Copy link
Copy Markdown
Contributor

PR Summary by Qodo

Add Gaffer links and Navigator deep-linking from the embedded UI

✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

Grey Divider

AI Description

• Add Gaffer outbound links on projections list/detail pages and the Tools sidebar.
• Turn Navigator sidebar entry into a kurrentdb:// deep link with download fallback.
• Add link-building helpers, icon asset, CSS hover affordance, and bUnit/xUnit coverage.
Diagram

graph TD
  PR["Projections pages"] --> GL("(GafferLink)") --> GA{{"Gaffer web"}}
  NM["NavMenu (Tools)"] --> NL("(NavigatorLink)") --> JS["kurrentNavigator JS"] --> OS{{"OS protocol handler"}} --> NAV{{"Navigator app"}}
  JS --> NDL{{"Navigator download"}}
  NM --> GL

  subgraph Legend
    direction LR
    _ui["UI component"] ~~~ _helper("(Link helper)") ~~~ _ext{{"External"}}
  end
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Keep a dedicated /ui/navigator page (two-step launch)
  • ➕ Avoids protocol-handler detection heuristics and timing sensitivity
  • ➕ Keeps JS interop out of shared App layout
  • ➕ Allows richer explanatory copy and troubleshooting guidance
  • ➖ Adds extra click and interrupts the workflow from the Tools sidebar
  • ➖ Retains static assets/content that can go stale (e.g., screenshots)
  • ➖ Still ultimately depends on external protocol handling to launch the app
2. Always open download page alongside the deep link (no heuristic)
  • ➕ No need to infer whether a protocol handler ran
  • ➕ Guarantees users see the download/learn-more option immediately
  • ➖ Creates noisy UX (an extra tab even when Navigator is installed)
  • ➖ More likely to trigger popup blockers / be perceived as spammy behavior
3. Attempt deep link via hidden iframe (classic custom-scheme pattern)
  • ➕ Sometimes reduces navigation disruption compared to location.href
  • ➕ Can be isolated from the main page URL bar/history
  • ➖ Inconsistent across modern browsers and can be blocked for security reasons
  • ➖ Harder to reason about and test; often breaks under tightened browser policies

Recommendation: The PR’s approach (direct deep link with a short, gesture-friendly fallback) is a pragmatic balance for modern browser constraints, and it eliminates an extra navigation step. Keep the heuristic as implemented, but reviewers should pay attention to edge cases around focus/visibility events and timeout behavior across browsers (especially Safari) since there is no reliable protocol-handler success signal.

Files changed (12) +329 / -2

Enhancement (9) +138 / -2
App.razorAdd JS helper to open Navigator protocol deep links with fallback +23/-0

Add JS helper to open Navigator protocol deep links with fallback

• Defines 'window.kurrentNavigator.open(deepLink, fallbackUrl)' which navigates to the custom scheme and uses blur/visibility change plus a short timeout to decide whether to open the download fallback in a new tab.

src/KurrentDB/Components/App.razor

NavMenu.razorAdd Gaffer tool link and deep-link Navigator from the sidebar +17/-1

Add Gaffer tool link and deep-link Navigator from the sidebar

• Replaces the Navigator nav route with an on-click JS invocation that launches Navigator for the currently-reached node address, and adds a new Tools entry linking to Gaffer with consistent UTM attribution.

src/KurrentDB/Components/Layout/NavMenu.razor

ProjectionDetail.razorAdd contextual Gaffer link next to projection Source for user projections +16/-1

Add contextual Gaffer link next to projection Source for user projections

• Renders a small external link beside the Source heading that opens Gaffer in a new tab, gated to non-system projections to avoid offering an authoring link where none applies.

src/KurrentDB/Components/Projections/ProjectionDetail.razor

ProjectionDetail.razor.csIntroduce system-projection detection for Gaffer link gating +4/-0

Introduce system-projection detection for Gaffer link gating

• Adds an 'IsSystemProjection' helper using the '$' prefix convention so the detail view can conditionally suppress the Gaffer link for built-in projections.

src/KurrentDB/Components/Projections/ProjectionDetail.razor.cs

Projections.razorAdd a dense, clickable Gaffer ribbon under the projections header +15/-0

Add a dense, clickable Gaffer ribbon under the projections header

• Introduces a full-width MudAlert ribbon wrapped in a MudLink to Gaffer, placed only within the projections-available/leader branch to avoid stacking with higher-urgency notices.

src/KurrentDB/Components/Projections/Projections.razor

GafferLink.csCentralize UTM-tagged Gaffer URL construction +14/-0

Centralize UTM-tagged Gaffer URL construction

• Adds an internal helper to generate Gaffer links with consistent 'utm_source/medium/campaign/content' values so referrals aggregate cleanly across admin surfaces.

src/KurrentDB/Components/Shared/GafferLink.cs

NavigatorLink.csCentralize Navigator deep-link and download fallback construction +31/-0

Centralize Navigator deep-link and download fallback construction

• Adds an internal helper to build 'kurrentdb://host:port' (or 'kurrentdb+discover://' for clusters) using the browser-reached address, with 'tls=false' for HTTP and a UTM-tagged download fallback URL.

src/KurrentDB/Components/Shared/NavigatorLink.cs

KurrentIcons.csAdd Gaffer logomark icon +8/-0

Add Gaffer logomark icon

• Ports the Gaffer SVG mark into the theme icon set, including transform/clip-path mapping to MudIcon’s 24x24 coordinate space.

src/KurrentDB/UI/Theme/KurrentIcons.cs

app.cssAdd hover styling for the Gaffer ribbon link wrapper +10/-0

Add hover styling for the Gaffer ribbon link wrapper

• Introduces a subtle hover background transition on the ribbon’s MudAlert to provide an affordance when the entire banner is a link without underline or link color.

src/KurrentDB/wwwroot/css/app.css

Tests (2) +188 / -0
NavigatorLinkTests.csAdd unit tests for Navigator deep-link and fallback URLs +50/-0

Add unit tests for Navigator deep-link and fallback URLs

• Introduces xUnit coverage for scheme selection ('kurrentdb' vs 'kurrentdb+discover'), TLS flag behavior on HTTP, explicit port formatting, and UTM-tagged fallback URL.

src/KurrentDB.Components.Tests/NavigatorLinkTests.cs

ProjectionsGafferRibbonTests.csAdd bUnit tests for Gaffer ribbon/link presence and gating +138/-0

Add bUnit tests for Gaffer ribbon/link presence and gating

• Adds component tests ensuring the projections list ribbon appears in normal states but not when projections are disabled, and that the projection detail link is shown only for user projections (not '$' system projections).

src/KurrentDB.Components.Tests/ProjectionsGafferRibbonTests.cs

Other (1) +3 / -0
KurrentDB.csprojExpose internals to component test project +3/-0

Expose internals to component test project

• Adds 'InternalsVisibleTo' for 'KurrentDB.Components.Tests' to enable testing of internal URL-building helpers without making them public API.

src/KurrentDB/KurrentDB.csproj

@qodo-code-review

qodo-code-review Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Unencoded UTM query values ✓ Resolved 🐞 Bug ≡ Correctness
Description
GafferLink.For interpolates campaign and content directly into a query string without
URL-encoding. If a future caller passes reserved characters (e.g. &, =) or spaces, the resulting
URL can be parsed incorrectly and attribution parameters can be truncated or altered.
Code

src/KurrentDB/Components/Shared/GafferLink.cs[R12-13]

+	public static string For(string content, string campaign = "projections") =>
+		$"{Home}?utm_source=embedded-ui&utm_medium=referral&utm_campaign={campaign}&utm_content={content}";
Evidence
The new helper constructs the URL by directly inserting campaign and content into the query
string, with no escaping/encoding step, so reserved characters in those values would affect the
query structure.

src/KurrentDB/Components/Shared/GafferLink.cs[6-13]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`GafferLink.For` builds a query string via string interpolation and does not URL-encode the `campaign` and `content` values. This is fragile: any future use that includes reserved characters or whitespace can produce a malformed URL or change query parsing semantics.

### Issue Context
The helper is newly introduced and is intended as a reusable outbound-link builder.

### Fix Focus Areas
- src/KurrentDB/Components/Shared/GafferLink.cs[10-13]

### Suggested fix
Encode `campaign` and `content` as query-component values, e.g.:

```csharp
public static string For(string content, string campaign = "projections") =>
   $"{Home}?utm_source=embedded-ui&utm_medium=referral&utm_campaign={Uri.EscapeDataString(campaign)}&utm_content={Uri.EscapeDataString(content)}";
```

(Alternatively, use a query builder utility if one is already used elsewhere in the repo.)

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. IsSystemProjection masks null Name ✓ Resolved 📘 Rule violation ≡ Correctness
Description
IsSystemProjection uses a null-conditional check on the required route parameter Name, silently
treating a null Name as “not system” instead of failing fast. This can hide binding/config bugs
and violates the requirement to avoid fallback behavior for required inputs.
Code

src/KurrentDB/Components/Projections/ProjectionDetail.razor.cs[R36-38]

+	// System projections are built into the server, so there is nothing to author locally and no Gaffer link.
+	// Same `$` prefix test Navigator splits user from system on.
+	bool IsSystemProjection => Name?.StartsWith('$') == true;
Evidence
PR Compliance ID 7 requires required inputs to be explicit and to avoid silent fallbacks when null
indicates a bug. The added IsSystemProjection logic uses a null-conditional check
(Name?.StartsWith(...)) that converts a potentially invalid null Name into a benign false
result instead of failing fast.

CLAUDE.md: Do Not Use Silent Default/Fallback Parameter Values for Required Inputs
src/KurrentDB/Components/Projections/ProjectionDetail.razor.cs[36-38]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`IsSystemProjection` currently evaluates to `false` when `Name` is null via `Name?.StartsWith('$') == true`, which is a silent fallback for a required input.

## Issue Context
`Name` is a route-bound parameter (`/ui/projections/{Name}`) and is expected to always be present; when it is missing/null, that should be treated as an error rather than being silently coerced.

## Fix Focus Areas
- src/KurrentDB/Components/Projections/ProjectionDetail.razor.cs[36-38]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

3. setTimeout uses magic 1000 📘 Rule violation ⚙ Maintainability
Description
The Navigator handoff timeout is hard-coded as 1000, which is a magic number embedded in logic.
This reduces maintainability and violates the constant-usage convention requirement.
Code

src/KurrentDB/Components/App.razor[R76-77]

+				if (!handedOff) window.open(fallbackUrl, "_blank", "noopener");
+			}, 1000);
Evidence
PR Compliance ID 9 requires avoiding magic numbers by using named constants where feasible. The
added JS uses a hard-coded 1000 millisecond timeout in the Navigator deep-link/fallback flow.

CLAUDE.md: Follow Naming and Constant Usage Conventions (Versioned Types, Accurate Names, Named Booleans, No Magic Numbers)
src/KurrentDB/Components/App.razor[73-77]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Navigator fallback timeout is encoded as the literal `1000` in `setTimeout`, making the behavior harder to audit and adjust.

## Issue Context
The comment indicates the timeout value has behavioral/security implications (gesture-driven popup blockers), so it should be a named constant to make intent explicit.

## Fix Focus Areas
- src/KurrentDB/Components/App.razor[73-77]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/KurrentDB/Components/Projections/ProjectionDetail.razor.cs Outdated
Comment thread src/KurrentDB/Components/App.razor
Comment thread src/KurrentDB/Components/Shared/GafferLink.cs Outdated
IsSystemProjection tested `Name` with a null-conditional, implying a nullability
the route parameter does not have: OnInitializedAsync already dereferences it
via Uri.UnescapeDataString, and LeaderUrl escapes it unguarded, so a null would
have thrown well before this property was read. Dereference it directly, per the
convention against fallbacks for values that are always expected.

GafferLink interpolated its campaign and content straight into the query string.
No caller passes a reserved character, but one that did would split the query
and truncate the attribution silently rather than fail, so escape both. Covered
by tests, including that nothing leaks into a new parameter.
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.

1 participant