EntityListController::buildRow() takes care of adding operations to the row, but it doesn't add the label.

I am struggling to understand why this is. Surely more entities you'd want to list would have labels? Not having this means that EntityListController isn't usable as a list controller OOTB. It would be good DX if EntityListController were reasonably usable for basic entities.

This quick debug output confirms that all the entity types that DON'T have labels also don't use a list controller, so having the label in this class wouldn't have a negative impact:

  $labels = array();
  foreach (entity_get_info() as $type => $entity_info) {
    $label = isset($entity_info['entity_keys']['label']) ? $entity_info['entity_keys']['label'] : 'NOLABEL';
    $list_controller = isset($entity_info['controllers']['list']) ? $entity_info['controllers']['list'] : "NOLIST";
    
    $labels[$type] = "$label - $list_controller";
  }
  dsm($labels);

Furthermore, it would allow the removal of this method in VocabularyListController and probably quite a few similar copies elsewhere:

  /**
   * {@inheritdoc}
   */
  public function buildRow(EntityInterface $entity) {
    $row['label'] = $this->getLabel($entity);
    return $row + parent::buildRow($entity);
  }

Comments

joachim’s picture

Status: Active » Needs review
StatusFileSize
new860 bytes

This is just a quick patch to see what the testbot says in principle.

A proper fix for this would require the now pointless methods in subclasses to be removed too.

joachim’s picture

Issue tags: +Novice

Tests pass!

It would be good to get an opinion on this from someone who understands the D8 entity system better, to make sure this is okay to do.

Tagging this as novice in anticipation :) -- it's just a question of finding implementations of those two methods in subclasses that are made obsolete by the change in the parent.

andypost’s picture

We removed label in #1855402: Add generic weighting (tabledrag) support for config entities (DraggableListController) to allow other list controllers easily override listings so probably it's a step back

joachim’s picture

I had a feeling that might be the case. I can see why this class should be as abstract as possible.

It's a shame though that (AFAICT) Core doesn't have an entity list controller class that's usable OOTB.

joachim’s picture

Status: Needs review » Closed (won't fix)