Fix bugs with candidates when moving election posts (#437)

* Attempted fix for candidates being left behind when moving election posts

* Fixed bug where strange behaviour when moving posts

Author: georgelgeback

routes/sub_election_router.py

@@ -1,4 +1,6 @@
 from fastapi import APIRouter, HTTPException, status
+from db_models.candidate_model import Candidate_DB
+from db_models.candidate_post_model import Candidation_DB
 from database import DB_dependency
 from db_models.sub_election_model import SubElection_DB
 from db_models.election_model import Election_DB
@@ -186,6 +188,11 @@ def move_election_post(sub_election_id: int, data: MovePostRequest, db: DB_depen
             status_code=status.HTTP_400_BAD_REQUEST, detail="New sub-election is not in the same election"
         )
 
+    if new_sub_election.sub_election_id == sub_election.sub_election_id:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST, detail="You are trying to move to the same sub-election"
+        )
+
     # Check if the post is already assigned to the new sub-election
     existing_post = (
         db.query(ElectionPost_DB)
@@ -201,14 +208,59 @@ def move_election_post(sub_election_id: int, data: MovePostRequest, db: DB_depen
     # Move the post to the new sub-election
     election_post.sub_election = new_sub_election
 
-    # Update all the candidations and candidates to point to the new sub-election
+    # Update all the candidations to point to the new sub-election
     for candidation in election_post.candidations:
         candidation.sub_election = new_sub_election
-        candidation.candidate.sub_election = new_sub_election
 
     # Update all the nominations to point to the new sub-election
     for nomination in election_post.nominations:
         nomination.sub_election = new_sub_election
 
+    candidations_to_move = list(election_post.candidations)
+    candidate_candidations: dict[int, list[Candidation_DB]] = {}
+    candidate_lookup: dict[int, Candidate_DB] = {}
+    for candidation in candidations_to_move:
+        key = candidation.candidate_id
+        candidate_candidations.setdefault(key, []).append(candidation)
+        candidate_lookup.setdefault(key, candidation.candidate)
+
+    # Reassign candidates while keeping candidations tied to the correct sub-election
+    for candidate_id, candidate_candidations_to_move in candidate_candidations.items():
+        candidate = candidate_lookup[candidate_id]
+
+        # Check if a candidate from the same user already exists in the target sub-election
+        existing_candidate = (
+            db.query(Candidate_DB)
+            .filter(Candidate_DB.sub_election_id == new_sub_election.sub_election_id)
+            .filter(Candidate_DB.user_id == candidate.user_id)
+            .one_or_none()
+        )
+
+        # True if all candidations are being moved
+        # candidate.candidations should be the same candidate mentioned
+        # in candidate_candidations_to_move, so if lengths match, all are being moved
+        moving_only_candidations = len(candidate.candidations) == len(candidate_candidations_to_move)
+
+        if existing_candidate is None and moving_only_candidations:
+            # Move the candidate wholesale
+            candidate.sub_election = new_sub_election
+            target_candidate = candidate
+        else:
+            if existing_candidate is None:
+                target_candidate = Candidate_DB(
+                    sub_election_id=new_sub_election.sub_election_id,
+                    user_id=candidate.user_id,
+                )
+                db.add(target_candidate)
+                db.flush()
+            else:
+                target_candidate = existing_candidate
+
+            for candidation in candidate_candidations_to_move:
+                candidation.candidate = target_candidate
+
+            if moving_only_candidations:
+                db.delete(candidate)
+
     db.commit()
     return new_sub_election

tests/test_elections.py

@@ -553,3 +553,93 @@ def test_move_election_post_retains_nominations(
         noms = moved_post.get("nominations", [])
         matching = [n for n in noms if n.get("nominee_email") == admin_user.email]
         assert len(matching) == 1
+
+
+def test_move_election_post_keeps_remaining_candidations_in_original_sub_election(
+    admin_token, admin_user, client, admin_post, member_post, open_election
+):
+    # Create sub-election with two posts
+    resp_sub = create_sub_election(
+        client,
+        open_election.election_id,
+        token=admin_token,
+        title_sv="Source",
+        title_en="Source",
+        post_ids=[admin_post.id, member_post.id],
+    )
+    assert resp_sub.status_code in (200, 201), resp_sub.text
+    sub_election = resp_sub.json()
+    sub_election_id = sub_election["sub_election_id"]
+
+    # Create candidations for both posts for the same user
+    resp_cand_admin = create_candidation(
+        client,
+        sub_election_id=sub_election_id,
+        post_id=admin_post.id,
+        token=admin_token,
+        user_id=admin_user.id,
+    )
+    assert resp_cand_admin.status_code in (200, 201), resp_cand_admin.text
+
+    resp_cand_member = create_candidation(
+        client,
+        sub_election_id=sub_election_id,
+        post_id=member_post.id,
+        token=admin_token,
+        user_id=admin_user.id,
+    )
+    assert resp_cand_member.status_code in (200, 201), resp_cand_member.text
+    candidate = resp_cand_member.json()
+    candidate_id = candidate["candidate_id"]
+    post_ids_before_move = {candidation["post_id"] for candidation in candidate["candidations"]}
+    assert post_ids_before_move == {admin_post.id, member_post.id}
+
+    # Destination sub-election without posts
+    resp_dest = create_sub_election(
+        client,
+        open_election.election_id,
+        token=admin_token,
+        title_sv="Destination",
+        title_en="Destination",
+    )
+    assert resp_dest.status_code in (200, 201), resp_dest.text
+    dest_sub_election = resp_dest.json()
+    dest_sub_election_id = dest_sub_election["sub_election_id"]
+
+    # Resolve election_post identifier for the admin post
+    election_post_admin = next(ep for ep in sub_election["election_posts"] if ep["post_id"] == admin_post.id)
+    election_post_id = election_post_admin["election_post_id"]
+
+    # Move election post to the destination sub-election
+    resp_move = client.patch(
+        f"/sub-election/{sub_election_id}/move-election-post",
+        json={
+            "election_post_id": election_post_id,
+            "new_sub_election_id": dest_sub_election_id,
+        },
+        headers=auth_headers(admin_token),
+    )
+    assert resp_move.status_code in (200, 204), resp_move.text
+
+    # Original sub-election should still have the candidate with the remaining post
+    resp_candidates_source = client.get(f"/candidate/sub-election/{sub_election_id}", headers=auth_headers(admin_token))
+    assert resp_candidates_source.status_code == 200, resp_candidates_source.text
+    candidates_source = resp_candidates_source.json()
+    original_candidate = next(c for c in candidates_source if c["candidate_id"] == candidate_id)
+    remaining_post_ids = {candidation["post_id"] for candidation in original_candidate["candidations"]}
+    assert remaining_post_ids == {member_post.id}
+    assert all(candidation["sub_election_id"] == sub_election_id for candidation in original_candidate["candidations"])
+
+    # Destination sub-election should have a candidate for the moved post only
+    resp_candidates_dest = client.get(
+        f"/candidate/sub-election/{dest_sub_election_id}", headers=auth_headers(admin_token)
+    )
+    assert resp_candidates_dest.status_code == 200, resp_candidates_dest.text
+    candidates_dest = resp_candidates_dest.json()
+    moved_candidate = next(c for c in candidates_dest if c["user_id"] == admin_user.id)
+    moved_post_ids = {candidation["post_id"] for candidation in moved_candidate["candidations"]}
+    assert moved_post_ids == {admin_post.id}
+    assert all(
+        candidation["sub_election_id"] == dest_sub_election_id for candidation in moved_candidate["candidations"]
+    )
+    assert moved_candidate["candidate_id"] != candidate_id