From 76b4b96e16560da5abe21192308ba9d6bb923fd5 Mon Sep 17 00:00:00 2001 From: Maurice Date: Fri, 9 Oct 2026 15:01:54 +0200 Subject: [PATCH] refactor(budget): share the category reorder between the slice and the remote handler The optimistic category reorder in budgetSlice and the budget:reordered handler for another member's reorder grouped the items the same way in two copies. Both now call orderBudgetByCategories from budgetOrder.ts, so the local and the remote reorder cannot drift apart. --- client/src/store/slices/budgetOrder.test.ts | 30 +++++++++++++++++++ client/src/store/slices/budgetOrder.ts | 27 +++++++++++++++++ client/src/store/slices/budgetSlice.ts | 19 ++---------- client/src/store/slices/remoteEventHandler.ts | 18 ++--------- 4 files changed, 61 insertions(+), 33 deletions(-) create mode 100644 client/src/store/slices/budgetOrder.test.ts create mode 100644 client/src/store/slices/budgetOrder.ts diff --git a/client/src/store/slices/budgetOrder.test.ts b/client/src/store/slices/budgetOrder.test.ts new file mode 100644 index 000000000..29bf1427f --- /dev/null +++ b/client/src/store/slices/budgetOrder.test.ts @@ -0,0 +1,30 @@ +import { describe, expect, it } from 'vitest'; +import type { BudgetItem } from '../../types'; +import { orderBudgetByCategories } from './budgetOrder'; + +const item = (id: number, category: string | null): BudgetItem => + ({ id, category, name: `item ${id}`, sort_order: id }) as unknown as BudgetItem; + +describe('orderBudgetByCategories', () => { + it('groups the items in the order the categories are given, keeping their order inside each', () => { + const items = [item(1, 'Food'), item(2, 'Hotel'), item(3, 'Food'), item(4, 'Hotel')]; + expect(orderBudgetByCategories(items, ['Hotel', 'Food']).map((i) => i.id)).toEqual([2, 4, 1, 3]); + }); + + it('counts an item without a category as Other', () => { + const items = [item(1, null), item(2, 'Food'), item(3, '')]; + expect(orderBudgetByCategories(items, ['Other', 'Food']).map((i) => i.id)).toEqual([1, 3, 2]); + }); + + it('puts the categories the list does not name last, in the order they were first seen', () => { + const items = [item(1, 'Fuel'), item(2, 'Food'), item(3, 'Tickets'), item(4, 'Fuel')]; + expect(orderBudgetByCategories(items, ['Food']).map((i) => i.id)).toEqual([2, 1, 4, 3]); + }); + + it('ignores a named category no item has and leaves the items untouched', () => { + const items = [item(1, 'Food')]; + const out = orderBudgetByCategories(items, ['Ghost', 'Food']); + expect(out).toEqual(items); + expect(out[0]).toBe(items[0]); + }); +}); diff --git a/client/src/store/slices/budgetOrder.ts b/client/src/store/slices/budgetOrder.ts new file mode 100644 index 000000000..2c5d59000 --- /dev/null +++ b/client/src/store/slices/budgetOrder.ts @@ -0,0 +1,27 @@ +import type { BudgetItem } from '../../types'; + +/** + * The budget items regrouped by category, in the order the categories are given. + * + * Items keep their order inside a category, an item without one counts as + * 'Other', and a category the list does not name follows the named ones in the + * order it was first seen. Shared by the local reorder and the reorder another + * member's client broadcasts, so both land on the same list. + */ +export function orderBudgetByCategories(items: BudgetItem[], orderedCategories: string[]): BudgetItem[] { + const grouped = new Map(); + for (const item of items) { + const cat = item.category || 'Other'; + if (!grouped.has(cat)) grouped.set(cat, []); + grouped.get(cat)!.push(item); + } + const reordered: BudgetItem[] = []; + for (const cat of orderedCategories) { + const inCategory = grouped.get(cat); + if (inCategory) reordered.push(...inCategory); + } + for (const [cat, inCategory] of grouped) { + if (!orderedCategories.includes(cat)) reordered.push(...inCategory); + } + return reordered; +} diff --git a/client/src/store/slices/budgetSlice.ts b/client/src/store/slices/budgetSlice.ts index d1a9e8f66..054483a4d 100644 --- a/client/src/store/slices/budgetSlice.ts +++ b/client/src/store/slices/budgetSlice.ts @@ -7,6 +7,7 @@ import type { BudgetCreateItemRequest, BudgetFallbackFx, BudgetFreezeRatesRespon import { getApiErrorMessage } from '../../types' import { withFallbackFx } from '../../hooks/useExchangeRates' import { notify } from '../notify' +import { orderBudgetByCategories } from './budgetOrder' type SetState = StoreApi['setState'] type GetState = StoreApi['getState'] @@ -138,23 +139,7 @@ export const createBudgetSlice = (set: SetState, get: GetState): BudgetSlice => reorderBudgetCategories: async (tripId, orderedCategories) => { // Optimistic: reorder items by new category order (Map preserves insertion order for numeric keys) - set(state => { - const grouped = new Map() - for (const item of state.budgetItems) { - const cat = item.category || 'Other' - if (!grouped.has(cat)) grouped.set(cat, []) - grouped.get(cat)!.push(item) - } - const reordered: BudgetItem[] = [] - for (const cat of orderedCategories) { - const items = grouped.get(cat) - if (items) reordered.push(...items) - } - for (const [cat, items] of grouped) { - if (!orderedCategories.includes(cat)) reordered.push(...items) - } - return { budgetItems: reordered } - }) + set(state => ({ budgetItems: orderBudgetByCategories(state.budgetItems, orderedCategories) })) try { await budgetApi.reorderCategories(tripId, orderedCategories) } catch (err: unknown) { diff --git a/client/src/store/slices/remoteEventHandler.ts b/client/src/store/slices/remoteEventHandler.ts index 38acd8e3c..f719bbc06 100644 --- a/client/src/store/slices/remoteEventHandler.ts +++ b/client/src/store/slices/remoteEventHandler.ts @@ -6,6 +6,7 @@ import { offlineDb } from '../../db/offlineDb' import { useAuthStore } from '../authStore' import { mergeAssignmentPlace } from './placesSlice' import { withoutDay } from './daysSlice' +import { orderBudgetByCategories } from './budgetOrder' type SetState = StoreApi['setState'] type GetState = StoreApi['getState'] @@ -439,22 +440,7 @@ export const STATE_APPLIERS: Partial> return { budgetItems: [...reordered, ...remaining].map((item, idx) => ({ ...item, sort_order: idx })) } } if (payload.orderedCategories) { - const orderedCategories = payload.orderedCategories as string[] - const grouped = new Map() - for (const item of state.budgetItems) { - const cat = item.category || 'Other' - if (!grouped.has(cat)) grouped.set(cat, []) - grouped.get(cat)!.push(item) - } - const reordered: BudgetItem[] = [] - for (const cat of orderedCategories) { - const items = grouped.get(cat) - if (items) reordered.push(...items) - } - for (const [cat, items] of grouped) { - if (!orderedCategories.includes(cat)) reordered.push(...items) - } - return { budgetItems: reordered } + return { budgetItems: orderBudgetByCategories(state.budgetItems, payload.orderedCategories as string[]) } } return {} },