Repository navigation
fix: Connect Microsoft 365 reached a missing page; lighten the share dialog - #1248
Merged
Merged
Conversation
…dialog Three user-reported problems with Share ⇒ as email (shipped in #1222). 1. The 404. The route was NEVER wrong — /auth/ea/connect is real and registered (EaConsentController [Route("auth/ea")] + [HttpGet("connect")], MapControllers at MemexConfiguration:1248). The defect was the navigation MODE: WithNavigateToHref makes ButtonView call NavigationManager.NavigateTo with no forceLoad, so Blazor routes it client-side, matches its own catch-all page, and reports "does not match any registered address pattern" — a server endpoint is unreachable that way by construction. Fixed with the shipped idiom ctx.NavigateTo(url, forceLoad: true), and consent now carries a returnUrl back to the dialog. Root-caused one level deeper too. The path was a string literal in a framework project that cannot know whether the host serves it (three copies across two assemblies, and MeshWeaver.Markdown.Export has no /auth/ea/* at all on a non-memex host). It is now one constant on the controller that the route attributes are built from, surfaced across the boundary as IEmailSender.ConnectAsUserHref — null when a deployment has no connect flow, so no connect affordance is offered rather than a dead link. InstanceSyncLayoutArea's /connect/instance button had the identical live defect and is fixed the same way. 2. Dialog too heavy. Dropped the three-sentence intro. Recipient is now a plain email field FIRST with the mesh-user picker as the secondary affordance beneath it (was the other way round). The identity question is deferred to the moment it matters: a connected user sees one line naming the sending address and no connect UI at all; an unconnected user is not lectured up front and is asked once, on Send, to connect or to send from the shared mailbox — explicitly, with reply-to set. The honesty guarantee holds: the shared mailbox is never used silently. 3. Menu icons. The export/share/approval block was the only icon-less group in the node menu: Export to PDF 📄, Export to DOCX 📝, Share ⇒ as email 📤, Request Approval ✅ — emoji, matching every sibling (a Fluent icon NAME would render as a broken <img>). DeckExportMenuProvider also had no LabelKeys, so a Deck's entries rendered English for German viewers. Guards, because "looks wired, isn't" is the failure mode here: - EaConnectRouteTest builds a real controller pipeline and asserts the constant names a route the routing table actually contains, that the callback is registered too, that the Blazor catch-all declines it, and — non-vacuously — that a made-up path is absent. - MarkdownExportMenuTest asserts every non-separator node-menu entry has an emoji icon, as an invariant over the whole menu, so a future icon-less entry fails rather than ships. Localization: 4 catalogs kept in lockstep (server en/de + react vendored copies); 7 keys retired, 5 added, 2 reworded. Verified: 5 touched projects build -c Release -warnaserror with 0 warnings; EaConnectRouteTest 4/4, LocalizationTest 33/33, WhatsNewEntryIntegrityTest 1/1, MarkdownExportMenuTest 3/3. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes two “Connect” navigation paths that were incorrectly routed client-side in Blazor (causing the mesh catch-all “page not found”), and refines the Share ⇒ as email dialog UX while tightening the contract between UI and server endpoints (single source of truth for consent routes + host-provided connect affordance).
Changes:
- Replace client-side navigation with
forceLoad: truefor server-side connect endpoints (EA consent + instance sync), and centralize the EA consent path as constants used by both routing attributes and consumers. - Simplify the share-by-email dialog (recipient email first; connect only prompted at send-time; shared mailbox fallback is explicit) and update localization strings accordingly.
- Add/standardize node-menu icons (emoji) and fix missing
LabelKeys; add tests + docs + What’s New entries to guard the new invariants.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/MeshWeaver.Security.Test/MarkdownExportMenuTest.cs | Adds an invariant test enforcing emoji icons for all actionable node-menu entries. |
| test/Memex.Portal.Shared.Test/EaConnectRouteTest.cs | New test ensuring EA consent routes are actually registered and not swallowed by the Blazor catch-all. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.en.json | Updates share-dialog copy and keys (English) for the lighter UX + shared-mailbox prompt. |
| src/MeshWeaver.Messaging.Hub/Localization/strings.de.json | Same as above for German localization. |
| clients/react/src/i18n/strings.en.json | Mirrors updated share-dialog localization keys/copy for the React client bundle. |
| clients/react/src/i18n/strings.de.json | Mirrors updated share-dialog localization keys/copy for the React client bundle. |
| src/MeshWeaver.Mesh.Contract/IEmailSender.cs | Introduces ConnectAsUserHref (default null) so UI can avoid offering dead links on hosts without connect flow. |
| src/MeshWeaver.Mesh.Contract/HubEmailExtensions.cs | Exposes ConnectAsUserHref() on IMessageHub for UI consumption. |
| src/MeshWeaver.Markdown.Export/Layout/SendDocumentLayoutArea.cs | Reworks the share-by-email dialog UX; defers connect/shared-mailbox decision to send-time; uses forceLoad for connect. |
| src/MeshWeaver.Markdown.Export/Layout/MarkdownExportMenuProvider.cs | Adds emoji icons to export/share menu entries (and documents the “emoji not Fluent icon name” constraint). |
| src/MeshWeaver.Markdown.Export/Layout/DeckExportMenuProvider.cs | Aligns Deck menu entries with Markdown’s (adds icons + missing LabelKeys). |
| src/MeshWeaver.InstanceSync/InstanceSyncLayoutArea.cs | Fixes /connect/instance navigation to use forceLoad: true so the server endpoint is reachable. |
| src/MeshWeaver.Graph/ApprovalExtensions.cs | Adds an emoji icon for “Request Approval” to match node-menu icon conventions. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-12-sharing-a-document-by-email-is-quicker-to-fill-in.md | What’s New entry for the dialog UX improvements + icons. |
| src/MeshWeaver.Documentation/Data/WhatsNew/2026-08-12-connect-microsoft-365-no-longer-leads-to-a-missing-page.md | What’s New entry for the connect navigation fix + path centralization. |
| src/MeshWeaver.Documentation/Data/GUI/NodeMenu.md | Documents the “every entry has an emoji icon” rule and the reasoning; references the guarding test. |
| memex/Memex.Portal.Shared/Email/GraphEmailSender.cs | Implements ConnectAsUserHref when the Graph integration is configured. |
| memex/Memex.Portal.Shared/Email/ExecutiveAssistantPlugin.cs | Uses the centralized EA consent path constant when emitting an agent-facing consent link. |
| memex/Memex.Portal.Shared/Authentication/EaConsentController.cs | Centralizes EA consent route parts into constants; builds attributes and consumers from one definition. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
User's decision on the label question: "just pdf, email, docx". Labels become bare format/action names and the block is ordered as the user listed them — PDF (27), Email (28), DOCX (29). Request Approval (30) and the separators at 20/40 are untouched; the block neither moves nor grows. Export to PDF -> 📄 PDF Share ⇒ as email -> 📤 Email Export to DOCX -> 📝 DOCX The explanation the labels no longer carry moves to tooltips, via the TooltipKey field NodeMenuItemDefinition already had — no new plumbing. That is the AGENTS.md-preferred shape (language-neutral glyph + short label + translated tooltip), and it shrinks the translation surface to the tooltips alone: - PDF and DOCX are FORMAT NAMES and are byte-identical in both catalogs — they are not translated, in either language. - Email -> German "E-Mail" (hyphen, capital M) is a real word, so it is. - All three tooltips are translated. Once a label is this short the tooltip is the only remaining explanation, so TooltipKey stops being optional polish. The new test asserts the group reads PDF, Email, DOCX as a SEQUENCE (the Order numbers are an implementation detail; the reading order is the contract) and that each entry carries a non-empty tooltip. Catalogs: 895 keys, server en/de and the react vendored copies verified equal. Verified: MeshWeaver.Markdown.Export builds -c Release -warnaserror with 0 warnings; MarkdownExportMenuTest 4/4, EaConnectRouteTest 4/4, LocalizationTest 33/33, WhatsNewEntryIntegrityTest 1/1. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Test Results (shard 0)707 tests 707 ✅ 13m 18s ⏱️ Results for commit c91ee05. |
Contributor
Test Results (shard 5)1 798 tests 1 606 ✅ 7m 12s ⏱️ Results for commit c91ee05. |
Contributor
Test Results (shard 1)1 288 tests 1 288 ✅ 6m 1s ⏱️ Results for commit c91ee05. |
Contributor
Test Results (shard 3) 11 files 11 suites 7m 53s ⏱️ Results for commit c91ee05. |
Contributor
Test Results (shard 4)1 306 tests 1 296 ✅ 12m 1s ⏱️ Results for commit c91ee05. |
Contributor
Test Results (shard 2)2 068 tests 2 067 ✅ 9m 11s ⏱️ Results for commit c91ee05. |
Contributor
Test Results 65 files 65 suites 55m 38s ⏱️ Results for commit c91ee05. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three user-reported problems with the node menu's document-email feature (shipped in #1222 as
dad8615), reviewed in the live portal.1. The 404 — and why the brief's premise was wrong
The route was never wrong.
/auth/ea/connectis real and registered:EaConsentControllercarries[Route("auth/ea")]+[HttpGet("connect")], andMemexConfiguration.cs:1248callsapp.MapControllers(). The 404 text the user saw comes from the mesh catch-all page (NavigationStatus.cs:99), not from ASP.NET.The defect is the navigation mode.
WithNavigateToHrefmakesButtonView.razor:46callNavigationManager.NavigateTo(href)with noforceLoad, so Blazor routes it client-side, finds no@page, falls into the mesh catch-all and reports "does not match any registered address pattern". A server-side endpoint is unreachable that way by construction — no route string could have fixed it.Fixed with the framework's shipped idiom,
ctx.NavigateTo(url, forceLoad: true), and consent now carries areturnUrlso the user lands back on the dialog instead of/.Root-caused one level deeper. The path was a string literal in a framework project that has no way to know whether the host serves it — three copies across two assemblies, and
MeshWeaver.Markdown.Exporthas no/auth/ea/*at all on a non-memex host. It is now:EaConsentController.ConnectPath) that the route attributes themselves are built from — a rename breaks compilation instead of shipping a 404;IEmailSender.ConnectAsUserHref—nullwhen a deployment has no connect flow, so the UI offers no affordance rather than a dead link.InstanceSyncLayoutArea's/connect/instancebutton had the identical live defect and is fixed the same way.2. Dialog too heavy
The honesty guarantee holds and is arguably stronger: the shared mailbox is never used silently, and it is now a visible, explicit choice at the moment of sending (with reply-to set to the user) rather than a hard refusal.
3. Menu look and feel — icons, labels, order
The export/share group was the only icon-less block in the node menu and the only one whose entries were sentences. Both fixed, using the existing convention — emoji in
NodeMenuItemDefinition.Icon(a Fluent icon name renders as a broken<img src="Name">; two deadIcon:values elsewhere prove the trap):Export to PDFShare ⇒ as emailExport to DOCXRequest approvalRequest Approvaland the surrounding separators (Order 20 / 40) are untouched. 📤 rather than an envelope deliberately: ✉️ already belongs to Invite people in the same menu.Localization follows the AGENTS.md-preferred shape — language-neutral glyph + short label + translated tooltip, which shrinks the translation surface to the tooltips:
PDFandDOCXare format names → identical in both catalogs, not translated.Email→ GermanE-Mail(hyphen, capital M) — a real word, so it is translated.Also fixed while here:
DeckExportMenuProvidercarried noLabelKeys at all, so a Deck's export entries rendered English regardless of locale while the Markdown twins translated.Guards — because "looks wired, isn't" is the failure mode
EaConnectRouteTestbuilds a real controller pipeline and asserts the constant names a route the routing table actually contains; that the callback is registered too; that the Blazor catch-all declines it (so a full browser load reaches the server); and — non-vacuously — that a made-up path is absent.MarkdownExportMenuTestgains two: the group reads PDF, Email, DOCX in that sequence and each entry carries a tooltip; and — as an invariant over the whole menu, not per item — every non-separator entry has an emoji icon, so a future icon-less entry fails rather than ships.Verification
-c Release -warnaserror— 0 warningsEaConnectRouteTest·LocalizationTest·WhatsNewEntryIntegrityTest·MarkdownExportMenuTest— all greenWhat's New: one Fix (the 404) + one Feature (the dialog, labels and icons).
🤖 Generated with Claude Code