fix(planner): place edit and delete controls follow place_edit, not day_edit

With places set to "Trip owner" a member who may edit days still saw Edit and
Delete on a place in the day plan's context menu, the inspector's action row,
its inline rename and the phone timeline's edit circle, and every one of them
ended in the server's 403. The day plan gates the two entries on place_edit
(taking a place off the day stays a day right), the inspector drops its edit,
delete and update handlers without the right the way the places pool already
does, the phone timeline offers the pencil only with it, and openPlaceEditor
and handleDeletePlace guard like their siblings.

Closes #2446
This commit is contained in:
Maurice
2026-09-22 10:53:54 +02:00
parent e510ff25ab
commit a44df6cf11
9 changed files with 97 additions and 14 deletions
@@ -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<string>() }))
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(<DayPlanSidebar {...makeDefaultProps({ days: [day], places: [place], assignments: { '10': [assignment] } })} />)
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 () => {
@@ -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 => {
@@ -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(<PlaceInspector {...defaultProps} onEdit={onEdit} onDelete={onDelete} onUpdatePlace={onUpdatePlace} />);
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(<PlaceInspector {...defaultProps} onEdit={onEdit} onDelete={onDelete} onUpdatePlace={onUpdatePlace} />);
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
@@ -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')
@@ -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)}
@@ -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
</div>
{chrome.editing && (
<span className="flex flex-none items-center gap-1.5">
<ActionCircle label={t('common.edit')} onClick={onEdit}>
<Pencil size={14} strokeWidth={2} />
</ActionCircle>
{onEdit && (
<ActionCircle label={t('common.edit')} onClick={onEdit}>
<Pencil size={14} strokeWidth={2} />
</ActionCircle>
)}
<ActionCircle label={t('planner.removeFromDay')} onClick={onRemove}>
<X size={14} strokeWidth={2} />
</ActionCircle>
@@ -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()
@@ -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
@@ -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' })