fix(massif): set reviewer on update to fix recent-changes feed

[ClemRz]

Fixes massif edits always appearing as "created" in the recent-changes feed.

Why

The change_massif DB trigger uses id_reviewer IS NULL to decide whether a row has ever been edited:

elsif NEW.is_deleted = false AND NEW.id_reviewer is null then
    type_change := 'create';
elsif NEW.is_deleted = false then
    type_change := 'update';

massif/update.js never set reviewer, so id_reviewer stayed NULL permanently. Every edit — first or hundredth — was classified as create in the feed. All other entity update controllers (cave, entrance, organization, rigging, etc.) already set reviewer: req.token.id.

What

• Add reviewer: req.token.id to the update payload in api/controllers/v1/massif/update.js
• Add a reviewer assertion to the existing update integration test

Related

• Closes Recent changes: massif edits always appear as 'created' due to missing reviewer in update controller #1769

[Paul-AUB]

Verified the fix against the actual trigger definition and the surrounding code paths:

• sql/0_triggers.sql:222-227 — change_massif() classifies as create exactly when id_reviewer IS NULL, so populating reviewer is the right fix, and it matches the entrance/update.js:19-23 pattern precisely.
• sql/0_triggers.sql:203 — histo_update_massif() sets NEW.date_reviewed := now() itself, so not setting dateReviewed in the controller is correct.
• MassifService.getConvertedDataFromClientRequest (api/services/MassifService.js:224-232) returns an explicit whitelist with no reviewer key, so the spread cannot clobber the new field.
• config/policies.js:240 — v1/massif/update is behind tokenAuth, so req.token.id is always defined.
• converters.js uses convertIfObject(source.reviewer, …), so massif responses now returning a populated reviewer instead of null is a safe, intended change.
• Other massif write paths are already correct: MassifService.setSensitivity (api/services/MassifService.js:436-440) sets reviewer + dateReviewed, and create.js correctly leaves reviewer null so the first feed entry stays create.

The test fixture creates the massif with reviewer: 2 while the acting user is user1@user1.com, so the new assertion is not vacuous — it genuinely proves the reviewer was reassigned by the update.

Nice, minimal, well-targeted fix. A few non-blocking notes below.

Suggestions (Should Consider)

1. [api/controllers/v1/massif/update.js:74-81] The inline TName.updateOne(...) block does not set reviewer, whereas the equivalent block in entrance/update.js:97-101 does. histo_update_name() copies NEW.id_reviewer into h_name (sql/0_triggers.sql:908,923), so renaming a massif currently writes a history row attributed to the previous reviewer (or NULL). There is no last_change_name trigger, so this has no effect on the recent-changes feed and is not a regression from this PR — but it is the same attribution gap the PR is closing, and it might be worth fixing in the same pass:

   const nameUpdate = { reviewer: req.token.id };

2. [test/integration/4_routes/Massifs/update.test.js:112-115] The assertion proves the column is populated, which is a good proxy, but it does not cover the behaviour the issue actually describes — that a second edit is classified as update rather than create in the feed. Consider a regression test that issues two consecutive PUTs and asserts the resulting t_last_change rows (or the /api/v1/changes/recent payload) contain an update entry for the massif. test/integration/4_routes/Changes/get-recent.test.js already exercises that endpoint and could serve as a starting point. This would lock in the fix even if someone later refactors the payload construction.

Nitpicks (Optional)

3. [api/controllers/v1/massif/update.js:15-18] cave/update.js:26-28, entrance/update.js:20-21 and rigging/update.js:23-24 all carry a short comment next to reviewer explaining that dateReviewed is handled by the SQL historisation trigger. Adding the same line here would keep the four controllers visually consistent and preempt the "shouldn't we also set dateReviewed?" question on a future read:

   const cleanedData = {
     reviewer: req.token.id,
     // dateReviewed will be updated automatically by the SQL historisation trigger
     ...MassifService.getConvertedDataFromClientRequest(req),
   };

4. [api/controllers/v1/massif/update.js:15-18] Placing reviewer before the spread matches entrance/update.js, so the current ordering is defensible on consistency grounds. That said, organization/update.js:18-22 puts it after the spread, which makes the field structurally impossible to shadow if getConvertedDataFromClientRequest ever gains a reviewer key. Purely defensive — there is no client-controlled override path today since the service returns a fixed whitelist.

[ClemRz]

All four points addressed in the follow-up commit:

1. TName.updateOne reviewer — fixed. nameUpdate now initialises with reviewer: req.token.id, so name history rows in h_name are attributed to the acting user. Also dropped the now-redundant Object.keys guard since reviewer is always present.

2. Regression test — added. Creates a null-reviewer massif (matching the real-world state at the time of the bug), issues two consecutive PUTs, and asserts reviewer is non-null after both. Direct t_last_change querying isn't possible in the test DB (Waterline migrate: drop builds tables from model definitions only — no triggers), but asserting reviewer stays non-null is both necessary and sufficient given the trigger's id_reviewer IS NULL → 'create' logic.

3. dateReviewed comment — added.

4. Reviewer after spread — applied (defensive ordering matching organization/update.js).