From bcd533ae84c7021ba0bd6ffad4ac89de2fb4d6fa Mon Sep 17 00:00:00 2001 From: Nicolas FRYDER Date: Sat, 22 Aug 2026 14:20:44 +0200 Subject: [PATCH] =?UTF-8?q?fix(g=C3=A9ocodage):=20les=20corrections=20manu?= =?UTF-8?q?elles=20n'=C3=A9taient=20prot=C3=A9g=C3=A9es=20par=20rien?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Corriger à la main les coordonnées d'un groupe écrit un band_locations avec geocode_provider='admin' et confiance 1.0 : la donnée la plus autoritaire du système, saisie par un humain, impossible à régénérer. Elle était pourtant la moins protégée. Le trigger de la migration 013 supprime toutes les band_locations d'un groupe dès que son location_text change — override compris. locked_fields ne couvre que les colonnes de `bands`, jamais les points : une correction disparaissait donc silencieusement à la première modification du lieu, y compris quand ce lieu venait d'être verrouillé par l'admin lui-même. Trois routes de reset l'effaçaient également. Le code raisonnait explicitement sur la protection des saisies manuelles pour choisir le statut écrit ('done' et non 'manual', afin que reset-llm ne les emporte pas), mais ni reset-all ni requeue-all(include_done) n'excluaient le provider : la protection tenait sur un bouton et manquait sur les deux autres. Le trigger épargne désormais les points 'admin', et les trois routes les excluent. Trois tests d'intégration contre un vrai PostGIS couvrent ce trigger et celui de la migration 016, qui n'en avaient aucun. Co-Authored-By: Claude Opus 5 --- .../018_protect_admin_locations.sql | 32 +++++++++++ apps/api/src/adminRoutes.js | 11 +++- .../integration/migrate.integration.test.js | 55 +++++++++++++++++++ 3 files changed, 96 insertions(+), 2 deletions(-) create mode 100644 apps/api/migrations/018_protect_admin_locations.sql diff --git a/apps/api/migrations/018_protect_admin_locations.sql b/apps/api/migrations/018_protect_admin_locations.sql new file mode 100644 index 0000000..9c7fd20 --- /dev/null +++ b/apps/api/migrations/018_protect_admin_locations.sql @@ -0,0 +1,32 @@ +-- 018_protect_admin_locations.sql +-- Les corrections manuelles de géocodage n'étaient protégées par rien. +-- +-- Corriger à la main les coordonnées d'un groupe écrit un band_locations avec +-- geocode_provider='admin' et geocode_confidence=1.0 : la donnée la plus +-- autoritaire du système, saisie par un humain, impossible à régénérer. +-- +-- Or le trigger de la migration 013 supprime TOUTES les band_locations d'un +-- groupe dès que son location_text change — override compris. Et locked_fields +-- ne protège que les colonnes de `bands`, jamais les points. Une correction +-- disparaissait donc silencieusement à la première modification du lieu, y +-- compris quand ce lieu venait d'être verrouillé par l'admin lui-même. +-- +-- Le trigger épargne désormais les points posés par un admin. Ils restent +-- supprimables explicitement (DELETE depuis la route, reset-all ciblé), mais +-- plus par effet de bord. + +CREATE OR REPLACE FUNCTION bands_geocode_dirty() RETURNS trigger AS $$ +BEGIN + IF TG_OP = 'UPDATE' AND OLD.location_text IS DISTINCT FROM NEW.location_text THEN + DELETE FROM band_locations + WHERE ma_id = NEW.ma_id + AND COALESCE(geocode_provider, '') <> 'admin'; + END IF; + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +DROP TRIGGER IF EXISTS trg_bands_geocode_dirty ON bands; +CREATE TRIGGER trg_bands_geocode_dirty +AFTER UPDATE OF location_text ON bands +FOR EACH ROW EXECUTE FUNCTION bands_geocode_dirty(); diff --git a/apps/api/src/adminRoutes.js b/apps/api/src/adminRoutes.js index 05f96c8..ee40db4 100644 --- a/apps/api/src/adminRoutes.js +++ b/apps/api/src/adminRoutes.js @@ -624,6 +624,9 @@ export default async function adminRoutes(fastify, opts) { geocode_tries_geo=0, geocode_tries_llm=0, geocode_next_at=now(), updated_at=now() WHERE is_country_only = FALSE + -- Les saisies manuelles sont épargnées : elles n'ont pas de source + -- automatique à re-jouer, les effacer les perd définitivement. + AND COALESCE(geocode_provider, '') <> 'admin' `); const count = r.rowCount; await pool.query( @@ -882,8 +885,9 @@ export default async function adminRoutes(fastify, opts) { UPDATE band_locations SET geocode_status='queued', geocode_tries_geo=0, geocode_error=NULL, geocode_next_at=now(), updated_at=now() - WHERE geocode_status = 'error' - OR (geocode_status = 'processing' AND updated_at < now() - interval '5 minutes') + WHERE (geocode_status = 'error' + OR (geocode_status = 'processing' AND updated_at < now() - interval '5 minutes')) + AND COALESCE(geocode_provider, '') <> 'admin' `); const count = r.rowCount; await pool.query( @@ -929,6 +933,9 @@ export default async function adminRoutes(fastify, opts) { geocode_query=NULL, geocode_error=NULL, geocode_next_at=now(), updated_at=now() WHERE geocode_status = ANY($1::text[]) AND is_country_only = FALSE + -- Idem reset-all : include_done englobait les corrections manuelles, + -- que rien ne permet de reconstituer. + AND COALESCE(geocode_provider, '') <> 'admin' `, [statuses]); const count = r.rowCount; await pool.query( diff --git a/apps/api/test/integration/migrate.integration.test.js b/apps/api/test/integration/migrate.integration.test.js index 2a0b292..00bd68a 100644 --- a/apps/api/test/integration/migrate.integration.test.js +++ b/apps/api/test/integration/migrate.integration.test.js @@ -137,3 +137,58 @@ describe("migrate.js", () => { } }, 60000); }); + +describe("triggers de bands — comportement réel", () => { + beforeAll(async () => { + await migrate(); + await client.query("DELETE FROM band_locations"); + await client.query("DELETE FROM bands"); + }, 120000); + + it("updated_at ne bouge pas sur un upsert sans changement réel", async () => { + // Le crawler fait un ON CONFLICT DO UPDATE sans WHERE : Postgres exécute + // l'UPDATE pour chaque ligne vue, même identique. Quand le trigger bumpait + // updated_at sans condition, get_bands_to_enrich considérait toute la table + // comme « à ré-enrichir » après chaque crawl (migration 016). + await client.query( + `INSERT INTO bands (ma_id, name, country, location_text) VALUES (901, 'A', 'FR', 'Paris')` + ); + const avant = (await client.query(`SELECT updated_at FROM bands WHERE ma_id=901`)).rows[0].updated_at; + + await client.query(` + INSERT INTO bands (ma_id, name, country, location_text) VALUES (901, 'A', 'FR', 'Paris') + ON CONFLICT (ma_id) DO UPDATE SET + name = COALESCE(EXCLUDED.name, bands.name), + country = COALESCE(EXCLUDED.country, bands.country), + location_text = COALESCE(EXCLUDED.location_text, bands.location_text) + `); + const apres = (await client.query(`SELECT updated_at FROM bands WHERE ma_id=901`)).rows[0].updated_at; + expect(apres).toEqual(avant); + }); + + it("updated_at bouge sur un changement réel", async () => { + const avant = (await client.query(`SELECT updated_at FROM bands WHERE ma_id=901`)).rows[0].updated_at; + await client.query(`UPDATE bands SET genre='Black Metal' WHERE ma_id=901`); + const apres = (await client.query(`SELECT updated_at FROM bands WHERE ma_id=901`)).rows[0].updated_at; + expect(apres.getTime()).toBeGreaterThan(avant.getTime()); + }); + + it("changer le lieu efface les points automatiques mais épargne la saisie admin", async () => { + // Une correction manuelle (provider='admin', confiance 1.0) ne peut pas être + // régénérée : le trigger de nettoyage ne doit pas l'emporter au passage + // (migration 018). + await client.query(` + INSERT INTO band_locations + (ma_id, step_order, location_raw, is_country_only, lat, lon, geocode_status, geocode_provider, geocode_confidence) + VALUES (901, 0, 'Paris', FALSE, 48.85, 2.35, 'done', 'geoapify', 0.9), + (901, -1, 'override', FALSE, 48.86, 2.34, 'done', 'admin', 1.0) + `); + + await client.query(`UPDATE bands SET location_text='Lyon' WHERE ma_id=901`); + + const r = await client.query( + `SELECT geocode_provider FROM band_locations WHERE ma_id=901 ORDER BY geocode_provider` + ); + expect(r.rows.map((x) => x.geocode_provider)).toEqual(["admin"]); + }); +});