fix: miscellaneous bug fixes (#1139)

* fix(share): serve place thumbnails in shared trip links (#1100) Google-sourced place photos are stored as image_url pointing at the JWT-guarded /api/maps/place-photo/:placeId/bytes endpoint, so they 401 for an unauthenticated shared-trip viewer and render as broken images. Rewrite place image_url values in the shared payload to a public, token-scoped proxy (/api/shared/:token/place-photo/:placeId/bytes) and add an unguarded SharedController route that validates the token and that the place belongs to its trip before streaming the cached bytes. Mirrors the existing JourneyPublicController precedent. No client changes needed. * fix(atlas): replace Natural Earth with geoBoundaries for up-to-date regions (#1119) Atlas sourced country and sub-national boundaries from Natural Earth's GitHub `master` at runtime. That data is stale (e.g. it still shows Norway's pre-2020 counties such as Oppland/Hordaland) and depicts some contested territory in unwanted ways (nvkelso/natural-earth-vector#391), so Natural Earth is dropped entirely. - Country borders (admin0) now come from the geoBoundaries CGAZ composite; sub-national regions (admin1) from per-country gbOpen, which carries ISO 3166-2 codes. A new script (server/scripts/build-atlas-geo.mjs) normalizes and quantizes them into committed gzipped bundles under server/assets/atlas, read server-side at runtime (no network at boot, no GitHub CSP allowlist entry). - New GET /addons/atlas/countries/geo serves the country layer; the client fetches it from the API instead of GitHub. - A migration reconciles manually-marked visited_regions against the new bundle (valid code -> keep; region name still matches -> re-code; curated merge crosswalk for renamed reforms; else leave intact), with UNIQUE-safe dedup. bucket_list and visited_countries hold only invariant alpha-2 country codes, so they are untouched. - Attribution added (NOTICE.md + README) per geoBoundaries CC BY 4.0. Closes #1119 * fix(packing): make templates admin-only to create, usable by members Creating a packing-list template was gated only by trip access, so any trip member could create one from the Lists feature, while applying a template silently failed for non-admins because the apply dropdown was populated from the AdminGuard-protected /api/admin/packing-templates endpoint. - save-as-template now returns 403 for non-admins; the Save-as-Template button is hidden unless the user is an admin (both the TripPlanner toolbar and the inline packing header). - add member-accessible GET /api/trips/:tripId/packing/templates so the apply dropdown lists templates for any trip member; client fetches from it instead of the admin endpoint. Closes #1120 Closes #1121 * fix(packing): show bag tracking to non-admin members The global Bag Tracking toggle was only readable via the admin-gated GET /api/admin/bag-tracking, so non-admin trip members got 403 and the weight fields, bag circles, and BAGS sidebar never rendered (#1124). Surface the flag through the already-authenticated GET /api/addons (loaded into the client addon store on app start for every user); the packing hook reads it from the store instead of the admin endpoint. The admin write path stays admin-gated and unchanged.
2026-06-20 22:01:45 +00:00 · 2026-06-09 16:02:37 +02:00
parent 49b3af8b0d
commit 3c040fab11
41 changed files with 1061 additions and 277 deletions
@@ -30,11 +30,12 @@ vi.mock('../../src/db/database', () => ({
  db, canAccessTrip: vi.fn(), isOwner: vi.fn(), getPlaceWithTags: vi.fn(), closeDb: () => {}, reinitialize: () => {},
 }));

-const { getCollabFeatures, getPhotoProviderConfig } = vi.hoisted(() => ({
+const { getCollabFeatures, getBagTracking, getPhotoProviderConfig } = vi.hoisted(() => ({
  getCollabFeatures: vi.fn(() => ({ chat: true, notes: true, polls: true, whatsnext: true })),
+  getBagTracking: vi.fn(() => ({ enabled: true })),
  getPhotoProviderConfig: vi.fn(() => ({ url: 'https://immich.example' })),
 }));
-vi.mock('../../src/services/adminService', () => ({ getCollabFeatures }));
+vi.mock('../../src/services/adminService', () => ({ getCollabFeatures, getBagTracking }));
 vi.mock('../../src/services/memories/helpersService', () => ({ getPhotoProviderConfig }));

 import { AddonsModule } from '../../src/nest/addons/addons.module';
@@ -72,11 +73,15 @@ describe('GET /api/addons e2e (real auth guard + temp SQLite)', () => {
    expect((await request(server).get('/api/addons')).status).toBe(401);
  });

+  // Session 1 is a default-role ('user') account — i.e. a non-admin. Asserting the
+  // global bagTracking flag here is present is the #1124 regression guard: reading the
+  // toggle must not require admin.
  it('200 returns enabled addons + photo providers (disabled addon excluded)', async () => {
    const res = await request(server).get('/api/addons').set('Cookie', sessionCookie(1));
    expect(res.status).toBe(200);
    expect(res.body).toEqual({
      collabFeatures: { chat: true, notes: true, polls: true, whatsnext: true },
+      bagTracking: true,
      addons: [
        { id: 'packing', name: 'Packing', type: 'trip', icon: 'Backpack', enabled: true },
        {
@@ -33,6 +33,7 @@ const { mocks } = vi.hoisted(() => ({
    unmarkRegionVisited: vi.fn(),
    getVisitedRegions: vi.fn(),
    getRegionGeo: vi.fn(),
+    getCountryGeo: vi.fn(),
    listBucketList: vi.fn(),
    createBucketItem: vi.fn(),
    updateBucketItem: vi.fn(),
@@ -75,6 +76,14 @@ describe('Atlas e2e (real auth guard + temp SQLite)', () => {
    expect(res.status).toBe(401);
  });

+  it('200 countries/geo returns the admin-0 FeatureCollection', async () => {
+    mocks.getCountryGeo.mockReturnValue({ type: 'FeatureCollection', features: [{ id: 'NO' }] });
+    const res = await request(server).get('/api/addons/atlas/countries/geo').set('Cookie', sessionCookie(1));
+    expect(res.status).toBe(200);
+    expect(res.body.type).toBe('FeatureCollection');
+    expect(res.headers['cache-control']).toContain('max-age=86400');
+  });
+
  it('200 stats for an authenticated user', async () => {
    const res = await request(server).get('/api/addons/atlas/stats').set('Cookie', sessionCookie(1));
    expect(res.status).toBe(200);
@@ -8,6 +8,9 @@
 import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest';
 import request from 'supertest';
 import cookieParser from 'cookie-parser';
+import os from 'node:os';
+import path from 'node:path';
+import fs from 'node:fs';
 import type { Server } from 'http';
 import { Test } from '@nestjs/testing';
 import { seedUser, sessionCookie } from './harness';
@@ -28,7 +31,7 @@ const { checkPermission } = vi.hoisted(() => ({ checkPermission: vi.fn() }));
 vi.mock('../../src/services/permissions', () => ({ checkPermission }));

 const { shareSvc } = vi.hoisted(() => ({
-  shareSvc: { createOrUpdateShareLink: vi.fn(), getShareLink: vi.fn(), deleteShareLink: vi.fn(), getSharedTripData: vi.fn() },
+  shareSvc: { createOrUpdateShareLink: vi.fn(), getShareLink: vi.fn(), deleteShareLink: vi.fn(), getSharedTripData: vi.fn(), getSharedPlacePhotoPath: vi.fn() },
 }));
 vi.mock('../../src/services/shareService', () => shareSvc);

@@ -106,4 +109,29 @@ describe('Share-link e2e (real auth guard + temp SQLite)', () => {
    expect(res.status).toBe(404);
    expect(res.body).toEqual({ error: 'Invalid or expired link' });
  });
+
+  describe('public place-photo proxy (/api/shared/:token/place-photo/:placeId/bytes)', () => {
+    const photoFile = path.join(os.tmpdir(), 'trek-share-photo.e2e.jpg');
+    const photoBytes = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]); // JPEG-ish header
+
+    beforeAll(() => fs.writeFileSync(photoFile, photoBytes));
+    afterAll(() => { try { fs.unlinkSync(photoFile); } catch { /* ignore */ } });
+
+    it('streams cached bytes with no cookie (unguarded) for a valid token + place', async () => {
+      shareSvc.getSharedPlacePhotoPath.mockReturnValueOnce(photoFile);
+      const res = await request(server).get('/api/shared/tok/place-photo/ChIJabc/bytes');
+      expect(res.status).toBe(200);
+      expect(res.headers['content-type']).toContain('image/jpeg');
+      expect(res.headers['cache-control']).toContain('immutable');
+      expect(Buffer.from(res.body)).toEqual(photoBytes);
+      expect(shareSvc.getSharedPlacePhotoPath).toHaveBeenCalledWith('tok', 'ChIJabc');
+    });
+
+    it('404 when the token/place does not resolve to a cached photo', async () => {
+      shareSvc.getSharedPlacePhotoPath.mockReturnValueOnce(null);
+      const res = await request(server).get('/api/shared/bad/place-photo/ChIJabc/bytes');
+      expect(res.status).toBe(404);
+      expect(res.body).toEqual({ error: 'Photo not cached' });
+    });
+  });
 });
@@ -448,8 +448,8 @@ describe('Packing — apply-template, bag members, save-as-template', () => {
    expect(res.body.error).toBeDefined();
  });

-  it('PACK-017 — POST /save-as-template saves packing list as a template', async () => {
-    const { user } = createUser(testDb);
+  it('PACK-017 — POST /save-as-template saves packing list as a template (admin)', async () => {
+    const { user } = createUser(testDb, { role: 'admin' });
    const trip = createTrip(testDb, user.id);

    // Add an item so the trip has something to save
@@ -465,8 +465,8 @@ describe('Packing — apply-template, bag members, save-as-template', () => {
    expect(res.body.template.name).toBe('My Summer Template');
  });

-  it('PACK-017b — POST /save-as-template without name returns 400', async () => {
-    const { user } = createUser(testDb);
+  it('PACK-017b — POST /save-as-template without name returns 400 (admin)', async () => {
+    const { user } = createUser(testDb, { role: 'admin' });
    const trip = createTrip(testDb, user.id);

    const res = await request(app)
@@ -478,8 +478,8 @@ describe('Packing — apply-template, bag members, save-as-template', () => {
    expect(res.body.error).toBeDefined();
  });

-  it('PACK-017c — POST /save-as-template when trip has no items returns 400', async () => {
-    const { user } = createUser(testDb);
+  it('PACK-017c — POST /save-as-template when trip has no items returns 400 (admin)', async () => {
+    const { user } = createUser(testDb, { role: 'admin' });
    const trip = createTrip(testDb, user.id);

    const res = await request(app)
@@ -490,4 +490,37 @@ describe('Packing — apply-template, bag members, save-as-template', () => {
    expect(res.status).toBe(400);
    expect(res.body.error).toBeDefined();
  });
+
+  it('PACK-017d — POST /save-as-template is forbidden for non-admins (403)', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    createPackingItem(testDb, trip.id);
+
+    const res = await request(app)
+      .post(`/api/trips/${trip.id}/packing/save-as-template`)
+      .set('Cookie', authCookie(user.id))
+      .send({ name: 'My Summer Template' });
+
+    expect(res.status).toBe(403);
+    expect(res.body.error).toBe('Admin access required');
+  });
+
+  it('PACK-017e — GET /packing/templates lists templates for a trip member', async () => {
+    const { user: admin } = createUser(testDb, { role: 'admin' });
+    const trip = createTrip(testDb, admin.id);
+    createPackingItem(testDb, trip.id);
+    await request(app)
+      .post(`/api/trips/${trip.id}/packing/save-as-template`)
+      .set('Cookie', authCookie(admin.id))
+      .send({ name: 'Shared Template' });
+
+    const res = await request(app)
+      .get(`/api/trips/${trip.id}/packing/templates`)
+      .set('Cookie', authCookie(admin.id));
+
+    expect(res.status).toBe(200);
+    expect(Array.isArray(res.body.templates)).toBe(true);
+    expect(res.body.templates.some((t: { name: string }) => t.name === 'Shared Template')).toBe(true);
+    expect(res.body.templates[0]).toHaveProperty('item_count');
+  });
 });
@@ -49,6 +49,8 @@ import { runMigrations } from '../../src/db/migrations';
 import { resetTestDb, resetRateLimits } from '../helpers/test-db';
 import { createUser, createTrip, addTripMember, createDay, createPlace, createDayAssignment, createDayNote } from '../helpers/factories';
 import { authCookie } from '../helpers/auth';
+import * as placePhotoCache from '../../src/services/placePhotoCache';
+import fs from 'node:fs';

 let nestApp: INestApplication;
 let app: Application;
@@ -351,3 +353,78 @@ describe('Shared trip — ordering parity (issue #981)', () => {
    expect(reservation.day_positions[day.id]).toBe(1.5);
  });
 });
+
+describe('Shared trip — place photos in shared links (issue #1100)', () => {
+  const PLACE_ID = 'ChIJsharedPhoto1100';
+  const PROXY_URL = `/api/maps/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`;
+  const photoBytes = Buffer.from([0xff, 0xd8, 0xff, 0xe0, 0x00, 0x10]);
+  let cachedFilePath: string;
+
+  afterAll(() => { try { if (cachedFilePath) fs.unlinkSync(cachedFilePath); } catch { /* ignore */ } });
+
+  async function setupSharedPlaceWithPhoto() {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const place = createPlace(testDb, trip.id, { name: 'Photo Place' });
+    testDb.prepare('UPDATE places SET image_url = ?, google_place_id = ? WHERE id = ?').run(PROXY_URL, PLACE_ID, place.id);
+
+    const { body: { token } } = await request(app)
+      .post(`/api/trips/${trip.id}/share-link`)
+      .set('Cookie', authCookie(user.id))
+      .send({});
+    return { token, place };
+  }
+
+  it('SHARE-016 — shared payload rewrites place image_url to the public token-scoped proxy', async () => {
+    const { token } = await setupSharedPlaceWithPhoto();
+    const res = await request(app).get(`/api/shared/${token}`);
+    expect(res.status).toBe(200);
+    const place = res.body.places.find((p: any) => p.image_url);
+    expect(place.image_url).toBe(`/api/shared/${token}/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`);
+    expect(place.image_url.startsWith('/api/maps/')).toBe(false);
+  });
+
+  it('SHARE-017 — shared payload rewrites assignment place image_url too', async () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+    const day = createDay(testDb, trip.id, { date: '2025-10-01' });
+    const place = createPlace(testDb, trip.id, { name: 'Assigned Photo Place' });
+    testDb.prepare('UPDATE places SET image_url = ? WHERE id = ?').run(PROXY_URL, place.id);
+    createDayAssignment(testDb, day.id, place.id, {});
+
+    const { body: { token } } = await request(app)
+      .post(`/api/trips/${trip.id}/share-link`)
+      .set('Cookie', authCookie(user.id))
+      .send({});
+
+    const res = await request(app).get(`/api/shared/${token}`);
+    expect(res.status).toBe(200);
+    expect(res.body.assignments[day.id][0].place.image_url)
+      .toBe(`/api/shared/${token}/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`);
+  });
+
+  it('SHARE-018 — public proxy streams cached bytes for a valid token + place (no cookie)', async () => {
+    const { token } = await setupSharedPlaceWithPhoto();
+    const cached = await placePhotoCache.put(PLACE_ID, photoBytes, null);
+    cachedFilePath = cached.filePath;
+
+    const res = await request(app).get(`/api/shared/${token}/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`);
+    expect(res.status).toBe(200);
+    expect(res.headers['content-type']).toContain('image/jpeg');
+    expect(Buffer.from(res.body)).toEqual(photoBytes);
+  });
+
+  it('SHARE-019 — public proxy 404s for a placeId not in the shared trip', async () => {
+    const { token } = await setupSharedPlaceWithPhoto();
+    const res = await request(app).get(`/api/shared/${token}/place-photo/ChIJnotInTrip/bytes`);
+    expect(res.status).toBe(404);
+    expect(res.body).toEqual({ error: 'Photo not cached' });
+  });
+
+  it('SHARE-020 — public proxy 404s for an invalid token', async () => {
+    await setupSharedPlaceWithPhoto();
+    const res = await request(app).get(`/api/shared/bad-token/place-photo/${encodeURIComponent(PLACE_ID)}/bytes`);
+    expect(res.status).toBe(404);
+    expect(res.body).toEqual({ error: 'Photo not cached' });
+  });
+});
@@ -0,0 +1,119 @@
+/**
+ * Unit test for the Atlas region-code reconciliation migration (#1119).
+ *
+ * After Atlas swapped Natural Earth for geoBoundaries, manually-marked regions
+ * (`visited_regions`) held the old Natural Earth ISO-3166-2 codes. The final migration
+ * reconciles each row against the shipped admin-1 bundle: valid codes are kept, codes
+ * whose region NAME still matches are re-coded, renamed-merge cases use a curated
+ * crosswalk, and anything else is left untouched. We exercise the real migration by
+ * running all migrations, seeding rows, rewinding schema_version by one, and re-running
+ * so only the last (reconciliation) migration fires.
+ */
+import { describe, it, expect } from 'vitest';
+import Database from 'better-sqlite3';
+import { createTables } from '../../../src/db/schema';
+import { runMigrations } from '../../../src/db/migrations';
+import { createUser } from '../../helpers/factories';
+
+function freshDb() {
+  const db = new Database(':memory:');
+  db.exec('PRAGMA journal_mode = WAL');
+  db.exec('PRAGMA foreign_keys = ON');
+  createTables(db);
+  runMigrations(db);
+  return db;
+}
+
+function mark(db: Database.Database, userId: number, code: string, name: string, country = 'NO') {
+  db.prepare(
+    'INSERT INTO visited_regions (user_id, region_code, region_name, country_code) VALUES (?, ?, ?, ?)'
+  ).run(userId, code, name, country);
+}
+
+// Rewind one migration and re-run so only the reconciliation (the last migration) executes.
+function rerunLastMigration(db: Database.Database) {
+  const version = (db.prepare('SELECT version FROM schema_version').get() as { version: number }).version;
+  db.prepare('UPDATE schema_version SET version = ?').run(version - 1);
+  runMigrations(db);
+}
+
+describe('Atlas region-code reconciliation migration', () => {
+  it('CROSSWALK-001: remaps a renamed-merge county via the curated crosswalk', () => {
+    const db = freshDb();
+    const { user } = createUser(db);
+    mark(db, user.id, 'NO-05', 'Oppland'); // merged into Innlandet, name changed
+
+    rerunLastMigration(db);
+
+    const rows = db.prepare('SELECT region_code, region_name FROM visited_regions WHERE user_id = ?').all(user.id);
+    expect(rows).toEqual([{ region_code: 'NO-34', region_name: 'Innlandet' }]);
+    db.close();
+  });
+
+  it('CROSSWALK-002: merges two old counties that map to the same new region (no UNIQUE clash)', () => {
+    const db = freshDb();
+    const { user } = createUser(db);
+    mark(db, user.id, 'NO-04', 'Hedmark'); // → Innlandet
+    mark(db, user.id, 'NO-05', 'Oppland'); // → Innlandet
+
+    rerunLastMigration(db);
+
+    const rows = db.prepare('SELECT region_code FROM visited_regions WHERE user_id = ?').all(user.id);
+    expect(rows).toEqual([{ region_code: 'NO-34' }]);
+    db.close();
+  });
+
+  it('CROSSWALK-003: leaves a still-valid code untouched', () => {
+    const db = freshDb();
+    const { user } = createUser(db);
+    mark(db, user.id, 'NO-03', 'Oslo'); // present in the new bundle
+
+    rerunLastMigration(db);
+
+    const rows = db.prepare('SELECT region_code, region_name FROM visited_regions WHERE user_id = ?').all(user.id);
+    expect(rows).toEqual([{ region_code: 'NO-03', region_name: 'Oslo' }]);
+    db.close();
+  });
+
+  it('CROSSWALK-004: re-codes a stale code whose region NAME still matches the bundle', () => {
+    // Not in any crosswalk: a bogus code but a name ("Oslo") that the bundle still carries
+    // for NO → reconciled to the bundle's code for that name (NO-03) by the name-match path.
+    const db = freshDb();
+    const { user } = createUser(db);
+    mark(db, user.id, 'NO-99', 'Oslo');
+
+    rerunLastMigration(db);
+
+    const rows = db.prepare('SELECT region_code, region_name FROM visited_regions WHERE user_id = ?').all(user.id);
+    expect(rows).toEqual([{ region_code: 'NO-03', region_name: 'Oslo' }]);
+    db.close();
+  });
+
+  it('CROSSWALK-005: leaves an unresolvable row as-is (no code, no name, no crosswalk match)', () => {
+    const db = freshDb();
+    const { user } = createUser(db);
+    mark(db, user.id, 'ZZ-99', 'Nowhere', 'ZZ');
+
+    rerunLastMigration(db);
+
+    const rows = db.prepare('SELECT region_code, region_name FROM visited_regions WHERE user_id = ?').all(user.id);
+    expect(rows).toEqual([{ region_code: 'ZZ-99', region_name: 'Nowhere' }]);
+    db.close();
+  });
+
+  it('CROSSWALK-006: does not touch bucket_list or visited_countries (no region identifier there)', () => {
+    const db = freshDb();
+    const { user } = createUser(db);
+    db.prepare('INSERT INTO bucket_list (user_id, name, country_code) VALUES (?, ?, ?)').run(user.id, 'Oppland', 'NO');
+    db.prepare('INSERT INTO visited_countries (user_id, country_code) VALUES (?, ?)').run(user.id, 'NO');
+    mark(db, user.id, 'NO-05', 'Oppland'); // ensure the migration actually runs its body
+
+    rerunLastMigration(db);
+
+    const bucket = db.prepare('SELECT name, country_code FROM bucket_list WHERE user_id = ?').all(user.id);
+    expect(bucket).toEqual([{ name: 'Oppland', country_code: 'NO' }]); // free-text name untouched
+    const countries = db.prepare('SELECT country_code FROM visited_countries WHERE user_id = ?').all(user.id);
+    expect(countries).toEqual([{ country_code: 'NO' }]);
+    db.close();
+  });
+});
@@ -135,6 +135,8 @@ const ALLOWED_DESTRUCTIVE: Record<string, string> = {
    "Migration 121: DELETE ... WHERE title IN ('Gallery','[Trip Photos]') — remove synthetic wrapper entries replaced by the gallery model.",
  'DELETE FROM place_regions':
    'Atlas enclave fix: DELETE ... WHERE place_id IN (places inside specific enclave boxes) — invalidate stale region cache; re-resolved on next request.',
+  'DELETE FROM visited_regions':
+    'Atlas geoBoundaries swap (#1119): DELETE ... WHERE id = ? — after UPDATE OR IGNORE re-codes a manually-marked region to its current code, drop only the single leftover row whose UNIQUE(user_id, region_code) collision caused the update to be skipped (a duplicate of a region the user already has).',
 };

 describe('migration hygiene — destructive operation guard', () => {
@@ -53,6 +53,13 @@ describe('AtlasController (parity with the legacy /api/addons/atlas route)', ()
    });
  });

+  it('GET /countries/geo delegates to the service', () => {
+    const fc = { type: 'FeatureCollection', features: [{ id: 'NO' }] };
+    const countryGeo = vi.fn().mockReturnValue(fc);
+    expect(makeController({ countryGeo }).countryGeo()).toBe(fc);
+    expect(countryGeo).toHaveBeenCalledWith();
+  });
+
  describe('country', () => {
    it('GET /country/:code upper-cases the code', () => {
      const countryPlaces = vi.fn().mockReturnValue([]);
@@ -5,6 +5,7 @@ import type { PackingService } from '../../../src/nest/packing/packing.service';
 import type { User } from '../../../src/types';

 const user = { id: 1, role: 'user', email: 'u@example.test' } as User;
+const admin = { id: 1, role: 'admin', email: 'a@example.test' } as User;
 const trip = { id: 5, user_id: 1 };

 /** Service mock with trip access granted + edit allowed by default. */
@@ -119,6 +120,14 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
  });

  describe('templates', () => {
+    it('GET /templates returns the template list for an accessible trip', () => {
+      const listTemplates = vi.fn().mockReturnValue([{ id: 1, name: 'Beach', item_count: 4 }]);
+      const svc = makeService({ listTemplates } as Partial<PackingService>);
+      expect(new PackingController(svc).listTemplates(user, '5')).toEqual({
+        templates: [{ id: 1, name: 'Beach', item_count: 4 }],
+      });
+    });
+
    it('404 when applying a missing/empty template (POST stays 200 otherwise)', () => {
      const svc = makeService({ applyTemplate: vi.fn().mockReturnValue(null) } as Partial<PackingService>);
      expect(thrown(() => new PackingController(svc).applyTemplate(user, '5', 't1'))).toEqual({
@@ -126,12 +135,30 @@ describe('PackingController (parity with the legacy /api/trips/:tripId/packing r
      });
    });

-    it('400 saving a template with no items', () => {
-      const svc = makeService({ saveAsTemplate: vi.fn().mockReturnValue(null) } as Partial<PackingService>);
+    it('403 when a non-admin tries to save a template', () => {
+      const saveAsTemplate = vi.fn();
+      const svc = makeService({ saveAsTemplate } as Partial<PackingService>);
      expect(thrown(() => new PackingController(svc).saveAsTemplate(user, '5', 'My template'))).toEqual({
+        status: 403, body: { error: 'Admin access required' },
+      });
+      expect(saveAsTemplate).not.toHaveBeenCalled();
+    });
+
+    it('400 when an admin saves a template with no items', () => {
+      const svc = makeService({ saveAsTemplate: vi.fn().mockReturnValue(null) } as Partial<PackingService>);
+      expect(thrown(() => new PackingController(svc).saveAsTemplate(admin, '5', 'My template'))).toEqual({
        status: 400, body: { error: 'No items to save' },
      });
    });
+
+    it('saves a template for an admin', () => {
+      const saveAsTemplate = vi.fn().mockReturnValue({ id: 7, name: 'My template' });
+      const svc = makeService({ saveAsTemplate } as Partial<PackingService>);
+      expect(new PackingController(svc).saveAsTemplate(admin, '5', 'My template')).toEqual({
+        template: { id: 7, name: 'My template' },
+      });
+      expect(saveAsTemplate).toHaveBeenCalledWith('5', admin.id, 'My template');
+    });
  });

  describe('category assignees', () => {
@@ -31,7 +31,7 @@ import { createTables } from '../../../src/db/schema';
 import { runMigrations } from '../../../src/db/migrations';
 import { resetTestDb } from '../../helpers/test-db';
 import { createUser, createTrip } from '../../helpers/factories';
-import { getStats, getCached, setCache, getCountryFromCoords, getCountryFromAddress, reverseGeocodeCountry, getRegionGeo, getCountryPlaces, getVisitedRegions } from '../../../src/services/atlasService';
+import { getStats, getCached, setCache, getCountryFromCoords, getCountryFromAddress, reverseGeocodeCountry, getRegionGeo, getCountryGeo, getCountryPlaces, getVisitedRegions } from '../../../src/services/atlasService';

 function insertPlace(db: any, tripId: number, name: string, address: string | null = null) {
  const cat = db.prepare('SELECT id FROM categories LIMIT 1').get() as { id: number } | undefined;
@@ -243,38 +243,57 @@ describe('reverseGeocodeCountry', () => {

 // ── getRegionGeo ────────────────────────────────────────────────────────────

+// These read the committed geoBoundaries bundle (server/assets/atlas/admin1.geojson.gz),
+// so they double as a guard that the bundle ships current sub-national data (#1119).
 describe('getRegionGeo', () => {
-  it('ATLAS-SVC-017: returns empty FeatureCollection when fetch throws a network error', async () => {
-    // Override the default stub to throw so loadAdmin1Geo's .catch handler runs,
-    // returning null — which causes getRegionGeo to return the empty FeatureCollection.
-    // (The default ok:false stub does NOT trigger the catch; it still resolves json()
-    // to {}, which loadAdmin1Geo caches as a non-null truthy value.)
-    vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('Network failure')));
-    const result = await getRegionGeo(['DE', 'FR']);
+  it('ATLAS-SVC-017: returns an empty FeatureCollection for a country with no admin-1 features', async () => {
+    const result = await getRegionGeo(['ZZ']);
    expect(result).toEqual({ type: 'FeatureCollection', features: [] });
  });

-  it('ATLAS-SVC-018: returns filtered features for matching country codes when fetch returns mock GeoJSON', async () => {
-    // ATLAS-SVC-017 ran with a throwing fetch, so admin1GeoCache is null and
-    // admin1GeoLoading is null — this test's fetch override will be called.
-    const mockGeoJson = {
-      type: 'FeatureCollection',
-      features: [
-        { type: 'Feature', properties: { iso_a2: 'DE' }, geometry: {} },
-        { type: 'Feature', properties: { iso_a2: 'FR' }, geometry: {} },
-      ],
-    };
-    vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
-      ok: true,
-      json: async () => mockGeoJson,
-    }));
-
-    // Pass lowercase 'de' — getRegionGeo uppercases internally for matching
-    const result = await getRegionGeo(['de']);
+  it('ATLAS-SVC-018: returns the current geoBoundaries regions for a country, case-insensitively', async () => {
+    // Pass lowercase 'no' — getRegionGeo uppercases internally for matching.
+    const result = await getRegionGeo(['no']);

    expect(result.type).toBe('FeatureCollection');
-    expect(result.features).toHaveLength(1);
-    expect(result.features[0].properties.iso_a2).toBe('DE');
+    expect(result.features.length).toBeGreaterThan(0);
+    expect(result.features.every((f: any) => f.properties.iso_a2 === 'NO')).toBe(true);
+
+    const names = result.features.map((f: any) => f.properties.name);
+    const codes = result.features.map((f: any) => f.properties.iso_3166_2);
+    // Post-2020 reform is present…
+    expect(codes).toContain('NO-34'); // Innlandet
+    expect(codes).toContain('NO-46'); // Vestland
+    // …and the merged-away pre-2020 counties are gone (the original #1119 bug).
+    expect(names).not.toContain('Oppland');
+    expect(names).not.toContain('Hordaland');
+    expect(names).not.toContain('Sogn og Fjordane');
+  });
+});
+
+describe('getCountryGeo', () => {
+  it('ATLAS-SVC-019: returns the admin-0 FeatureCollection with ISO_A2/ADM0_A3 properties', () => {
+    const geo = getCountryGeo();
+    expect(geo.type).toBe('FeatureCollection');
+    expect(geo.features.length).toBeGreaterThan(0);
+    const no = geo.features.find((f: any) => f.properties.ISO_A2 === 'NO');
+    expect(no).toBeDefined();
+    expect(no.properties.ADM0_A3).toBe('NOR');
+    expect(no.properties.NAME).toBe('Norway');
+  });
+
+  it('ATLAS-SVC-020: includes territories that the curated list dropped (Greenland + Svalbard)', () => {
+    const geo = getCountryGeo();
+    // Greenland is its own feature.
+    expect(geo.features.some((f: any) => f.properties.ISO_A2 === 'GL')).toBe(true);
+    // Svalbard has no separate ISO entity in geoBoundaries; it sits inside Norway's
+    // geometry (lat ~74-81°N). Guard that the country polygon reaches those latitudes.
+    const no = geo.features.find((f: any) => f.properties.ISO_A2 === 'NO');
+    const maxLat = (function max(coords: any): number {
+      if (typeof coords[0] === 'number') return coords[1];
+      return Math.max(...coords.map(max));
+    })(no.geometry.coordinates);
+    expect(maxLat).toBeGreaterThan(78);
  });
 });

@@ -37,6 +37,7 @@ import { createUser, createTrip } from '../../helpers/factories';
 import {
  saveAsTemplate,
  applyTemplate,
+  listTemplates,
  setBagMembers,
  createBag,
  deleteBag,
@@ -92,6 +93,27 @@ describe('saveAsTemplate', () => {
  });
 });

+// ── listTemplates ───────────────────────────────────────────────────────────────
+
+describe('listTemplates', () => {
+  it('PACK-SVC-LIST-001: returns templates with id, name and item_count', () => {
+    const { user } = createUser(testDb);
+    const trip = createTrip(testDb, user.id);
+
+    testDb.prepare('INSERT INTO packing_items (trip_id, name, category, checked, sort_order) VALUES (?, ?, ?, 0, ?)').run(trip.id, 'Shirt', 'Clothes', 0);
+    testDb.prepare('INSERT INTO packing_items (trip_id, name, category, checked, sort_order) VALUES (?, ?, ?, 0, ?)').run(trip.id, 'Toothbrush', 'Toiletries', 1);
+    const saved = saveAsTemplate(trip.id, user.id, 'Weekend');
+
+    const templates = listTemplates();
+    expect(templates).toHaveLength(1);
+    expect(templates[0]).toMatchObject({ id: saved!.id, name: 'Weekend', item_count: 2 });
+  });
+
+  it('PACK-SVC-LIST-002: returns an empty array when no templates exist', () => {
+    expect(listTemplates()).toEqual([]);
+  });
+});
+
 // ── applyTemplate ─────────────────────────────────────────────────────────────

 describe('applyTemplate', () => {