Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
In #2337191: Split up EntityManager into many services the entity manager was refactored and splitted into many services.
In the refactor you forgot to add the return statement in EntityManager::clearDisplayModeInfo(). For confirmation see
EntityDisplayRepository::clearDisplayModeInfo() and what is stated by the interface:
/**
* Clears the gathered display mode info.
*
* @return $this
*/
public function clearDisplayModeInfo();
Comment | File | Size | Author |
---|---|---|---|
#20 | interdiff-18-20.txt | 2.04 KB | kishor_kolekar |
#20 | missing_return-2605904-20.patch | 1.82 KB | kishor_kolekar |
#18 | interdiff_12-18.txt | 1.62 KB | swatichouhan012 |
#18 | 2605904-18.patch | 1.83 KB | swatichouhan012 |
#12 | missing_return-2605904-12.patch | 1.49 KB | jmikii |
Comments
Comment #2
willzyx CreditAttribution: willzyx commentedComment #3
willzyx CreditAttribution: willzyx commentedComment #10
BerdirGood catch, now that we have a legacy test for this method, we could assert quite easily that it returns itself again in \Drupal\Tests\Core\Entity\EntityManagerTest::testClearDisplayModeInfo.
Comment #11
vacho CreditAttribution: vacho at Skilld commentedOnly updating this patch. (rerolled)
Comment #12
jmikii CreditAttribution: jmikii as a volunteer commentedModified the test to check the return output
Comment #14
rosinegrean CreditAttribution: rosinegrean at PitechPlus commentedComment #16
joachim CreditAttribution: joachim commentedComment #17
alexpottThis method needs to return an instance of EntityManager. This would return that entity display repository service.
This should return $this->entityDisplayRepository.
We should assert that it equals $this->entityManager.
Comment #18
swatichouhan012 CreditAttribution: swatichouhan012 at Valuebound for Valuebound commentedHi, i have updated patch according comment #17, kindly review.
Comment #19
BerdirYou need to change the willReturn() call above to $this->entityManager, and now you don't call the method anymore inside EntityManager.
Comment #20
kishor_kolekar CreditAttribution: kishor_kolekar as a volunteer and at QED42 for Drupal India Association commentedkindly review new patch.
Comment #21
BerdirYeah, that's better. The willReturn() is correct although it technically doesn't matter anymore as we ignore that return.
Comment #22
alexpottCommitted and pushed 96a7c8f714 to 8.9.x and c9d2c9bbec to 8.8.x. Thanks!
I guess it is important to have EntityManager work the same as the EntityDisplayRepositoty...
Comment #23
alexpott