From 5e686eac06a362737c2c148d3a26ece1032ac115 Mon Sep 17 00:00:00 2001 From: Konstantinos Thermos Date: Fri, 17 Jul 2026 18:47:58 +0200 Subject: [PATCH] fix(packing): apply templates into the active list tab (#1581) --- client/src/api/client.ts | 4 +- .../Packing/ApplyTemplateButton.tsx | 5 +- .../Packing/PackingListPanel.test.tsx | 42 +++++++++++++ .../components/Packing/usePackingListPanel.ts | 11 +++- client/src/pages/TripPlannerPage.tsx | 4 +- server/src/nest/packing/packing.controller.ts | 6 +- server/src/nest/packing/packing.service.ts | 4 +- server/src/services/packingService.ts | 13 +++- .../unit/nest/packing.controller.test.ts | 35 ++++++++--- .../tests/unit/nest/packing.service.test.ts | 2 +- .../unit/services/packingService.test.ts | 59 +++++++++++++++++++ shared/src/packing/packing.schema.ts | 5 ++ 12 files changed, 167 insertions(+), 23 deletions(-) diff --git a/client/src/api/client.ts b/client/src/api/client.ts index 8b3d30a36..d2f716143 100644 --- a/client/src/api/client.ts +++ b/client/src/api/client.ts @@ -31,7 +31,7 @@ import { type PlaceBulkUpdateRequest, type DayNoteCreateRequest, type DayNoteUpdateRequest, type PackingImportRequest, type PackingBagMembersRequest, type PackingUpdateBagRequest, - type PackingCategoryAssigneesRequest, + type PackingCategoryAssigneesRequest, type PackingApplyTemplateRequest, type BudgetUpdateMembersRequest, type BudgetToggleMemberPaidRequest, type BudgetReorderCategoriesRequest, type TodoCategoryAssigneesRequest, type CollabNoteCreateRequest, type CollabNoteUpdateRequest, type CollabPollCreateRequest, @@ -449,7 +449,7 @@ export const packingApi = { getCategoryAssignees: (tripId: number | string) => apiClient.get(`/trips/${tripId}/packing/category-assignees`).then(r => r.data), setCategoryAssignees: (tripId: number | string, categoryName: string, userIds: number[]) => apiClient.put(`/trips/${tripId}/packing/category-assignees/${encodeURIComponent(categoryName)}`, { user_ids: userIds } satisfies PackingCategoryAssigneesRequest).then(r => r.data), listTemplates: (tripId: number | string) => apiClient.get(`/trips/${tripId}/packing/templates`).then(r => r.data), - applyTemplate: (tripId: number | string, templateId: number) => apiClient.post(`/trips/${tripId}/packing/apply-template/${templateId}`).then(r => r.data), + applyTemplate: (tripId: number | string, templateId: number, visibility: 'common' | 'personal' = 'common') => apiClient.post(`/trips/${tripId}/packing/apply-template/${templateId}`, { visibility } satisfies PackingApplyTemplateRequest).then(r => r.data), saveAsTemplate: (tripId: number | string, name: string) => apiClient.post(`/trips/${tripId}/packing/save-as-template`, { name }).then(r => r.data), setBagMembers: (tripId: number | string, bagId: number, userIds: number[]) => apiClient.put(`/trips/${tripId}/packing/bags/${bagId}/members`, { user_ids: userIds } satisfies PackingBagMembersRequest).then(r => r.data), listBags: (tripId: number | string) => apiClient.get(`/trips/${tripId}/packing/bags`).then(r => r.data), diff --git a/client/src/components/Packing/ApplyTemplateButton.tsx b/client/src/components/Packing/ApplyTemplateButton.tsx index cd2ea129e..7fa920f98 100644 --- a/client/src/components/Packing/ApplyTemplateButton.tsx +++ b/client/src/components/Packing/ApplyTemplateButton.tsx @@ -13,13 +13,14 @@ interface Template { interface ApplyTemplateButtonProps { tripId: number + visibility: 'common' | 'personal' style: React.CSSProperties className?: string } // Dropdown-Button um ein Packing-Template auf den aktuellen Trip anzuwenden. // Rendert nichts wenn keine Templates existieren. -export default function ApplyTemplateButton({ tripId, style, className }: ApplyTemplateButtonProps): React.ReactElement | null { +export default function ApplyTemplateButton({ tripId, visibility, style, className }: ApplyTemplateButtonProps): React.ReactElement | null { const [templates, setTemplates] = useState([]) const [open, setOpen] = useState(false) const [applying, setApplying] = useState(false) @@ -43,7 +44,7 @@ export default function ApplyTemplateButton({ tripId, style, className }: ApplyT const handleApply = async (templateId: number) => { setApplying(true) try { - const data = await packingApi.applyTemplate(tripId, templateId) + const data = await packingApi.applyTemplate(tripId, templateId, visibility) useTripStore.setState(s => ({ packingItems: [...s.packingItems, ...(data.items || [])] })) toast.success(t('packing.templateApplied', { count: data.count })) setOpen(false) diff --git a/client/src/components/Packing/PackingListPanel.test.tsx b/client/src/components/Packing/PackingListPanel.test.tsx index 881a2ca85..bd15afb4f 100644 --- a/client/src/components/Packing/PackingListPanel.test.tsx +++ b/client/src/components/Packing/PackingListPanel.test.tsx @@ -1159,6 +1159,48 @@ describe('PackingListPanel', () => { await waitFor(() => expect(applyCalled).toBe(true)); }); + // #1565: the template used to always land in the shared pool, whichever tab was open. + it('FE-COMP-PACKING-061a: applying a template from "My List" sends the personal view', async () => { + const user = userEvent.setup(); + let applyBody: Record | null = null; + server.use( + http.get('/api/trips/:id/packing/templates', () => + HttpResponse.json({ templates: [{ id: 5, name: 'Beach Trip', item_count: 12 }] }) + ), + http.post('/api/trips/1/packing/apply-template/5', async ({ request }) => { + applyBody = (await request.json()) as Record; + return HttpResponse.json({ items: [], count: 12 }); + }) + ); + render(); + + await user.click(await screen.findByText('My list')); + await user.click(await screen.findByText('Apply template')); + await user.click(await screen.findByText('Beach Trip')); + + await waitFor(() => expect(applyBody).toEqual({ visibility: 'personal' })); + }); + + it('FE-COMP-PACKING-061b: applying a template from the shared tab sends the common view', async () => { + const user = userEvent.setup(); + let applyBody: Record | null = null; + server.use( + http.get('/api/trips/:id/packing/templates', () => + HttpResponse.json({ templates: [{ id: 5, name: 'Beach Trip', item_count: 12 }] }) + ), + http.post('/api/trips/1/packing/apply-template/5', async ({ request }) => { + applyBody = (await request.json()) as Record; + return HttpResponse.json({ items: [], count: 12 }); + }) + ); + render(); + + await user.click(await screen.findByText('Apply template')); + await user.click(await screen.findByText('Beach Trip')); + + await waitFor(() => expect(applyBody).toEqual({ visibility: 'common' })); + }); + it('FE-COMP-PACKING-062: handleBulkImport calls import API and closes modal', async () => { const user = userEvent.setup(); let importBody: Record | null = null; diff --git a/client/src/components/Packing/usePackingListPanel.ts b/client/src/components/Packing/usePackingListPanel.ts index b4e04c24e..f4f388954 100644 --- a/client/src/components/Packing/usePackingListPanel.ts +++ b/client/src/components/Packing/usePackingListPanel.ts @@ -33,6 +33,9 @@ export interface PackingListPanelProps { clearCheckedSignal?: number saveTemplateSignal?: number inlineHeader?: boolean + // Lifted so an out-of-panel Apply Template button knows the active view (#1565). + view?: 'common' | 'personal' + onViewChange?: (view: 'common' | 'personal') => void } /** @@ -42,11 +45,13 @@ export interface PackingListPanelProps { * sections below render header, filters, the grouped list, the bag sidebar/ * modal and the import dialog. */ -export function usePackingList({ tripId, items, openImportSignal = 0, clearCheckedSignal = 0, saveTemplateSignal = 0, inlineHeader = true }: PackingListPanelProps) { +export function usePackingList({ tripId, items, openImportSignal = 0, clearCheckedSignal = 0, saveTemplateSignal = 0, inlineHeader = true, view: viewProp, onViewChange }: PackingListPanelProps) { const [filter, setFilter] = useState('alle') // 'alle' | 'offen' | 'erledigt' // Three-tier sharing (#858): 'common' = the group pool (where existing items // live — non-breaking), 'personal' = my own list (private + shared-to-me). - const [view, setView] = useState<'common' | 'personal'>('common') + const [ownView, setOwnView] = useState<'common' | 'personal'>('common') + const view = viewProp ?? ownView + const setView = onViewChange ?? setOwnView const [addingCategory, setAddingCategory] = useState(false) const [newCatName, setNewCatName] = useState('') const { addPackingItem, updatePackingItem, deletePackingItem, togglePackingItem, reorderPackingItems, @@ -307,7 +312,7 @@ export function usePackingList({ tripId, items, openImportSignal = 0, clearCheck const handleApplyTemplate = async (templateId: number) => { setApplyingTemplate(true) try { - const data = await packingApi.applyTemplate(tripId, templateId) + const data = await packingApi.applyTemplate(tripId, templateId, view) useTripStore.setState(s => ({ packingItems: [...s.packingItems, ...(data.items || [])] })) toast.success(t('packing.templateApplied', { count: data.count })) setShowTemplateDropdown(false) diff --git a/client/src/pages/TripPlannerPage.tsx b/client/src/pages/TripPlannerPage.tsx index 25564cfdb..ad56bcf74 100644 --- a/client/src/pages/TripPlannerPage.tsx +++ b/client/src/pages/TripPlannerPage.tsx @@ -60,6 +60,7 @@ function ListsContainer({ tripId, packingItems, todoItems }: { tripId: number; p const [clearCheckedSignal, setClearCheckedSignal] = useState(0) const [saveTemplateSignal, setSaveTemplateSignal] = useState(0) const [addTodoSignal, setAddTodoSignal] = useState(0) + const [packingView, setPackingView] = useState<'common' | 'personal'>('common') const { t } = useTranslation() const isAdmin = useAuthStore(s => s.user?.role === 'admin') @@ -127,6 +128,7 @@ function ListsContainer({ tripId, packingItems, todoItems }: { tripId: number; p )} @@ -167,7 +169,7 @@ function ListsContainer({ tripId, packingItems, todoItems }: { tripId: number; p
- {subTab === 'packing' && } + {subTab === 'packing' && } {subTab === 'todo' && }
diff --git a/server/src/nest/packing/packing.controller.ts b/server/src/nest/packing/packing.controller.ts index 6905319a8..2e16ba2f1 100644 --- a/server/src/nest/packing/packing.controller.ts +++ b/server/src/nest/packing/packing.controller.ts @@ -348,15 +348,17 @@ export class PackingController { @CurrentUser() user: User, @Param('tripId') tripId: string, @Param('templateId') templateId: string, + @Body() body: { visibility?: 'common' | 'personal' }, @Headers('x-socket-id') socketId?: string, ) { const trip = this.requireTrip(tripId, user); this.requireEdit(trip, user); - const added = this.packing.applyTemplate(tripId, templateId); + const visibility = body?.visibility === 'personal' ? 'personal' : 'common'; + const added = this.packing.applyTemplate(tripId, templateId, visibility, user.id); if (!added) { throw new HttpException({ error: 'Template not found or empty' }, 404); } - this.packing.broadcast(tripId, 'packing:template-applied', { items: added }, socketId); + this.packing.broadcastItem(tripId, 'packing:template-applied', { items: added }, added[0], socketId); return { items: added, count: added.length }; } diff --git a/server/src/nest/packing/packing.service.ts b/server/src/nest/packing/packing.service.ts index f54f5bd00..580acda43 100644 --- a/server/src/nest/packing/packing.service.ts +++ b/server/src/nest/packing/packing.service.ts @@ -126,8 +126,8 @@ export class PackingService { return svc.listTemplates(); } - applyTemplate(tripId: string, templateId: string) { - return svc.applyTemplate(tripId, templateId); + applyTemplate(tripId: string, templateId: string, visibility: 'common' | 'personal', ownerId: number) { + return svc.applyTemplate(tripId, templateId, visibility, ownerId); } saveAsTemplate(tripId: string, userId: number, name: string) { diff --git a/server/src/services/packingService.ts b/server/src/services/packingService.ts index 09f51e0c4..29a8def6c 100644 --- a/server/src/services/packingService.ts +++ b/server/src/services/packingService.ts @@ -390,7 +390,12 @@ export function listTemplates() { // ── Apply Template ───────────────────────────────────────────────────────── -export function applyTemplate(tripId: string | number, templateId: string | number) { +export function applyTemplate( + tripId: string | number, + templateId: string | number, + visibility: 'common' | 'personal' = 'common', + ownerId?: number, +) { const templateItems = db.prepare(` SELECT ti.name, tc.name as category FROM packing_template_items ti @@ -403,11 +408,13 @@ export function applyTemplate(tripId: string | number, templateId: string | numb const maxOrder = db.prepare('SELECT MAX(sort_order) as max FROM packing_items WHERE trip_id = ?').get(tripId) as { max: number | null }; let sortOrder = (maxOrder.max !== null ? maxOrder.max : -1) + 1; + const isPrivate = ownerId != null ? visibilityToPrivate(visibility) : 0; + const owner = isPrivate ? ownerId! : null; - const insert = db.prepare('INSERT INTO packing_items (trip_id, name, checked, category, sort_order, updated_at) VALUES (?, ?, 0, ?, ?, CURRENT_TIMESTAMP)'); + const insert = db.prepare('INSERT INTO packing_items (trip_id, name, checked, category, sort_order, is_private, owner_id, updated_at) VALUES (?, ?, 0, ?, ?, ?, ?, CURRENT_TIMESTAMP)'); const added: any[] = []; for (const ti of templateItems) { - const result = insert.run(tripId, ti.name, ti.category, sortOrder++); + const result = insert.run(tripId, ti.name, ti.category, sortOrder++, isPrivate, owner); const item = db.prepare('SELECT * FROM packing_items WHERE id = ?').get(result.lastInsertRowid); added.push(item); } diff --git a/server/tests/unit/nest/packing.controller.test.ts b/server/tests/unit/nest/packing.controller.test.ts index 869ee1725..6877c3486 100644 --- a/server/tests/unit/nest/packing.controller.test.ts +++ b/server/tests/unit/nest/packing.controller.test.ts @@ -383,18 +383,39 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r it('404 when applying a missing/empty template (POST stays 200 otherwise)', () => { const svc = makeService({ applyTemplate: vi.fn().mockReturnValue(null) } as Partial); - expect(thrown(() => new PackingController(svc).applyTemplate(user, '5', 't1'))).toEqual({ + expect(thrown(() => new PackingController(svc).applyTemplate(user, '5', 't1', {}))).toEqual({ status: 404, body: { error: 'Template not found or empty' }, }); }); it('applies a template, broadcasts the added items and reports the count', () => { - const applyTemplate = vi.fn().mockReturnValue([{ id: 1 }, { id: 2 }, { id: 3 }]); - const broadcast = vi.fn(); - const svc = makeService({ applyTemplate, broadcast } as Partial); - const res = new PackingController(svc).applyTemplate(user, '5', 't1', 'sock'); - expect(res).toEqual({ items: [{ id: 1 }, { id: 2 }, { id: 3 }], count: 3 }); - expect(broadcast).toHaveBeenCalledWith('5', 'packing:template-applied', { items: [{ id: 1 }, { id: 2 }, { id: 3 }] }, 'sock'); + const items = [{ id: 1 }, { id: 2 }, { id: 3 }]; + const applyTemplate = vi.fn().mockReturnValue(items); + const broadcastItem = vi.fn(); + const svc = makeService({ applyTemplate, broadcastItem } as Partial); + const res = new PackingController(svc).applyTemplate(user, '5', 't1', {}, 'sock'); + expect(res).toEqual({ items, count: 3 }); + expect(applyTemplate).toHaveBeenCalledWith('5', 't1', 'common', user.id); + expect(broadcastItem).toHaveBeenCalledWith('5', 'packing:template-applied', { items }, items[0], 'sock'); + }); + + // #1565: the template must follow the tab the user is on, and a personal apply + // must not be broadcast to the rest of the trip. + it('applies a template into the personal list and keeps the broadcast to its owner', () => { + const items = [{ id: 1, is_private: 1, owner_id: user.id }]; + const applyTemplate = vi.fn().mockReturnValue(items); + const broadcastItem = vi.fn(); + const svc = makeService({ applyTemplate, broadcastItem } as Partial); + new PackingController(svc).applyTemplate(user, '5', 't1', { visibility: 'personal' }, 'sock'); + expect(applyTemplate).toHaveBeenCalledWith('5', 't1', 'personal', user.id); + expect(broadcastItem).toHaveBeenCalledWith('5', 'packing:template-applied', { items }, items[0], 'sock'); + }); + + it('falls back to the common pool for an unknown visibility', () => { + const applyTemplate = vi.fn().mockReturnValue([{ id: 1 }]); + const svc = makeService({ applyTemplate } as Partial); + new PackingController(svc).applyTemplate(user, '5', 't1', { visibility: 'bogus' } as never); + expect(applyTemplate).toHaveBeenCalledWith('5', 't1', 'common', user.id); }); it('400 when an admin saves a template with no name (whitespace)', () => { diff --git a/server/tests/unit/nest/packing.service.test.ts b/server/tests/unit/nest/packing.service.test.ts index bf50e6589..081667ccd 100644 --- a/server/tests/unit/nest/packing.service.test.ts +++ b/server/tests/unit/nest/packing.service.test.ts @@ -83,7 +83,7 @@ describe('PackingService (wrapper delegation + helpers)', () => { s.deleteBag('5', '2'); expect(pk.deleteBag).toHaveBeenCalledWith('5', '2'); s.setBagMembers('5', '2', [1, 2]); expect(pk.setBagMembers).toHaveBeenCalledWith('5', '2', [1, 2]); s.listTemplates(); expect(pk.listTemplates).toHaveBeenCalled(); - s.applyTemplate('5', 't1'); expect(pk.applyTemplate).toHaveBeenCalledWith('5', 't1'); + s.applyTemplate('5', 't1', 'personal', 1); expect(pk.applyTemplate).toHaveBeenCalledWith('5', 't1', 'personal', 1); s.saveAsTemplate('5', 1, 'Tpl'); expect(pk.saveAsTemplate).toHaveBeenCalledWith('5', 1, 'Tpl'); s.getCategoryAssignees('5'); expect(pk.getCategoryAssignees).toHaveBeenCalledWith('5'); s.updateCategoryAssignees('5', 'Clothes', [2]); expect(pk.updateCategoryAssignees).toHaveBeenCalledWith('5', 'Clothes', [2]); diff --git a/server/tests/unit/services/packingService.test.ts b/server/tests/unit/services/packingService.test.ts index 0db2184ed..160326023 100644 --- a/server/tests/unit/services/packingService.test.ts +++ b/server/tests/unit/services/packingService.test.ts @@ -125,6 +125,16 @@ describe('listTemplates', () => { // ── applyTemplate ───────────────────────────────────────────────────────────── +/** A one-category template with the given item names. Returns its id. */ +function seedTemplate(userId: number, itemNames: string[]): number { + const templateId = testDb.prepare('INSERT INTO packing_templates (name, created_by) VALUES (?, ?)').run('Camping', userId).lastInsertRowid as number; + const catId = testDb.prepare('INSERT INTO packing_template_categories (template_id, name, sort_order) VALUES (?, ?, ?)').run(templateId, 'Gear', 0).lastInsertRowid as number; + itemNames.forEach((name, i) => { + testDb.prepare('INSERT INTO packing_template_items (category_id, name, sort_order) VALUES (?, ?, ?)').run(catId, name, i); + }); + return templateId; +} + describe('applyTemplate', () => { it('PACK-SVC-003: adds template items to a trip packing list', () => { const { user } = createUser(testDb); @@ -163,6 +173,55 @@ describe('applyTemplate', () => { expect(result).toBeNull(); }); + + // #1565: the applied items must land in the view the user is on, not always Common. + it('PACK-SVC-046: applies into the personal list when visibility is personal', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const templateId = seedTemplate(user.id, ['Tent']); + + const result = applyTemplate(trip.id, templateId, 'personal', user.id) as any[]; + + expect(result[0].is_private).toBe(1); + expect(result[0].owner_id).toBe(user.id); + expect(listItems(trip.id, user.id).filter((i: any) => i.is_private)).toHaveLength(1); + }); + + it('PACK-SVC-047: a personally applied template stays hidden from other members', () => { + const { user } = createUser(testDb); + const { user: other } = createUser(testDb, { username: 'other' }); + const trip = createTrip(testDb, user.id); + testDb.prepare('INSERT INTO trip_members (trip_id, user_id) VALUES (?, ?)').run(trip.id, other.id); + const templateId = seedTemplate(user.id, ['Tent']); + + applyTemplate(trip.id, templateId, 'personal', user.id); + + expect(listItems(trip.id, other.id)).toHaveLength(0); + }); + + it('PACK-SVC-048: applies into the common pool by default, leaving items unowned', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const templateId = seedTemplate(user.id, ['Tent']); + + const result = applyTemplate(trip.id, templateId, 'common', user.id) as any[]; + + expect(result[0].is_private).toBe(0); + // Unowned, so any member may still re-share it (setItemSharing claims a null owner). + expect(result[0].owner_id).toBeNull(); + }); + + it('PACK-SVC-049: falls back to common when no owner is given, so items stay visible', () => { + const { user } = createUser(testDb); + const trip = createTrip(testDb, user.id); + const templateId = seedTemplate(user.id, ['Tent']); + + // A private item with no owner would be invisible to everyone. + const result = applyTemplate(trip.id, templateId, 'personal') as any[]; + + expect(result[0].is_private).toBe(0); + expect(listItems(trip.id, user.id)).toHaveLength(1); + }); }); // ── createBag / deleteBag ───────────────────────────────────────────────────── diff --git a/shared/src/packing/packing.schema.ts b/shared/src/packing/packing.schema.ts index 7b58260fa..25656fbf4 100644 --- a/shared/src/packing/packing.schema.ts +++ b/shared/src/packing/packing.schema.ts @@ -147,6 +147,11 @@ export const packingSaveTemplateRequestSchema = z.object({ }); export type PackingSaveTemplateRequest = z.infer; +export const packingApplyTemplateRequestSchema = z.object({ + visibility: z.enum(['common', 'personal']).optional(), +}); +export type PackingApplyTemplateRequest = z.infer; + export const packingTemplateSummarySchema = z.object({ id: z.number(), name: z.string(),