mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-19 05:26:47 +00:00
fix(server): let a copied packing item keep its weight
createItem never accepted weight_grams or bag_id — not in its signature, not in the INSERT — so cloneItem had nothing to pass on and every copy arrived empty. For a group trip that meant re-entering every weight by hand for each traveller, which is what people were doing. The bag needs more care than the weight. Bags belong to a trip, but they can be assigned to a person, so inheriting the original bag would drop your copy into somebody else luggage and inflate their total. A copy keeps the bag only when nobody owns it or when the copier is one of its members.
This commit is contained in:
@@ -170,7 +170,7 @@ export class PackingService {
|
||||
|
||||
createItem(
|
||||
tripId: string | number,
|
||||
data: { name: string; category?: string; checked?: boolean; quantity?: number; is_private?: boolean; visibility?: PackingVisibility; recipient_ids?: number[] },
|
||||
data: { name: string; category?: string; checked?: boolean; quantity?: number; weight_grams?: number | null; bag_id?: number | null; is_private?: boolean; visibility?: PackingVisibility; recipient_ids?: number[] },
|
||||
ownerId?: number,
|
||||
) {
|
||||
const maxOrder = this.db.get<{ max: number | null }>('SELECT MAX(sort_order) as max FROM packing_items WHERE trip_id = ?', tripId)!;
|
||||
@@ -180,8 +180,8 @@ export class PackingService {
|
||||
|
||||
const itemId = this.db.transaction(() => {
|
||||
const result = this.db.run(
|
||||
'INSERT INTO packing_items (trip_id, name, checked, category, sort_order, quantity, is_private, owner_id, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)',
|
||||
tripId, data.name, data.checked ? 1 : 0, data.category || 'Other', sortOrder, qty, isPrivate, ownerId ?? null
|
||||
'INSERT INTO packing_items (trip_id, name, checked, category, sort_order, quantity, weight_grams, bag_id, is_private, owner_id, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP)',
|
||||
tripId, data.name, data.checked ? 1 : 0, data.category || 'Other', sortOrder, qty, data.weight_grams ?? null, data.bag_id ?? null, isPrivate, ownerId ?? null
|
||||
);
|
||||
const id = Number(result.lastInsertRowid);
|
||||
// "Shared with specific people" — record the recipients it covers.
|
||||
@@ -253,7 +253,7 @@ export class PackingService {
|
||||
|
||||
/** Loads an item scoped to its trip (the trip-access check happens in the controller). */
|
||||
private getItemInTrip(tripId: string | number, id: string | number) {
|
||||
return this.db.get<{ id: number; owner_id: number | null; is_private: number; name: string; category: string | null; quantity: number }>(
|
||||
return this.db.get<{ id: number; owner_id: number | null; is_private: number; name: string; category: string | null; quantity: number; weight_grams: number | null; bag_id: number | null }>(
|
||||
'SELECT * FROM packing_items WHERE id = ? AND trip_id = ?', id, tripId
|
||||
);
|
||||
}
|
||||
@@ -305,11 +305,37 @@ export class PackingService {
|
||||
return this.enrichItems([this.db.get('SELECT * FROM packing_items WHERE id = ?', id)])[0];
|
||||
}
|
||||
|
||||
/** Clone a (Common) item onto the caller's Personal list as a private starting point. */
|
||||
/**
|
||||
* A copy keeps the original's bag only when that bag is the caller's to pack: one nobody
|
||||
* owns, or one they belong to. Inheriting someone else's bag would drop the copy into
|
||||
* their luggage and inflate their weight (#207).
|
||||
*/
|
||||
private bagForCloner(tripId: string | number, bagId: number | null, userId: number): number | null {
|
||||
if (bagId == null) return null;
|
||||
const bag = this.db.get<{ user_id: number | null }>('SELECT user_id FROM packing_bags WHERE id = ? AND trip_id = ?', bagId, tripId);
|
||||
if (!bag) return null;
|
||||
if (bag.user_id === userId) return bagId;
|
||||
const members = this.db.all<{ user_id: number }>('SELECT user_id FROM packing_bag_members WHERE bag_id = ?', bagId);
|
||||
if (bag.user_id == null && members.length === 0) return bagId; // shared bag, nobody's in particular
|
||||
return members.some(m => m.user_id === userId) ? bagId : null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Clone a (Common) item onto the caller's Personal list as a private starting point.
|
||||
* Weight comes along — it is a property of the thing, and re-entering it by hand for
|
||||
* every traveller was the whole complaint in #207.
|
||||
*/
|
||||
cloneItem(tripId: string | number, id: string | number, userId: number) {
|
||||
const item = this.getItemInTrip(tripId, id);
|
||||
if (!item) return null;
|
||||
return this.createItem(tripId, { name: item.name, category: item.category || undefined, quantity: item.quantity, visibility: 'personal' }, userId);
|
||||
return this.createItem(tripId, {
|
||||
name: item.name,
|
||||
category: item.category || undefined,
|
||||
quantity: item.quantity,
|
||||
weight_grams: item.weight_grams,
|
||||
bag_id: this.bagForCloner(tripId, item.bag_id, userId),
|
||||
visibility: 'personal',
|
||||
}, userId);
|
||||
}
|
||||
|
||||
deleteItem(tripId: string | number, id: string | number) {
|
||||
|
||||
@@ -60,7 +60,7 @@ vi.mock('../../../src/nest/notifications/notifications.bridge', () => ({ send })
|
||||
import { createTables } from '../../../src/db/schema';
|
||||
import { runMigrations } from '../../../src/db/migrations';
|
||||
import { resetTestDb } from '../../helpers/test-db';
|
||||
import { createUser, createAdmin, createTrip } from '../../helpers/factories';
|
||||
import { createUser, createAdmin, createTrip, addTripMember } from '../../helpers/factories';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
import type { PermissionsService } from '../../../src/nest/permissions/permissions.service';
|
||||
import { PackingService } from '../../../src/nest/packing/packing.service';
|
||||
@@ -584,6 +584,61 @@ describe('three-tier packing sharing (#858)', () => {
|
||||
expect(names(svc.listItems(trip.id, owner.id) as any[])).toEqual(['Travel adapter']); // owner sees only the common one
|
||||
expect(names(svc.listItems(trip.id, cloner.id) as any[])).toEqual(['Travel adapter', 'Travel adapter']); // common + own clone
|
||||
});
|
||||
|
||||
// #207: "one person curates the list, everyone copies it" meant re-entering every
|
||||
// weight by hand, because a copy arrived empty.
|
||||
it('PACK-SVC-073: cloneItem carries the weight over', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: cloner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const common = svc.createItem(trip.id, { name: 'Tent', visibility: 'common', weight_grams: 2400, quantity: 2 }, owner.id) as any;
|
||||
|
||||
const clone = svc.cloneItem(trip.id, common.id, cloner.id) as any;
|
||||
|
||||
expect(clone.weight_grams).toBe(2400);
|
||||
expect(clone.quantity).toBe(2);
|
||||
});
|
||||
|
||||
it('PACK-SVC-074: cloneItem keeps a bag nobody owns', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: cloner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
const bag = svc.createBag(trip.id, { name: 'Car boot' }) as any;
|
||||
const common = svc.createItem(trip.id, { name: 'Cool box', visibility: 'common', weight_grams: 3000, bag_id: bag.id }, owner.id) as any;
|
||||
|
||||
const clone = svc.cloneItem(trip.id, common.id, cloner.id) as any;
|
||||
|
||||
expect(clone.bag_id).toBe(bag.id);
|
||||
});
|
||||
|
||||
it('PACK-SVC-075: cloneItem drops a bag that belongs to someone else', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: cloner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, cloner.id);
|
||||
const bag = svc.createBag(trip.id, { name: 'Owner backpack' }) as any;
|
||||
svc.setBagMembers(trip.id, bag.id, [owner.id]);
|
||||
const common = svc.createItem(trip.id, { name: 'Rope', visibility: 'common', weight_grams: 900, bag_id: bag.id }, owner.id) as any;
|
||||
|
||||
const clone = svc.cloneItem(trip.id, common.id, cloner.id) as any;
|
||||
|
||||
// Weight still comes along — only the foreign bag is dropped, so the copy cannot
|
||||
// land in someone else's luggage and inflate their total.
|
||||
expect(clone.weight_grams).toBe(900);
|
||||
expect(clone.bag_id).toBeNull();
|
||||
});
|
||||
|
||||
it('PACK-SVC-076: cloneItem keeps a bag the cloner is a member of', () => {
|
||||
const { user: owner } = createUser(testDb);
|
||||
const { user: cloner } = createUser(testDb);
|
||||
const trip = createTrip(testDb, owner.id);
|
||||
addTripMember(testDb, trip.id, cloner.id);
|
||||
const bag = svc.createBag(trip.id, { name: 'Shared duffel' }) as any;
|
||||
svc.setBagMembers(trip.id, bag.id, [owner.id, cloner.id]);
|
||||
const common = svc.createItem(trip.id, { name: 'Stove', visibility: 'common', bag_id: bag.id }, owner.id) as any;
|
||||
|
||||
expect((svc.cloneItem(trip.id, common.id, cloner.id) as any).bag_id).toBe(bag.id);
|
||||
});
|
||||
});
|
||||
|
||||
// ── Post-migration fixes over the legacy quirks ───────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user