mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(packing): a bag weighs what everyone put in it, not just what you can see
A bag's weight was summed in the browser over the item list the viewer is allowed to see. Personal items belonging to other members are filtered out of that list by design (#858), so their weight vanished with them: two people looking at the same shared bag saw two different totals, and neither saw the real one. That number is then measured against `weight_limit_grams`, an absolute airline limit. A per-viewer subtotal there is not merely incomplete but wrong — a shared bag could sit over its limit and warn nobody. The sum now happens in SQL over every row and ships as `total_weight_grams` on the bag, with `unassigned_weight_grams` alongside so the "no bag" pile and the grand total follow the same rule; a screen mixing true totals with per-viewer ones would be worse than either. Only integers cross the wire: a member learns that a bag is heavier than the items they can see, never a name, category, quantity or owner. That disclosure is deliberate and bounded, and it is what the issue asks for. The item lists stay viewer-scoped, so #1767 ("someone shared it with you, but they are the one bringing it") is unchanged — it just no longer decides what a bag weighs. Totals also had to stay live, because the reported repro is two people looking at the same screen. A private item's event reaches only its owner, which is the very rule that made the totals wrong, so item writes now also ping the room with a content-free `packing:bag-totals` and everyone re-reads the numbers. All three surfaces — the bag sidebar, the bag modal and the mobile sheet — read the server figure and fall back to the local sum only for a bag cached offline before this change. Closes #2191 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S5VwmiBQ9sPCJwfmpv64r3
This commit is contained in:
Claude
@@ -1,18 +1,22 @@
|
||||
import { X, Plus } from 'lucide-react'
|
||||
import type { PackingState } from './usePackingListPanel'
|
||||
import { bagFillPct, countsTowardsMyLoad, itemWeight } from './packingListPanel.helpers'
|
||||
import { bagFillPct, bagTotalWeight, countsTowardsMyLoad, unassignedTotalWeight } from './packingListPanel.helpers'
|
||||
import { BagCard } from './PackingListPanelBagCard'
|
||||
|
||||
export function BagModal(S: PackingState) {
|
||||
const {
|
||||
setShowBagModal, t, bags, items, tripId, tripMembers, canEdit, currentUserId, handleDeleteBag, handleUpdateBag, handleSetBagMembers,
|
||||
showAddBag, setShowAddBag, newBagName, setNewBagName, handleCreateBag,
|
||||
showAddBag, setShowAddBag, newBagName, setNewBagName, handleCreateBag, unassignedWeightGrams,
|
||||
} = S
|
||||
// These numbers describe what you are carrying. An item someone shared with you stays
|
||||
// in your list, but they are the one bringing it, so it is not your weight (#1767).
|
||||
// The ITEM LISTS still describe what you are carrying — an item someone shared
|
||||
// with you stays in your list, but they are the one bringing it (#1767).
|
||||
const myItems = items.filter(i => countsTowardsMyLoad(i, currentUserId))
|
||||
// The WEIGHTS no longer do. A bag's load is the bag's, whoever packed it and
|
||||
// whether or not you may see the items, so it comes from the server (#2191).
|
||||
const bagWeightOf = (bag: typeof bags[number]) =>
|
||||
bagTotalWeight(bag, myItems.filter(i => i.bag_id === bag.id))
|
||||
// Reference for bags without a limit of their own — computed once instead of per bag.
|
||||
const heaviestBagWeight = Math.max(...bags.map(b => myItems.filter(i => i.bag_id === b.id).reduce((s, i) => s + itemWeight(i), 0)), 1)
|
||||
const heaviestBagWeight = Math.max(...bags.map(bagWeightOf), 1)
|
||||
return (
|
||||
<div style={{ position: 'fixed', inset: 0, zIndex: 100, background: 'rgba(0,0,0,0.3)', display: 'flex', alignItems: 'flex-start', justifyContent: 'center', padding: 20, paddingTop: 140, paddingBottom: 'calc(20px + var(--bottom-nav-h))', overflowY: 'auto' }}
|
||||
role="button" tabIndex={0} aria-label={t('common.close')}
|
||||
@@ -30,7 +34,7 @@ export function BagModal(S: PackingState) {
|
||||
|
||||
{bags.map(bag => {
|
||||
const bagItems = myItems.filter(i => i.bag_id === bag.id)
|
||||
const totalWeight = bagItems.reduce((sum, i) => sum + itemWeight(i), 0)
|
||||
const totalWeight = bagWeightOf(bag)
|
||||
const pct = bagFillPct(totalWeight, bag.weight_limit_grams, heaviestBagWeight)
|
||||
return (
|
||||
<BagCard key={bag.id} bag={bag} bagItems={bagItems} totalWeight={totalWeight} pct={pct} tripId={tripId} tripMembers={tripMembers} canEdit={canEdit} onDelete={() => handleDeleteBag(bag.id)} onUpdate={handleUpdateBag} onSetMembers={handleSetBagMembers} t={t} />
|
||||
@@ -40,7 +44,7 @@ export function BagModal(S: PackingState) {
|
||||
{/* Unassigned */}
|
||||
{(() => {
|
||||
const unassigned = myItems.filter(i => !i.bag_id)
|
||||
const unassignedWeight = unassigned.reduce((s, i) => s + itemWeight(i), 0)
|
||||
const unassignedWeight = unassignedTotalWeight(unassignedWeightGrams, unassigned)
|
||||
if (unassigned.length === 0) return null
|
||||
return (
|
||||
<div style={{ marginBottom: 16, opacity: 0.6 }}>
|
||||
@@ -60,7 +64,13 @@ export function BagModal(S: PackingState) {
|
||||
<div style={{ borderTop: '1px solid var(--border-secondary)', paddingTop: 12, marginTop: 8 }}>
|
||||
<div style={{ display: 'flex', justifyContent: 'space-between', fontSize: 'calc(14px * var(--fs-scale-body, 1))', fontWeight: 700, color: 'var(--text-primary)' }}>
|
||||
<span>{t('packing.totalWeight')}</span>
|
||||
<span>{(() => { const w = myItems.reduce((s, i) => s + itemWeight(i), 0); return w >= 1000 ? `${(w / 1000).toFixed(1)} kg` : `${w} g` })()}</span>
|
||||
<span>{(() => {
|
||||
// Same rule as the rows above it: a grand total mixing true bag
|
||||
// weights with a per-viewer remainder would be worse than either.
|
||||
const w = bags.reduce((s, b) => s + bagWeightOf(b), 0)
|
||||
+ unassignedTotalWeight(unassignedWeightGrams, myItems.filter(i => !i.bag_id))
|
||||
return w >= 1000 ? `${(w / 1000).toFixed(1)} kg` : `${w} g`
|
||||
})()}</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -1,18 +1,21 @@
|
||||
import { Plus } from 'lucide-react'
|
||||
import type { PackingState } from './usePackingListPanel'
|
||||
import { bagFillPct, countsTowardsMyLoad, itemWeight } from './packingListPanel.helpers'
|
||||
import { bagFillPct, bagTotalWeight, countsTowardsMyLoad, unassignedTotalWeight } from './packingListPanel.helpers'
|
||||
import { BagCard } from './PackingListPanelBagCard'
|
||||
|
||||
export function BagSidebar(S: PackingState) {
|
||||
const {
|
||||
t, bags, items, tripId, tripMembers, canEdit, currentUserId, handleDeleteBag, handleUpdateBag, handleSetBagMembers,
|
||||
showAddBag, setShowAddBag, newBagName, setNewBagName, handleCreateBag,
|
||||
showAddBag, setShowAddBag, newBagName, setNewBagName, handleCreateBag, unassignedWeightGrams,
|
||||
} = S
|
||||
// These numbers describe what you are carrying. An item someone shared with you stays
|
||||
// in your list, but they are the one bringing it, so it is not your weight (#1767).
|
||||
// The ITEM LISTS still describe what you are carrying — an item someone shared
|
||||
// with you stays in your list, but they are the one bringing it (#1767).
|
||||
const myItems = items.filter(i => countsTowardsMyLoad(i, currentUserId))
|
||||
// The WEIGHTS no longer do: a bag's load is the bag's, whoever packed it (#2191).
|
||||
const bagWeightOf = (bag: typeof bags[number]) =>
|
||||
bagTotalWeight(bag, myItems.filter(i => i.bag_id === bag.id))
|
||||
// Reference for bags without a limit of their own — computed once instead of per bag.
|
||||
const heaviestBagWeight = Math.max(...bags.map(b => myItems.filter(i => i.bag_id === b.id).reduce((s, i) => s + itemWeight(i), 0)), 1)
|
||||
const heaviestBagWeight = Math.max(...bags.map(bagWeightOf), 1)
|
||||
return (
|
||||
<div className="hidden xl:block" style={{ width: 260, marginLeft: 16, borderLeft: '1px solid var(--border-secondary)', overflowY: 'auto', padding: 16, flexShrink: 0 }}>
|
||||
<div style={{ fontSize: 'calc(11px * var(--fs-scale-caption, 1))', fontWeight: 700, textTransform: 'uppercase', letterSpacing: '0.05em', color: 'var(--text-faint)', marginBottom: 12 }}>
|
||||
@@ -21,7 +24,7 @@ export function BagSidebar(S: PackingState) {
|
||||
|
||||
{bags.map(bag => {
|
||||
const bagItems = myItems.filter(i => i.bag_id === bag.id)
|
||||
const totalWeight = bagItems.reduce((sum, i) => sum + itemWeight(i), 0)
|
||||
const totalWeight = bagWeightOf(bag)
|
||||
const pct = bagFillPct(totalWeight, bag.weight_limit_grams, heaviestBagWeight)
|
||||
return (
|
||||
<BagCard key={bag.id} bag={bag} bagItems={bagItems} totalWeight={totalWeight} pct={pct} tripId={tripId} tripMembers={tripMembers} canEdit={canEdit} onDelete={() => handleDeleteBag(bag.id)} onUpdate={handleUpdateBag} onSetMembers={handleSetBagMembers} t={t} compact />
|
||||
@@ -31,7 +34,7 @@ export function BagSidebar(S: PackingState) {
|
||||
{/* Unassigned */}
|
||||
{(() => {
|
||||
const unassigned = myItems.filter(i => !i.bag_id)
|
||||
const unassignedWeight = unassigned.reduce((s, i) => s + itemWeight(i), 0)
|
||||
const unassignedWeight = unassignedTotalWeight(unassignedWeightGrams, unassigned)
|
||||
if (unassigned.length === 0) return null
|
||||
return (
|
||||
<div style={{ marginBottom: 14, opacity: 0.6 }}>
|
||||
@@ -51,7 +54,12 @@ export function BagSidebar(S: PackingState) {
|
||||
<div style={{ borderTop: '1px solid var(--border-secondary)', paddingTop: 10, marginTop: 6 }}>
|
||||
<div style={{ display: 'flex', justifyContent: 'space-between', fontSize: 'calc(12px * var(--fs-scale-body, 1))', fontWeight: 700, color: 'var(--text-primary)' }}>
|
||||
<span>{t('packing.totalWeight')}</span>
|
||||
<span>{(() => { const w = myItems.reduce((s, i) => s + itemWeight(i), 0); return w >= 1000 ? `${(w / 1000).toFixed(1)} kg` : `${w} g` })()}</span>
|
||||
<span>{(() => {
|
||||
// Same rule as the rows above it (#2191).
|
||||
const w = bags.reduce((s, b) => s + bagWeightOf(b), 0)
|
||||
+ unassignedTotalWeight(unassignedWeightGrams, myItems.filter(i => !i.bag_id))
|
||||
return w >= 1000 ? `${(w / 1000).toFixed(1)} kg` : `${w} g`
|
||||
})()}</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { katColor, itemWeight, bagFillPct, countsTowardsMyLoad, parseCsvLine, parseImportLines } from './packingListPanel.helpers'
|
||||
import { katColor, itemWeight, bagFillPct, bagTotalWeight, countsTowardsMyLoad, parseCsvLine, parseImportLines, unassignedTotalWeight } from './packingListPanel.helpers'
|
||||
import { KAT_COLORS } from './packingListPanel.constants'
|
||||
|
||||
describe('packingListPanel.helpers', () => {
|
||||
@@ -128,4 +128,29 @@ describe('packingListPanel.helpers', () => {
|
||||
expect(rows[0].name).toBe('Passport')
|
||||
})
|
||||
})
|
||||
|
||||
describe('bagTotalWeight / unassignedTotalWeight (#2191)', () => {
|
||||
it('prefers the server total over anything summable locally', () => {
|
||||
// The whole point: the local list is privacy-filtered, so it can only ever
|
||||
// be the part of the bag this viewer is allowed to see.
|
||||
expect(bagTotalWeight({ total_weight_grams: 1000 }, [{ weight_grams: 800, quantity: 1 }])).toBe(1000)
|
||||
})
|
||||
|
||||
it('keeps a server-reported zero instead of falling back to the local sum', () => {
|
||||
// An empty bag really weighs 0; only an ABSENT field means "not told".
|
||||
expect(bagTotalWeight({ total_weight_grams: 0 }, [{ weight_grams: 800, quantity: 1 }])).toBe(0)
|
||||
})
|
||||
|
||||
it('falls back to the local sum for a bag cached before the field existed', () => {
|
||||
expect(bagTotalWeight({}, [{ weight_grams: 250, quantity: 3 }, { weight_grams: 50 }])).toBe(800)
|
||||
expect(bagTotalWeight({ total_weight_grams: null }, [{ weight_grams: 120 }])).toBe(120)
|
||||
})
|
||||
|
||||
it('applies the same rule to the unassigned pile', () => {
|
||||
expect(unassignedTotalWeight(150, [{ weight_grams: 900 }])).toBe(150)
|
||||
expect(unassignedTotalWeight(0, [{ weight_grams: 900 }])).toBe(0)
|
||||
expect(unassignedTotalWeight(null, [{ weight_grams: 900 }])).toBe(900)
|
||||
expect(unassignedTotalWeight(undefined, [])).toBe(0)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -30,6 +30,32 @@ export const countsTowardsMyLoad = (
|
||||
return i.owner_id == null || i.owner_id === currentUserId
|
||||
}
|
||||
|
||||
/**
|
||||
* What a bag weighs, as shown (#2191).
|
||||
*
|
||||
* The server sums every member's items — including the private ones this viewer
|
||||
* may not see — and sends the figure on the bag itself. Adding it up locally
|
||||
* could only ever produce the part of the bag the viewer is allowed to look at,
|
||||
* which is the wrong number to measure against an airline's weight limit.
|
||||
*
|
||||
* The local sum stays as the fallback for exactly one case: a bag that reached
|
||||
* the client without the field, i.e. an offline Dexie row cached before #2191.
|
||||
* Note `?? 0` semantics — an empty bag legitimately weighs 0 and must not fall
|
||||
* through to the local sum.
|
||||
*/
|
||||
export const bagTotalWeight = (
|
||||
bag: { total_weight_grams?: number | null },
|
||||
visibleItems: { weight_grams?: number | null; quantity?: number | null }[],
|
||||
): number =>
|
||||
bag.total_weight_grams ?? visibleItems.reduce((sum, i) => sum + itemWeight(i), 0)
|
||||
|
||||
/** The same rule for the pile that is in no bag (#2191). */
|
||||
export const unassignedTotalWeight = (
|
||||
serverTotal: number | null | undefined,
|
||||
visibleItems: { weight_grams?: number | null; quantity?: number | null }[],
|
||||
): number =>
|
||||
serverTotal ?? visibleItems.reduce((sum, i) => sum + itemWeight(i), 0)
|
||||
|
||||
/**
|
||||
* How full a bag's bar reads. A bag with a weight limit is measured against that limit —
|
||||
* that is the number an airline cares about. Without one there is nothing absolute to
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useState, useMemo, useRef, useEffect } from 'react'
|
||||
import { useState, useMemo, useRef, useEffect, useCallback } from 'react'
|
||||
import type { ChangeEvent } from 'react'
|
||||
import { useTripStore } from '../../store/tripStore'
|
||||
import { useCanDo } from '../../store/permissionsStore'
|
||||
@@ -7,6 +7,7 @@ import { useToast } from '../shared/Toast'
|
||||
import { useTranslation } from '../../i18n'
|
||||
import { packingApi, tripsApi } from '../../api/client'
|
||||
import { useAddonStore } from '../../store/addonStore'
|
||||
import { addListener, removeListener } from '../../api/websocket'
|
||||
import type { PackingItem, PackingBag } from '../../types'
|
||||
import { BAG_COLORS, PACKING_PLACEHOLDER_NAME } from './packingListPanel.constants'
|
||||
import { parseImportLines } from './packingListPanel.helpers'
|
||||
@@ -210,6 +211,8 @@ export function usePackingList({ tripId, items, openImportSignal = 0, clearCheck
|
||||
const addonsLoaded = useAddonStore(s => s.loaded)
|
||||
const loadAddons = useAddonStore(s => s.loadAddons)
|
||||
const [bags, setBags] = useState<PackingBag[]>([])
|
||||
/** Server-summed weight of everything in no bag (#2191); null until the first load. */
|
||||
const [unassignedWeightGrams, setUnassignedWeightGrams] = useState<number | null>(null)
|
||||
const [newBagName, setNewBagName] = useState('')
|
||||
const [showAddBag, setShowAddBag] = useState(false)
|
||||
const [showBagModal, setShowBagModal] = useState(false)
|
||||
@@ -218,10 +221,32 @@ export function usePackingList({ tripId, items, openImportSignal = 0, clearCheck
|
||||
if (!addonsLoaded) loadAddons()
|
||||
}, [addonsLoaded, loadAddons])
|
||||
|
||||
useEffect(() => {
|
||||
if (bagTrackingEnabled) packingApi.listBags(tripId).then(r => setBags(r.bags || [])).catch(() => {})
|
||||
const reloadBags = useCallback(() => {
|
||||
if (!bagTrackingEnabled) return
|
||||
packingApi.listBags(tripId)
|
||||
.then(r => {
|
||||
setBags(r.bags || [])
|
||||
setUnassignedWeightGrams(r.unassigned_weight_grams ?? null)
|
||||
})
|
||||
.catch(() => {})
|
||||
}, [tripId, bagTrackingEnabled])
|
||||
|
||||
useEffect(() => { reloadBags() }, [reloadBags])
|
||||
|
||||
// Bag weights are summed server-side across every member (#2191), so an item
|
||||
// this viewer may not even see still moves them. The item events cannot carry
|
||||
// that — a private item is delivered only to its owner, which is the very rule
|
||||
// that made the totals wrong — so the server pings the room content-free and
|
||||
// we re-read the numbers.
|
||||
useEffect(() => {
|
||||
if (!bagTrackingEnabled) return
|
||||
const handler = (event: Record<string, unknown>) => {
|
||||
if (event.type === 'packing:bag-totals') reloadBags()
|
||||
}
|
||||
addListener(handler)
|
||||
return () => removeListener(handler)
|
||||
}, [bagTrackingEnabled, reloadBags])
|
||||
|
||||
const handleCreateBag = async () => {
|
||||
if (!newBagName.trim()) return
|
||||
try {
|
||||
@@ -373,7 +398,7 @@ export function usePackingList({ tripId, items, openImportSignal = 0, clearCheck
|
||||
filter, setFilter, addingCategory, setAddingCategory, newCatName, setNewCatName,
|
||||
tripMembers, categoryAssignees, handleSetAssignees, allCategories, gruppiert, abgehakt, fortschritt,
|
||||
handleAddItemToCategory, handleAddNewCategory, handleRenameCategory, handleDeleteCategory, handleDeleteItem, handleClearChecked,
|
||||
bagTrackingEnabled, bags, newBagName, setNewBagName, showAddBag, setShowAddBag, showBagModal, setShowBagModal,
|
||||
bagTrackingEnabled, bags, unassignedWeightGrams, newBagName, setNewBagName, showAddBag, setShowAddBag, showBagModal, setShowBagModal,
|
||||
handleCreateBag, handleCreateBagByName, handleDeleteBag, handleUpdateBag, handleSetBagMembers,
|
||||
availableTemplates, showTemplateDropdown, setShowTemplateDropdown, applyingTemplate,
|
||||
showSaveTemplate, setShowSaveTemplate, saveTemplateName, setSaveTemplateName,
|
||||
|
||||
@@ -6,14 +6,16 @@ import { FIELD_CLS, FormSheetHeader } from '../sheets/PlSheetChrome'
|
||||
import { avatarSrc } from '../../../../utils/avatarSrc'
|
||||
import type { PackingBag, PackingItem, TripMember } from '../../../../types'
|
||||
import type { TripPlanner } from '../MTripShell'
|
||||
import { formatWeight, packingItemWeight } from './listsModel'
|
||||
import { bagFillPct, countsTowardsMyLoad } from '../../../../components/Packing/packingListPanel.helpers'
|
||||
import { formatWeight } from './listsModel'
|
||||
import { bagFillPct, bagTotalWeight, countsTowardsMyLoad, unassignedTotalWeight } from '../../../../components/Packing/packingListPanel.helpers'
|
||||
|
||||
export interface MBagsSheetProps {
|
||||
planner: TripPlanner
|
||||
open: boolean
|
||||
onClose: () => void
|
||||
bags: PackingBag[]
|
||||
/** Server-summed weight of everything in no bag (#2191); null when unknown. */
|
||||
unassignedWeightGrams?: number | null
|
||||
items: PackingItem[]
|
||||
tripMembers: TripMember[]
|
||||
canEdit: boolean
|
||||
@@ -31,20 +33,23 @@ export interface MBagsSheetProps {
|
||||
* is the caller's business — this sheet just renders whatever bags it's given.
|
||||
*/
|
||||
export default function MBagsSheet({
|
||||
planner, open, onClose, bags, items, tripMembers, canEdit, currentUserId, onCreateBag, onUpdateBag, onDeleteBag, onSetBagMembers,
|
||||
planner, open, onClose, bags, items, unassignedWeightGrams, tripMembers, canEdit, currentUserId, onCreateBag, onUpdateBag, onDeleteBag, onSetBagMembers,
|
||||
}: MBagsSheetProps) {
|
||||
const { t } = planner
|
||||
const [addingBag, setAddingBag] = useState(false)
|
||||
const [newBagName, setNewBagName] = useState('')
|
||||
|
||||
// These numbers describe what you are carrying. An item someone shared with you stays
|
||||
// in your list, but they are the one bringing it, so it is not your weight (#1767).
|
||||
// The ITEM LISTS still describe what you are carrying — an item someone shared
|
||||
// with you stays in your list, but they are the one bringing it (#1767).
|
||||
const myItems = items.filter(i => countsTowardsMyLoad(i, currentUserId))
|
||||
// The WEIGHTS no longer do: a bag's load is the bag's, whoever packed it (#2191).
|
||||
const bagWeightOf = (bag: PackingBag) =>
|
||||
bagTotalWeight(bag, myItems.filter(i => i.bag_id === bag.id))
|
||||
const unassigned = myItems.filter(i => !i.bag_id)
|
||||
const unassignedWeight = unassigned.reduce((s, i) => s + packingItemWeight(i), 0)
|
||||
const totalWeight = myItems.reduce((s, i) => s + packingItemWeight(i), 0)
|
||||
const unassignedWeight = unassignedTotalWeight(unassignedWeightGrams, unassigned)
|
||||
const totalWeight = bags.reduce((s, b) => s + bagWeightOf(b), 0) + unassignedWeight
|
||||
// Reference for bags without a limit of their own — computed once instead of per bag.
|
||||
const heaviestBagWeight = Math.max(...bags.map(b => myItems.filter(i => i.bag_id === b.id).reduce((s, i) => s + packingItemWeight(i), 0)), 1)
|
||||
const heaviestBagWeight = Math.max(...bags.map(bagWeightOf), 1)
|
||||
|
||||
const submitNewBag = () => {
|
||||
if (!newBagName.trim()) return
|
||||
@@ -60,7 +65,7 @@ export default function MBagsSheet({
|
||||
<div className="min-h-0 flex-1 overflow-y-auto px-[18px] pb-[6px] pt-1">
|
||||
{bags.map(bag => {
|
||||
const bagItems = myItems.filter(i => i.bag_id === bag.id)
|
||||
const bagWeight = bagItems.reduce((s, i) => s + packingItemWeight(i), 0)
|
||||
const bagWeight = bagWeightOf(bag)
|
||||
const pct = bagFillPct(bagWeight, bag.weight_limit_grams, heaviestBagWeight)
|
||||
return (
|
||||
<BagRow
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { useEffect, useMemo, useState } from 'react'
|
||||
import { useEffect, useMemo, useState, useCallback } from 'react'
|
||||
import type { LucideIcon } from 'lucide-react'
|
||||
import {
|
||||
Briefcase, Check, CheckCheck, ChevronDown, ChevronUp, HandHelping,
|
||||
@@ -9,6 +9,7 @@ import { useAuthStore } from '../../../../store/authStore'
|
||||
import { useAddonStore } from '../../../../store/addonStore'
|
||||
import { useTripStore } from '../../../../store/tripStore'
|
||||
import { packingApi } from '../../../../api/client'
|
||||
import { addListener, removeListener } from '../../../../api/websocket'
|
||||
import type { PackingUpdateBagRequest } from '@trek/shared'
|
||||
import type { PackingBag, PackingItem, TripMember } from '../../../../types'
|
||||
import type { TripPlanner } from '../MTripShell'
|
||||
@@ -62,13 +63,33 @@ export default function MPackingListTab({ planner }: { planner: TripPlanner }) {
|
||||
const [deleteCategoryTarget, setDeleteCategoryTarget] = useState<{ name: string; items: PackingItem[] } | null>(null)
|
||||
|
||||
const [bags, setBags] = useState<PackingBag[]>([])
|
||||
/** Server-summed weight of everything in no bag (#2191); null until first load. */
|
||||
const [unassignedWeightGrams, setUnassignedWeightGrams] = useState<number | null>(null)
|
||||
const [templates, setTemplates] = useState<PackingTemplate[]>([])
|
||||
const [categoryAssignees, setCategoryAssignees] = useState<Record<string, CategoryAssignee[]>>({})
|
||||
|
||||
const reloadBags = useCallback(() => {
|
||||
if (!bagTrackingEnabled) return
|
||||
packingApi.listBags(tripId)
|
||||
.then(r => {
|
||||
setBags(r.bags || [])
|
||||
setUnassignedWeightGrams(r.unassigned_weight_grams ?? null)
|
||||
})
|
||||
.catch(() => {})
|
||||
}, [tripId, bagTrackingEnabled])
|
||||
|
||||
useEffect(() => { reloadBags() }, [reloadBags])
|
||||
|
||||
// Bag weights are server-summed across every member (#2191), so an item this
|
||||
// viewer cannot see still moves them; the server pings the room content-free.
|
||||
useEffect(() => {
|
||||
if (!bagTrackingEnabled) return
|
||||
packingApi.listBags(tripId).then(r => setBags(r.bags || [])).catch(() => {})
|
||||
}, [tripId, bagTrackingEnabled])
|
||||
const handler = (event: Record<string, unknown>) => {
|
||||
if (event.type === 'packing:bag-totals') reloadBags()
|
||||
}
|
||||
addListener(handler)
|
||||
return () => removeListener(handler)
|
||||
}, [bagTrackingEnabled, reloadBags])
|
||||
|
||||
useEffect(() => {
|
||||
packingApi.listTemplates(tripId).then(r => setTemplates(r.templates || [])).catch(() => {})
|
||||
@@ -470,6 +491,7 @@ export default function MPackingListTab({ planner }: { planner: TripPlanner }) {
|
||||
onClose={() => setShowBagsSheet(false)}
|
||||
bags={bags}
|
||||
items={items}
|
||||
unassignedWeightGrams={unassignedWeightGrams}
|
||||
tripMembers={tripMembers}
|
||||
canEdit={canEdit}
|
||||
currentUserId={currentUserId}
|
||||
|
||||
@@ -85,6 +85,7 @@ export class PackingController {
|
||||
for (const item of created) {
|
||||
this.packing.broadcastItem(tripId, 'packing:created', { item }, item, socketId);
|
||||
}
|
||||
this.packing.broadcastBagTotals(tripId);
|
||||
return { items: created, count: created.length };
|
||||
}
|
||||
|
||||
@@ -104,6 +105,7 @@ export class PackingController {
|
||||
throw new HttpException({ error: 'Bag not found' }, 400);
|
||||
}
|
||||
this.packing.emitToViewers(tripId, 'packing:created', { item }, item, socketId);
|
||||
this.packing.broadcastBagTotals(tripId);
|
||||
return { item };
|
||||
}
|
||||
|
||||
@@ -149,6 +151,7 @@ export class PackingController {
|
||||
throw new HttpException({ error: 'Bag not found' }, 400);
|
||||
}
|
||||
this.packing.broadcastUpdate(tripId, id, updated as PackingItemRow, !!before?.is_private, socketId);
|
||||
this.packing.broadcastBagTotals(tripId);
|
||||
return { item: updated };
|
||||
}
|
||||
|
||||
@@ -166,6 +169,7 @@ export class PackingController {
|
||||
}
|
||||
// Scope the delete to the people who could see it (owner + recipients, #858).
|
||||
this.packing.emitToViewers(tripId, 'packing:deleted', { itemId: Number(id) }, deleted as PackingItemRow, socketId);
|
||||
this.packing.broadcastBagTotals(tripId);
|
||||
return { success: true };
|
||||
}
|
||||
|
||||
@@ -207,6 +211,7 @@ export class PackingController {
|
||||
}
|
||||
// The clone is personal to the caller — only their sockets need it.
|
||||
this.packing.emitToViewers(tripId, 'packing:created', { item }, item, socketId);
|
||||
this.packing.broadcastBagTotals(tripId);
|
||||
return { item };
|
||||
}
|
||||
|
||||
@@ -248,7 +253,13 @@ export class PackingController {
|
||||
|
||||
@Get('bags')
|
||||
listBags(@CurrentUser() user: User, @Param('tripId') tripId: string) {
|
||||
return { bags: this.packing.listBags(tripId) };
|
||||
// unassigned_weight_grams rides along so the "no bag" pile and the grand
|
||||
// total follow the same rule as the bags themselves (#2191) — a screen
|
||||
// mixing true totals with per-viewer ones would be worse than either.
|
||||
return {
|
||||
bags: this.packing.listBags(tripId),
|
||||
unassigned_weight_grams: this.packing.unassignedWeightGrams(tripId),
|
||||
};
|
||||
}
|
||||
|
||||
@RequirePermission('packing_edit')
|
||||
@@ -324,6 +335,7 @@ export class PackingController {
|
||||
throw new HttpException({ error: 'Template not found or empty' }, 404);
|
||||
}
|
||||
this.packing.broadcastItem(tripId, 'packing:template-applied', { items: added }, added[0], socketId);
|
||||
this.packing.broadcastBagTotals(tripId);
|
||||
return { items: added, count: added.length };
|
||||
}
|
||||
|
||||
|
||||
@@ -112,6 +112,21 @@ export class PackingService {
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Tell the whole room its bag weights moved (#2191).
|
||||
*
|
||||
* Deliberately NOT excluding the originating socket, which every other
|
||||
* broadcast here does: the payload carries nothing to echo, and the sender's
|
||||
* own client cannot recompute a server-side total from the item it just
|
||||
* wrote either. Everyone refetches, everyone gets numbers.
|
||||
*
|
||||
* Called for item writes, not bag writes — a bag's own create/update already
|
||||
* broadcasts the row, and neither changes what is inside it.
|
||||
*/
|
||||
broadcastBagTotals(tripId: string): void {
|
||||
this.broadcast(tripId, 'packing:bag-totals', {}, undefined);
|
||||
}
|
||||
|
||||
/**
|
||||
* The four public/private transitions after an update (#858). `wasPrivate` must be
|
||||
* read BEFORE the write: getting it wrong leaks a freshly-privatized item to the
|
||||
@@ -489,8 +504,42 @@ export class PackingService {
|
||||
|
||||
// ── Bags ───────────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* What each bag actually weighs (#2191).
|
||||
*
|
||||
* Every weight TREK showed used to be a client-side sum over `listItems`,
|
||||
* which is privacy-filtered — so a bag's "total" silently omitted the private
|
||||
* items of every other member, and no one but their owner could ever see the
|
||||
* real figure. That number is then measured against `weight_limit_grams`, an
|
||||
* absolute airline limit, which makes a per-viewer subtotal not merely
|
||||
* incomplete but wrong: a shared bag could sit over its limit and warn nobody.
|
||||
*
|
||||
* So the sum is computed here, over EVERY row, and only integers cross the
|
||||
* wire. A member learns that a bag is heavier than the items they can see —
|
||||
* never a name, category, quantity or owner. That is a deliberate, bounded
|
||||
* disclosure and the point of the issue.
|
||||
*
|
||||
* Keyed by bag id, with the unassigned pile under `null` — the same shape the
|
||||
* "no bag" row on every packing surface needs.
|
||||
*/
|
||||
private bagWeightTotals(tripId: string | number): Map<number | null, number> {
|
||||
const rows = this.db.all<{ bag_id: number | null; total: number | null }>(`
|
||||
SELECT bag_id, SUM(COALESCE(weight_grams, 0) * COALESCE(quantity, 1)) AS total
|
||||
FROM packing_items
|
||||
WHERE trip_id = ?
|
||||
GROUP BY bag_id
|
||||
`, tripId);
|
||||
return new Map(rows.map(r => [r.bag_id, r.total ?? 0]));
|
||||
}
|
||||
|
||||
/** The weight of everything in the trip that is in no bag (#2191). */
|
||||
unassignedWeightGrams(tripId: string | number): number {
|
||||
return this.bagWeightTotals(tripId).get(null) ?? 0;
|
||||
}
|
||||
|
||||
listBags(tripId: string | number) {
|
||||
const bags = this.db.all<any>('SELECT * FROM packing_bags WHERE trip_id = ? ORDER BY sort_order, id', tripId);
|
||||
const totals = this.bagWeightTotals(tripId);
|
||||
const members = this.db.all<{ bag_id: number; user_id: number; username: string; avatar: string | null }>(`
|
||||
SELECT bm.bag_id, bm.user_id, COALESCE(u.display_name, u.username) AS username, u.avatar
|
||||
FROM packing_bag_members bm
|
||||
@@ -506,6 +555,7 @@ export class PackingService {
|
||||
return bags.map(b => ({
|
||||
...b,
|
||||
members: (membersByBag.get(b.id) || []).map(m => ({ ...m, avatar: avatarUrl(m) })),
|
||||
total_weight_grams: totals.get(b.id) ?? 0,
|
||||
}));
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,8 @@ function makeService(overrides: Partial<PackingService> = {}): PackingService {
|
||||
broadcast: vi.fn(),
|
||||
broadcastItem: vi.fn(),
|
||||
broadcastToViewers: vi.fn(),
|
||||
// Content-free "the bag weights moved" ping fired after every item write (#2191).
|
||||
broadcastBagTotals: vi.fn(),
|
||||
// Real viewer logic so the emit-to-viewers routing is exercised faithfully.
|
||||
viewersOf: (item: { is_private?: number; owner_id?: number | null; recipients?: { user_id: number }[] } | null | undefined) =>
|
||||
!item || !item.is_private ? null : [item.owner_id, ...(item.recipients || []).map(r => r.user_id)].filter((x): x is number => x != null),
|
||||
@@ -316,10 +318,14 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
|
||||
});
|
||||
|
||||
describe('bags', () => {
|
||||
it('GET /bags lists bags for the trip', () => {
|
||||
const listBags = vi.fn().mockReturnValue([{ id: 3, name: 'Carry-on' }]);
|
||||
const svc = makeService({ listBags } as Partial<PackingService>);
|
||||
expect(new PackingController(svc).listBags(user, '5')).toEqual({ bags: [{ id: 3, name: 'Carry-on' }] });
|
||||
it('GET /bags lists bags for the trip, with the unassigned weight alongside (#2191)', () => {
|
||||
const listBags = vi.fn().mockReturnValue([{ id: 3, name: 'Carry-on', total_weight_grams: 800 }]);
|
||||
const unassignedWeightGrams = vi.fn().mockReturnValue(150);
|
||||
const svc = makeService({ listBags, unassignedWeightGrams } as Partial<PackingService>);
|
||||
expect(new PackingController(svc).listBags(user, '5')).toEqual({
|
||||
bags: [{ id: 3, name: 'Carry-on', total_weight_grams: 800 }],
|
||||
unassigned_weight_grams: 150,
|
||||
});
|
||||
});
|
||||
|
||||
it('400 on bag create with blank name (bespoke check — the schema cannot see whitespace)', () => {
|
||||
|
||||
@@ -697,6 +697,55 @@ describe('create-path fields + bag trip scope (#2154)', () => {
|
||||
expect(item).toMatchObject({ weight_grams: 250, bag_id: bag.id, quantity: 3 });
|
||||
});
|
||||
|
||||
it('PACK-SVC-077: a bag weighs what EVERY member put in it, private items included (#2191)', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const { user: buddy } = createUser(testDb, { username: 'bag-buddy' });
|
||||
const trip = createTrip(testDb, user.id);
|
||||
addTripMember(testDb, trip.id, buddy.id);
|
||||
const bag = svc.createBag(trip.id, { name: 'Duffel' }) as any;
|
||||
|
||||
// The reporter's repro: 500 g of mine (personal) + 300 g common + 200 g of
|
||||
// my buddy's personal list. Their item is invisible to me by design (#858)
|
||||
// and used to be missing from the weight along with it.
|
||||
svc.createItem(trip.id, { name: 'Boots', weight_grams: 500, bag_id: bag.id, visibility: 'personal' }, user.id);
|
||||
svc.createItem(trip.id, { name: 'Stove', weight_grams: 300, bag_id: bag.id }, user.id);
|
||||
svc.createItem(trip.id, { name: 'Book', weight_grams: 200, bag_id: bag.id, visibility: 'personal' }, buddy.id);
|
||||
|
||||
// What I am allowed to SEE is still only my own two items...
|
||||
const visible = svc.listItems(trip.id, user.id) as any[];
|
||||
expect(visible.map(i => i.name).sort()).toEqual(['Boots', 'Stove']);
|
||||
// ...but the bag weighs all three.
|
||||
expect((svc.listBags(trip.id) as any[])[0].total_weight_grams).toBe(1000);
|
||||
});
|
||||
|
||||
it('PACK-SVC-078: bag weight multiplies by quantity and survives null weights (#2191)', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const bag = svc.createBag(trip.id, { name: 'Carry-On' }) as any;
|
||||
|
||||
svc.createItem(trip.id, { name: 'Socks', weight_grams: 60, bag_id: bag.id, quantity: 3 }, user.id);
|
||||
// No weight and no quantity: contributes nothing rather than NaN.
|
||||
svc.createItem(trip.id, { name: 'Passport', bag_id: bag.id }, user.id);
|
||||
|
||||
expect((svc.listBags(trip.id) as any[])[0].total_weight_grams).toBe(180);
|
||||
});
|
||||
|
||||
it('PACK-SVC-079: an empty bag reports 0, and the unassigned pile is summed too (#2191)', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
const bag = svc.createBag(trip.id, { name: 'Empty' }) as any;
|
||||
|
||||
// 0, never undefined — the client's fallback keys off the field being absent,
|
||||
// so an empty bag must not read as "the server did not tell me".
|
||||
expect((svc.listBags(trip.id) as any[])[0].total_weight_grams).toBe(0);
|
||||
expect(svc.unassignedWeightGrams(trip.id)).toBe(0);
|
||||
|
||||
svc.createItem(trip.id, { name: 'Loose sandwich', weight_grams: 150 }, user.id);
|
||||
expect(svc.unassignedWeightGrams(trip.id)).toBe(150);
|
||||
// Still empty: the loose item belongs to no bag.
|
||||
expect((svc.listBags(trip.id) as any[]).find(b => b.id === bag.id).total_weight_grams).toBe(0);
|
||||
});
|
||||
|
||||
it('PACK-SVC-055: createItem refuses a bag off the trip with the invalidBag sentinel, inserting nothing', () => {
|
||||
const { user } = createUser(testDb);
|
||||
const trip = createTrip(testDb, user.id);
|
||||
|
||||
@@ -73,6 +73,15 @@ export const packingBagSchema = z.object({
|
||||
assigned_username: z.string().nullable().optional(),
|
||||
created_at: z.string().optional(),
|
||||
members: z.array(packingBagMemberSchema).optional(),
|
||||
/**
|
||||
* What the bag really weighs, summed server-side over every member's items
|
||||
* (#2191). The client used to add this up from the item list it was allowed
|
||||
* to see, which silently dropped other members' private items and left the
|
||||
* figure measured against an absolute weight limit wrong for everyone but
|
||||
* its owner. Optional: an offline-cached bag from before #2191 has no such
|
||||
* field, and the surfaces fall back to their local sum when it is absent.
|
||||
*/
|
||||
total_weight_grams: z.number().optional(),
|
||||
});
|
||||
export type PackingBag = z.infer<typeof packingBagSchema>;
|
||||
|
||||
|
||||
@@ -41,6 +41,7 @@ const FIXTURES: Record<TrekWsEventName, Record<string, unknown>> = {
|
||||
'packing:bag-members-updated': { bagId: 1, members: [{ user_id: 1 }] },
|
||||
'packing:assignees': { category: 'clothes', assignees: [1, 2] },
|
||||
'packing:template-applied': { items: [{ id: 7 }] },
|
||||
'packing:bag-totals': {},
|
||||
'todo:created': { item: { id: 4 } },
|
||||
'todo:updated': { item: { id: 4 } },
|
||||
'todo:deleted': { itemId: 4 },
|
||||
@@ -134,10 +135,11 @@ const DRIFT_VARIANTS: Partial<Record<TrekWsEventName, Record<string, unknown>[]>
|
||||
};
|
||||
|
||||
describe('@trek/shared realtime event registry', () => {
|
||||
it('WSEVT-REG-001: pins the authoritative inventory counts (65 trip + 32 user = 97)', () => {
|
||||
expect(TREK_WS_TRIP_EVENT_NAMES).toHaveLength(65);
|
||||
it('WSEVT-REG-001: pins the authoritative inventory counts (66 trip + 32 user = 98)', () => {
|
||||
// 66th trip event: packing:bag-totals (#2191).
|
||||
expect(TREK_WS_TRIP_EVENT_NAMES).toHaveLength(66);
|
||||
expect(TREK_WS_USER_EVENT_NAMES).toHaveLength(32);
|
||||
expect(TREK_WS_EVENT_NAMES).toHaveLength(97);
|
||||
expect(TREK_WS_EVENT_NAMES).toHaveLength(98);
|
||||
});
|
||||
|
||||
it('WSEVT-REG-002: every name is domain:action shaped and outside the reserved plugin: namespace', () => {
|
||||
|
||||
@@ -114,6 +114,17 @@ export const TREK_WS_EVENTS = {
|
||||
payload: z.object({ category: z.string(), assignees: z.array(z.unknown()) }),
|
||||
},
|
||||
'packing:template-applied': { scope: 'trip', payload: z.object({ items: z.array(z.unknown()) }) },
|
||||
/**
|
||||
* "A bag's weight may have changed" — deliberately content-free (#2191).
|
||||
*
|
||||
* Bag totals are summed server-side across every member's items, so a private
|
||||
* item nobody else may see still moves the number everybody else reads. The
|
||||
* item events themselves cannot carry that: a private one is delivered only
|
||||
* to its owner, which is exactly the privacy rule that made the totals wrong
|
||||
* in the first place. This ping goes to the whole room and says nothing at
|
||||
* all; receivers refetch the bags and get numbers, never contents.
|
||||
*/
|
||||
'packing:bag-totals': { scope: 'trip', payload: z.object({}) },
|
||||
|
||||
// ── Todo ─────────────────────────────────────────────────────────────────
|
||||
'todo:created': { scope: 'trip', payload: z.object({ item: entity }) },
|
||||
|
||||
Reference in New Issue
Block a user