fix(collections): moving a place out of a list needs the same role as deleting it

Moving a saved place to another list takes it out of the source list, but
updatePlace only asked for edit rights there, so an editor could empty a
shared list they may not delete from. The source now takes owner or admin,
like a delete, and the target still takes edit rights. REST and the
update_collection_place tool share the service path. The Move to list
action on desktop and phone follows the delete permission.

client/src/mobile/screens/collections/MCollections.tsx

@@ -383,7 +383,7 @@ export default function MCollections() {
                 <button type="button" onClick={c.openCopyForSelection} disabled={c.selectedIds.length === 0} className={selectChip(c.selectedIds.length === 0)}>
                   <Copy size={13} strokeWidth={2.2} /> {t('collections.copyToTrip')}
                 </button>
-                {c.canEdit && (
+                {c.canDelete && (
                   <button type="button" onClick={() => c.setListPickerMode('move')} disabled={c.selectedIds.length === 0} className={selectChip(c.selectedIds.length === 0)}>
                     <FolderInput size={13} strokeWidth={2.2} /> {t('collections.moveToList')}
                   </button>

client/src/pages/CollectionsPage.tsx

@@ -277,7 +277,7 @@ function CollectionsPageDesktop(): React.ReactElement {
                         <Tags size={14} /> {t('collections.labels.assign')}
                       </button>
                     )}
-                    {c.canEdit && (
+                    {c.canDelete && (
                       <button type="button" onClick={() => c.setListPickerMode('move')} disabled={c.selectedIds.length === 0} className="col-selbar-btn">
                         <FolderInput size={14} /> {t('collections.moveToList')}
                       </button>

client/src/pages/CollectionsPage.wiring.test.tsx

@@ -479,6 +479,15 @@ describe('CollectionsPage — select mode', () => {
     expect(screen.getByRole('button', { name: /collections.copyToTrip/ })).toBeInTheDocument()
   })
 
+  it('FE-PAGE-COLLPAGE-021b: an editor keeps the edit actions but cannot move places out of the list', () => {
+    renderPage({ ...selecting, canEdit: true, canDelete: false })
+    expect(screen.queryByRole('button', { name: /collections.moveToList/ })).toBeNull()
+    expect(screen.queryByRole('button', { name: /common.delete/ })).toBeNull()
+    expect(screen.getByRole('button', { name: /collections.labels.assign/ })).toBeInTheDocument()
+    expect(screen.getByRole('button', { name: /collections.duplicateToList/ })).toBeInTheDocument()
+    expect(screen.getByRole('button', { name: /collections.copyToTrip/ })).toBeInTheDocument()
+  })
+
   it('FE-PAGE-COLLPAGE-022: bulk buttons are disabled while nothing is selected', () => {
     renderPage({ selectMode: true, selectedIds: [] })
     expect(screen.getByRole('button', { name: /collections.moveToList/ })).toBeDisabled()

client/tests/unit/mobile/collections/MCollections.test.tsx

@@ -435,6 +435,16 @@ describe('MCollections', () => {
     expect(screen.getByRole('button', { name: /collections.duplicateToList/ })).toBeInTheDocument()
   })
 
+  it('FE-MOB-COLSCR-017b: an editor keeps copy and duplicate but cannot move places out of the list', () => {
+    mocks.coll = makeHook({ selectMode: true, selectedIds: [10], canEdit: true, canDelete: false, isOwner: false })
+    render(<MCollections />)
+
+    expect(screen.queryByRole('button', { name: /collections.moveToList/ })).not.toBeInTheDocument()
+    expect(screen.queryByRole('button', { name: /common.delete/ })).not.toBeInTheDocument()
+    expect(screen.getByRole('button', { name: /collections.duplicateToList/ })).toBeInTheDocument()
+    expect(screen.getByRole('button', { name: /collections.copyToTrip/ })).toBeInTheDocument()
+  })
+
   it('FE-MOB-COLSCR-018: the list view renders rows, the loading state and both empty states', () => {
     render(<MCollections />)
     expect(screen.getByText('Elbphilharmonie')).toBeInTheDocument()

server/src/nest/collections/collections.mcp.ts

@@ -304,7 +304,7 @@ export class CollectionsMcp {
   @Tool({
     name: 'update_collection_place',
     description:
-      "Update a saved place's name, address, coordinates (lat/lng), description, notes, status, category, links, tags, labels, image, or move it to another collection (set collection_id).",
+      "Update a saved place's name, address, coordinates (lat/lng), description, notes, status, category, links, tags, labels, image, or move it to another collection (set collection_id). Moving a place out of a list needs the same role there as deleting it (owner or admin).",
     inputSchema: { placeId: idSchema, ...collectionPlaceUpdateRequestSchema.shape, links: linksInput },
     annotations: TOOL_ANNOTATIONS_WRITE,
     when: collectionsAddonOn,

server/src/nest/collections/collections.service.ts

@@ -198,9 +198,19 @@ export class CollectionsService {
 
   /** Delete a place — owner or admin only. */
   async assertCanDelete(userId: number, collectionId: number): Promise<void> {
+    await this.assertOwnerOrAdmin(userId, collectionId, 'Only an admin can delete places from this list');
+  }
+
+  /** Move a place out of a list to another one. It leaves the list just as a
+   *  delete would, so it takes the same role. */
+  private async assertCanMoveOut(userId: number, collectionId: number): Promise<void> {
+    await this.assertOwnerOrAdmin(userId, collectionId, 'Only an admin can move places out of this list');
+  }
+
+  private async assertOwnerOrAdmin(userId: number, collectionId: number, refusal: string): Promise<void> {
     const r = await this.roleOf(userId, collectionId);
     if (r === null) httpError(404, 'Collection not found');
-    if (r !== 'owner' && r !== 'admin') httpError(403, 'Only an admin can delete places from this list');
+    if (r !== 'owner' && r !== 'admin') httpError(403, refusal);
   }
 
   private async ownerOf(collectionId: number): Promise<number> {
@@ -919,6 +929,7 @@ export class CollectionsService {
 
     let movedTo: number | null = null;
     if (body.collection_id !== undefined && body.collection_id !== currentCollection) {
+      await this.assertCanMoveOut(userId, currentCollection);
       await this.assertCanEdit(userId, body.collection_id);
       write.collection_id = body.collection_id;
       write.owner_id = await this.ownerOf(body.collection_id);

server/tests/e2e/collections.e2e.test.ts

@@ -711,6 +711,65 @@ describe('Collections e2e (real auth guard + real service + temp SQLite)', () =>
     ).toBe(400);
   });
 
+  // ── move out of a shared list ────────────────────────────────────────────
+  // Moving a place out removes it from the source list, so it takes the same
+  // role there as deleting it: an editor is refused, an admin is not.
+  it('COLLECTIONS-E2E-051: moving a place out of a shared list needs admin, not editor', async () => {
+    const shared = (
+      await request(server)
+        .post('/api/addons/collections')
+        .set('Cookie', sessionCookie(ownerId))
+        .send({ name: 'Shared moves' })
+    ).body;
+    const editorId = createUser(db as never).user.id;
+    const adminId = createUser(db as never).user.id;
+    const join = async (userId: number, role: 'editor' | 'admin') => {
+      await request(server)
+        .post('/api/addons/collections/invite')
+        .set('Cookie', sessionCookie(ownerId))
+        .send({ collection_id: shared.id, user_id: userId, role });
+      await request(server)
+        .post('/api/addons/collections/invite/accept')
+        .set('Cookie', sessionCookie(userId))
+        .send({ collection_id: shared.id });
+    };
+    await join(editorId, 'editor');
+    await join(adminId, 'admin');
+    const place = (
+      await request(server)
+        .post('/api/addons/collections/places')
+        .set('Cookie', sessionCookie(ownerId))
+        .send({ collection_id: shared.id, name: 'Pergamon' })
+    ).body.place;
+    const editorList = (
+      await request(server)
+        .post('/api/addons/collections')
+        .set('Cookie', sessionCookie(editorId))
+        .send({ name: 'Editor own' })
+    ).body;
+    const adminList = (
+      await request(server)
+        .post('/api/addons/collections')
+        .set('Cookie', sessionCookie(adminId))
+        .send({ name: 'Admin own' })
+    ).body;
+
+    const refused = await request(server)
+      .patch(`/api/addons/collections/places/${place.id}`)
+      .set('Cookie', sessionCookie(editorId))
+      .send({ collection_id: editorList.id });
+    expect(refused.status).toBe(403);
+    expect(refused.body.error).toBe('Only an admin can move places out of this list');
+    expect((await findRow(orm, CollectionPlaces, { id: place.id }))?.collection_id).toBe(shared.id);
+
+    const moved = await request(server)
+      .patch(`/api/addons/collections/places/${place.id}`)
+      .set('Cookie', sessionCookie(adminId))
+      .send({ collection_id: adminList.id });
+    expect(moved.status).toBe(200);
+    expect((await findRow(orm, CollectionPlaces, { id: place.id }))?.collection_id).toBe(adminList.id);
+  });
+
   // ── delete ───────────────────────────────────────────────────────────────
   it('COLLECTIONS-E2E-050: owner deletes; non-owner member cannot (403)', async () => {
     const col = (

server/tests/unit/mcp/tools-collections.test.ts

@@ -477,6 +477,24 @@ describe('Tool: update_collection_place / set_collection_place_status / rate_col
       expect(cleared.place.rating_count).toBe(0);
     });
   });
+
+  it('update_collection_place refuses an editor moving a place out of a shared list', async () => {
+    const { user: owner } = createUser(testDb);
+    const { user: editor } = createUser(testDb);
+    const shared = await seedCollection(owner.id, 'Shared');
+    await addMember(shared, editor.id, 'editor');
+    const own = await seedCollection(editor.id, 'Mine');
+    const pid = await seedPlace(shared, owner.id, 'Gate');
+    await withHarness(editor.id, async (h) => {
+      const res = await h.client.callTool({
+        name: 'update_collection_place',
+        arguments: { placeId: pid, collection_id: own },
+      });
+      expect(res.isError).toBe(true);
+      expect(errorText(res)).toBe('Only an admin can move places out of this list');
+    });
+    expect((await findRow(orm, CollectionPlaces, { id: pid }))?.collection_id).toBe(shared);
+  });
 });
 
 // ---------------------------------------------------------------------------

server/tests/unit/nest/collections.service.test.ts

@@ -461,6 +461,64 @@ describe('status + updatePlace move', () => {
     await expect(svc.updatePlace(owner.id, p.id, { collection_id: b.id })).rejects.toThrow();
   });
 
+  // Moving a place out takes it away from the source list, so the source needs
+  // the same role as deleting it there (owner or admin), not just edit rights.
+  it('COLLECTIONS-SVC-018b: an editor of the source list cannot move a place out of it', async () => {
+    const owner = createUser(testDb).user;
+    const shared = await svc.createCollection(owner.id, { name: 'Shared' });
+    const editor = createUser(testDb).user;
+    await joinCollection(shared.id, editor.id, 'editor');
+    const own = await svc.createCollection(editor.id, { name: 'Mine' });
+    const l = await svc.createLabel(owner.id, shared.id, 'Berlin');
+    const { place: p } = await svc.savePlace(owner.id, { collection_id: shared.id, name: 'Kept' });
+    await svc.updatePlace(owner.id, p.id, { label_ids: [l.id] });
+
+    await expect(svc.updatePlace(editor.id, p.id, { collection_id: own.id })).rejects.toThrow(
+      'Only an admin can move places out of this list',
+    );
+    const row = await findRow(await orm(), CollectionPlaces, { id: p.id });
+    expect(row?.collection_id).toBe(shared.id);
+    expect(row?.owner_id).toBe(owner.id);
+    expect((await svc.getCollection(owner.id, shared.id)).places.find(x => x.id === p.id)?.label_ids).toEqual([l.id]);
+  });
+
+  it('COLLECTIONS-SVC-018c: an admin of the source list may move a place out of it', async () => {
+    const owner = createUser(testDb).user;
+    const shared = await svc.createCollection(owner.id, { name: 'Shared' });
+    const admin = createUser(testDb).user;
+    await joinCollection(shared.id, admin.id, 'admin');
+    const own = await svc.createCollection(admin.id, { name: 'Mine' });
+    const { place: p } = await svc.savePlace(owner.id, { collection_id: shared.id, name: 'Movable' });
+
+    const moved = await svc.updatePlace(admin.id, p.id, { collection_id: own.id });
+    expect(moved.collection_id).toBe(own.id);
+  });
+
+  it('COLLECTIONS-SVC-018d: an editor may still edit a place when collection_id names the list it is in', async () => {
+    const owner = createUser(testDb).user;
+    const shared = await svc.createCollection(owner.id, { name: 'Shared' });
+    const editor = createUser(testDb).user;
+    await joinCollection(shared.id, editor.id, 'editor');
+    const { place: p } = await svc.savePlace(owner.id, { collection_id: shared.id, name: 'Old' });
+
+    const res = await svc.updatePlace(editor.id, p.id, { collection_id: shared.id, name: 'New' });
+    expect(res.name).toBe('New');
+    expect(res.collection_id).toBe(shared.id);
+  });
+
+  it('COLLECTIONS-SVC-018e: a viewer of the source list still gets the read-only refusal on a move', async () => {
+    const owner = createUser(testDb).user;
+    const shared = await svc.createCollection(owner.id, { name: 'Shared' });
+    const viewer = createUser(testDb).user;
+    await joinCollection(shared.id, viewer.id, 'viewer');
+    const own = await svc.createCollection(viewer.id, { name: 'Mine' });
+    const { place: p } = await svc.savePlace(owner.id, { collection_id: shared.id, name: 'X' });
+
+    await expect(svc.updatePlace(viewer.id, p.id, { collection_id: own.id })).rejects.toThrow(
+      'You have read-only access to this list',
+    );
+  });
+
   // #1870: the address column was missing from the UPDATE set, so a typo or a
   // moved restaurant could only be fixed by deleting and re-adding the place.
   it('COLLECTIONS-SVC-019: updatePlace corrects the address and clears it with null', async () => {

wiki/Collections.md

@@ -69,7 +69,7 @@ Above the places sit compact filters: by **status**, by **category**, by average
 - **Select all** the currently filtered places.
 - **Assign label**: add one or more of the list's labels to every selected place at once.
 - **Copy to trip**: copies the selected places into any of your trips (carrying their name, description, category, notes, price, coordinates, photo and tags).
-- **Move to list** or **Duplicate to list**: the selection goes into another of your lists.
+- **Move to list** or **Duplicate to list**: the selection goes into another of your lists. Moving takes the places out of the current list, so in a shared list it needs the same role as deleting them (Admin or owner).
 - **Delete** the selection.
 
 ## Custom labels
@@ -94,7 +94,7 @@ When sharing, the owner assigns each member a permission role, and can change it
 |---|---|
 | **Viewer** | View the list, cast their own star rating on its places, copy its places into their own trips, and filter by label; no other changes to the list. |
 | **Editor** *(default)* | Add new places and edit existing ones, and manage + assign the list's labels. |
-| **Admin** | Everything an editor can, plus delete places. |
+| **Admin** | Everything an editor can, plus delete places and move them out to another list. |
 
 The owner always has full control. The owner can also remove a member, and a member can leave a shared list themselves. Permissions are enforced on the server, so a role can only ever do what it is allowed to.
 

wiki/MCP-Addon-Tools.md

@@ -107,7 +107,7 @@ Requires `collections:read` or `collections:write` scope.
 | `reorder_collections` | Reorder the user's collections — pass every collection id in the desired order. |
 | `save_place_to_collection` | Save a place into a collection from a raw payload. Returns a duplicate marker instead of saving when a similar place already exists, unless `force` is true. |
 | `save_trip_places_to_collection` | Copy one or more existing trip places into a collection. Duplicates are skipped unless `force` is true. |
-| `update_collection_place` | Update a saved place's name, address, coordinates, description, notes, status, category, links, tags, labels, image, or move it to another collection. |
+| `update_collection_place` | Update a saved place's name, address, coordinates, description, notes, status, category, links, tags, labels, image, or move it to another collection (moving out of a list needs the owner or admin role there). |
 | `set_collection_place_status` | Set a saved place's status: `idea`, `want`, or `visited`. |
 | `set_collection_place_status_from_trip` | Set a status on every saved copy of the given trip places, in every list they are on, for marking places visited after a day out. Ids are trip place ids. Lists you may only read are skipped. Returns how many saved places changed and how many of the trip places were found in at least one list. |
 | `rate_collection_place` | Set or clear the current user's 1-5 star rating on a saved place. Every member rates independently; pass `null` to remove the vote. |