[DimitrovK]

A few things in plan management didn't work the way the admin form says they do.

What was wrong

• features got an array cast in feat: Add plan-based feature limits (quotas) #351 (the limits PR), but the column is a string, the seeder writes "A, B, C", the admin TagsInput uses a comma separator and both the pricing section and checkout do explode(',', $plan->features). So:
  • seeded plans read back as null: the admin form opens with no features and saving the plan wipes them, and the pricing page renders one empty bullet
  • plans saved from admin since then are stored double-encoded ("A,B" with the quotes)
  • plans created with an array (Plan::create(['features' => [...]]), like PlanFeatureLimitsTest does) break the pricing page with explode(): Argument #2 must be of type string, array given
• Limits entered in admin are strings. getLimit() only treats the integer -1 as unlimited, so "-1" became a limit of -1 and canUseFeature() always returned false, the opposite of "explicitly unlimited". An empty value became 0 (disabled), though the form says "Leave empty for unlimited". A null default in config/limits.php (documented as unlimited) also became 0. HasPlanFeatures had its own copy of the same logic.
• The subscriptions plan_id foreign key cascades, so deleting a plan from the table, the bulk action or the edit page silently deleted every subscription on it, active ones included.
• Nothing called Plan::clearCache(), so admin edits took up to 30 minutes to show on the pricing page and in subscribedToPlan(). clearCache() also only forgot names still in the table, so a renamed or deleted plan's wave_plan_{name} entry stayed.

Changes

• features is a comma-separated string again, through an accessor/mutator instead of the cast. JSON values written while the cast was there (["A","B"] or "A,B") are read back as the comma string and rewritten on the next save, so there's no migration. Arrays passed in are joined. New featureList() returns the trimmed, non-empty list and both views use it.
• Plan::normalizeLimit(): null, empty and -1/"-1" mean unlimited, anything else is an int. getLimit() and the trait's plan and default lookups all go through it. The limits field validates values as empty, -1 or a whole number of 0 or more.
• Plan delete actions (row, bulk, edit page) refuse when the plan has any subscriptions and show a notification suggesting turning off "Active" instead. Bulk delete refuses the whole selection.
• Plan clears its caches on saved and deleted, including the previous name.

Worth a look

• Going back to a string means code that loops over $plan->features as an array would need featureList(). Nothing in the repo does.
• Any subscription blocks deletion, including cancelled ones, since those rows are billing history wave:stats reads. The guard is admin only: the cascade FK is unchanged, so $plan->delete() in code still cascades. Changing it to restrict would mean a migration rebuilding the FK, happy to do that separately if you want it.
• hasLimit() still means "key is defined" (true for -1 or empty). Nothing outside tests calls it, so I didn't change its meaning.

Tests

PlanAdminTest: seeded features load in the admin form; admin-saved, array and old JSON features render on the pricing page; -1, "-1", empty and null defaults count as unlimited and 0 as disabled; invalid limits are rejected; row, bulk and edit page deletes are refused for plans with subscriptions and still work for plans without; caches are cleared after edits and deletes. They fail on main except the "still deletes unused plans" guard. Full suite passes serial and --parallel.

Changelog line, if useful: "Plan features from the seeder or older versions show correctly again, -1 and empty limits mean unlimited as documented, plans with subscriptions can no longer be deleted from the admin, and plan changes show up immediately."

[bobbyiliev]

nice one, thanks Kalin! the features cast regression was a sneaky one. couple of small notes inline, nothing blocking

[bobbyiliev]

nice, saved fires before syncOriginal() so the old name is still there here, good catch with the rename case

[bobbyiliev]

nit: a non-numeric value from an older row like "ten" still ends up as 0 (disabled) here. probably fine now that the form validates it, just mentioning

[bobbyiliev]

re the FK question in the description: I'd keep the cascade as is for now, the admin guard covers the real world case. no need for the migration