Move explored relationship from caves to entrances

[ClemRz]

🤔 What

Move the "explored caves" relationship from the cave level (j_caver_cave_explorer) to the entrance level (j_caver_entrance_explorer).

Closes #1698

• New junction table j_caver_entrance_explorer with composite PK and indexes
• New PUT/DELETE /api/v1/entrances/:entranceId/cavers/:caverId endpoints replace the old cave-based ones
• CaverService.getCaver() now returns a flat exploredEntrances array (no more exploredNetworks split)
• Caver delete/merge handles the new junction table
• Data migration script expands each caver-cave link into caver-entrance links for all entrances of that cave
• Old j_caver_cave_explorer table preserved (rows cleaned up on caver delete) for rollback safety

🤷‍♂️ Why

The cave-level explorer relationship was an inaccurate model. A caver explores specific entrances, not abstract cave entities. For multi-entrance caves (networks), the old model couldn't express which entrance(s) the caver actually used. Moving to entrance-level granularity aligns the data model with physical reality and simplifies the API response (no more entrance/network bifurcation).

🔍 How

• Schema: New j_caver_entrance_explorer table (migration 9_13), mock data (9_14), and data migration from the old table (9_15). Table also added to 0_tables.sql.
• Model: New JCaverEntranceExplorer Waterline model. TCaver.exploredEntrances now points through it; TEntrance.explorerCavers added as the inverse.
• Controllers: add-explored-entrance.js and remove-explored-entrance.js use raw SQL for precise conflict/existence checks (409 for duplicates, 404 for missing relationships).
• Service: CaverService.getCaver() simplified — directly populates and returns exploredEntrances with names.
• Converter: Dead exploredNetworks mapping removed from toCaver.
• Routes: Entrance explorer routes placed in the Entrance section.
• Swagger: Updated with proper 404/409 error codes.
• Tests: Model association tests, service tests, and comprehensive route tests (auth, permissions, CRUD, conflict, not-found).

🧪 Testing

npm run dev:clean   # Rebuild DB with new table + migration
npm run test

The migration 9_15 is idempotent (ON CONFLICT DO NOTHING) — safe to re-run.

📸 Previews

N/A — backend-only change.

[Paul-AUB]

Issues (Must Fix)

1. [api/controllers/v1/caver/add-explored-entrance.js:34-55] Race condition: the SELECT-then-INSERT pattern has a TOCTOU window. Two concurrent PUT requests for the same (entranceId, caverId) pair can both find zero rows in the SELECT, then both attempt the INSERT. The second INSERT hits the composite PK constraint and throws an unhandled PostgreSQL error → 500 response to the caller instead of 409. Consider replacing the two-query flow with a single atomic statement:

   const insertQuery = `
     INSERT INTO j_caver_entrance_explorer (id_entrance, id_caver)
     VALUES ($1, $2)
     ON CONFLICT (id_entrance, id_caver) DO NOTHING
   `;
   const result = await sails.sendNativeQuery(insertQuery, [entranceId, caverId]);
   if (result.rowCount === 0) {
     return res.conflict('Caver is already registered as an explorer of this entrance.');
   }
   return res.status(204).send();

   This eliminates the race entirely and removes the extra round-trip.

Suggestions (Should Consider)

2. [test/integration/4_routes/Cavers/exploredEntrances.test.js:118] The DELETE describe block has no test for a non-existing caverId (expected: 404). The PUT block has this case at line 77. The remove-explored-entrance controller does check caver.isDeleted and returns 404, so the path exists — it's just untested. Adding a symmetric case would complete the DELETE coverage.

Nitpicks (Optional)

3. [api/controllers/v1/caver/delete.js:163-167] The j_caver_cave_explorer raw SQL cleanup runs sequentially after the Promise.all on line 153. There is no data dependency between them, so it could be moved inside the Promise.all call for a slight efficiency gain (one less sequential round-trip on caver delete/merge).

4. [test/integration/0_models/JCaverEntranceExplorer.test.js:32-35] The outer after destroys testEntrance before testCaver without first clearing any lingering junction rows. If an inner after hook fails mid-run, leftover j_caver_entrance_explorer rows would prevent a hard delete of testEntrance (FK constraint), leaving test state partially dirty. A safety await TCaver.removeFromCollection(testCaver.id, 'exploredEntrances', testEntrance.id) at the top of the outer after (idempotent — safe to call when rows are already gone) would make cleanup resilient to inner-hook failures.

[Paul-AUB]

Thanks for the implementation of this feature @ClemRz :D

[ClemRz]

All four review items addressed in d495d57:

1. Race condition (must fix) — Replaced the two-query SELECT+INSERT with a single atomic INSERT ... ON CONFLICT (id_entrance, id_caver) DO NOTHING + rowCount check. Also added the corresponding unique index to test/customSQL.js since Waterline's migrate: drop doesn't create the composite PK from SQL migrations.

2. Missing DELETE test for non-existing caverId (should consider) — Added symmetric should return 404 on non-existing caver test in the DELETE block.

3. Sequential cleanup in delete.js (nitpick) — Moved the j_caver_cave_explorer cleanup into the existing Promise.all call.

4. Cleanup resilience in model test (nitpick) — Added defensive TCaver.removeFromCollection() at the top of the outer after hook.

All 3088 tests passing, lint clean.

[Paul-AUB]

All four items from the previous review have been addressed in d495d57:

1. Race condition (must fix) — The two-query SELECT+INSERT has been replaced with a single atomic INSERT ... ON CONFLICT (id_entrance, id_caver) DO NOTHING + rowCount check in add-explored-entrance.js. The corresponding unique index was also added to test/customSQL.js so the composite PK is enforced in the test environment. ✅

2. Missing DELETE test for non-existing caverId — A symmetric should return 404 on non-existing caver test was added to the DELETE describe block in exploredEntrances.test.js. ✅

3. Sequential cleanup in delete.js — The j_caver_cave_explorer raw SQL cleanup is now inside the existing Promise.all call alongside the other parallel operations. ✅

4. Cleanup resilience in model test — A defensive TCaver.removeFromCollection() call was added at the top of the outer after hook in JCaverEntranceExplorer.test.js. ✅

The implementation looks solid. The entrance-level model is correctly wired up in both directions (TCaver → exploredEntrances, TEntrance → explorerCavers), the data migration is idempotent, and test coverage is comprehensive.

[ClemRz]

🚀 Deployment Procedure

This PR includes a database schema migration that must be executed on the production database before or during the deployment of the new code.

Pre-deployment (run against production DB)

Execute the migration scripts in order:

1. sql/9_13_2026_06_23_caver_entrance_explorer.sql — Creates the new j_caver_entrance_explorer table with composite PK and indexes.
2. sql/9_15_2026_06_23_migrate_cave_explorer_to_entrance.sql — Migrates existing data from j_caver_cave_explorer to j_caver_entrance_explorer by expanding each caver-cave link into caver-entrance links for all entrances of that cave. Uses ON CONFLICT DO NOTHING so it's idempotent and safe to re-run.

⚠️ Do NOT run 9_14 on production — that's mock data for the dev/test environment only.

Deployment

Deploy the new application code. The old j_caver_cave_explorer table is preserved (not dropped) for rollback safety. The new code no longer reads from it, but the caver delete handler cleans up legacy rows.

Post-deployment verification

-- Confirm the new table has data
SELECT COUNT(*) FROM j_caver_entrance_explorer;

-- Spot-check: compare row counts (new should be >= old since multi-entrance caves expand)
SELECT COUNT(*) FROM j_caver_cave_explorer;

Rollback plan

If a rollback is needed:

• The old j_caver_cave_explorer table and its data are untouched — simply redeploy the previous code version.
• The new j_caver_entrance_explorer table can be dropped without data loss since it's derived from the old table:

  DROP TABLE IF EXISTS j_caver_entrance_explorer;

API breaking changes

Old endpoint	New endpoint
PUT /api/v1/caves/:caveId/cavers/:caverId	PUT /api/v1/entrances/:entranceId/cavers/:caverId
DELETE /api/v1/caves/:caveId/cavers/:caverId	DELETE /api/v1/entrances/:entranceId/cavers/:caverId

The GET /api/v1/cavers/:id response no longer includes the exploredNetworks field. exploredEntrances now contains entrance objects directly (no longer derived from caves).

Coordinate with the front-end to update API calls before or alongside this deployment.