k1nq/TREK
1
0
Fork 0
mirror of https://github.com/mauriceboe/TREK.git synced 2026-10-18 21:16:47 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(files): keep each view's id rules in the shared link toggle

Browse Source 
The shared toggle had mixed the two views. Desktop again treats any empty
column as free and finds a link record by comparing ids as they come. The
phone link sheet again treats only a null column as free and compares the
numeric value of a record id.

Starring stays per view. It is a single filesApi call with different
error handling and refresh on each side, so a shared wrapper would add a
layer without removing any logic.
This commit is contained in:
Maurice
2026-10-09 11:20:03 +02:00
parent a809de1b7a
commit c9c3cc6175
3 changed files with 69 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files
client/src/components/Files/fileActions.test.ts
+38 -8
View File
@@ -1,10 +1,16 @@
// FE-FILES-ACTIONS-001 to FE-FILES-ACTIONS-010: the paste and link rules the desktop file
// manager and the phone's files tab and link sheet share.
// FE-FILES-ACTIONS-001 to FE-FILES-ACTIONS-012: the paste and link rules the desktop file
// manager and the phone's files tab and link sheet share, with the id reading each keeps.
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { buildTripFile } from '../../../tests/helpers/factories';
import { filesApi } from '../../api/client';
import { filesFromClipboard, planFileLinkToggle, runFileLinkRecordStep, toggleFileLink } from './fileActions';
import {
PHONE_FILE_LINK_RULES,
filesFromClipboard,
planFileLinkToggle,
runFileLinkRecordStep,
toggleFileLink,
} from './fileActions';
function clipboard(items: { kind: string; file: File | null }[]): DataTransfer {
return { items: items.map((i) => ({ kind: i.kind, getAsFile: () => i.file })) } as unknown as DataTransfer;
@@ -54,6 +60,12 @@ describe('planFileLinkToggle', () => {
expect(planFileLinkToggle(file, 'place_id', 5)).toEqual({ kind: 'removeLink' });
expect(planFileLinkToggle(file, 'reservation_id', 7)).toEqual({ kind: 'removeLink' });
});
it('FE-FILES-ACTIONS-011: a zero column is free on desktop and taken on the phone', () => {
const file = buildTripFile({ place_id: 0 });
expect(planFileLinkToggle(file, 'place_id', 5)).toEqual({ kind: 'update', data: { place_id: 5 } });
expect(planFileLinkToggle(file, 'place_id', 5, PHONE_FILE_LINK_RULES)).toEqual({ kind: 'addLink' });
});
});
describe('running a link toggle', () => {
@@ -80,9 +92,9 @@ describe('running a link toggle', () => {
expect(filesApi.addLink).toHaveBeenNthCalledWith(2, 1, 9, { reservation_id: 7 });
});
it('FE-FILES-ACTIONS-008: removing a record looks it up by the target, numeric or not', async () => {
await runFileLinkRecordStep(1, 9, 'place_id', 5, 'removeLink');
await runFileLinkRecordStep(1, 9, 'reservation_id', 7, 'removeLink');
it('FE-FILES-ACTIONS-008: on the phone a record is looked up by the numeric target', async () => {
await runFileLinkRecordStep(1, 9, 'place_id', 5, 'removeLink', PHONE_FILE_LINK_RULES);
await runFileLinkRecordStep(1, 9, 'reservation_id', 7, 'removeLink', PHONE_FILE_LINK_RULES);
expect(filesApi.getLinks).toHaveBeenCalledWith(1, 9);
expect(filesApi.removeLink).toHaveBeenNthCalledWith(1, 1, 9, 50);
expect(filesApi.removeLink).toHaveBeenNthCalledWith(2, 1, 9, 70);
@@ -95,13 +107,31 @@ describe('running a link toggle', () => {
expect(filesApi.removeLink).not.toHaveBeenCalled();
});
it('FE-FILES-ACTIONS-012: on desktop a record is looked up by the target as it comes', async () => {
await runFileLinkRecordStep(1, 9, 'place_id', 5, 'removeLink');
await runFileLinkRecordStep(1, 9, 'reservation_id', 7, 'removeLink');
expect(filesApi.removeLink).toHaveBeenCalledTimes(1);
expect(filesApi.removeLink).toHaveBeenCalledWith(1, 9, 70);
});
it('FE-FILES-ACTIONS-010: toggleFileLink writes the column or the record, and rethrows', async () => {
await toggleFileLink(1, buildTripFile({ id: 9, place_id: null }), 'place_id', 5);
await toggleFileLink(1, buildTripFile({ id: 9, place_id: null }), 'place_id', 5, PHONE_FILE_LINK_RULES);
expect(filesApi.update).toHaveBeenCalledWith(1, 9, { place_id: 5 });
await toggleFileLink(1, buildTripFile({ id: 9, place_id: 1 }), 'place_id', 5);
await toggleFileLink(1, buildTripFile({ id: 9, place_id: 1 }), 'place_id', 5, PHONE_FILE_LINK_RULES);
expect(filesApi.addLink).toHaveBeenCalledWith(1, 9, { place_id: 5 });
await toggleFileLink(1, buildTripFile({ id: 9, place_id: 1, linked_place_ids: [5] }), 'place_id', 5);
expect(filesApi.removeLink).not.toHaveBeenCalled();
await toggleFileLink(
1,
buildTripFile({ id: 9, place_id: 1, linked_place_ids: [5] }),
'place_id',
5,
PHONE_FILE_LINK_RULES
);
expect(filesApi.removeLink).toHaveBeenCalledWith(1, 9, 50);
vi.mocked(filesApi.update).mockRejectedValue(new Error('nope'));
await expect(toggleFileLink(1, buildTripFile({ id: 9, reservation_id: 7 }), 'reservation_id', 7)).rejects.toThrow(
'nope'
client/src/components/Files/fileActions.ts
+29 -7
View File
@@ -30,19 +30,37 @@ export type FileLinkStep =
| { kind: 'addLink' }
| { kind: 'removeLink' };
/**
* How a caller reads the ids, which differs between the views. The phone link sheet treats
* only a null column as free and finds a link record by the numeric value of its id; the
* desktop manager treats any empty column as free and compares the record id as it comes.
*/
export interface FileLinkRules {
freeOnlyWhenNull?: boolean;
matchRecordsByNumber?: boolean;
}
export const PHONE_FILE_LINK_RULES: FileLinkRules = { freeOnlyWhenNull: true, matchRecordsByNumber: true };
function linkedIds(file: TripFile, field: FileLinkField): number[] {
const own = file[field];
const extra = field === 'place_id' ? file.linked_place_ids : file.linked_reservation_ids;
return [...(own != null ? [own] : []), ...(extra || []).filter((id) => id != null)];
}
export function planFileLinkToggle(file: TripFile, field: FileLinkField, targetId: number): FileLinkStep {
export function planFileLinkToggle(
file: TripFile,
field: FileLinkField,
targetId: number,
rules: FileLinkRules = {}
): FileLinkStep {
const column = (value: number | null) => (field === 'place_id' ? { place_id: value } : { reservation_id: value });
if (linkedIds(file, field).includes(targetId)) {
if (file[field] === targetId) return { kind: 'update', data: column(null) };
return { kind: 'removeLink' };
}
if (!file[field]) return { kind: 'update', data: column(targetId) };
const free = rules.freeOnlyWhenNull ? file[field] == null : !file[field];
if (free) return { kind: 'update', data: column(targetId) };
return { kind: 'addLink' };
}
@@ -58,7 +76,8 @@ export async function runFileLinkRecordStep(
fileId: number,
field: FileLinkField,
targetId: number,
step: 'addLink' | 'removeLink'
step: 'addLink' | 'removeLink',
rules: FileLinkRules = {}
): Promise<void> {
if (step === 'addLink') {
await filesApi.addLink(
@@ -69,7 +88,9 @@ export async function runFileLinkRecordStep(
return;
}
const linksRes = (await filesApi.getLinks(tripId, fileId)) as { links?: FileLinkRecord[] };
const link = (linksRes.links || []).find((l) => Number(l[field]) === targetId);
const link = (linksRes.links || []).find((l) =>
rules.matchRecordsByNumber ? Number(l[field]) === targetId : l[field] === targetId
);
if (link) await filesApi.removeLink(tripId, fileId, link.id);
}
@@ -78,9 +99,10 @@ export async function toggleFileLink(
tripId: number,
file: TripFile,
field: FileLinkField,
targetId: number
targetId: number,
rules: FileLinkRules = {}
): Promise<void> {
const step = planFileLinkToggle(file, field, targetId);
const step = planFileLinkToggle(file, field, targetId, rules);
if (step.kind === 'update') await filesApi.update(tripId, file.id, step.data);
else await runFileLinkRecordStep(tripId, file.id, field, targetId, step.kind);
else await runFileLinkRecordStep(tripId, file.id, field, targetId, step.kind, rules);
}
client/src/mobile/screens/trip/tabs/MFileLinkSheet.tsx
+2 -2
View File
@@ -2,7 +2,7 @@ import { useRef, useState } from 'react'
import { Check, Link2, Loader2, MapPin, Ticket, TrainFront } from 'lucide-react'
import type { LucideIcon } from 'lucide-react'
import MSheet from '../../../components/MSheet'
import { toggleFileLink, type FileLinkField } from '../../../../components/Files/fileActions'
import { PHONE_FILE_LINK_RULES, toggleFileLink, type FileLinkField } from '../../../../components/Files/fileActions'
import type { TripFile } from '../../../../types'
import type { TripPlanner } from '../MTripShell'
import { Eyebrow, TileHeader } from '../sheets/MTripSheetUi'
@@ -48,7 +48,7 @@ export default function MFileLinkSheet({ planner, file, onClose }: MFileLinkShee
const key = `${field === 'place_id' ? 'p' : 'r'}${targetId}`
setBusyKey(key)
try {
await toggleFileLink(tripId, shown, field, targetId)
await toggleFileLink(tripId, shown, field, targetId, PHONE_FILE_LINK_RULES)
refresh()
} catch {
toast.error(t('files.toast.assignError'))