[5.x] Plans::_getAllPlans() memoization broken when plans table is empty

[john-henry]

Commerce version: 5.6.2
Craft version: 5.9.20

Plans::_getAllPlans() uses a null-check to memoize results, but the memoization never activates when the craft_commerce_plans table is empty. This causes the method to issue a database query on every call within a request, and on every subsequent request, rather than querying once and caching the result.

Root cause

  // services/Plans.php                                                                                                                                                                                                                                                   
  private function _getAllPlans(): array
  {
      if ($this->_allPlans === null) {
          $plans = $this->_createPlansQuery()->all();
                                                                                                                                                                                                                                                                          
          if (!empty($plans)) {          // ← never true when table is empty                                                                                                                                                                                              
              $this->_allPlans = [];                                                                                                                                                                                                                                      
              $plans = $this->_populatePlans($plans);                                                                                                                                                                                                                     
              foreach ($plans as $plan) {
                  $this->_allPlans[$plan->id] = $plan;
              }                                                                                                                                                                                                                                                           
          }
          // _allPlans remains null — memoization never applied                                                                                                                                                                                                           
      }                                                                                                                                                                                                                                                                   
   
      return $this->_allPlans ?? [];                                                                                                                                                                                                                                      
  }       

When no plans exist, $this->_allPlans is never assigned, so the === null guard never clears on subsequent calls.

Impact

Plugin::getCpNavItem()calls getAllPlans() on every CP request to decide whether to show the Subscriptions subnav item. On a site with no subscription plans this means a redundant craft_commerce_plans query fires on every single CP request. Because Craft's CP element index triggers ~40 AJAX sub-requests per page load, a single page visit produces ~40 identical database queries that should produce zero after the first.

Expected behaviour

_getAllPlans() queries the database exactly once per request regardless of whether the plans table is empty or not.

Suggested fix

Unconditionally assign _allPlans before the !empty check so an empty result set is also memoized:

private function _getAllPlans(): array                                                                                                                                                                                                                                  
 {                                                                                                                                                                                                                                                                       
     if ($this->_allPlans === null) {
         $this->_allPlans = [];                                                                                                                                                                                                                                          
         $plans = $this->_createPlansQuery()->all();

         if (!empty($plans)) {
             $plans = $this->_populatePlans($plans);
             foreach ($plans as $plan) {                                                                                                                                                                                                                                 
                 $this->_allPlans[$plan->id] = $plan;
             }                                                                                                                                                                                                                                                           
         }       
     }

     return $this->_allPlans;                                                                                                                                                                                                                                            
 }

This is a one-line fix and eliminates the redundant queries entirely.