diff --git a/client/src/components/Planner/DayPlanSidebar.test.tsx b/client/src/components/Planner/DayPlanSidebar.test.tsx index 7a6b3f884..dc9f6c591 100644 --- a/client/src/components/Planner/DayPlanSidebar.test.tsx +++ b/client/src/components/Planner/DayPlanSidebar.test.tsx @@ -101,14 +101,15 @@ vi.mock('../shared/Toast', () => ({ // ── Permissions mock ──────────────────────────────────────────────────────── -// Flipped per test to render the read-only variants of the day rows. -const mockPermissions = vi.hoisted(() => ({ canEdit: true })) +// Flipped per test to render the read-only variants of the day rows; `denied` +// takes single actions away from an otherwise editing member. +const mockPermissions = vi.hoisted(() => ({ canEdit: true, denied: new Set() })) vi.mock('../../store/permissionsStore', async (importOriginal) => { const actual = await importOriginal() as any return { ...actual, - useCanDo: () => () => mockPermissions.canEdit, + useCanDo: () => (action: string) => mockPermissions.canEdit && !mockPermissions.denied.has(action), } }) @@ -182,6 +183,7 @@ beforeEach(() => { resetAllStores() vi.clearAllMocks() mockPermissions.canEdit = true + mockPermissions.denied.clear() // clearAllMocks keeps implementations, so tests that swap the router out would // otherwise leak into the ones after them. vi.mocked(calculateRouteWithLegs).mockImplementation(waypoints => Promise.resolve({ @@ -938,6 +940,18 @@ describe('DayPlanSidebar', () => { expect(onEditPlace).toHaveBeenCalledWith(place, assignment.id) }) + it('FE-PLANNER-DAYPLAN-214: without place_edit the menu still removes from the day but neither edits nor deletes the place (#2446)', () => { + mockPermissions.denied.add('place_edit') + const place = buildPlace({ id: 42, name: 'Louvre Museum' }) + const day = buildDay({ id: 10, date: '2025-06-01', title: 'Day 1' }) + const assignment = buildAssignment({ id: 99, day_id: 10, order_index: 0, place }) + render() + fireEvent.contextMenu(screen.getByText('Louvre Museum')) + expect(screen.getByText(/Remove from day/i)).toBeInTheDocument() + expect(screen.queryByText('Edit')).not.toBeInTheDocument() + expect(screen.queryByText('Delete')).not.toBeInTheDocument() + }) + // ── Arrow reorder buttons ──────────────────────────────────────────────── it('FE-PLANNER-DAYPLAN-041: arrow down button reorders day assignments', async () => { diff --git a/client/src/components/Planner/DayPlanSidebar.tsx b/client/src/components/Planner/DayPlanSidebar.tsx index f0296f8d3..749b8b561 100644 --- a/client/src/components/Planner/DayPlanSidebar.tsx +++ b/client/src/components/Planner/DayPlanSidebar.tsx @@ -188,6 +188,9 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { const tripActions = useRef(useTripStore.getState()).current const can = useCanDo() const canEditDays = can('day_edit', trip) + // Editing or deleting the place itself is a place right; taking it off the + // day stays a day right (#2446). + const canEditPlaces = can('place_edit', trip) // The calendar subscription hands out a link that reads the trip without an // account, so it sits behind the same permission as the public share link. const canManageShare = can('share_manage', trip) @@ -1148,6 +1151,7 @@ function useDayPlanSidebar(props: DayPlanSidebarProps) { tripActions, can, canEditDays, + canEditPlaces, canManageShare, noteUi, setNoteUi, @@ -1348,6 +1352,7 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP tripActions, can, canEditDays, + canEditPlaces, canManageShare, noteUi, setNoteUi, @@ -2097,13 +2102,13 @@ const DayPlanSidebar = React.memo(function DayPlanSidebar(props: DayPlanSidebarP // does not exist here. const navTargets = getNavigationTargets(place) ctxMenu.open(e, [ - canEditDays && onEditPlace && { label: t('common.edit'), icon: Pencil, onClick: () => onEditPlace(place, assignment.id) }, + canEditPlaces && onEditPlace && { label: t('common.edit'), icon: Pencil, onClick: () => onEditPlace(place, assignment.id) }, canEditDays && onRemoveAssignment && { label: t('planner.removeFromDay'), icon: Trash2, onClick: () => onRemoveAssignment(day.id, assignment.id) }, safeHttpUrl(place.website) && { label: t('inspector.website'), icon: ExternalLink, onClick: () => window.open(safeHttpUrl(place.website)!, '_blank', 'noopener,noreferrer') }, ...navTargets.map(target => ({ label: target.label, icon: Navigation, onClick: () => openNavigationTarget(target) })), collectionsEnabled && { label: t('inspector.saveToCollection'), icon: Bookmark, onClick: () => useSaveToCollectionStore.getState().open(placeToSaveTarget(place)) }, { divider: true }, - canEditDays && onDeletePlace && { label: t('common.delete'), icon: Trash2, danger: true, onClick: () => onDeletePlace(place.id) }, + canEditPlaces && onDeletePlace && { label: t('common.delete'), icon: Trash2, danger: true, onClick: () => onDeletePlace(place.id) }, ]) }} onMouseEnter={e => { diff --git a/client/src/components/Planner/PlaceInspector.test.tsx b/client/src/components/Planner/PlaceInspector.test.tsx index f3478d578..facc0317b 100644 --- a/client/src/components/Planner/PlaceInspector.test.tsx +++ b/client/src/components/Planner/PlaceInspector.test.tsx @@ -8,6 +8,7 @@ import { useSettingsStore } from '../../store/settingsStore'; import { useAddonStore } from '../../store/addonStore'; import { usePluginStore } from '../../store/pluginStore'; import { useSaveToCollectionStore } from '../../store/saveToCollectionStore'; +import { usePermissionsStore } from '../../store/permissionsStore'; import { http, HttpResponse } from 'msw'; import { server } from '../../../tests/helpers/msw/server'; import type { AssignmentsMap } from '../../types'; @@ -1478,6 +1479,25 @@ describe('PlaceInspector', () => { expect(screen.getByText('Museum Ticket').closest('[role="button"]')).toHaveAttribute('data-no-press'); }); + it('FE-PLANNER-INSPECTOR-102: a member without place_edit gets neither Edit nor Delete nor the inline rename (#2446)', () => { + // buildUser() is a plain member and buildTrip({ id: 1 }) belongs to somebody else. + usePermissionsStore.setState({ permissions: { place_edit: 'trip_owner' } }); + const onEdit = vi.fn(); + const onDelete = vi.fn(); + const onUpdatePlace = vi.fn(); + const member = render(); + expect(screen.queryByText('Edit')).not.toBeInTheDocument(); + expect(screen.queryByText('Delete')).not.toBeInTheDocument(); + fireEvent.doubleClick(screen.getByText(place.name)); + expect(screen.queryByRole('textbox')).not.toBeInTheDocument(); + member.unmount(); + // the same trip owned by this member shows them again + seedStore(useTripStore, { trip: buildTrip({ id: 1, user_id: useAuthStore.getState().user!.id }) }); + render(); + expect(screen.getByText('Edit')).toBeInTheDocument(); + expect(screen.getByText('Delete')).toBeInTheDocument(); + }); + it('FE-PLANNER-INSPECTOR-017b: a stop a booked night wrote is neither removed nor doubled here', () => { // Taking it off the day would leave the booking behind with nothing on the drive and diff --git a/client/src/components/Planner/PlaceInspector.tsx b/client/src/components/Planner/PlaceInspector.tsx index 692276803..aed903a36 100644 --- a/client/src/components/Planner/PlaceInspector.tsx +++ b/client/src/components/Planner/PlaceInspector.tsx @@ -32,6 +32,7 @@ import type { Place, Category, Day, Assignment, Reservation, TripFile, Assignmen import type { CollectionStatus } from '@trek/shared' import { splitReservationDateTime, formatTime, formatMoney } from '../../utils/formatters' import { useTripStore } from '../../store/tripStore' +import { useCanDo } from '../../store/permissionsStore' import { formatDistance, formatElevation } from '../../utils/units' import { getNavigationTargets, openNavigationTarget } from './placeNavigation' import { TRANSPORT_TYPES, getAssignmentReservations } from '../../utils/dayMerge' @@ -184,11 +185,21 @@ interface PlaceInspectorProps { export default function PlaceInspector({ place, categories, mode = 'trip', days = [], selectedDayId = null, selectedAssignmentId = null, assignments = {}, reservations = [], onEditTransport, onEditReservation, - onClose, onEdit, onDelete, onAssignToDay, onRemoveAssignment, - files = [], onFileUpload, tripMembers = [], onSetParticipants, onUpdatePlace, onUploadImage, onRate, + onClose, onEdit: editPlace, onDelete: deletePlace, onAssignToDay, onRemoveAssignment, + files = [], onFileUpload, tripMembers = [], onSetParticipants, onUpdatePlace: updatePlace, onUploadImage, onRate, leftWidth = 0, rightWidth = 0, collectionStatus, onCopyToTrip, onSetStatus, onRemoveFromList, roadtripEndDay, roadtripStay, roadtripActive, }: PlaceInspectorProps) { + // Editing the place is a place right. The planner hands the handlers over + // regardless, and a member without the right saw Edit, Delete and the inline + // rename and got the server's 403 for each (#2446). Collection mode gates + // its own actions. + const can = useCanDo() + const trip = useTripStore(s => s.trip) + const mayEditPlace = mode !== 'trip' || can('place_edit', trip) + const onEdit = mayEditPlace ? editPlace : undefined + const onDelete = mayEditPlace ? deletePlace : undefined + const onUpdatePlace = mayEditPlace ? updatePlace : undefined // Plugins that declared a place-detail slot mount at the bottom of this panel, // scoped to the open place (trip mode only). Inline-filter like the other sites. const placeDetailPlugins = usePluginStore((s) => s.plugins).filter((p) => p.type === 'widget' && p.slot === 'place-detail') diff --git a/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx b/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx index 2baed3029..a118a2b30 100644 --- a/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx +++ b/client/src/mobile/screens/trip/plan/MPlanTimeline.tsx @@ -43,6 +43,7 @@ export default function MPlanTimeline({ planner, shell }: MPlanTimelineProps) { const { t, trip, can } = planner const canEdit = can('day_edit', trip) const editing = shell.mode === 'edit' && canEdit + const canEditPlaces = can('place_edit', trip) // Per-segment travel mode (#1281): tap a connector → pick the leg's mode. const legMenu = useContextMenu() const modeIcon = (key: string) => (key === 'walking' ? Footprints : key.startsWith('plugin:') ? Zap : Car) @@ -202,7 +203,7 @@ export default function MPlanTimeline({ planner, shell }: MPlanTimelineProps) { reorder={reorderFor(row.item)} drag={dragFor(row)} onOpen={() => openPlace(row.assignment)} - onEdit={() => tl.editAssignment(row.assignment)} + onEdit={canEditPlaces ? () => tl.editAssignment(row.assignment) : undefined} onRemove={() => tl.removeAssignment(row.assignment)} /> {dayScheduleFor('assignment', row.assignment.id)} diff --git a/client/src/mobile/screens/trip/plan/MPlanTimelineRows.tsx b/client/src/mobile/screens/trip/plan/MPlanTimelineRows.tsx index 1ea35adc9..b86e12e14 100644 --- a/client/src/mobile/screens/trip/plan/MPlanTimelineRows.tsx +++ b/client/src/mobile/screens/trip/plan/MPlanTimelineRows.tsx @@ -124,7 +124,7 @@ export function PlaceRow({ assignment, fullPlace, linkedReservations, chrome, re reorder: ReactNode drag?: RowDrag onOpen: () => void - onEdit: () => void + onEdit?: () => void onRemove: () => void }) { const { t } = chrome @@ -210,9 +210,11 @@ export function PlaceRow({ assignment, fullPlace, linkedReservations, chrome, re {chrome.editing && ( - - - + {onEdit && ( + + + + )} diff --git a/client/src/pages/tripPlanner/useTripPlanner.test.tsx b/client/src/pages/tripPlanner/useTripPlanner.test.tsx index eb3037fd5..c6428278b 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.test.tsx +++ b/client/src/pages/tripPlanner/useTripPlanner.test.tsx @@ -2092,6 +2092,21 @@ describe('useTripPlanner — misc state', () => { expect(result.current.prefillCoords).toBeNull() }) + it('FE-TP-HOOK-120: without place_edit rights the place editor and the delete dialog stay shut (#2446)', async () => { + seedStore(useAuthStore, { user: buildUser({ id: 2, role: 'user' }) }) + usePermissionsStore.setState({ permissions: { place_edit: 'admin' } }) + const place = buildPlace({ id: 1, lat: 1, lng: 2 }) + seedTrip({ places: [place], assignments: { '7': [buildAssignment({ id: 10, day_id: 7, place })] } }) + + const { result } = await renderPlanner() + act(() => { result.current.openPlaceEditor(place, 10) }) + act(() => { result.current.handleDeletePlace(1) }) + + expect(result.current.showPlaceForm).toBe(false) + expect(result.current.editingPlace).toBeNull() + expect(result.current.deletePlaceId).toBeNull() + }) + it('FE-TP-HOOK-102: a member roster refresh replaces the cached list', async () => { seedTrip() const { result } = await renderPlanner() diff --git a/client/src/pages/tripPlanner/useTripPlanner.ts b/client/src/pages/tripPlanner/useTripPlanner.ts index 243422e90..d6bd740fd 100644 --- a/client/src/pages/tripPlanner/useTripPlanner.ts +++ b/client/src/pages/tripPlanner/useTripPlanner.ts @@ -2095,6 +2095,7 @@ export function useTripPlanner() { // place's lone assignment to hydrate & persist its times; with 0 or 2+ // assignments the time is ambiguous and the modal hides the fields (#1247). const openPlaceEditor = useCallback((place: Place, preferredAssignmentId: number | null = null) => { + if (!can('place_edit', trip)) return if (roadtripActive && (isServiceStopType(place.stop_type) || tripAccommodations.some(stay => stay.place_id === place.id)) && typeof place.lat === 'number' && typeof place.lng === 'number') { const visitId = preferredAssignmentId ?? resolvePoolAssignmentId(assignments, place.id) const entry = Object.entries(assignments).find(([, visits]) => visits.some(visit => visit.id === visitId)) @@ -2119,7 +2120,7 @@ export function useTripPlanner() { setPlaceFormDayId(null) setServiceStopForm(false) setShowPlaceForm(true) - }, [assignments, roadtripActive, tripAccommodations, days, overnightOptions, roadtripRoutes.days]) + }, [can, trip, assignments, roadtripActive, tripAccommodations, days, overnightOptions, roadtripRoutes.days]) /** * How long the drive stands here, for every stop alike. @@ -2134,8 +2135,9 @@ export function useTripPlanner() { }, []) const handleDeletePlace = useCallback((placeId) => { + if (!can('place_edit', trip)) return setDeletePlaceId(placeId) - }, []) + }, [can, trip]) const confirmDeletePlace = useCallback(async () => { if (!deletePlaceId) return diff --git a/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx b/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx index f458b9fbe..33319c6a0 100644 --- a/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx +++ b/client/tests/unit/mobile/trip/MPlanTimeline.test.tsx @@ -622,6 +622,19 @@ describe('MPlanTimeline', () => { expect(mocks.tl.removeAssignment).toHaveBeenCalledWith(MUSEUM) }) + it('FE-MOB-PLTL-048: without place_edit a place row keeps remove and reorder but loses its edit circle (#2446)', () => { + renderTimeline({}, { can: vi.fn((action: string) => action !== 'place_edit') as TripPlanner['can'] }, { mode: 'edit' }) + + // the place row keeps its remove circle and loses the edit one; the + // transport and note rows keep theirs, editing those is a day right + const row = screen.getByText('Museum').closest('[role="button"]') as HTMLElement + expect(within(row).getByLabelText('planner.removeFromDay')).toBeInTheDocument() + expect(within(row).queryByLabelText('common.edit')).not.toBeInTheDocument() + fireEvent.click(within(row).getByLabelText('planner.removeFromDay')) + expect(mocks.tl.removeAssignment).toHaveBeenCalledWith(MUSEUM) + expect(mocks.tl.editAssignment).not.toHaveBeenCalled() + }) + it('FE-MOB-PLTL-037: the transit row opens the journey view from its edit circle', () => { renderTimeline({}, {}, { mode: 'edit' })