Merge pull request #76 from lucletoffe/feat/calendar-sort-order

feat(calendar): order calendars by sortOrder and move them from the drawer

src/components/calendar/CalendarSidebarDrawer.tsx

@@ -12,7 +12,7 @@ import {
 } from 'react-native';
 import { SafeAreaView } from 'react-native-safe-area-context';
 import {
-  Check, X, Upload, Rss, Shuffle, Star, Plus, Pencil, Share2, Eraser, Trash2, RefreshCw,
+  Check, X, Upload, Rss, Shuffle, Star, Plus, Pencil, Share2, Eraser, Trash2, RefreshCw, ArrowUp, ArrowDown,
   type AppIcon,
 } from '../icons';
 import type { Calendar } from '../../api/types';
@@ -55,6 +55,9 @@ interface CalendarSidebarDrawerProps {
   onShare?: (calendar: Calendar) => void;
   onClear?: (calendar: Calendar) => void;
   onDelete?: (calendar: Calendar) => void;
+  // Move one of the user's own calendars next to `targetId`, a neighbour in
+  // the same section (before it when moving up, after it when moving down).
+  onMove?: (calendar: Calendar, targetId: string) => void;
   // Client-side iCal subscriptions are managed from the subscriptions sheet.
   isSubscriptionCalendar?: (calendarId: string) => boolean;
 }
@@ -79,6 +82,7 @@ export function CalendarSidebarDrawer({
   onShare,
   onClear,
   onDelete,
+  onMove,
   isSubscriptionCalendar,
 }: CalendarSidebarDrawerProps) {
   const c = useColors();
@@ -148,6 +152,7 @@ export function CalendarSidebarDrawer({
     onShare,
     onClear,
     onDelete,
+    onMove,
     isSubscriptionCalendar,
   };
 
@@ -284,6 +289,7 @@ function Section({
   onShare,
   onClear,
   onDelete,
+  onMove,
   isSubscriptionCalendar,
 }: {
   title: string;
@@ -299,11 +305,14 @@ function Section({
   onShare?: (calendar: Calendar) => void;
   onClear?: (calendar: Calendar) => void;
   onDelete?: (calendar: Calendar) => void;
+  onMove?: (calendar: Calendar, targetId: string) => void;
   isSubscriptionCalendar?: (calendarId: string) => boolean;
 }) {
   const c = useColors();
   const styles = React.useMemo(() => makeStyles(c), [c]);
   const t = useLocaleStore((s) => s.t);
+  // Neighbours a calendar can move past: the server calendars of this section.
+  const orderable = calendars.filter((cal) => cal.id !== BIRTHDAY_CALENDAR_ID);
   return (
     <View style={styles.section}>
       <Text style={styles.sectionTitle}>{title}</Text>
@@ -320,7 +329,11 @@ function Section({
         const canClear = !!onClear && isOwn && isWritableCalendar(cal);
         // Like webmail: the default calendar and shared calendars can't be deleted.
         const canDelete = !!onDelete && isOwn && !cal.isDefault && (!cal.myRights || cal.myRights.mayDelete !== false);
-        const hasActions = canSetDefault || canRecolor || canRename || canShare || canClear || canDelete;
+        const orderIndex = orderable.indexOf(cal);
+        const previousId = isOwn && orderIndex > 0 ? orderable[orderIndex - 1].id : undefined;
+        const nextId = isOwn && orderIndex >= 0 ? orderable[orderIndex + 1]?.id : undefined;
+        const canMove = !!onMove && (!!previousId || !!nextId);
+        const hasActions = canSetDefault || canRecolor || canRename || canShare || canClear || canDelete || canMove;
         const expanded = expandedId === cal.id && hasActions;
         return (
           <View key={cal.id}>
@@ -355,6 +368,27 @@ function Section({
 
             {expanded && (
               <View style={styles.actionsPanel}>
+                {/* The panel stays open so a calendar can be moved several places. */}
+                {canMove && previousId && (
+                  <Pressable
+                    onPress={() => onMove!(cal, previousId)}
+                    style={({ pressed }) => [styles.panelRow, pressed && styles.rowPressed]}
+                    accessibilityRole="button"
+                  >
+                    <ArrowUp size={16} color={c.textSecondary} />
+                    <Text style={styles.panelRowText}>{t('settings.account.accounts.move_up', 'Move up')}</Text>
+                  </Pressable>
+                )}
+                {canMove && nextId && (
+                  <Pressable
+                    onPress={() => onMove!(cal, nextId)}
+                    style={({ pressed }) => [styles.panelRow, pressed && styles.rowPressed]}
+                    accessibilityRole="button"
+                  >
+                    <ArrowDown size={16} color={c.textSecondary} />
+                    <Text style={styles.panelRowText}>{t('settings.account.accounts.move_down', 'Move down')}</Text>
+                  </Pressable>
+                )}
                 {canSetDefault && (
                   <Pressable
                     onPress={() => { onExpand(null); onSetDefault!(cal); }}

src/lib/__tests__/calendar-sort.test.ts

@@ -0,0 +1,94 @@
+import { describe, it, expect } from 'vitest';
+import { compareCalendars, moveCalendar, sortCalendars, sortOrderUpdates } from '../calendar-sort';
+import type { Calendar } from '../../api/types';
+
+function cal(overrides: Partial<Calendar>): Calendar {
+  return { id: 'c', name: 'Cal', sortOrder: 0, ...overrides };
+}
+
+const names = (list: Calendar[]) => list.map((c) => c.name);
+const ids = (list: Calendar[]) => list.map((c) => c.id);
+
+describe('sortCalendars', () => {
+  it('orders by sortOrder, whatever order the server listed them in', () => {
+    const list = [
+      cal({ id: 'b', name: 'Default', sortOrder: 20 }),
+      cal({ id: 'n', name: 'Holidays', sortOrder: 30 }),
+      cal({ id: 'e', name: 'Work', sortOrder: 1 }),
+      cal({ id: 'd', name: 'Personal', sortOrder: 10 }),
+    ];
+    expect(names(sortCalendars(list))).toEqual(['Work', 'Personal', 'Default', 'Holidays']);
+  });
+
+  it('sorts equal sortOrder by name, ignoring case and accents, numbers by value', () => {
+    const list = [
+      cal({ id: '1', name: 'zeta' }),
+      cal({ id: '2', name: 'Team 10' }),
+      cal({ id: '3', name: 'Éclair' }),
+      cal({ id: '4', name: 'alpha' }),
+      cal({ id: '5', name: 'Team 2' }),
+      cal({ id: '6', name: 'Beta' }),
+    ];
+    expect(names(sortCalendars(list))).toEqual(['alpha', 'Beta', 'Éclair', 'Team 2', 'Team 10', 'zeta']);
+  });
+
+  it('treats a missing sortOrder as 0', () => {
+    const list = [cal({ id: 'a', name: 'B', sortOrder: 1 }), cal({ id: 'b', name: 'A', sortOrder: undefined })];
+    expect(names(sortCalendars(list))).toEqual(['A', 'B']);
+  });
+
+  it('keeps each account together, in the order the accounts came', () => {
+    const list = [
+      cal({ id: 'own-2', name: 'Own 2', sortOrder: 2 }),
+      cal({ id: 'own-1', name: 'Own 1', sortOrder: 1 }),
+      cal({ id: 'x:b', name: 'Shared B', sortOrder: 0, isShared: true, accountId: 'x' }),
+      cal({ id: 'y:a', name: 'Other A', sortOrder: 0, isShared: true, accountId: 'y' }),
+      cal({ id: 'x:a', name: 'Shared A', sortOrder: 5, isShared: true, accountId: 'x' }),
+    ];
+    expect(ids(sortCalendars(list))).toEqual(['own-1', 'own-2', 'x:b', 'x:a', 'y:a']);
+  });
+
+  it('does not change the list it is given', () => {
+    const list = [cal({ id: 'b', sortOrder: 2 }), cal({ id: 'a', sortOrder: 1 })];
+    sortCalendars(list);
+    expect(ids(list)).toEqual(['b', 'a']);
+  });
+
+  it('compareCalendars puts the lower sortOrder first', () => {
+    expect(compareCalendars(cal({ sortOrder: 1, name: 'Z' }), cal({ sortOrder: 2, name: 'A' }))).toBeLessThan(0);
+  });
+});
+
+describe('sortOrderUpdates', () => {
+  it('numbers the calendars 1, 2, 3 in the given order and skips unchanged ones', () => {
+    const list = [cal({ id: 'a', sortOrder: 1 }), cal({ id: 'b', sortOrder: 2 }), cal({ id: 'c', sortOrder: 3 })];
+    expect(sortOrderUpdates(list, ['a', 'c', 'b'])).toEqual([
+      { id: 'c', sortOrder: 2 },
+      { id: 'b', sortOrder: 3 },
+    ]);
+  });
+});
+
+describe('moveCalendar', () => {
+  const list = [
+    cal({ id: 'work', name: 'Work', sortOrder: 1 }),
+    cal({ id: 'home', name: 'Home', sortOrder: 10 }),
+    cal({ id: 'tasks', name: 'Tasks', sortOrder: 15 }),
+    cal({ id: 'holidays', name: 'Holidays', sortOrder: 30 }),
+    cal({ id: 'x:team', name: 'Team', sortOrder: 0, isShared: true, accountId: 'x' }),
+  ];
+
+  it('moving up puts the calendar just before the target', () => {
+    expect(moveCalendar(list, 'holidays', 'home')).toEqual(['work', 'holidays', 'home', 'tasks']);
+  });
+
+  it('moving down puts the calendar just after the target', () => {
+    expect(moveCalendar(list, 'work', 'home')).toEqual(['home', 'work', 'tasks', 'holidays']);
+  });
+
+  it('refuses a move across accounts, onto itself or to an unknown id', () => {
+    expect(moveCalendar(list, 'work', 'x:team')).toBeNull();
+    expect(moveCalendar(list, 'work', 'work')).toBeNull();
+    expect(moveCalendar(list, 'work', 'gone')).toBeNull();
+  });
+});

src/lib/calendar-sort.ts

@@ -0,0 +1,69 @@
+import type { Calendar } from '../api/types';
+
+// Names compare the way the user reads them: accents and case are ignored, and
+// "Team 2" comes before "Team 10".
+const nameCollator = new Intl.Collator(undefined, { sensitivity: 'base', numeric: true });
+
+/**
+ * JMAP Calendars: a calendar with a lower `sortOrder` is shown first, and
+ * calendars with equal `sortOrder` are sorted by name. Mirrors the webmail.
+ */
+export function compareCalendars(a: Calendar, b: Calendar): number {
+  return (a.sortOrder ?? 0) - (b.sortOrder ?? 0) || nameCollator.compare(a.name ?? '', b.name ?? '');
+}
+
+/**
+ * Calendars of one account stay together: the user's own first, then each
+ * sharing account, as fetched.
+ */
+export function calendarAccountKey(calendar: Calendar): string {
+  return calendar.isShared ? calendar.accountId ?? '' : '';
+}
+
+/**
+ * The calendars in display order: grouped by account in the order the
+ * accounts first appear, and by `sortOrder`, then name, within each account.
+ */
+export function sortCalendars(calendars: Calendar[]): Calendar[] {
+  const accountRank = new Map<string, number>();
+  for (const calendar of calendars) {
+    const key = calendarAccountKey(calendar);
+    if (!accountRank.has(key)) accountRank.set(key, accountRank.size);
+  }
+  return [...calendars].sort((a, b) =>
+    accountRank.get(calendarAccountKey(a))! - accountRank.get(calendarAccountKey(b))! || compareCalendars(a, b),
+  );
+}
+
+/**
+ * `sortOrder` values that put `orderedIds` in that order: 1, 2, 3... Only the
+ * calendars whose value changes are returned.
+ */
+export function sortOrderUpdates(calendars: Calendar[], orderedIds: string[]): { id: string; sortOrder: number }[] {
+  const byId = new Map(calendars.map((calendar) => [calendar.id, calendar]));
+  const updates: { id: string; sortOrder: number }[] = [];
+  orderedIds.forEach((id, index) => {
+    const calendar = byId.get(id);
+    if (calendar && calendar.sortOrder !== index + 1) updates.push({ id, sortOrder: index + 1 });
+  });
+  return updates;
+}
+
+/**
+ * The ids of the calendar's account in display order once it is moved next to
+ * `targetId`: before the target when it moves up, after it when it moves down.
+ * Null when the move is not possible (unknown ids, or another account).
+ */
+export function moveCalendar(calendars: Calendar[], calendarId: string, targetId: string): string[] | null {
+  const sorted = sortCalendars(calendars);
+  const moving = sorted.find((calendar) => calendar.id === calendarId);
+  const target = sorted.find((calendar) => calendar.id === targetId);
+  if (!moving || !target || moving === target) return null;
+  const key = calendarAccountKey(moving);
+  if (calendarAccountKey(target) !== key) return null;
+  const group = sorted.filter((calendar) => calendarAccountKey(calendar) === key);
+  const movingDown = group.indexOf(moving) < group.indexOf(target);
+  const rest = group.filter((calendar) => calendar !== moving);
+  rest.splice(rest.indexOf(target) + (movingDown ? 1 : 0), 0, moving);
+  return rest.map((calendar) => calendar.id);
+}

src/screens/CalendarScreen.tsx

@@ -96,6 +96,7 @@ import {
   truncateRecurrenceRules,
 } from '../lib/recurrence-overrides';
 import { generateBirthdayEvents, createBirthdayCalendar, BIRTHDAY_CALENDAR_ID } from '../lib/birthday-calendar';
+import { moveCalendar } from '../lib/calendar-sort';
 import { useContactsStore } from '../stores/contacts-store';
 import { useLocaleStore } from '../stores/locale-store';
 import { useUserCalendarAddresses } from '../lib/calendar-user-addresses';
@@ -262,6 +263,7 @@ export default function CalendarScreen() {
   const setDefaultCalendar = useCalendarStore((s) => s.setDefaultCalendar);
   const createCalendar = useCalendarStore((s) => s.createCalendar);
   const updateCalendar = useCalendarStore((s) => s.updateCalendar);
+  const reorderCalendars = useCalendarStore((s) => s.reorderCalendars);
   const removeCalendar = useCalendarStore((s) => s.removeCalendar);
   const clearCalendarEvents = useCalendarStore((s) => s.clearCalendarEvents);
   const shareCalendar = useCalendarStore((s) => s.shareCalendar);
@@ -954,6 +956,14 @@ export default function CalendarScreen() {
     [setSharedCalendarColor, updateCalendar, reportError],
   );
 
+  const handleMoveCalendar = React.useCallback(
+    (cal: Calendar, targetId: string) => {
+      const orderedIds = moveCalendar(storeCalendars, cal.id, targetId);
+      if (orderedIds) reorderCalendars(orderedIds).catch(reportError);
+    },
+    [storeCalendars, reorderCalendars, reportError],
+  );
+
   const handleClearCalendar = React.useCallback(
     (cal: Calendar) => {
       Alert.alert(
@@ -1277,6 +1287,7 @@ export default function CalendarScreen() {
         onShare={(cal) => { setSidebarVisible(false); setShareTarget(cal); }}
         onClear={handleClearCalendar}
         onDelete={handleDeleteCalendar}
+        onMove={handleMoveCalendar}
         isSubscriptionCalendar={isSubscriptionCalendar}
       />
 

src/stores/__tests__/calendar-store-sort-order.test.ts

@@ -0,0 +1,84 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+vi.mock('../../api/calendar', () => ({
+  getCalendars: vi.fn(),
+  updateCalendar: vi.fn(),
+  createCalendar: vi.fn(),
+  isCalendarAccessDenied: vi.fn(() => false),
+  noteCalendarAccessError: vi.fn(() => false),
+  resetCalendarAccessDenied: vi.fn(),
+  supportsSyntheticCalendarIds: vi.fn(async () => false),
+  resetSyntheticIdSupport: vi.fn(),
+}));
+
+vi.mock('../../api/jmap-client', () => ({
+  jmapClient: { accountId: 'acc-1', isConnected: true },
+}));
+
+vi.mock('../../device-sync/app/request', () => ({ requestDeviceSync: vi.fn() }));
+
+import * as calendarApi from '../../api/calendar';
+import { useCalendarStore } from '../calendar-store';
+import type { Calendar } from '../../api/types';
+
+const mockGetCalendars = calendarApi.getCalendars as ReturnType<typeof vi.fn>;
+const mockUpdateCalendar = calendarApi.updateCalendar as ReturnType<typeof vi.fn>;
+
+const cal = (overrides: Partial<Calendar>): Calendar => ({ id: 'c', name: 'Cal', sortOrder: 0, ...overrides });
+const names = () => useCalendarStore.getState().calendars.map((c) => c.name);
+
+beforeEach(() => {
+  vi.clearAllMocks();
+  useCalendarStore.setState({ calendars: [], error: null });
+});
+
+describe('calendar order', () => {
+  it('fetchCalendars lists the calendars by sortOrder, not in server order', async () => {
+    mockGetCalendars.mockResolvedValue([
+      cal({ id: 'b', name: 'Default', sortOrder: 20 }),
+      cal({ id: 'n', name: 'Holidays', sortOrder: 30 }),
+      cal({ id: 'e', name: 'Work', sortOrder: 1 }),
+    ]);
+
+    await useCalendarStore.getState().fetchCalendars();
+
+    expect(names()).toEqual(['Work', 'Default', 'Holidays']);
+  });
+
+  it('reorderCalendars writes only the sortOrders that change, to the owning account', async () => {
+    useCalendarStore.setState({
+      calendars: [
+        cal({ id: 'a', name: 'A', sortOrder: 1 }),
+        cal({ id: 'b', name: 'B', sortOrder: 2 }),
+        cal({ id: 'x:c', originalId: 'c', name: 'C', sortOrder: 0, isShared: true, accountId: 'x' }),
+      ],
+    });
+    mockUpdateCalendar.mockResolvedValue(undefined);
+
+    const done = useCalendarStore.getState().reorderCalendars(['b', 'a']);
+    expect(names()).toEqual(['B', 'A', 'C']);
+    await done;
+
+    expect(mockUpdateCalendar.mock.calls).toEqual([
+      ['b', { sortOrder: 1 }, undefined],
+      ['a', { sortOrder: 2 }, undefined],
+    ]);
+  });
+
+  it('after a failed write, shows the order the server holds and rethrows', async () => {
+    useCalendarStore.setState({
+      calendars: [
+        cal({ id: 'a', name: 'A', sortOrder: 1 }),
+        cal({ id: 'b', name: 'B', sortOrder: 2 }),
+        cal({ id: 'c', name: 'C', sortOrder: 3 }),
+      ],
+    });
+    mockUpdateCalendar.mockResolvedValueOnce(undefined).mockRejectedValueOnce(new Error('forbidden'));
+
+    await expect(useCalendarStore.getState().reorderCalendars(['c', 'a', 'b'])).rejects.toThrow('forbidden');
+
+    expect(useCalendarStore.getState().calendars.map((c) => [c.id, c.sortOrder])).toEqual([
+      ['a', 1], ['c', 1], ['b', 2],
+    ]);
+  });
+});

src/stores/calendar-store.ts

@@ -48,6 +48,7 @@ import {
 import { findTasksOnlyCalendarIds, isTaskLikeObject } from '../lib/calendar-component-detection';
 import { mergeRangeEvents, mergedSpanLimit, planRangeLoad, sameRange } from '../lib/calendar-range-cache';
 import { requestDeviceSync } from '../device-sync/app/request';
+import { sortCalendars, sortOrderUpdates } from '../lib/calendar-sort';
 import { CALENDAR_AUTHORITY } from '../device-sync/types';
 
 // extendRange merges up to twice the requested range (at least half a year,
@@ -266,6 +267,8 @@ export interface CalendarState {
   importEvents: (events: Partial<CalendarEvent>[], calendarId: string) => Promise<ImportResult>;
   createCalendar: (name: string, color?: string, description?: string) => Promise<Calendar>;
   updateCalendar: (id: string, updates: CalendarUpdates) => Promise<void>;
+  // Store `orderedIds` in that order by renumbering their sortOrder.
+  reorderCalendars: (orderedIds: string[]) => Promise<void>;
   removeCalendar: (id: string) => Promise<void>;
   clearCalendarEvents: (id: string) => Promise<number>;
   shareCalendar: (id: string, principalId: string, rights: CalendarRights | null) => Promise<void>;
@@ -572,7 +575,7 @@ export const useCalendarStore = create<CalendarState>()(
     if (calendarsInFlight) return calendarsInFlight;
     calendarsInFlight = (async () => {
       try {
-        const calendars = (await fetchCalendars()) ?? [];
+        const calendars = sortCalendars((await fetchCalendars()) ?? []);
         set({ calendars });
       } catch (err) {
         set({ error: err instanceof Error ? err.message : 'Failed to load calendars' });
@@ -1013,7 +1016,7 @@ export const useCalendarStore = create<CalendarState>()(
 
   createCalendar: async (name, color, description) => {
     const created = await apiCreateCalendar(name, color, description);
-    set({ calendars: [...get().calendars, created] });
+    set({ calendars: sortCalendars([...get().calendars, created]) });
     requestDeviceSync(CALENDAR_AUTHORITY);
     return created;
   },
@@ -1022,11 +1025,36 @@ export const useCalendarStore = create<CalendarState>()(
     const cal = get().calendars.find((c) => c.id === id);
     await apiUpdateCalendar(cal?.originalId || id, updates, cal?.accountId);
     set({
-      calendars: get().calendars.map((c) => (c.id === id ? { ...c, ...updates } as Calendar : c)),
+      calendars: sortCalendars(get().calendars.map((c) => (c.id === id ? { ...c, ...updates } as Calendar : c))),
     });
     requestDeviceSync(CALENDAR_AUTHORITY);
   },
 
+  reorderCalendars: async (orderedIds) => {
+    const updates = sortOrderUpdates(get().calendars, orderedIds);
+    if (updates.length === 0) return;
+    const before = get().calendars;
+    const withSortOrders = (sortOrders: Map<string, number>) => sortCalendars(before.map((c) =>
+      sortOrders.has(c.id) ? { ...c, sortOrder: sortOrders.get(c.id)! } : c));
+    // Optimistic, so the list moves at once.
+    set({ calendars: withSortOrders(new Map(updates.map((u) => [u.id, u.sortOrder]))) });
+    const saved = new Map<string, number>();
+    try {
+      for (const { id, sortOrder } of updates) {
+        const cal = before.find((c) => c.id === id);
+        await apiUpdateCalendar(cal?.originalId || id, { sortOrder }, cal?.accountId);
+        saved.set(id, sortOrder);
+      }
+    } catch (err) {
+      // Show the order the server now holds: the moves saved before the
+      // failure stay, the rest are undone.
+      set({ calendars: withSortOrders(saved) });
+      throw err;
+    } finally {
+      if (saved.size > 0) requestDeviceSync(CALENDAR_AUTHORITY);
+    }
+  },
+
   removeCalendar: async (id) => {
     const cal = get().calendars.find((c) => c.id === id);
     await apiDeleteCalendar(cal?.originalId || id, cal?.accountId);