feat(access-control): human-readable group subscription names

[dkoo]

Summary

Adds an editable field to set human-readable names for Group subscriptions (which defaults to <subscription owner>'s Group), and adds admin UX enhancements for the WooCommerce > Subscriptions list view:

Subscription edit page:

Subscriptions list view:

• Group name in Subscription column: Group-enabled subscriptions show <group name> (<x> of <y> members) instead of the standard #<id> for <name> format, with y showing the member limit or "unlimited" if no limit is set.
• Group subscription filter dropdown: A new dropdown filter alongside the existing product/customer/payment method filters with options for "Group subscriptions" and "Non-group subscriptions". Captures subscriptions with group enabled directly and those inheriting group status from their (non-variable) product, while respecting subscription-level opt-outs. Supports both HPOS and legacy CPT modes.
• Group name in search: The group name meta is now searchable from the subscription search box, covering the HPOS orders table, the WCS data store search, and standard WP_Query searches.
• Unit tests: 4 new tests covering the group name setting (default name, custom override, update-and-read-back, empty fallback).

Group filter precedence

The filter respects the same precedence as get_subscription_settings():

1. Subscription meta enabled = yes → included (explicit opt-in)
2. Subscription meta enabled = no → excluded (explicit opt-out overrides product)
3. No subscription meta + product meta enabled = yes → included (inheritance)
4. No subscription meta + no product meta → excluded

Variable parent products are skipped in the inheritance path since variations have their own independent group settings.

Performance

• The group subscription ID list is cached in a 5-minute transient (newspack_group_subscription_ids)
• Cache is invalidated when a subscription's or product's group settings change
• Queries use WCS API functions (wcs_get_subscriptions, wcs_get_subscriptions_for_product) which properly handle meta queries in both CPT and HPOS storage modes

Test plan

Prerequisites

• A Newspack site with WooCommerce Subscriptions active
• At least one subscription product with "Group subscription" enabled
• At least one active subscription with a custom group name set and members added
• At least one regular (non-group) subscription

Subscription column

• Navigate to WooCommerce > Subscriptions
• Verify group-enabled subscriptions show the group name and member count (e.g. "Acme Newsroom (2 of 10 members)")
• Verify subscriptions with no member limit show "unlimited" (e.g. "Test Group (0 of unlimited members)")
• Verify non-group subscriptions still show the standard format (#ID for Name)
• Verify the group name links to the subscription edit page

Group filter

• Verify a "Group subscriptions" / "Non-group subscriptions" dropdown appears in the filter bar
• Select "Group subscriptions" and verify only group-enabled subscriptions appear (including those inheriting group status from their product)
• Select "Non-group subscriptions" and verify group subscriptions are excluded
• Verify the filter works in combination with other filters (product, customer, etc.)
• Reset to "All subscriptions" and verify all subscriptions appear again
• Set a subscription's group enabled setting to "no" while its product is group-enabled - verify it's excluded from "Group subscriptions"
• Test with both HPOS enabled and disabled sites

Search

• Search for a group subscription by its custom group name
• Verify the subscription appears in search results
• Search for a term that does not match any group name and verify it does not return false positives

Unit tests

• Run n test-php --filter Test_Group_Subscriptions and verify all 49 tests pass

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds support for a human-readable “Group subscription name” on WooCommerce Subscriptions, and enhances the subscriptions admin list to display/filter/search using that group context.

Changes:

• Adds a name setting to group subscription settings, including admin edit UI and save handling.
• Customizes the Subscriptions list “Subscription” column for group subscriptions and adds a group/non-group filter.
• Extends subscription search to include the group name meta key, and adds unit tests for name behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 7 comments.

File	Description
includes/plugins/woocommerce-subscriptions/group-subscription/class-group-subscription-settings.php	Adds group name setting, list-table display/filter/search hooks, and query helpers for group subscriptions.
tests/unit-tests/content-gate/group-subscriptions.php	Adds unit tests covering default/custom/updated/empty-fallback group name behavior.
tests/mocks/wc-mocks.php	Extends the WC_Subscription test mock with get_formatted_billing_full_name() used by the new default-name logic.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[dkoo]

Note: pushed fixes to handle some edge cases:

• 5e9f22f fixes querying for group and non-group subscriptions on sites not using HPOS
• 6acdadf fixes querying for product variations—these should not inherit Group settings from their parent product or any other sibling variations
• 5772d59 fixes an edge case where a subscription has the "enabled" setting set to "no" when its product is set to "yes"—the subscription-level "no" setting should override the product-inherited setting.

[adekbadek]

Tested in an isolated env (CPT and HPOS) — column rendering, filter dropdown (Group / Non-group / Reset), and group-name search all behave per the test plan. Precedence works end-to-end: a subscription with _newspack_group_subscription_enabled = no against a group-enabled product is correctly excluded from Group subscriptions; subscriptions inheriting from the product are correctly included. Default-name fallback (<owner>’s Group) renders, custom names persist and are searchable. CI green on the latest commit. Outcome: comments only — no blockers.

Automated reviewers used: deep-review (Opus) and standards-review (superpowers:code-reviewer). Codex was unavailable (CLI not installed locally) so no third opinion this round.

Most worth addressing before merge:

1. Perf scaling of get_group_subscription_ids() — three full-table scans with posts_per_page => -1 per cache miss. Inline note on the function.
2. Cache invalidation — busts on every subscription save (because the meta box always posts enabled), and there is no hook for trash/delete. Inline note.
3. Regex rewrite of WP's search WHERE — fragile and silently no-ops on shape changes; also untested.
4. Test coverage — the four new tests cover the name setting only; the precedence rules and search injection (the most subtle code in the diff) are not exercised.
5. wp_kses_post( woocommerce_wp_text_input(...) ) — woocommerce_wp_text_input() echoes and returns null, so the kses wrapper is a no-op. On PHP 8.1+ it triggers preg_replace(): Passing null to parameter #3 ... deprecated — I observed this live in debug.log while the meta box was rendering. Pre-existing pattern in this file, but the PR extends it.

Rest is polish (a11y label on the filter dropdown, missing cache invalidation on subscription delete, the column markup replacing rather than appending, brittle $_GET['page'] guard for HPOS search, DRY of the two near-identical filter methods, and a few nits) — all in inline notes.

Verified browser screenshots and a screen recording are saved locally; happy to attach if useful.

[adekbadek]

Perf scaling. Each cache miss runs three queries with posts_per_page => -1 and subscription_status => 'any': subs with enabled=yes, subs with enabled=no, and all product/product_variation posts with enabled=yes, then loops wcs_get_subscriptions_for_product() per product. On a publisher with thousands of subscriptions/products this stalls the admin list. Consider building the ID list in a single SQL/joined-meta query, or skipping the full ID list for non-group mode and using meta_query with compare = NOT EXISTS / value = no instead.

(Combined with the cache-invalidation note below, this is the most worth-fixing finding — every save invalidates the cache, so the cold path is hit constantly.)

[dkoo]

Deferring this for now. Combined with the cache-invalidation fix in e981408 (which addresses the cache being busted on every subscription save), the 5-minute transient should hold for the full TTL on most installs and the cold path becomes much rarer.

Re-architecting around a single joined query would mean dropping the WCS API helpers (wcs_get_subscriptions, wcs_get_subscriptions_for_product) which are useful for correctness across CPT/HPOS modes. The NOT EXISTS shortcut for non-group mode also doesn't cover the product-inheritance + subscription-level opt-out cases cleanly.

Will track this as a follow-up if it shows up as an actual bottleneck on a publisher with many group subscriptions.

[adekbadek]

Cache invalidation is too eager. The meta box always posts _newspack_group_subscription_enabled (whether checked or not), so isset( $settings['enabled'] ) is always true on subscription save. The transient is busted on every save, even when the value didn't change — defeating the 5-min TTL and exposing the perf cost in get_group_subscription_ids() above.

Clear only when the value actually changed (compare against $previous_settings['enabled']).

[dkoo]

Fixed in e981408. Two bugs uncovered while tracing this:

1. update_subscription_settings() was comparing wc_bool_to_string($value) ('yes'/'no') against $previous_settings[$key] (bool), so for the enabled key the strict comparison was always true and $should_save flipped on every save — even when nothing changed.
2. The cache-clear condition was isset( $settings['enabled'] ), which was always true since the meta box always passes the key.

Now we track which keys actually changed (after normalizing both sides of the comparison) and only invalidate the cache if enabled is among them.

[adekbadek]

Missing cache invalidation on subscription delete/trash. When a group subscription is trashed/deleted, the cached ID list lingers for up to 5 minutes — Group subscriptions will still match against IDs that no longer exist. Consider adding wp_trash_post / before_delete_post hooks (gated on 'shop_subscription' === get_post_type( $post_id )) to clear the transient.

[dkoo]

Fixed in e981408. Added four hooks to cover both storage modes:

• wp_trash_post and before_delete_post (CPT) — gated on 'shop_subscription' === get_post_type( $post_id )
• woocommerce_trash_subscription and woocommerce_delete_subscription (HPOS)

[adekbadek]

Fragile search rewrite. WP_Query::parse_search() produces clauses shaped differently with exact, sentence, multi-term, or other plugins' posts_search filters running before this one — the exact bracketing has shifted between WP versions. When neither regex matches, preg_replace silently returns the original $search unchanged: search-by-group-name fails quietly with no signal.

Safer alternatives:

• Wrap the original clause from outside: ( {$search} OR ( np_group_name.meta_value LIKE %s ) ).
• Or run a separate ID lookup and inject via pre_get_posts post__in.

Also worth covering this code path with a unit test — it's the trickiest piece in the diff and currently has zero coverage.

[dkoo]

Fixed in e981408. Switched to wrapping the existing clause from outside:

$inner  = preg_replace( '/^\s*AND\s+/', '', $search );
$search = " AND ( {$inner} OR {$or_clause} ) ";

This is robust against exact/sentence/multi-term and other plugins' posts_search filters running before ours, since we don't rely on the inner shape.

Also added five unit tests covering: non-subscription queries (skip), empty search (skip), simple clause wrapping, multi-term clauses, and special-character escaping.

[adekbadek]

i18n nit on the unnamed-owner fallback.

With a billing name → "Jane Doe’s Group". With no billing name → "Unnamed Group" (no ’s). The asymmetry is harmless, but composing the phrase in PHP via sprintf( '%s Group', ... ) means translators can't reorder X's Group → Group de X in other locales — only the trailing word can vary.

Consider making the entire phrases translatable instead, e.g.:

$settings['name'] = $owner_name
    ? sprintf( __( '%s’s Group', 'newspack-plugin' ), $owner_name )
    : __( 'Unnamed group', 'newspack-plugin' );

Minor: the meta is fetched twice on the previous two lines — assigning to a local first would help readability.

[dkoo]

Fixed in e981408. Made the whole phrases translatable per your suggestion:

if ( $name_meta ) {
    $settings['name'] = $name_meta;
} elseif ( $owner_name ) {
    $settings['name'] = sprintf( __( '%s’s Group', 'newspack-plugin' ), $owner_name );
} else {
    $settings['name'] = __( 'Unnamed group', 'newspack-plugin' );
}

Also fetching each meta value into a local once instead of twice.

[adekbadek]

a11y: filter dropdown is missing an accessible label. Other WC filters in the same toolbar use <label class="screen-reader-text" for="…">. Worth matching here for parity (and screen-reader users):

printf( '<label class="screen-reader-text" for="_newspack_group_subscription">%s</label>', esc_html__( 'Filter by group subscription status', 'newspack-plugin' ) );

[dkoo]

Fixed in 24bfee4 — added the <label class="screen-reader-text"> for the dropdown to match the pattern used by other WC filters in the toolbar.

[adekbadek]

HPOS guard is brittle. Coupling to the literal page slug wc-orders--shop_subscription means a future WC slug change silently disables group-name search — and AJAX/REST contexts that don't carry page in the query string skip the filter entirely. Worth at least a // @TODO replace with a stabler signal if WC exposes one. comment, or hooking off current_screen instead.

[dkoo]

Fixed in e50cf8f. Now uses get_current_screen() with wcs_get_page_screen_id( 'shop_subscription' ) — same pattern as admin_enqueue_scripts in this file. Returns the meta keys unchanged in AJAX/REST contexts (where get_current_screen() is null), which is the correct no-op since those aren't subscription list-table searches.

[adekbadek]

DRY. filter_subscriptions_by_group() and filter_subscriptions_by_group_cpt() are ~30 lines of mirrored post__in/post__not_in logic — only the early guard differs. Extract a private helper that takes array $args and returns the mutated args; both filters call into it.

[dkoo]

Fixed in e50cf8f. Extracted a private apply_group_filter( $args, $filter, $group_ids ) helper. Both filter_subscriptions_by_group() (HPOS) and filter_subscriptions_by_group_cpt() (CPT) keep their distinct entry guards and call into the shared helper for the merge logic.

[adekbadek]

Test coverage gap. The four new tests exercise the name setting (default fallback / custom override / update-and-read-back / empty fallback). None of the substantively new behavior is covered:

• The filter precedence rules: subscription yes overrides product, subscription no overrides product inheritance, variable parents are skipped, etc.
• get_group_subscription_ids() itself.
• Cache invalidation paths.
• Search injection.

Per the project's TDD/spec philosophy, the precedence logic in particular is the highest-value addition — it's the most subtle code in this PR and the most likely to regress quietly.

[dkoo]

Partially addressed in e50cf8f — added two tests covering the transient cache hit path and clear_group_subscription_ids_cache() invalidation.

The full precedence-rule tests (subscription yes overrides product, no overrides product inheritance, variable parents skipped, etc.) require real shop_subscription and product posts in the test database, since wcs_get_subscriptions() and wcs_get_subscriptions_for_product() query the actual posts/order tables. Our current tests/mocks/wc-mocks.php stores subscriptions in a $subscriptions_database global that those WCS functions don't see.

I'll open a follow-up issue to build out the integration test infrastructure needed for end-to-end coverage of get_group_subscription_ids(). Agreed it's the most subtle code in this PR and deserves real coverage.

[dkoo]

Issue created: NPPD-1496

[adekbadek]

Nit: the ternary is a no-op. trim( '' ) already returns '', so return $name ? $name : ''; simplifies to return $name; (or just return trim( "$first $last" ); directly).

[dkoo]

Fixed in e50cf8f. Simplified to return trim( "$first $last" ); — the ternary was a workaround for a phpcs rule against short ternaries (?:) but isn't needed at all since trim('') already returns ''.

[dkoo]

@adekbadek thanks for the review and feedback! I've pushed some changes to address your comments, so this is ready for a re-review.