Dual-write samples to Mongo and Postgres

[igboyes]

Summary

• Step 2 of the samples → Postgres migration: every sample mutation (create, delete, finalize, update) now writes a consistent legacy_samples row plus label and subtraction join rows in the same transaction as the Mongo write. Mongo remains the read authority; no read paths change.
• Moved the rights update out of the API handler into a dual-writing update_rights data-layer method (with a new get_owner_id, replacing get_sample_owner). The endpoint now returns the full Sample it already declared instead of a partial rights projection.
• Cascade label and subtraction deletions to legacy_sample_labels / legacy_sample_subtractions atomically with their Mongo $pull.

[linear-code]

VIR-2524

[sourcery-ai]

Hey - I've found 1 issue, and left some high level feedback:

• In update_rights, the new logic treats any non-None group value as a real group (including strings like 'none'), which will now raise a ResourceConflictError instead of clearing the group as before; if clearing via 'none' or similar is still a supported API contract, consider explicitly handling that sentinel before the DB lookup.
• The join-row cleanup logic for labels and subtractions (eg, in SamplesData.update, LabelsData.delete, and SubtractionsData.delete) is now spread across several places with slightly different behaviors; consider extracting a shared helper or documenting the intended invariants to reduce the risk of subtle divergence over time.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- In `update_rights`, the new logic treats any non-`None` `group` value as a real group (including strings like `'none'`), which will now raise a `ResourceConflictError` instead of clearing the group as before; if clearing via `'none'` or similar is still a supported API contract, consider explicitly handling that sentinel before the DB lookup.
- The join-row cleanup logic for labels and subtractions (eg, in `SamplesData.update`, `LabelsData.delete`, and `SubtractionsData.delete`) is now spread across several places with slightly different behaviors; consider extracting a shared helper or documenting the intended invariants to reduce the risk of subtle divergence over time.

## Individual Comments

### Comment 1
<location path="virtool/samples/api.py" line_range="330-339" />
<code_context>
         ):
             raise APIInsufficientRights("Must be administrator or sample owner")

-        group = data.get("group")
-
-        if group is not None and group != "none":
-            async with AsyncSession(pg) as session:
-                result = await session.execute(
-                    select(SQLGroup.id).where(
-                        (SQLGroup.id == group)
-                        if isinstance(group, int)
-                        else (SQLGroup.legacy_id == group),
-                    ),
-                )
-
-                if not result.scalars().one_or_none():
-                    raise APIBadRequest("Group does not exist")
-
</code_context>
<issue_to_address>
**issue:** The previous special casing of `group == "none"` has been removed; this may change semantics for clients using that sentinel.

Previously, `group` was only resolved when `group is not None and group != "none"`, allowing clients to use the literal "none" to mean "no group" without a lookup. The new `update_rights` path treats any non-`None` `group` as an id and will try to resolve `legacy_id == "none"`, leading to `ResourceConflictError`/`APIBadRequest` for existing clients that still send "none".

If "none" is a supported client convention, consider preserving it by normalizing `group` before calling `update_rights` (e.g., mapping "none" to `None`) or explicitly handling this sentinel inside `update_rights`.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[igboyes]

Re: sharing a helper for the label/subtraction join-row cleanup across SamplesData.update, LabelsData.delete, and SubtractionsData.delete — declining. These are three different operations on different FK axes: update deletes+reinserts by sample_id (per-sample replace), while the two delete paths cascade by label_id/subtraction_id across all samples. Each is a single delete().where() on a distinct table/column with no shared logic, so a helper would add indirection without removing duplication.