mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-31 00:12:39 +00:00
Merge pull request #777 from hildebrandttk/fix/consistent-cross-account-id-namespacing
Fix/consistent cross account id namespacing
This commit is contained in:
@@ -669,7 +669,7 @@ export function CalendarApp({ linkSegments }: CalendarAppProps = {}) {
|
||||
const { dateRange: currentRange } = useCalendarStore.getState();
|
||||
if (!currentRange) return;
|
||||
if (multiAccountEnabled && accountClients.length > 0) {
|
||||
await fetchAllAccountsEventsFn(accountClients, activeAccountId, currentRange.start, currentRange.end);
|
||||
await fetchAllAccountsEventsFn(accountClients, currentRange.start, currentRange.end);
|
||||
return;
|
||||
}
|
||||
await fetchEvents(client, currentRange.start, currentRange.end);
|
||||
@@ -682,7 +682,7 @@ export function CalendarApp({ linkSegments }: CalendarAppProps = {}) {
|
||||
onRefresh: async () => {
|
||||
if (!client) return;
|
||||
const calendarRefresh = multiAccountEnabled && accountClients.length > 0 && activeAccountId
|
||||
? fetchAllAccountsCalendarsFn(accountClients, activeAccountId)
|
||||
? fetchAllAccountsCalendarsFn(accountClients)
|
||||
: fetchCalendars(client);
|
||||
await Promise.all([
|
||||
calendarRefresh,
|
||||
|
||||
@@ -242,6 +242,10 @@ export function CalendarSidebarPanel({
|
||||
<button
|
||||
onClick={() => onToggleVisibility(cal.id)}
|
||||
onContextMenu={hasMenu ? (e) => openContextMenu(e, cal) : undefined}
|
||||
data-testid="calendar-item"
|
||||
data-calendar-name={cal.name}
|
||||
data-account={cal.accountName ?? ''}
|
||||
data-visible={isVisible}
|
||||
className={cn(
|
||||
"flex items-center gap-2 w-full px-1.5 py-1 rounded-md text-sm transition-colors duration-150",
|
||||
"hover:bg-muted"
|
||||
|
||||
@@ -224,8 +224,8 @@ export function ContactsApp({ linkSegments }: ContactsAppProps = {}) {
|
||||
if (activeId) {
|
||||
const { fetchAllAccountsContacts, fetchAllAccountsAddressBooks } = useContactStore.getState();
|
||||
await Promise.all([
|
||||
fetchAllAccountsAddressBooks(accountClients, activeId),
|
||||
fetchAllAccountsContacts(accountClients, activeId),
|
||||
fetchAllAccountsAddressBooks(accountClients),
|
||||
fetchAllAccountsContacts(accountClients),
|
||||
]);
|
||||
return;
|
||||
}
|
||||
@@ -331,7 +331,7 @@ export function ContactsApp({ linkSegments }: ContactsAppProps = {}) {
|
||||
const activeId = useAuthStore.getState().activeAccountId;
|
||||
if (activeId) {
|
||||
const { fetchAllAccountsAddressBooks } = useContactStore.getState();
|
||||
await fetchAllAccountsAddressBooks(accountClients, activeId);
|
||||
await fetchAllAccountsAddressBooks(accountClients);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -846,6 +846,9 @@ function AddressBookItem({
|
||||
onDragOver={handleDragOver}
|
||||
onDragLeave={handleDragLeave}
|
||||
onDrop={handleDrop}
|
||||
data-testid="address-book-item"
|
||||
data-book-name={book.name}
|
||||
data-account={book.accountName ?? ''}
|
||||
className={cn(
|
||||
"w-full flex items-center gap-2 ps-5 pe-3 text-sm transition-colors",
|
||||
isActive
|
||||
|
||||
@@ -354,6 +354,7 @@ export function LayoutSettings() {
|
||||
<ToggleSwitch
|
||||
checked={proInterface}
|
||||
onChange={(v) => updateSetting('proInterface', v)}
|
||||
testId="setting-pro-interface"
|
||||
/>
|
||||
</SettingItem>
|
||||
</SettingsSection>
|
||||
|
||||
@@ -932,6 +932,7 @@ export function SettingsApp({ linkSegments }: SettingsAppProps = {}) {
|
||||
return (
|
||||
<div key={tab.id}>
|
||||
<button
|
||||
data-testid={`settings-tab-${tab.id}`}
|
||||
onClick={() => handleTabSelect(tab.id)}
|
||||
className="w-full flex items-center justify-between px-5 py-3.5 text-sm text-foreground hover:bg-muted transition-colors duration-150"
|
||||
>
|
||||
@@ -1072,6 +1073,7 @@ export function SettingsApp({ linkSegments }: SettingsAppProps = {}) {
|
||||
return (
|
||||
<div key={tab.id}>
|
||||
<button
|
||||
data-testid={`settings-tab-${tab.id}`}
|
||||
onClick={() => handleTabSelect(tab.id)}
|
||||
className={cn(
|
||||
'w-full text-start px-3 py-2 rounded-md text-sm transition-colors duration-150 flex items-center gap-2.5',
|
||||
|
||||
@@ -67,14 +67,17 @@ interface ToggleSwitchProps {
|
||||
disabled?: boolean;
|
||||
/** Overrides the enclosing SettingItem label as the accessible name. */
|
||||
ariaLabel?: string;
|
||||
/** Test hook (data-testid) for integration tests. */
|
||||
testId?: string;
|
||||
}
|
||||
|
||||
export function ToggleSwitch({ checked, onChange, disabled, ariaLabel }: ToggleSwitchProps) {
|
||||
export function ToggleSwitch({ checked, onChange, disabled, ariaLabel, testId }: ToggleSwitchProps) {
|
||||
const labelledBy = useContext(SettingLabelContext);
|
||||
return (
|
||||
<button
|
||||
type="button"
|
||||
role="switch"
|
||||
data-testid={testId}
|
||||
aria-checked={checked}
|
||||
aria-label={ariaLabel}
|
||||
aria-labelledby={ariaLabel ? undefined : labelledBy}
|
||||
|
||||
@@ -49,14 +49,14 @@ export function useProMultiAccountCalendars(start: string | null, end: string |
|
||||
// bootstrapping).
|
||||
useEffect(() => {
|
||||
if (!enabled || !activeAccountId || accountClients.length === 0) return;
|
||||
void fetchAllAccountsCalendars(accountClients, activeAccountId);
|
||||
void fetchAllAccountsCalendars(accountClients);
|
||||
}, [enabled, activeAccountId, accountClients, fetchAllAccountsCalendars]);
|
||||
|
||||
// Fetch events for the current visible date range across all accounts.
|
||||
useEffect(() => {
|
||||
if (!enabled || !activeAccountId || accountClients.length === 0) return;
|
||||
if (!start || !end) return;
|
||||
void fetchAllAccountsEvents(accountClients, activeAccountId, start, end);
|
||||
void fetchAllAccountsEvents(accountClients, start, end);
|
||||
}, [enabled, activeAccountId, accountClients, start, end, fetchAllAccountsEvents]);
|
||||
|
||||
return { enabled, accountClients };
|
||||
|
||||
@@ -40,8 +40,8 @@ export function useProMultiAccountContacts(): {
|
||||
|
||||
useEffect(() => {
|
||||
if (!enabled || !activeAccountId || accountClients.length === 0) return;
|
||||
void fetchAllAccountsAddressBooks(accountClients, activeAccountId);
|
||||
void fetchAllAccountsContacts(accountClients, activeAccountId);
|
||||
void fetchAllAccountsAddressBooks(accountClients);
|
||||
void fetchAllAccountsContacts(accountClients);
|
||||
}, [enabled, activeAccountId, accountClients, fetchAllAccountsAddressBooks, fetchAllAccountsContacts]);
|
||||
|
||||
return { enabled, accountClients };
|
||||
|
||||
@@ -0,0 +1,89 @@
|
||||
import { test, expect, type Page } from '@playwright/test';
|
||||
import { ACCOUNTS } from './helpers/config';
|
||||
import {
|
||||
login,
|
||||
addAccount,
|
||||
enableProInterface,
|
||||
openCalendar,
|
||||
calendarItem,
|
||||
calendarRows,
|
||||
} from './helpers/app';
|
||||
|
||||
/**
|
||||
* Multi-account calendar aggregation across an account switch.
|
||||
*
|
||||
* Regression for the cross-account id-namespacing bug: the Pro shell used to
|
||||
* leave the ACTIVE account's calendar ids raw while namespacing the others, so
|
||||
* switching accounts flipped the id form and reset the persisted calendar
|
||||
* visibility selection (and broke subscription/mutation routing). All accounts
|
||||
* are now namespaced consistently, so the selection survives a switch.
|
||||
*
|
||||
* Mail is unaffected (it keys per account + uses stable virtual ids), which is
|
||||
* why the user saw this only in Calendar and Address Book.
|
||||
*/
|
||||
const { alice, bob } = ACCOUNTS;
|
||||
|
||||
const switcher = (page: Page) => page.locator('[data-testid="account-switcher"]').first();
|
||||
|
||||
/** Read an account's id from the (stable, standard-shell) switcher popover. */
|
||||
async function accountId(page: Page, email: string): Promise<string> {
|
||||
await switcher(page).click();
|
||||
const id = await page
|
||||
.locator(`[data-testid="account-option"][data-account-email="${email}"]`)
|
||||
.first()
|
||||
.getAttribute('data-account-id');
|
||||
await page.keyboard.press('Escape');
|
||||
if (!id) throw new Error(`no account id for ${email}`);
|
||||
return id;
|
||||
}
|
||||
|
||||
/**
|
||||
* Switch active account robustly in the Pro shell. The switcher popover is a
|
||||
* portalled, repositioning element, so a normal click can race its layout;
|
||||
* force the click and verify via the switcher's data-active-account-id.
|
||||
*/
|
||||
async function switchTo(page: Page, id: string): Promise<void> {
|
||||
await switcher(page).click();
|
||||
await page.locator(`[data-testid="account-option"][data-account-id="${id}"]`).first().click({ force: true });
|
||||
await expect
|
||||
.poll(async () => switcher(page).getAttribute('data-active-account-id'), { timeout: 30000 })
|
||||
.toBe(id);
|
||||
}
|
||||
|
||||
test.describe('Multi-account calendar', () => {
|
||||
test('an account switch preserves the calendar visibility selection', async ({ page }) => {
|
||||
// Both accounts in the standard shell (stable switcher), capture their ids,
|
||||
// then flip on the Pro interface so the calendar aggregates across accounts.
|
||||
await login(page, alice);
|
||||
await addAccount(page, bob);
|
||||
const bobId = await accountId(page, bob.email);
|
||||
await enableProInterface(page);
|
||||
|
||||
await openCalendar(page);
|
||||
|
||||
// Both accounts' (default) calendars aggregate into one list with distinct
|
||||
// names, and every row is initially visible.
|
||||
const rows = await calendarRows(page);
|
||||
expect(rows.length).toBeGreaterThanOrEqual(2);
|
||||
expect(new Set(rows.map((r) => r.name)).size).toBeGreaterThanOrEqual(2);
|
||||
|
||||
// Deselect one calendar; remember it by name.
|
||||
const target = rows.find((r) => r.visible);
|
||||
expect(target, 'expected at least one visible calendar').toBeTruthy();
|
||||
const name = target!.name;
|
||||
await calendarItem(page, name).click();
|
||||
await expect(calendarItem(page, name)).toHaveAttribute('data-visible', 'false');
|
||||
|
||||
// Switch the active account. This alone triggered the bug: the just-
|
||||
// deselected calendar belonged to the previously-active account, whose ids
|
||||
// flipped raw->namespaced on the switch, so the reconciler dropped the
|
||||
// selection and reset visibility to "all".
|
||||
await switchTo(page, bobId);
|
||||
await openCalendar(page);
|
||||
|
||||
// The deselection survived the switch (the calendar is still aggregated in
|
||||
// and still hidden), and the list is still the cross-account set.
|
||||
await expect(calendarItem(page, name)).toHaveAttribute('data-visible', 'false');
|
||||
expect((await calendarRows(page)).length).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,70 @@
|
||||
import { test, expect, type Page } from '@playwright/test';
|
||||
import { ACCOUNTS } from './helpers/config';
|
||||
import {
|
||||
login,
|
||||
addAccount,
|
||||
enableProInterface,
|
||||
openContacts,
|
||||
addressBookRows,
|
||||
} from './helpers/app';
|
||||
|
||||
/**
|
||||
* Multi-account address-book aggregation across an account switch.
|
||||
*
|
||||
* Companion to the calendar spec for the cross-account id-namespacing fix.
|
||||
* Contacts keep no persisted id-selection (unlike calendars'
|
||||
* selectedCalendarIds), so there is no selection to reset; the observable here
|
||||
* is that the aggregated cross-account address-book list stays correct and
|
||||
* usable after switching the active account (the store now namespaces every
|
||||
* account consistently, so ids no longer flip form on a switch).
|
||||
*/
|
||||
const { alice, bob } = ACCOUNTS;
|
||||
|
||||
const switcher = (page: Page) => page.locator('[data-testid="account-switcher"]').first();
|
||||
|
||||
async function accountId(page: Page, email: string): Promise<string> {
|
||||
await switcher(page).click();
|
||||
const id = await page
|
||||
.locator(`[data-testid="account-option"][data-account-email="${email}"]`)
|
||||
.first()
|
||||
.getAttribute('data-account-id');
|
||||
await page.keyboard.press('Escape');
|
||||
if (!id) throw new Error(`no account id for ${email}`);
|
||||
return id;
|
||||
}
|
||||
|
||||
async function switchTo(page: Page, id: string): Promise<void> {
|
||||
await switcher(page).click();
|
||||
await page.locator(`[data-testid="account-option"][data-account-id="${id}"]`).first().click({ force: true });
|
||||
await expect
|
||||
.poll(async () => switcher(page).getAttribute('data-active-account-id'), { timeout: 30000 })
|
||||
.toBe(id);
|
||||
}
|
||||
|
||||
test.describe('Multi-account contacts', () => {
|
||||
test('address books from both accounts stay aggregated across a switch', async ({ page }) => {
|
||||
await login(page, alice);
|
||||
await addAccount(page, bob);
|
||||
const bobId = await accountId(page, bob.email);
|
||||
await enableProInterface(page);
|
||||
|
||||
await openContacts(page);
|
||||
|
||||
// Both accounts' (default) address books aggregate into one list with
|
||||
// distinct names.
|
||||
const before = await addressBookRows(page);
|
||||
expect(before.length).toBeGreaterThanOrEqual(2);
|
||||
const namesBefore = new Set(before.map((b) => b.name));
|
||||
expect(namesBefore.size).toBeGreaterThanOrEqual(2);
|
||||
|
||||
// Switch the active account: the previously-active account's ids flip
|
||||
// raw<->namespaced. The aggregated list must remain the same set (no loss,
|
||||
// no duplication, no corruption from the id-form change).
|
||||
await switchTo(page, bobId);
|
||||
await openContacts(page);
|
||||
|
||||
const after = await addressBookRows(page);
|
||||
expect(after.length).toBe(before.length);
|
||||
expect(new Set(after.map((b) => b.name))).toEqual(namesBefore);
|
||||
});
|
||||
});
|
||||
@@ -378,3 +378,76 @@ export async function emailContextAction(page: Page, subject: string, testId: st
|
||||
}).toPass({ timeout: 15000 });
|
||||
await item.click();
|
||||
}
|
||||
|
||||
// --- Calendar (multi-account) ---------------------------------------------
|
||||
|
||||
/**
|
||||
* Navigate to the calendar via the in-app nav (client-side routing) and wait
|
||||
* for the sidebar calendar list. A hard `page.goto('/calendar')` redirects to
|
||||
* login because the deep-link SSR path runs before client auth hydrates.
|
||||
*/
|
||||
export async function openCalendar(page: Page): Promise<void> {
|
||||
await page.locator('a[href$="/calendar"]').first().click();
|
||||
await page
|
||||
.locator('[data-testid="calendar-item"]')
|
||||
.first()
|
||||
.waitFor({ state: 'visible', timeout: 30000 });
|
||||
}
|
||||
|
||||
/** A sidebar calendar row by its (exact) name. */
|
||||
export function calendarItem(page: Page, name: string): Locator {
|
||||
return page.locator(`[data-testid="calendar-item"][data-calendar-name="${name}"]`).first();
|
||||
}
|
||||
|
||||
/** All sidebar calendars as {name, account, visible}. */
|
||||
export async function calendarRows(
|
||||
page: Page,
|
||||
): Promise<Array<{ name: string; account: string; visible: boolean }>> {
|
||||
return page.locator('[data-testid="calendar-item"]').evaluateAll((els) =>
|
||||
els.map((e) => ({
|
||||
name: e.getAttribute('data-calendar-name') || '',
|
||||
account: e.getAttribute('data-account') || '',
|
||||
visible: e.getAttribute('data-visible') === 'true',
|
||||
})),
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Turn on the Pro (multi-account) interface via the settings UI, so the
|
||||
* calendar/contacts aggregate across accounts. Done through the real toggle
|
||||
* (not a reload/seed) because the app doesn't survive a hard reload in tests
|
||||
* and seeding proInterface before login destabilises the add-account popover.
|
||||
*/
|
||||
export async function enableProInterface(page: Page): Promise<void> {
|
||||
await page.locator('a[href$="/settings"]').first().click();
|
||||
await page.locator('[data-testid="settings-tab-layout"]').first().click();
|
||||
const toggle = page.locator('[data-testid="setting-pro-interface"]').first();
|
||||
await toggle.waitFor({ state: 'visible', timeout: 30000 });
|
||||
if ((await toggle.getAttribute('aria-checked')) !== 'true') {
|
||||
await toggle.click();
|
||||
await expect(toggle).toHaveAttribute('aria-checked', 'true');
|
||||
}
|
||||
}
|
||||
|
||||
// --- Contacts / address books (multi-account) ------------------------------
|
||||
|
||||
/** Navigate to contacts via in-app nav and wait for the address-book list. */
|
||||
export async function openContacts(page: Page): Promise<void> {
|
||||
await page.locator('a[href$="/contacts"]').first().click();
|
||||
await page
|
||||
.locator('[data-testid="address-book-item"]')
|
||||
.first()
|
||||
.waitFor({ state: 'visible', timeout: 30000 });
|
||||
}
|
||||
|
||||
/** All sidebar address books as {name, account}. */
|
||||
export async function addressBookRows(
|
||||
page: Page,
|
||||
): Promise<Array<{ name: string; account: string }>> {
|
||||
return page.locator('[data-testid="address-book-item"]').evaluateAll((els) =>
|
||||
els.map((e) => ({
|
||||
name: e.getAttribute('data-book-name') || '',
|
||||
account: e.getAttribute('data-account') || '',
|
||||
})),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -0,0 +1,93 @@
|
||||
import { describe, it, expect, beforeEach } from 'vitest';
|
||||
import { useCalendarStore, reconcileSelectedIds } from '../calendar-store';
|
||||
import type { Calendar } from '@/lib/jmap/types';
|
||||
|
||||
function cal(overrides: Partial<Calendar>): Calendar {
|
||||
return {
|
||||
id: 'c', name: 'Cal', color: null, sortOrder: 0, isSubscribed: true, isVisible: true,
|
||||
isDefault: false, includeInAvailability: 'all', timeZone: null, shareWith: null,
|
||||
myRights: {
|
||||
mayReadFreeBusy: true, mayReadItems: true, mayWriteAll: true, mayWriteOwn: true,
|
||||
mayUpdatePrivate: true, mayRSVP: true, mayShare: true, mayDelete: true,
|
||||
},
|
||||
...overrides,
|
||||
} as Calendar;
|
||||
}
|
||||
|
||||
describe('reconcileSelectedIds (account-switch selection stability)', () => {
|
||||
it('keeps a selection whose id form flipped raw -> namespaced', () => {
|
||||
// Persisted while account A was active (raw "b"); after the fix A is
|
||||
// namespaced. The calendar is the same, only the id string changed.
|
||||
const calendars = [cal({ id: 'A@h::b', originalId: 'b', localAccountId: 'A@h' })];
|
||||
expect(reconcileSelectedIds(['b'], calendars)).toEqual(['A@h::b']);
|
||||
});
|
||||
|
||||
it('keeps an already-namespaced selection unchanged across a switch', () => {
|
||||
const calendars = [
|
||||
cal({ id: 'A@h::b', originalId: 'b', localAccountId: 'A@h' }),
|
||||
cal({ id: 'B@h::b', originalId: 'b', localAccountId: 'B@h' }),
|
||||
];
|
||||
// Both accounts have raw id "b"; the namespaced selection is unambiguous.
|
||||
expect(reconcileSelectedIds(['B@h::b'], calendars)).toEqual(['B@h::b']);
|
||||
});
|
||||
|
||||
it('remaps a just-created raw id to its namespaced form after refetch', () => {
|
||||
const calendars = [
|
||||
cal({ id: 'A@h::b', originalId: 'b', localAccountId: 'A@h' }),
|
||||
cal({ id: 'A@h::f', originalId: 'f', localAccountId: 'A@h' }),
|
||||
];
|
||||
// createCalendar added the raw "f"; the refetch namespaced it.
|
||||
expect(reconcileSelectedIds(['A@h::b', 'f'], calendars)).toEqual(['A@h::b', 'A@h::f']);
|
||||
});
|
||||
|
||||
it('drops ids that resolve to no calendar and de-dupes', () => {
|
||||
const calendars = [cal({ id: 'A@h::b', originalId: 'b', localAccountId: 'A@h' })];
|
||||
expect(reconcileSelectedIds(['b', 'A@h::b', 'gone'], calendars)).toEqual(['A@h::b']);
|
||||
});
|
||||
|
||||
it('preserves the birthday calendar id verbatim', () => {
|
||||
const calendars = [cal({ id: 'A@h::b', originalId: 'b', localAccountId: 'A@h' })];
|
||||
expect(reconcileSelectedIds(['__birthday-calendar__', 'b'], calendars)).toEqual(['__birthday-calendar__', 'A@h::b']);
|
||||
});
|
||||
|
||||
it('leaves single-account (raw, no localAccountId) selections untouched', () => {
|
||||
const calendars = [cal({ id: 'b' }), cal({ id: 'c' })];
|
||||
expect(reconcileSelectedIds(['b', 'c'], calendars)).toEqual(['b', 'c']);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isSubscriptionCalendar (namespace-robust, collision-safe)', () => {
|
||||
beforeEach(() => {
|
||||
useCalendarStore.setState({ calendars: [], icalSubscriptions: [] });
|
||||
});
|
||||
|
||||
it('matches a subscription across the raw->namespaced boundary', () => {
|
||||
useCalendarStore.setState({
|
||||
calendars: [cal({ id: 'A@h::sub1', originalId: 'sub1', localAccountId: 'A@h', accountId: 'acctA' })],
|
||||
// subs store the raw JMAP calendar id
|
||||
icalSubscriptions: [{ id: 's', url: 'x', calendarId: 'sub1', accountId: 'acctA', name: 'Feed', color: '#000', refreshInterval: 60, lastRefreshed: null }],
|
||||
});
|
||||
expect(useCalendarStore.getState().isSubscriptionCalendar('A@h::sub1')).toBe(true);
|
||||
});
|
||||
|
||||
it('does NOT false-positive when another account has the same raw id', () => {
|
||||
useCalendarStore.setState({
|
||||
calendars: [
|
||||
cal({ id: 'A@h::b', originalId: 'b', localAccountId: 'A@h', accountId: 'acctA' }), // A's subscription calendar
|
||||
cal({ id: 'B@h::b', originalId: 'b', localAccountId: 'B@h', accountId: 'acctB' }), // B's normal calendar, same raw id
|
||||
],
|
||||
icalSubscriptions: [{ id: 's', url: 'x', calendarId: 'b', accountId: 'acctA', name: 'Feed', color: '#000', refreshInterval: 60, lastRefreshed: null }],
|
||||
});
|
||||
expect(useCalendarStore.getState().isSubscriptionCalendar('A@h::b')).toBe(true);
|
||||
expect(useCalendarStore.getState().isSubscriptionCalendar('B@h::b')).toBe(false);
|
||||
});
|
||||
|
||||
it('matches a single-account (raw) subscription exactly', () => {
|
||||
useCalendarStore.setState({
|
||||
calendars: [cal({ id: 'sub1' })],
|
||||
icalSubscriptions: [{ id: 's', url: 'x', calendarId: 'sub1', name: 'Feed', color: '#000', refreshInterval: 60, lastRefreshed: null }],
|
||||
});
|
||||
expect(useCalendarStore.getState().isSubscriptionCalendar('sub1')).toBe(true);
|
||||
expect(useCalendarStore.getState().isSubscriptionCalendar('other')).toBe(false);
|
||||
});
|
||||
});
|
||||
+84
-34
@@ -42,6 +42,42 @@ function stripLocalAccountPrefix(id: string, localAccountId?: string): string {
|
||||
return id.startsWith(prefix) ? id.slice(prefix.length) : id;
|
||||
}
|
||||
|
||||
/** The raw identity of a store id, independent of any `<localAccountId>::` prefix. */
|
||||
function rawIdentityOf(id: string): string {
|
||||
const idx = id.indexOf(CROSS_ACCOUNT_ID_DELIMITER);
|
||||
return idx >= 0 ? id.slice(idx + CROSS_ACCOUNT_ID_DELIMITER.length) : id;
|
||||
}
|
||||
|
||||
/**
|
||||
* Reconcile persisted selectedCalendarIds against a freshly fetched calendars
|
||||
* list. A selected id can be stale in id-*form* (not existence): the raw->
|
||||
* namespaced transition (aggregation switched on, an account switch, or a
|
||||
* just-created calendar added with its raw id) changes the id string while the
|
||||
* calendar is the same. Remap by raw identity so the selection survives instead
|
||||
* of silently resetting to "all". Keeps BIRTHDAY_CALENDAR_ID; drops ids that
|
||||
* resolve to no calendar.
|
||||
*/
|
||||
export function reconcileSelectedIds(selectedCalendarIds: string[], calendars: Calendar[]): string[] {
|
||||
const byStoreId = new Set(calendars.map((c) => c.id));
|
||||
const rawToStoreId = new Map<string, string>();
|
||||
for (const c of calendars) {
|
||||
const raw = c.originalId ?? stripLocalAccountPrefix(c.id, c.localAccountId);
|
||||
if (!rawToStoreId.has(raw)) rawToStoreId.set(raw, c.id);
|
||||
}
|
||||
const out: string[] = [];
|
||||
const seen = new Set<string>();
|
||||
for (const id of selectedCalendarIds) {
|
||||
let mapped: string | undefined;
|
||||
if (id === BIRTHDAY_CALENDAR_ID || byStoreId.has(id)) mapped = id;
|
||||
else mapped = rawToStoreId.get(rawIdentityOf(id));
|
||||
if (mapped && !seen.has(mapped)) {
|
||||
out.push(mapped);
|
||||
seen.add(mapped);
|
||||
}
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
// In-flight refresh dedup. Concurrent callers (auto-interval +
|
||||
// manual refresh, two account-switch reloads, etc.) share the same
|
||||
// promise instead of double-fetching and racing the diff/import phase.
|
||||
@@ -56,10 +92,11 @@ export function isCalendarViewMode(value: unknown): value is CalendarViewMode {
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefix used to namespace calendar/event IDs that belong to a non-active
|
||||
* JMAP account when the Pro shell aggregates across accounts. The active
|
||||
* account's IDs are left untouched so existing single-account code paths
|
||||
* (links, deep-links, JMAP mutations) keep working unchanged.
|
||||
* Prefix used to namespace calendar/event IDs when the Pro shell aggregates
|
||||
* across accounts. EVERY aggregated account is namespaced (including the active
|
||||
* one) so an id stably identifies (account, entity) regardless of which account
|
||||
* is active - otherwise switching accounts flipped the id form. Single-account
|
||||
* (non-aggregated) code paths carry no localAccountId and keep raw ids.
|
||||
*/
|
||||
const CROSS_ACCOUNT_ID_DELIMITER = '::';
|
||||
|
||||
@@ -67,14 +104,18 @@ function buildCrossAccountIdPrefix(localAccountId: string): string {
|
||||
return `${localAccountId}${CROSS_ACCOUNT_ID_DELIMITER}`;
|
||||
}
|
||||
|
||||
// EVERY aggregated account is namespaced, including the active one. Leaving the
|
||||
// active account's IDs raw made an entity's ID depend on *which* account is
|
||||
// active, so switching accounts flipped the ID form and invalidated persisted
|
||||
// references (selectedCalendarIds, colors, subscriptions) - the account-switch
|
||||
// bug. The invariant is now: an entity is namespaced iff it carries a
|
||||
// `localAccountId`; `originalId` always holds the raw JMAP ID. Mutation/consumer
|
||||
// code already resolves the raw ID via `originalId || stripLocalAccountPrefix`,
|
||||
// so it keeps working unchanged.
|
||||
function prefixCalendarsWithLocalAccount(
|
||||
calendars: Calendar[],
|
||||
localAccountId: string,
|
||||
isActiveAccount: boolean,
|
||||
): Calendar[] {
|
||||
if (isActiveAccount) {
|
||||
return calendars.map((cal) => ({ ...cal, localAccountId }));
|
||||
}
|
||||
const prefix = buildCrossAccountIdPrefix(localAccountId);
|
||||
// Preserve each calendar's original `isShared` flag - it distinguishes
|
||||
// the user's own calendars on the other account from calendars shared
|
||||
@@ -83,6 +124,7 @@ function prefixCalendarsWithLocalAccount(
|
||||
return calendars.map((cal) => ({
|
||||
...cal,
|
||||
id: `${prefix}${cal.id}`,
|
||||
originalId: cal.originalId ?? cal.id,
|
||||
localAccountId,
|
||||
}));
|
||||
}
|
||||
@@ -90,15 +132,12 @@ function prefixCalendarsWithLocalAccount(
|
||||
function prefixEventsWithLocalAccount(
|
||||
events: CalendarEvent[],
|
||||
localAccountId: string,
|
||||
isActiveAccount: boolean,
|
||||
): CalendarEvent[] {
|
||||
if (isActiveAccount) {
|
||||
return events.map((event) => ({ ...event, localAccountId }));
|
||||
}
|
||||
const prefix = buildCrossAccountIdPrefix(localAccountId);
|
||||
return events.map((event) => ({
|
||||
...event,
|
||||
id: `${prefix}${event.id}`,
|
||||
originalId: event.originalId ?? event.id,
|
||||
localAccountId,
|
||||
calendarIds: event.calendarIds
|
||||
? Object.fromEntries(
|
||||
@@ -143,15 +182,21 @@ function mapServerEventToStoreEvent(
|
||||
.find((calendar): calendar is Calendar => Boolean(calendar));
|
||||
const resolvedAccountId = matchedCalendar?.accountId || targetAccountId;
|
||||
const isShared = matchedCalendar?.isShared || false;
|
||||
// Namespace the event id with the owning account when the store is aggregated
|
||||
// (the matched calendar carries a localAccountId). `calendarIds` are already
|
||||
// mapped to store ids above; `originalId` keeps the raw id for mutations.
|
||||
const localAccountId = matchedCalendar?.localAccountId;
|
||||
const baseId = isShared && resolvedAccountId ? `${resolvedAccountId}:${event.id}` : event.id;
|
||||
|
||||
return {
|
||||
...event,
|
||||
id: isShared && resolvedAccountId ? `${resolvedAccountId}:${event.id}` : event.id,
|
||||
id: localAccountId ? `${localAccountId}${CROSS_ACCOUNT_ID_DELIMITER}${baseId}` : baseId,
|
||||
originalId: event.id,
|
||||
originalCalendarIds: event.calendarIds,
|
||||
calendarIds: mappedCalendarIds,
|
||||
accountId: resolvedAccountId,
|
||||
accountName: matchedCalendar?.accountName,
|
||||
localAccountId,
|
||||
isShared,
|
||||
};
|
||||
}
|
||||
@@ -224,8 +269,8 @@ interface CalendarStore {
|
||||
setSupported: (supported: boolean) => void;
|
||||
fetchCalendars: (client: IJMAPClient) => Promise<void>;
|
||||
fetchEvents: (client: IJMAPClient, start: string, end: string) => Promise<void>;
|
||||
fetchAllAccountsCalendars: (accounts: CalendarAccountClient[], activeLocalAccountId: string) => Promise<void>;
|
||||
fetchAllAccountsEvents: (accounts: CalendarAccountClient[], activeLocalAccountId: string, start: string, end: string) => Promise<void>;
|
||||
fetchAllAccountsCalendars: (accounts: CalendarAccountClient[]) => Promise<void>;
|
||||
fetchAllAccountsEvents: (accounts: CalendarAccountClient[], start: string, end: string) => Promise<void>;
|
||||
createEvent: (client: IJMAPClient, event: Partial<CalendarEvent>, sendSchedulingMessages?: boolean) => Promise<CalendarEvent | null>;
|
||||
updateEvent: (client: IJMAPClient, id: string, updates: Partial<CalendarEvent>, sendSchedulingMessages?: boolean) => Promise<void>;
|
||||
deleteEvent: (client: IJMAPClient, id: string, sendSchedulingMessages?: boolean) => Promise<void>;
|
||||
@@ -284,12 +329,11 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
try {
|
||||
const calendars = await client.getAllCalendars();
|
||||
const { selectedCalendarIds } = get();
|
||||
const validIds = calendars.map(c => c.id);
|
||||
const stillValid = selectedCalendarIds.filter(id => validIds.includes(id) || id === BIRTHDAY_CALENDAR_ID);
|
||||
const stillValid = reconcileSelectedIds(selectedCalendarIds, calendars);
|
||||
set({
|
||||
calendars,
|
||||
isLoading: false,
|
||||
selectedCalendarIds: stillValid.length > 0 ? stillValid : validIds,
|
||||
selectedCalendarIds: stillValid.length > 0 ? stillValid : calendars.map(c => c.id),
|
||||
});
|
||||
} catch (error) {
|
||||
debug.error('Failed to fetch calendars:', error);
|
||||
@@ -332,18 +376,14 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
}
|
||||
},
|
||||
|
||||
fetchAllAccountsCalendars: async (accounts, activeLocalAccountId) => {
|
||||
fetchAllAccountsCalendars: async (accounts) => {
|
||||
set({ isLoading: true, error: null });
|
||||
try {
|
||||
const results = await Promise.all(
|
||||
accounts.map(async ({ client, localAccountId }) => {
|
||||
try {
|
||||
const list = await client.getAllCalendars();
|
||||
return prefixCalendarsWithLocalAccount(
|
||||
list,
|
||||
localAccountId,
|
||||
localAccountId === activeLocalAccountId,
|
||||
);
|
||||
return prefixCalendarsWithLocalAccount(list, localAccountId);
|
||||
} catch (error) {
|
||||
debug.error(`Failed to fetch calendars for account ${localAccountId}:`, error);
|
||||
return [] as Calendar[];
|
||||
@@ -352,12 +392,11 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
);
|
||||
const calendars = results.flat();
|
||||
const { selectedCalendarIds } = get();
|
||||
const validIds = calendars.map(c => c.id);
|
||||
const stillValid = selectedCalendarIds.filter(id => validIds.includes(id) || id === BIRTHDAY_CALENDAR_ID);
|
||||
const stillValid = reconcileSelectedIds(selectedCalendarIds, calendars);
|
||||
set({
|
||||
calendars,
|
||||
isLoading: false,
|
||||
selectedCalendarIds: stillValid.length > 0 ? stillValid : validIds,
|
||||
selectedCalendarIds: stillValid.length > 0 ? stillValid : calendars.map(c => c.id),
|
||||
});
|
||||
} catch (error) {
|
||||
debug.error('Failed to fetch all-account calendars:', error);
|
||||
@@ -365,7 +404,7 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
}
|
||||
},
|
||||
|
||||
fetchAllAccountsEvents: async (accounts, activeLocalAccountId, start, end) => {
|
||||
fetchAllAccountsEvents: async (accounts, start, end) => {
|
||||
set({ isLoadingEvents: true, error: null });
|
||||
try {
|
||||
const results = await Promise.all(
|
||||
@@ -376,11 +415,7 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
typeof e.start === 'string' && e.start && !isNaN(parseISO(e.start).getTime())
|
||||
);
|
||||
const expanded = expandRecurringEvents(valid, start, end);
|
||||
return prefixEventsWithLocalAccount(
|
||||
expanded,
|
||||
localAccountId,
|
||||
localAccountId === activeLocalAccountId,
|
||||
);
|
||||
return prefixEventsWithLocalAccount(expanded, localAccountId);
|
||||
} catch (error) {
|
||||
debug.error(`Failed to fetch events for account ${localAccountId}:`, error);
|
||||
return [] as CalendarEvent[];
|
||||
@@ -884,6 +919,8 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
set({ error: null });
|
||||
try {
|
||||
const created = await client.createCalendar(calendar);
|
||||
// Added with its raw id; the next aggregated refetch namespaces it and
|
||||
// reconcileSelectedIds() remaps the selection so it stays visible.
|
||||
set((state) => ({
|
||||
calendars: [...state.calendars, created],
|
||||
selectedCalendarIds: [...state.selectedCalendarIds, created.id],
|
||||
@@ -997,7 +1034,20 @@ export const useCalendarStore = create<CalendarStore>()(
|
||||
|
||||
// iCal subscriptions
|
||||
isSubscriptionCalendar: (calendarId) => {
|
||||
return get().icalSubscriptions.some(s => s.calendarId === calendarId);
|
||||
const subs = get().icalSubscriptions;
|
||||
// Fast path: exact match (single-account, or a sub stored with the same
|
||||
// id form as the query).
|
||||
if (subs.some((s) => s.calendarId === calendarId)) return true;
|
||||
// Aggregated: the query is a namespaced store id while subs store the raw
|
||||
// JMAP calendar id. Resolve the calendar and match by raw id, scoped to
|
||||
// the owning JMAP account so raw ids that collide across accounts (e.g.
|
||||
// Stalwart's "b"/"c") don't cause false positives.
|
||||
const cal = get().calendars.find((c) => c.id === calendarId);
|
||||
if (!cal) return false;
|
||||
const rawId = cal.originalId ?? stripLocalAccountPrefix(cal.id, cal.localAccountId);
|
||||
return subs.some(
|
||||
(s) => s.calendarId === rawId && (!s.accountId || !cal.accountId || s.accountId === cal.accountId),
|
||||
);
|
||||
},
|
||||
|
||||
addICalSubscription: async (client, url, name, color, refreshInterval = 60) => {
|
||||
|
||||
+17
-26
@@ -55,10 +55,9 @@ export interface ContactAccountClient {
|
||||
}
|
||||
|
||||
/**
|
||||
* Prefix used to namespace contact/address-book IDs that belong to a
|
||||
* non-active JMAP account when the Pro shell aggregates across accounts.
|
||||
* The active account's IDs are left untouched so existing single-account
|
||||
* code paths keep working unchanged.
|
||||
* Prefix used to namespace contact/address-book IDs when the Pro shell
|
||||
* aggregates across accounts. EVERY aggregated account is namespaced (including
|
||||
* the active one); single-account paths carry no localAccountId and stay raw.
|
||||
*/
|
||||
const CROSS_ACCOUNT_ID_DELIMITER = '::';
|
||||
|
||||
@@ -66,18 +65,21 @@ function buildCrossAccountIdPrefix(localAccountId: string): string {
|
||||
return `${localAccountId}${CROSS_ACCOUNT_ID_DELIMITER}`;
|
||||
}
|
||||
|
||||
// EVERY aggregated account is namespaced, including the active one, so an id
|
||||
// stably identifies (account, entity) regardless of which account is active -
|
||||
// otherwise switching accounts flipped the id form and broke mutation routing /
|
||||
// selection. Invariant: namespaced iff `localAccountId` is set; `originalId`
|
||||
// holds the raw id. Mutations already resolve raw via `originalId ||
|
||||
// stripLocalAccountPrefix`, so they keep working. Mirrors the calendar store.
|
||||
function prefixAddressBooksWithLocalAccount(
|
||||
books: AddressBook[],
|
||||
localAccountId: string,
|
||||
isActiveAccount: boolean,
|
||||
): AddressBook[] {
|
||||
if (isActiveAccount) {
|
||||
return books.map((b) => ({ ...b, localAccountId }));
|
||||
}
|
||||
const prefix = buildCrossAccountIdPrefix(localAccountId);
|
||||
return books.map((b) => ({
|
||||
...b,
|
||||
id: `${prefix}${b.id}`,
|
||||
originalId: b.originalId ?? b.id,
|
||||
localAccountId,
|
||||
}));
|
||||
}
|
||||
@@ -85,15 +87,12 @@ function prefixAddressBooksWithLocalAccount(
|
||||
function prefixContactsWithLocalAccount(
|
||||
contacts: ContactCard[],
|
||||
localAccountId: string,
|
||||
isActiveAccount: boolean,
|
||||
): ContactCard[] {
|
||||
if (isActiveAccount) {
|
||||
return contacts.map((c) => ({ ...c, localAccountId }));
|
||||
}
|
||||
const prefix = buildCrossAccountIdPrefix(localAccountId);
|
||||
return contacts.map((c) => ({
|
||||
...c,
|
||||
id: `${prefix}${c.id}`,
|
||||
originalId: c.originalId ?? c.id,
|
||||
localAccountId,
|
||||
addressBookIds: c.addressBookIds
|
||||
? Object.fromEntries(
|
||||
@@ -277,8 +276,8 @@ interface ContactStore {
|
||||
fetchContacts: (client: IJMAPClient) => Promise<void>;
|
||||
fetchDirectory: (client: IJMAPClient) => Promise<void>;
|
||||
fetchAddressBooks: (client: IJMAPClient) => Promise<void>;
|
||||
fetchAllAccountsContacts: (accounts: ContactAccountClient[], activeLocalAccountId: string) => Promise<void>;
|
||||
fetchAllAccountsAddressBooks: (accounts: ContactAccountClient[], activeLocalAccountId: string) => Promise<void>;
|
||||
fetchAllAccountsContacts: (accounts: ContactAccountClient[]) => Promise<void>;
|
||||
fetchAllAccountsAddressBooks: (accounts: ContactAccountClient[]) => Promise<void>;
|
||||
createContact: (client: IJMAPClient, contact: Partial<ContactCard>) => Promise<void>;
|
||||
updateContact: (client: IJMAPClient, id: string, updates: Partial<ContactCard>) => Promise<void>;
|
||||
deleteContact: (client: IJMAPClient, id: string) => Promise<void>;
|
||||
@@ -429,18 +428,14 @@ export const useContactStore = create<ContactStore>()(
|
||||
}
|
||||
},
|
||||
|
||||
fetchAllAccountsContacts: async (accounts, activeLocalAccountId) => {
|
||||
fetchAllAccountsContacts: async (accounts) => {
|
||||
set({ isLoading: true, error: null });
|
||||
try {
|
||||
const results = await Promise.all(
|
||||
accounts.map(async ({ client, localAccountId }) => {
|
||||
try {
|
||||
const list = await client.getAllContacts();
|
||||
return prefixContactsWithLocalAccount(
|
||||
list,
|
||||
localAccountId,
|
||||
localAccountId === activeLocalAccountId,
|
||||
);
|
||||
return prefixContactsWithLocalAccount(list, localAccountId);
|
||||
} catch (error) {
|
||||
debug.error(`Failed to fetch contacts for account ${localAccountId}:`, error);
|
||||
return [] as ContactCard[];
|
||||
@@ -454,17 +449,13 @@ export const useContactStore = create<ContactStore>()(
|
||||
}
|
||||
},
|
||||
|
||||
fetchAllAccountsAddressBooks: async (accounts, activeLocalAccountId) => {
|
||||
fetchAllAccountsAddressBooks: async (accounts) => {
|
||||
try {
|
||||
const results = await Promise.all(
|
||||
accounts.map(async ({ client, localAccountId }) => {
|
||||
try {
|
||||
const list = await client.getAllAddressBooks();
|
||||
return prefixAddressBooksWithLocalAccount(
|
||||
list,
|
||||
localAccountId,
|
||||
localAccountId === activeLocalAccountId,
|
||||
);
|
||||
return prefixAddressBooksWithLocalAccount(list, localAccountId);
|
||||
} catch (error) {
|
||||
debug.error(`Failed to fetch address books for account ${localAccountId}:`, error);
|
||||
return [] as AddressBook[];
|
||||
|
||||
Reference in New Issue
Block a user