Remove unused Admin::PlansFinder and run plan limits 404 specs on CE

V K Adhithiya Kumarrequested to merge
gitlab-community/gitlab-org/gitlab:dev-adhithiya/631753-remove-admin-plans-finder into master

What does this MR do and why?

Follow-up from !257670 (merged), addressing both non-blocking threads raised there by @marcogreg.

Remove Admin::PlansFinder

The current_plan helper in API::Admin::PlanLimits was the finder's only caller. Once that helper switched to SystemDefined::Plan the finder became dead code. A grep of the repository finds only the class, its spec, and allowlist and todo entries.

Removed:

  • app/finders/admin/plans_finder.rb
  • spec/finders/admin/plans_finder_spec.rb
  • The matching entries in spec/support/finder_collection_allowlist.yml, spec/support/rspec_order_todo.yml, .rubocop_todo/gitlab/bounded_contexts.yml, .rubocop_todo/rspec/instance_variable.yml, and .rubocop_todo/style/inline_disable_annotation.yml

Run the "no plans row" specs on both editions

The two when the plan has no plans row contexts in spec/requests/api/admin/plan_limits_spec.rb requested premium, which is not in all_plans on CE, so they carried an if: Gitlab.ee? guard. The 404 branch they cover is CE code, so the as-if-foss run never exercised it.

Each context now deletes the default plans row in a before block with Plan.by_name(Plan::DEFAULT).delete_all and requests default instead. The guard is gone and both editions run the examples.

Why delete_all on a relation rather than plan.delete: the plan record is a let_it_be shared across examples, and delete would mark that shared object as destroyed and frozen. The example transaction restores the row afterwards, and both foreign keys to plans are ON DELETE CASCADE, so the delete cannot fail on a constraint. Nothing in the plan limits request path calls Plan.default, which would otherwise recreate the row before the request reached the 404 branch.

How to verify

bin/rspec spec/requests/api/admin/plan_limits_spec.rb
FOSS_ONLY=1 bin/rspec spec/requests/api/admin/plan_limits_spec.rb

Both runs should include the two when the plan has no plans row examples and pass.

MR acceptance checklist

  • This MR has been reviewed against the MR acceptance checklist.
  • This contribution was prepared with the help of an AI assistant. I reviewed the changes and take responsibility for them.

Closes #631753 (closed)

Merge request reports