middleware/wordpress: verify merged-status tolerance across all consumers

CHANGELOG.md

@@ -2,6 +2,68 @@
 
 ## Unreleased
 
+- Verified `merged`-status tolerance across every consumer of `approval_status`
+  ahead of the reconciliation command (#310) — Track A2 of epic #307,
+  guardrail G4 of `docs/CANONICAL_IDENTITY_RFC.md` (#309). Audit checklist:
+  - **Safe as-is (exact-match/whitelist filters, verified with new tests where
+    applicable):** badge generation (`badges/runner.py`, double-filtered
+    server- and client-side against `{"approved"}` for both the bulk and
+    targeted `--chm-id` paths); `sync --type approvals`
+    (`sync/manager.py`, exact `!= APPROVAL_STATUS["APPROVED"]` skip for a
+    targeted sync and a server-side `approval_status=approved` filter for the
+    bulk fetch); `church_teams_export.py`'s force-resend categorization
+    (`_handle_force_resend`, an explicit whitelist of
+    pending/pending_approval/reapproval_required/validated); the `[vaysf_churches]`
+    and `[vaysf_badges]` shortcodes and the admin dashboard stat tiles
+    (`class-vaysf-statistics.php`, exact `approval_status = 'approved'`/`'denied'`
+    SQL matches); the REST `process-token` handler
+    (`class-vaysf-rest-approvals.php`), which already rejects any
+    non-`'pending'` approval — including `merged` — with a generic
+    "already processed" error, so a stale pastor-approval email token can't
+    resurrect a tombstoned row; validation-issue sync
+    (`sync/participants.py`), which is keyed by `participant_id` and never
+    branches on `approval_status`, and which — once #308's alias resolution
+    is live — never revisits a stale/merged `chm_id` during normal sync in
+    the first place.
+  - **Fixed (misrendered "Merged" as "Pending", or worse):** the admin
+    participants list, admin approvals list, and the `[vaysf_participants]`
+    shortcode (found during this audit; shares the same status-class helper)
+    all fell through their `switch`/`case` status-styling to the `pending`
+    bucket for any unrecognized status — a tombstoned duplicate would have
+    displayed as "awaiting action". Added an explicit `merged` case (and a
+    `.status-merged` style) to `get_status_class()` in
+    `includes/shortcodes.php`, `vaysf_format_approval_status()` in
+    `includes/functions.php`, and the inline switches in
+    `admin/class-vaysf-admin-participants.php` and
+    `admin/class-vaysf-admin-approvals.php`.
+  - **Fixed (functional hazard, not just cosmetic):** the admin Approvals
+    page's "Resend Email" action
+    (`admin/class-vaysf-admin-approvals.php` -> `vaysf_resend_approval_email()`
+    in `includes/functions.php`) unconditionally reset `approval_status` back
+    to `'pending'` and minted a fresh token/email — for *any* approval row,
+    including a `merged` one. Clicking Resend on a tombstoned duplicate would
+    have undone the reconciler's tombstone (RFC §4.3 step 3: "so an old email
+    token can't resurrect it") and mailed the pastor a stale approval link.
+    The row's "Resend Email" button is now hidden for `merged` approvals, and
+    the `action=resend` handler independently rejects the request with a
+    notice, so a direct URL hit can't bypass the UI guard either. Both guards
+    check the **participant's** status as well as the approval's, and both
+    queries now select `p.approval_status`: `apply-aliases` (#310) tombstones
+    the participant *before* its approval rows, so a partially-failed
+    reconciliation leaves the participant `merged` while its approval is still
+    `pending` — guarding on the approval row alone would let precisely that
+    row be resent, which is the resurrection case this guard exists to stop.
+  - The plugin schema needs no migration for any of this — `approval_status`
+    is already a plain VARCHAR(50) (`vaysf.php`) and the REST participant/
+    approval endpoints sanitize `approval_status` with `sanitize_text_field()`
+    rather than validating against an enum, so `merged` writes through with
+    zero plugin changes, exactly as the RFC anticipated.
+  - 6 new mock tests (`test_sync_manager.py`, `test_badges.py`,
+    `test_church_teams_export.py`) pin the middleware-side tolerance; the 4
+    touched PHP files pass `php -l`. No PHPUnit harness exists yet for the
+    plugin (tracked separately), so the PHP fixes are verified by lint plus
+    manual review of the audited call paths.
+
 - Hotfix 1.1.14: Fixed the public/admin `Approved Participants` stat
   under-reporting real approved-athlete counts (#181). `VAYSF_Statistics::get_overall_stats()`
   was counting `sf_approvals.approval_status = 'approved'` (a pastor-approval-token/sync

middleware/tests/test_badges.py

@@ -419,6 +419,29 @@ def test_runner_filters_out_non_approved(generator):
     assert any("3139537" in p.name for p in pngs)
 
 
+def test_runner_filters_out_merged(generator):
+    """Guardrail G4 (#309): a tombstoned 'merged' duplicate never gets a badge."""
+    parts = [_participant(), _participant(chmeetings_id="999", approval_status="merged")]
+    runner, chm, wp = _make_runner(parts, generator)
+    runner.run(force=True)
+    pngs = list(generator.output_dir.glob("*.png"))
+    assert len(pngs) == 1
+    assert any("3139537" in p.name for p in pngs)
+    assert not any("999" in p.name for p in pngs)
+
+
+def test_runner_targeted_chm_id_skips_merged(generator):
+    """The single-participant (--chm-id) badge path also re-checks approval status,
+    so a merged row targeted directly still produces nothing (#309)."""
+    participant = _participant(approval_status="merged")
+    runner, chm, wp = _make_runner([participant], generator)
+
+    ok = runner.run(force=True, chm_id="3139537")
+
+    assert ok is True
+    assert not list(generator.output_dir.glob("*.png"))
+
+
 def test_runner_uses_approval_only_when_payment_status_is_unreliable(generator):
     participant = _participant(payment_status="pending")
     runner, chm, wp = _make_runner([participant], generator)

middleware/tests/test_church_teams_export.py

@@ -191,6 +191,73 @@ def test_handle_force_resend_includes_reapproval_required(mock_connectors, mocke
     assert resend_count == 1
 
 
+def test_handle_force_resend_excludes_merged(mock_connectors, mocker):
+    """Guardrail G4 (#309): a tombstoned 'merged' row must never get a fresh
+    approval email — that would resurrect a retired duplicate identity."""
+    fake_sync_manager = MagicMock()
+    fake_sync_manager.wordpress_connector.get_churches.return_value = [
+        {"church_code": "RPC", "pastor_email": "pastor@rpc.org", "church_rep_email": "rep@rpc.org"}
+    ]
+    fake_sync_manager.__enter__.return_value = fake_sync_manager
+    fake_sync_manager.__exit__.return_value = None
+
+    mocker.patch("sync.manager.SyncManager", return_value=fake_sync_manager)
+
+    exporter = ChurchTeamsExporter()
+    contacts = [{
+        "ChMeetings ID": "3634001",
+        "First Name": "Ngoc",
+        "Last Name": "Le",
+        "Church Team": "RPC",
+        "Email": "ngoc@example.com",
+        "Approval_Status (WP)": "merged",
+    }]
+
+    resend_count = exporter._handle_force_resend(
+        contacts,
+        force_pending=True,
+        force_validated1=True,
+        force_validated2=True,
+        dry_run=True,
+    )
+
+    assert resend_count == 0
+
+
+def test_handle_force_resend_targeted_merged_id_is_still_excluded(mock_connectors, mocker):
+    """Even a targeted --resend-chm-id for a merged row's ChM ID resends nothing,
+    since the merged status never enters the resend candidate set (#309)."""
+    fake_sync_manager = MagicMock()
+    fake_sync_manager.wordpress_connector.get_churches.return_value = [
+        {"church_code": "RPC", "pastor_email": "pastor@rpc.org", "church_rep_email": "rep@rpc.org"}
+    ]
+    fake_sync_manager.__enter__.return_value = fake_sync_manager
+    fake_sync_manager.__exit__.return_value = None
+
+    mocker.patch("sync.manager.SyncManager", return_value=fake_sync_manager)
+
+    exporter = ChurchTeamsExporter()
+    contacts = [{
+        "ChMeetings ID": "3634001",
+        "First Name": "Ngoc",
+        "Last Name": "Le",
+        "Church Team": "RPC",
+        "Email": "ngoc@example.com",
+        "Approval_Status (WP)": "merged",
+    }]
+
+    resend_count = exporter._handle_force_resend(
+        contacts,
+        force_pending=True,
+        force_validated1=True,
+        force_validated2=True,
+        dry_run=True,
+        target_resend_chm_id="3634001",
+    )
+
+    assert resend_count == 0
+
+
 def test_resend_logs_existing_approval_metadata(mock_connectors, mocker):
     _, wp_connector = mock_connectors
 

middleware/tests/test_sync_manager.py

@@ -2272,6 +2272,60 @@ def test_sync_approvals_targeted_skips_non_approved(sync_manager, mocker):
     mock_get_approvals.assert_not_called()
     mock_update.assert_not_called()
 
+def test_sync_approvals_targeted_skips_merged(sync_manager, mocker):
+    """Guardrail G4 (#309): a tombstoned 'merged' participant must not sync to ChMeetings."""
+    participant = {
+        "participant_id": 75,
+        "chmeetings_id": "4363699",
+        "approval_status": "merged",
+        "first_name": "Janice",
+        "last_name": "Vu",
+    }
+    mock_get_participants = mocker.patch.object(
+        sync_manager.wordpress_connector,
+        "get_participants",
+        return_value=[participant],
+    )
+    mock_get_groups = mocker.patch.object(sync_manager.chm_connector, "get_groups", return_value=[])
+    mock_add = mocker.patch.object(sync_manager.chm_connector, "add_person_to_group", return_value=True)
+    mock_get_approvals = mocker.patch.object(
+        sync_manager.wordpress_connector,
+        "get_approvals",
+        return_value=[],
+    )
+    mock_update = mocker.patch.object(
+        sync_manager.wordpress_connector,
+        "update_approval",
+        return_value=True,
+    )
+
+    result = sync_manager.sync_approvals_to_chmeetings(chm_id_to_target="4363699")
+
+    assert result is True
+    mock_get_participants.assert_called_once_with(params={"chmeetings_id": "4363699"})
+    mock_get_groups.assert_not_called()
+    mock_add.assert_not_called()
+    mock_get_approvals.assert_not_called()
+    mock_update.assert_not_called()
+
+
+def test_sync_approvals_bulk_fetch_never_requests_merged(sync_manager, mocker):
+    """The bulk approval-sync page fetch filters server-side on approval_status=approved,
+    so a 'merged' row is never even requested (#309)."""
+    mock_get_participants = mocker.patch.object(
+        sync_manager.wordpress_connector,
+        "get_participants",
+        return_value=[],
+    )
+    mocker.patch.object(sync_manager.chm_connector, "get_groups", return_value=[])
+
+    sync_manager.sync_approvals_to_chmeetings()
+
+    mock_get_participants.assert_called_once_with(
+        params={"approval_status": "approved", "page": 1, "per_page": 100}
+    )
+
+
 def test_sync_rosters_soccer_coed_exhibition(sync_manager, mocker):
     """Soccer - Coed Exhibition arrives via the other_events checkbox; the comma-split
     loop must produce a single roster row with sport_format=Team and sport_gender=Mixed,

plugins/vaysf/admin/class-vaysf-admin-approvals.php

@@ -34,19 +34,34 @@ public function display_approvals_page() {
                         $resend_id = absint($_GET['id']);
                         $approval = $wpdb->get_row(
                                 $wpdb->prepare(
-                                        "SELECT a.*, p.first_name, p.last_name, c.church_name FROM $table_approvals a JOIN $table_participants p ON a.participant_id = p.participant_id JOIN $table_churches c ON a.church_id = c.church_id WHERE a.approval_id = %d",
+                                        "SELECT a.*, p.first_name, p.last_name, p.approval_status AS participant_approval_status, c.church_name FROM $table_approvals a JOIN $table_participants p ON a.participant_id = p.participant_id JOIN $table_churches c ON a.church_id = c.church_id WHERE a.approval_id = %d",
                                         $resend_id
                                 ),
                                 ARRAY_A
                         );
-                        if ($approval) {
+                        if (!$approval) {
+                                echo '<div class="notice notice-error"><p>Approval record not found.</p></div>';
+                        } elseif (
+                                $approval['approval_status'] === 'merged'
+                                || $approval['participant_approval_status'] === 'merged'
+                        ) {
+                                // Issue #309 (guardrail G4): a merged/tombstoned approval belongs to a
+                                // retired duplicate identity. Resending would reset it to 'pending' and
+                                // mail a fresh token, resurrecting exactly what the reconciler (#310)
+                                // retired it to prevent.
+                                //
+                                // The participant's status is checked as well as the approval's, and is
+                                // the more reliable signal: apply-aliases tombstones the participant
+                                // before the approval rows, so a partially-failed run leaves the
+                                // participant 'merged' while its approval is still 'pending'. Guarding
+                                // on the approval row alone would let precisely that row be resent.
+                                echo '<div class="notice notice-error"><p>Cannot resend: this approval belongs to a merged (retired duplicate) participant.</p></div>';
+                        } else {
                                 if (vaysf_resend_approval_email($approval)) {
                                         echo '<div class="notice notice-success"><p>Approval email resent successfully.</p></div>';
                                 } else {
                                         echo '<div class="notice notice-error"><p>Failed to resend approval email.</p></div>';
                                 }
-                        } else {
-                                echo '<div class="notice notice-error"><p>Approval record not found.</p></div>';
                         }
                 }
 
@@ -56,11 +71,11 @@ public function display_approvals_page() {
 
         // Get approvals
         $approvals = $wpdb->get_results(
-            "SELECT a.*, p.first_name, p.last_name, c.church_name 
-            FROM $table_approvals a 
-            JOIN $table_participants p ON a.participant_id = p.participant_id 
-            JOIN $table_churches c ON a.church_id = c.church_id 
-            $where_clause 
+            "SELECT a.*, p.first_name, p.last_name, p.approval_status AS participant_approval_status, c.church_name
+            FROM $table_approvals a
+            JOIN $table_participants p ON a.participant_id = p.participant_id
+            JOIN $table_churches c ON a.church_id = c.church_id
+            $where_clause
             ORDER BY a.created_at DESC",
             ARRAY_A
         );
@@ -109,6 +124,9 @@ public function display_approvals_page() {
                                         case 'denied':
                                             $status_class = 'status-denied';
                                             break;
+                                        case 'merged':
+                                            $status_class = 'status-merged';
+                                            break;
                                         default:
                                             $status_class = 'status-pending';
                                             break;
@@ -121,14 +139,43 @@ public function display_approvals_page() {
                                 <td><?php echo esc_html(date('Y-m-d H:i', strtotime($approval['created_at']))); ?></td>
                                 <td><?php echo esc_html(date('Y-m-d H:i', strtotime($approval['token_expiry']))); ?></td>
                                 <td>
-                                    <a href="<?php echo admin_url('admin.php?page=vaysf-approvals&action=resend&id=' . $approval['approval_id']); ?>" class="button button-small">Resend Email</a>
+                                    <?php if ($approval['approval_status'] === 'merged' || $approval['participant_approval_status'] === 'merged') : ?>
+                                        <span class="description">Merged — resend disabled</span>
+                                    <?php else : ?>
+                                        <a href="<?php echo admin_url('admin.php?page=vaysf-approvals&action=resend&id=' . $approval['approval_id']); ?>" class="button button-small">Resend Email</a>
+                                    <?php endif; ?>
                                 </td>
                             </tr>
                         <?php endforeach; ?>
                     <?php endif; ?>
                 </tbody>
             </table>
         </div>
+        <style>
+            .approval-status {
+                display: inline-block;
+                padding: 5px 10px;
+                border-radius: 3px;
+                font-weight: bold;
+            }
+            .status-approved {
+                background-color: #d4edda;
+                color: #155724;
+            }
+            .status-denied {
+                background-color: #f8d7da;
+                color: #721c24;
+            }
+            .status-pending {
+                background-color: #e2e3e5;
+                color: #383d41;
+            }
+            .status-merged {
+                background-color: #dcdcdc;
+                color: #55595c;
+                text-decoration: line-through;
+            }
+        </style>
         <?php
     }
 }

plugins/vaysf/admin/class-vaysf-admin-participants.php

@@ -117,6 +117,9 @@ public function display_participants_page() {
                                     case 'pending_approval':
                                         $status_class = 'status-pending-approval';
                                         break;
+                                    case 'merged':
+                                        $status_class = 'status-merged';
+                                        break;
                                     case 'pending':
                                     default:
                                         $status_class = 'status-pending';
@@ -165,6 +168,11 @@ public function display_participants_page() {
             background-color: #e2e3e5;
             color: #383d41;
         }
+        .status-merged {
+            background-color: #dcdcdc;
+            color: #55595c;
+            text-decoration: line-through;
+        }
     </style>
     <?php
 }

plugins/vaysf/includes/functions.php

@@ -404,6 +404,10 @@ function vaysf_format_approval_status($status) {
             return '<span class="approval-status status-validated">' . esc_html__('Validated', 'vaysf') . '</span>';
         case 'pending_approval':
             return '<span class="approval-status status-pending-approval">' . esc_html__('Pending Approval', 'vaysf') . '</span>';
+        case 'merged':
+            // Tombstoned by the canonical-identity reconciler (Issue #310): a retired
+            // duplicate row, not an athlete awaiting a decision. Must not read as "Pending".
+            return '<span class="approval-status status-merged">' . esc_html__('Merged', 'vaysf') . '</span>';
         case 'pending':
         default:
             return '<span class="approval-status status-pending">' . esc_html__('Pending', 'vaysf') . '</span>';

plugins/vaysf/includes/shortcodes.php

@@ -1147,6 +1147,8 @@ private function get_status_class($status) {
                 return 'status-validated';
             case 'pending_approval':
                 return 'status-pending-approval';
+            case 'merged':
+                return 'status-merged';
             default:
                 return 'status-pending';
         }
@@ -1668,6 +1670,12 @@ private function include_frontend_styles() {
                     color: #383d41;
                 }
 
+                .status-merged {
+                    background-color: #dcdcdc;
+                    color: #55595c;
+                    text-decoration: line-through;
+                }
+
                 .vaysf-live-schedule-filters,
                 .vaysf-advancement-filters {
                     display: flex;