MVC controller and action that handles favorite items in a session-based list
A few thoughts.


Consider building a result or response class to:


provide better structure around the returned result
provide possible re-use in other controllers
and clean up this code.



For example:

// instantiation
$result = new Result();

// usage
$result-&gt;setSuccess(true);
$result-&gt;setEntity($entity);


You could have this class implement JsonSerializable interface so you can pass it directly to JsonResponse() for serialization.  This also allows you to do things like add validation that setters are passed valid values (i.e. boolean for setSuccess())


You have a number of cases where you could remove else blocks and code nesting if you invert your conditionals.


For example:

    if (in_array($entity, $this-&gt;validEntites)) {
        $result['entity'] = $entity;
    } else {
        $result['success'] = false;
    }


can become:

   $result['success'] = false; 
   if (in_array($entity, $this-&gt;validEntites)) {
        $result['entity'] = $entity;
   }


or:

  if(!in_array($entity, $this-.validEntities)) {
      $result['success'] = false;
      return JsonResponse($result);
  }
  $result['entity'] = $entity;


Similarly:

if($result['success']) {
    // nested code
}


could become:

if(!$result['success']) {
    return JsonResponse($result);
}
// rest of code, now without nesting


Look for opportunities to remove code paths and complex nesting by getting rid of unnecessary else blocks (many of which truly are unnecessary) and exiting early by inverting conditions.  The fewer code paths you have in your code, the less fragile, easier to test, and easier to maintain it will be.


I think if you clean those things up, you will probably save 40% of the lines of code in this method. So it might not look like there is "too much" happening in this single method.  That being said, I think you could possibly fork the execution path along the lines of $action value , and hand off to protected/private class methods to finish that particular work.


For example:

switch($action) {
    case 'remove':
        return $this-&gt;removeFavorite(...);
    case 'add':
        return $this-&gt;addFavorite(...);
    default:
        $result['success'] = false;
        return JsonResponse($result);
} 


This would allow to more easily unit test those individual methods that represent some subset of logic on dealing with "favorites".


Similarly, you could perform all your upfront Request validation in a separate method as well to where you are getting towards something more like this in your main method.


Ex:

public function handleFavoritesAction(Request $request) {
    // here $result could be Result object as suggested above
    $result = $this-&gt;validateRequest($request);
    if($result-&gt;isSuccess === false) {
       return JsonResponse($result);
    }
    $action = $request-&gt;get('action');
    switch ($action) {
        // switch as shown above
    }
}



With regards to your documentation, I find many of your comments to be unnecessary. I think you generally have meaningfully named variables and appropriate use of vertical whitespace between logical sections of code to where comments become superfluous.  Especially, if you consider breaking this main method into several other well-named methods, you should not have much need to comment this code.