mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-20 05:56:53 +00:00
fix: the follow-ups the v4.2.1 audit turned up
Five findings from auditing the release branch, none of them a blocker for it, all of them things it would have shipped with. The mirrored booking price only survived an edit when the form happened to fill metadata of its own. Both reservation forms and the phone sheet sent `metadata: null` whenever the form had no metadata key to send, and null clears the column outright — so restaurant, event, tour, parking, other, a hotel with no check-in times and a transport with no carrier details all lost the price again on the next save. They send the object now: it still clears what the form dropped, and the server carries the price across (#2233). Your share counted expenses nobody has paid for, while the balances right below it stopped counting them. The tile contradicted the panel it sits in (#2225). App.tsx kept its own list of routes that must not mount authed-only widgets, and it had already drifted from the one the response interceptor uses: this one named /public/journey/, that one covers all of /public/. It reads isAuthPublicPath now, so a route cannot be public to one and not the other. LLM_TIMEOUT_MS and TREK_PLUGINS_IGNORE_TREK_RANGE shipped in .env.example but not in the compose file most installs actually read. And a typo in the Russian offline copy: Завершёна → Завершена.
This commit is contained in:
+10
-6
@@ -19,7 +19,7 @@ import ErrorBoundary from './components/shared/ErrorBoundary'
|
||||
import { lazyWithRetry } from './utils/lazyWithRetry'
|
||||
import { useIsPhone } from './mobile/useIsPhone'
|
||||
import { TranslationProvider, useTranslation } from './i18n'
|
||||
import { authApi } from './api/client'
|
||||
import { authApi, isAuthPublicPath } from './api/client'
|
||||
import { tripRepo } from './repo/tripRepo'
|
||||
import { readStartDestination, tripStartPath, DEFAULT_START_PAGE, DEFAULT_START_TRIP_TAB, SETTINGS_WAIT_MS, START_DESTINATION_ROUTE } from './utils/startDestination'
|
||||
import { usePermissionsStore, PermissionLevel } from './store/permissionsStore'
|
||||
@@ -402,11 +402,15 @@ export default function App() {
|
||||
|| location.pathname.startsWith('/register')
|
||||
|| location.pathname.startsWith('/forgot-password')
|
||||
|| location.pathname.startsWith('/reset-password')
|
||||
// Anonymous visitor routes: no session, so authenticated-only widgets (system
|
||||
// notices, background tasks, save-to-collection) have nothing to do here and
|
||||
// would otherwise fire a doomed authenticated request on mount.
|
||||
const isPublicVisitorPage = isSharedPage || location.pathname.startsWith('/public/journey/')
|
||||
const hideAuthedWidgets = isAuthPage || isPublicVisitorPage
|
||||
// No session on these, so authenticated-only widgets (system notices,
|
||||
// background tasks, save-to-collection) have nothing to do and would only fire
|
||||
// a doomed authenticated request on mount.
|
||||
//
|
||||
// Off isAuthPublicPath rather than a second hand-kept list: the two had already
|
||||
// drifted, this one naming only /public/journey/ while the response
|
||||
// interceptor's covers all of /public/. A route that is public to one and not
|
||||
// to the other is exactly the seam that puts a 401 back.
|
||||
const hideAuthedWidgets = isAuthPage || isAuthPublicPath(location.pathname)
|
||||
|
||||
return (
|
||||
<TranslationProvider>
|
||||
|
||||
@@ -295,6 +295,26 @@ describe('CostsPanel — settlements in the ledger', () => {
|
||||
expect(card).toHaveTextContent('120') // 120,00 € (locale separator), i.e. 90 + 30
|
||||
})
|
||||
|
||||
// #2225 follow-up — the ledger stopped charging payer-less expenses, but this
|
||||
// tile kept counting them, so it contradicted the balances right beside it.
|
||||
it('keeps a payer-less expense out of Your share, like the balances do', async () => {
|
||||
seedStore(useSettingsStore, { settings: { ...useSettingsStore.getState().settings, default_currency: 'EUR' } })
|
||||
seedStore(useAuthStore, { user: buildUser({ id: 1, username: 'alice' }), isAuthenticated: true })
|
||||
const paid = { ...buildBudgetItem({ trip_id: 1, category: 'food', name: 'Dinner' }), total_price: 60, payers: [{ user_id: 1, amount: 60, username: 'alice' }], members: [{ user_id: 1, username: 'alice', paid: 1 }] }
|
||||
const noPayer = { ...buildBudgetItem({ trip_id: 1, category: 'transport', name: 'Taxi' }), total_price: 40, payers: [], members: [{ user_id: 1, username: 'alice', paid: 0 }] }
|
||||
server.use(
|
||||
http.get('/api/trips/1/budget', () => HttpResponse.json({ items: [paid, noPayer] })),
|
||||
http.get('/api/trips/1/budget/settlement', () => HttpResponse.json({ balances: [], flows: [], settlements: [] })),
|
||||
)
|
||||
render(<CostsPanel tripId={1} tripMembers={tripMembers} />)
|
||||
|
||||
await screen.findByText('Taxi')
|
||||
// 60 from the paid expense only — the 40 nobody has paid for is not a share.
|
||||
const card = screen.getByText('Your share').closest('div[style*="border-radius: 22"]')
|
||||
expect(card).toHaveTextContent('60')
|
||||
expect(card).not.toHaveTextContent('100')
|
||||
})
|
||||
|
||||
it('records a recorded-total expense with nobody to split with (#1286)', async () => {
|
||||
seedStore(useAuthStore, { user: buildUser({ id: 1, username: 'alice' }), isAuthenticated: true })
|
||||
let posted: Record<string, unknown> | null = null
|
||||
|
||||
@@ -123,7 +123,14 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps
|
||||
// ── derived expense maths (everything converted to the base currency) ────
|
||||
const baseTotal = (e: BudgetItem) => convert(e.total_price || 0, curOf(e))
|
||||
const myPaidOf = (e: BudgetItem) => (e.payers || []).filter(p => p.user_id === me).reduce((a, p) => a + convert(p.amount, curOf(e)), 0)
|
||||
// "Unfinished": a recorded total nobody has paid yet — counts toward the trip
|
||||
// total but stays out of settlements until who-paid is filled in. A negative
|
||||
// total (a refund, #2176) is just as unfinished until its recipient is named.
|
||||
const isUnfinished = (e: BudgetItem) => baseTotal(e) !== 0 && (e.payers || []).filter(p => p.amount !== 0).length === 0
|
||||
const myShareOf = (e: BudgetItem) => {
|
||||
// Nobody paid, so nobody owes: the ledger skips these entirely (#2225), and
|
||||
// counting them here left the tile contradicting the balances right beside it.
|
||||
if (isUnfinished(e)) return 0
|
||||
const myMember = (e.members || []).find(m => m.user_id === me)
|
||||
if (!myMember) return 0
|
||||
if (myMember.amount !== null && myMember.amount !== undefined) {
|
||||
@@ -133,10 +140,6 @@ export default function CostsPanel({ tripId, tripMembers = [] }: CostsPanelProps
|
||||
const myShare = shares[me] || 0
|
||||
return convert(myShare, curOf(e))
|
||||
}
|
||||
// "Unfinished": a recorded total nobody has paid yet — counts toward the trip
|
||||
// total but stays out of settlements until who-paid is filled in. A negative
|
||||
// total (a refund, #2176) is just as unfinished until its recipient is named.
|
||||
const isUnfinished = (e: BudgetItem) => baseTotal(e) !== 0 && (e.payers || []).filter(p => p.amount !== 0).length === 0
|
||||
|
||||
const totals = useMemo(() => {
|
||||
const totalSpend = budgetItems.reduce((a, e) => a + baseTotal(e), 0)
|
||||
|
||||
@@ -254,7 +254,12 @@ export function ReservationModal({ isOpen, onClose, onSave, reservation, days, p
|
||||
// Hotels link a place through the accommodation record; every other type links
|
||||
// the picked trip place/activity directly on the reservation (#1353).
|
||||
place_id: form.type === 'hotel' ? null : (form.place_id || null),
|
||||
metadata: Object.keys(metadata).length > 0 ? metadata : null,
|
||||
// An empty object, not null: null clears the column outright, and that
|
||||
// took the mirrored booking price with it on every edit of a type that
|
||||
// fills no metadata of its own — restaurant, event, tour, parking, other,
|
||||
// a hotel without check-in times (#2233). An object still clears what the
|
||||
// form dropped, and lets the server carry the price across.
|
||||
metadata,
|
||||
// Omitted on an edit: the server replaces the endpoint set whenever the
|
||||
// key is present, and this form never edits endpoints, so sending an
|
||||
// empty list would drop a transit booking's stations (#2216).
|
||||
|
||||
@@ -543,7 +543,12 @@ export function TransportModal({ isOpen, onClose, onSave, reservation, days, sel
|
||||
location: null,
|
||||
confirmation_number: form.confirmation_number || null,
|
||||
notes: form.notes || null,
|
||||
metadata: Object.keys(metadata).length > 0 ? metadata : null,
|
||||
// An empty object, not null: null clears the column outright, and that
|
||||
// took the mirrored booking price with it on every edit of a type that
|
||||
// fills no metadata of its own — restaurant, event, tour, parking, other,
|
||||
// a hotel without check-in times (#2233). An object still clears what the
|
||||
// form dropped, and lets the server carry the price across.
|
||||
metadata,
|
||||
endpoints,
|
||||
needs_review: false,
|
||||
}
|
||||
|
||||
@@ -259,7 +259,12 @@ export default function MReservationSheet({ planner, onOpenExpense }: MReservati
|
||||
assignment_id: (isHotel && !form.accommodation_id) ? null : (assignmentId || null),
|
||||
accommodation_id: isHotel ? (form.accommodation_id || null) : null,
|
||||
place_id: isHotel ? null : (form.place_id || null),
|
||||
metadata: Object.keys(metadata).length > 0 ? metadata : null,
|
||||
// An empty object, not null: null clears the column outright, and that
|
||||
// took the mirrored booking price with it on every edit of a type that
|
||||
// fills no metadata of its own — restaurant, event, tour, parking, other,
|
||||
// a hotel without check-in times (#2233). An object still clears what the
|
||||
// form dropped, and lets the server carry the price across.
|
||||
metadata,
|
||||
// Omitted on an edit for the same reason as the desktop dialog: the
|
||||
// server swaps the whole endpoint set when the key is present, and this
|
||||
// sheet never edits endpoints (#2216).
|
||||
|
||||
Reference in New Issue
Block a user