Skip to content

Fix #550: one rules window for every account - #635

Open
CityDweller wants to merge 11 commits into
mainfrom
feat/unify-rules-window-550
Open

Fix #550: one rules window for every account#635
CityDweller wants to merge 11 commits into
mainfrom
feat/unify-rules-window-550

Conversation

@CityDweller

@CityDweller CityDweller commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

Closes #550.

The problem

MainWindow.BuildRulesWindow chose which rules window to open with a raw accounts.Any(a => a.BackendKind == BackendKind.MicrosoftGraph) test, while the VM's capability gate UnifiedRulesViewModel.AccountSupportsServerRules had been narrowed (#543) to work/school Graph only. The two disagreed: a user whose only Microsoft account is a personal Graph account got the unified server-rules window, where Move Up/Down can never enable and every new rule fired a modal "saved as a QuickMail rule" box.

The fix — one window, per Kelly's plan

Rather than reconcile two gates, collapse to a single rules window/engine. UnifiedRulesWindow is now the rules UI for every account; it already adapts per selected account via AccountSupportsServerRules, so an IMAP or personal-Graph account simply shows its client rules — the same set the retired client-only path showed. With no chooser, there is no second gate to drift.

  • BuildRulesWindow always builds UnifiedRulesWindow. Server-rule plumbing stays gated by the ServerRules feature flag: the service is passed only when the flag is on, else null and the VM degrades to client-only.
  • Extract UnifiedRulesViewModel.SupportsServerRules(account) — the named per-account capability test AccountSupportsServerRules now delegates to.
  • Drop the modal client-rule notice. On a client-only account the on-open hint already says rules run in QuickMail. On a server-capable account a rule that falls back to client (e.g. Mark as unread) is a genuine surprise, so it's announced (Result category, honors AnnounceResults) instead of a focus-stealing dialog.

Behavior change beyond the bug

A multi-account IMAP-only user previously got the client-only window that listed all accounts' rules together; they now get the one-account-at-a-time unified window (account picker, per-account list and Run-on-Existing). Intentional, and the User Guide's Rules section is updated to match.

Docs

  • docs/release-notes-v0.8.43.md — "Changed: one Rules window" entry.
  • docs/USER-GUIDE.md — the account picker / one-account-at-a-time behavior is generalized to any multi-account profile (no longer tied to Microsoft 365 being present); the server-vs-QuickMail marking and new-rule fallback note stay under the Microsoft 365 heading.

Follow-up (not in this PR)

The now-unreachable RulesManagerWindow / RulesManagerViewModel (and its 38 client-rule tests, plus the RulesManagerWindow arm in UiProbeDriver) are left for a separate retirement PR, so that test coverage can be ported to the unified VM deliberately rather than dropped.

Testing

  • Release build clean (analyzers-as-errors, 0 warnings).
  • UnifiedRulesViewModelTests 38/38, incl. new cases: server-capable fallback announces (Result), client-only account stays silent, the shared predicate agrees with the VM gate, and the serverRules: null production path saves a client rule without touching the absent service.
  • Adjacent suites green (RulesManager / XamlParse / ServerRule).
  • Independent pre-PR review run; its findings (release-note contradiction, null-service coverage) are folded in.

🤖 Generated with Claude Code

@kellylford

kellylford commented Sep 2, 2026

Copy link
Copy Markdown
Owner

@CityDweller I just tried this. Overall it seems fine to me with just one issue. When you create a rule for an account that supports client and server, you first get a message that the account supports only client, then you are told that it supports client and server.

@CityDweller

CityDweller commented Sep 2, 2026 via email

Copy link
Copy Markdown
Collaborator Author

CityDweller and others added 9 commits September 3, 2026 18:39
@CityDweller @claude
Fix #550: one rules window for every account
393ed0a 
The rules-window chooser opened the unified server-rules window whenever any
account was a Graph account (raw BackendKind test), while the VM's capability
gate AccountSupportsServerRules had narrowed to work/school Graph only (#543).
A personal-Graph-only user therefore landed in the unified window where reorder
can never enable and every new rule fired a modal "saved as a QuickMail rule"
box.

Rather than reconcile the two gates, collapse to one window (Kelly's plan): the
unified single-list manager is now THE rules UI for every account. It already
adapts per selected account via AccountSupportsServerRules, so an IMAP or
personal-Graph account simply shows its client rules — the same set the retired
client-only path showed. With no chooser there is no second gate to drift.

- MainWindow.BuildRulesWindow always builds UnifiedRulesWindow; server-rule
  plumbing stays gated by the ServerRules feature flag (serverRules: null when
  off, VM degrades to client-only).
- Extract UnifiedRulesViewModel.SupportsServerRules(account) — the named
  per-account capability test AccountSupportsServerRules now delegates to.
- Drop the modal client-rule notice. On a client-only account the on-open hint
  already says rules run in QuickMail. On a server-capable account a rule that
  falls back to client (e.g. Mark as unread) is a surprise, so announce it
  (Result category, honors AnnounceResults) instead of a focus-stealing dialog.

The now-unreachable RulesManagerWindow/RulesManagerViewModel are left for a
follow-up PR so their client-rule test coverage can be ported deliberately.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Release notes and user guide for the one-rules-window change
7b8874a 
Add the #550 "Changed" entry to the 0.8.43 release notes, and generalise the
User Guide's Rules section: the account picker (one account at a time) now
appears for any multi-account profile, not only when a Microsoft 365 account is
present, since one window serves every account. Keep the server-vs-QuickMail
marking and the new-rule fallback note under the Microsoft 365 heading, where
the distinction actually applies.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Address review: release-note accuracy and null-service coverage
a6c46e4 
- Release notes said personal accounts opened the simpler window, then said
  the personal account got the fuller window — a contradiction. Reword: the
  fuller vs simpler split was by connection (Microsoft 365 vs IMAP), and a
  personal account connected through Microsoft 365 wrongly got the fuller one.
- Add a test for the new production path where MainWindow constructs the VM
  with serverRules: null (ServerRules flag off / no Graph account): a new-rule
  save persists a client rule, stays silent, and never touches the absent
  server service.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Remove the ServerRules feature gate
4f9022f 
With one rules window serving every account and server-side rules a shipped,
tested capability, gating them behind a flag no longer earns its keep. The
GraphServerRuleService is always constructed and passed to the rules VM;
whether server rules actually load stays a per-account question
(AccountSupportsServerRules), so non-work/school-Graph accounts are unaffected.

- BuildRulesWindow always passes _serverRuleService (no flag check).
- Drop FeatureFlag.ServerRules and its ConfigFeatureGate default.

The EveryFeatureFlag_ResolvesToADefault invariant test iterates the enum, so
removing the member and its default together keeps it green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Rework rules wording, detail pane, and folder-name resolution
6d2c65a 
Screen-reader-driven pass over the unified rules window (feedback from testing
against a real Microsoft 365 account):

Wording
- Account hint on arrowing the account list: "This account supports both
  server-side and client-side rules." / "This account supports only
  client-side rules." (parallel, complete sentences).
- Save fallback (a work/school rule that goes client-side): "Saving as a
  client-side rule." — dropped the parenthetical and reason.
- Status bar: "N on client" (was "in QuickMail"); a client-only account drops
  the "0 on server" split entirely and reads "N client-side rules.".
- Per-rule row: "…, on client, …" (was "in QuickMail").

Detail pane
- The client rule detail now mirrors the server rule's structure ("Applies
  when:" / "Does:" sections) instead of its own ad-hoc shape, and drops the
  "runs client-side" line — where a rule runs is already the row label and the
  account hint, and the server pane has no such line.

Folder names
- A Graph rule (server or client) stores an opaque folder id; resolve it to the
  folder's display name from the account's folder cache so the prose reads
  "move to Deleted Items" rather than the id (server rules) or the raw blob
  (client rules). One shared ResolveFolderName; only fills a name that's empty,
  and falls back to the raw target (readable for IMAP) when unresolved.

Docs updated to match. 44 UnifiedRules tests pass; Release build clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Address review: IMAP folder rendering and folder-resolution test gaps
70f5811 
From the independent review of this branch:

- IMAP client move-to rules no longer collapse to the folder's leaf name.
  Folder-name resolution ran for every client move rule, including IMAP whose
  TargetFolder is already a readable path — resolving it returned the leaf
  DisplayName, so "Work/Archive" and "Personal/Archive" both read "Archive".
  Resolution is now gated to Graph accounts (opaque folder ids); IMAP keeps its
  path. Corrected the comments that wrongly described the old behavior.

- Tests: pin the copy-to server folder resolution (was entirely uncovered), the
  "an already-set folder name is not overwritten" guard, IMAP keeping its full
  path with the folder in cache, and the multi-condition "Applies when:" section
  formatting (";"-separated, no trailing ";").

Notification-wording findings (dropped save reason, present- vs past-tense,
announce-off feedback) are intentionally left for Kelly's review. 47 tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Fix UnifiedRuleRowTests for the "on client" row wording
ee0c6d8 
CI caught a test in UnifiedRuleRowTests (a separate class from the
UnifiedRulesViewModelTests I'd been running locally) still asserting the old
"in QuickMail" row label. Update it to "on client" and rename the method to
match; the raw-target fallback assertion (move to INBOX/Sorted) was already
correct.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Add temporary /debug diagnostic for the rule-mode announcement (#550)
c77f3bd 
Kelly reported hearing "this account supports only client-side rules" then
"…both server-side and client-side rules" when creating a rule on a
server-capable account. AccountSupportsServerRules is a pure function of the
selected account, so one account can't announce both — the log line records
which account each announcement is for, to tell "one account announced twice"
(a race) from "two accounts were visited" (expected on an account switch).

Debug-only (LogService.Debug, so /debug); clearly marked TEMP. Remove once the
report is understood.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Move the #550 release note to 0.8.45
0dd5e32 
Rebasing onto current main: 0.8.43 and 0.8.44 shipped while #635 was open, so
the "one Rules window" note no longer belongs in release-notes-v0.8.43.md (its
shipped content is restored). Add it to the in-progress 0.8.45 notes instead.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller
CityDweller force-pushed the feat/unify-rules-window-550 branch from d2920c4 to 0dd5e32 Compare September 3, 2026 22:43
@CityDweller @claude
Update the rule-folder-creation call-site test for the single window (#…
ba83cd2 
…550)

The #645 test asserted MainWindow constructs BOTH RulesManagerWindow and
UnifiedRulesWindow with a FolderCreationSupport. Since #550 collapsed to one
window, MainWindow always constructs UnifiedRulesWindow and no longer builds
RulesManagerWindow, so the loop's RulesManagerWindow arm failed. Check only the
UnifiedRulesWindow construction; the other tests in this file read
RulesManagerWindow.xaml.cs (still present, deferred-dead) and are unaffected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CityDweller @claude
Never show a Graph rule's raw folder id when it can't be resolved (#550)
32b17ff 
Adversarial-review finding: a Graph client rule whose move-to folder isn't in
the cache — not yet synced, or its id drifted (#366) — printed the raw
"AQMkAD…" id, the exact blob folder-name resolution exists to avoid.

Thread a targetIsOpaque flag (true for a Graph account, whose TargetFolder is
an opaque id) into UnifiedRuleRow. When no display name resolved and the target
is opaque, the summary reads "another folder" — matching how a server rule
renders an unresolved move target — instead of falling back to the raw id. IMAP
(readable path) is unaffected: it still shows its TargetFolder.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kellylford kellylford Awaiting requested review from kellylford kellylford is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Rules window chooser still gates on raw Graph backend, disagreeing with AccountSupportsServerRules

2 participants

@CityDweller @kellylford