Trashed live donations remain in default Donations list because live-mode OR is not grouped

[g3sf]

Bug description

In GiveWP 4.16.3, trashing a live donation succeeds, but the trashed donation remains in the default (unfiltered) Donations list after a full page reload.

The row status changes to Trash, the trash endpoint returns HTTP 200, and the donation statistics correctly exclude the trashed record. However, the default donations endpoint still returns that row.

Environment

• WordPress 7.0.1
• GiveWP 4.16.3
• New Donations list table
• Live donation mode (testMode=false)

The same query structure is still present on the current develop branch.

Steps to reproduce

1. Create or use a live donation.
2. In GiveWP > Donations, use the row action to trash it.
3. Confirm the trash action.
4. Reload the Donations page with no status filters active.
5. Observe the request:

   GET /wp-json/give-api/v2/admin/donations?page=1&perPage=30&sortColumn=id&sortDirection=desc&locale=en-US&testMode=false
   

6. The trashed donation is still returned and rendered with status Trash.

Expected behavior

The default Donations list should exclude donations with post_status = trash. Trashed donations should only appear when the explicit Show trashed filter is enabled.

Actual behavior

The default list includes trashed live donations. Because no status=trash filter is active, DonationRowActions also renders Trash again instead of Restore / Delete for the already-trashed row.

Technical diagnosis

ListDonations::getWhereConditions() correctly adds:

$query->where('post_status', DonationStatus::TRASH, '<>');

But the default live-mode branch later adds an ungrouped OR:

$query->whereIsNull('give_donationmeta_attach_meta_mode.meta_value')
    ->orWhere('give_donationmeta_attach_meta_mode.meta_value', DonationMode::TEST, '<>');

This produces logic equivalent to:

WHERE post_type = 'give_payment'
  AND post_status <> 'trash'
  AND mode IS NULL
   OR mode <> 'test'

Since AND binds more tightly than OR, any live-mode row satisfying mode <> 'test' is admitted even when its status is trash.

Source:

givewp/src/Donations/Endpoints/ListDonations.php

Lines 264 to 332 in 5093236

	$hasWhereConditions = $search || $start || $end || $campaignId || $subscriptionId || $donor || $status;
	$query->where('post_type', 'give_payment');
	if (!empty($status)) {
	$query->whereIn('post_status', $status);
	} else {
	// Default behavior: exclude trash donations
	$query->where('post_status', DonationStatus::TRASH, '<>');
	}
	if ($search) {
	if (ctype_digit($search)) {
	$query->where('id', $search);
	} elseif (strpos($search, '@') !== false) {
	$query
	->whereLike('give_donationmeta_attach_meta_email.meta_value', $search);
	$dependencies[] = DonationMetaKeys::EMAIL();
	} else {
	$query
	->whereLike('give_donationmeta_attach_meta_firstName.meta_value', $search)
	->orWhereLike('give_donationmeta_attach_meta_lastName.meta_value', $search);
	$dependencies[] = DonationMetaKeys::FIRST_NAME();
	$dependencies[] = DonationMetaKeys::LAST_NAME();
	}
	}
	if ($donor) {
	if (ctype_digit($donor)) {
	$query
	->where('give_donationmeta_attach_meta_donorId.meta_value', $donor);
	$dependencies[] = DonationMetaKeys::DONOR_ID();
	} else {
	$query
	->whereLike('give_donationmeta_attach_meta_firstName.meta_value', $donor)
	->orWhereLike('give_donationmeta_attach_meta_lastName.meta_value', $donor);
	$dependencies[] = DonationMetaKeys::FIRST_NAME();
	$dependencies[] = DonationMetaKeys::LAST_NAME();
	}
	}
	if ($campaignId) {
	$query
	->where('give_donationmeta_attach_meta_campaignId.meta_value', $campaignId);
	$dependencies[] = DonationMetaKeys::CAMPAIGN_ID();
	}
	if ($subscriptionId) {
	$query
	->where('give_donationmeta_attach_meta_subscriptionId.meta_value', $subscriptionId);
	$dependencies[] = DonationMetaKeys::SUBSCRIPTION_ID();
	}
	if ($start && $end) {
	$query->whereBetween('post_date', $start, $end);
	} elseif ($start) {
	$query->where('post_date', $start, '>=');
	} elseif ($end) {
	$query->where('post_date', $end, '<=');
	}
	if ($hasWhereConditions) {
	$query->havingRaw('HAVING COALESCE(give_donationmeta_attach_meta_mode.meta_value, %s) = %s', DonationMode::LIVE, $testMode ? DonationMode::TEST : DonationMode::LIVE);
	} elseif ($testMode) {
	$query->where('give_donationmeta_attach_meta_mode.meta_value', DonationMode::TEST);
	} else {
	$query->whereIsNull('give_donationmeta_attach_meta_mode.meta_value')
	->orWhere('give_donationmeta_attach_meta_mode.meta_value', DonationMode::TEST, '<>');
	}

Suggested correction

Group the live-mode alternatives so they remain subordinate to the post type and status clauses, for example:

$query->where(function ($query) {
    $query
        ->whereIsNull('give_donationmeta_attach_meta_mode.meta_value')
        ->orWhere('give_donationmeta_attach_meta_mode.meta_value', DonationMode::TEST, '<>');
});

It would also be safer for DonationRowActions to consider item.status when choosing between Trash and Restore/Delete, rather than relying only on the active table filter:
https://github.com/impress-org/givewp/blob/50932367b7a0c87e640179822d17bde0b815a194/src/Donations/resources/components/DonationRowActions.tsx