_comment_get_comment_fields() is weird, super costly and unused

- weird: fetches a list of Field config entities, and force-adds a ->bundles property on them - doesn't seem kosher
- super costly: entity_load_multiple_by_properties('field_entity') means loading all field definitions,
then *for each resulting field*, entity_load_multiple_by_properties('field_instance') means loading all instance definitions.
- unused: it's never called currently ? Plus, its phpdoc mentions it as a "maintenance mode" replacement for comment_get_comment_fields(), which itself doesn't exist...

CommentFileSizeAuthor
#1 comment_get_comment_fields-2143589-1.patch1.13 KByched

Comments

yched’s picture

yched commented
StatusFileSize
new comment_get_comment_fields-2143589-1.patch1.13 KB

Let's see what breaks if we remove it ?

yched’s picture

yched commented
Status: Active » Needs review
longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

Looked at the commit where this was first introduced, it wasn't called then either, so seems like this can be safely removed.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Yeah it was moved to the manager.
We should look at the performance of that too.

andypost’s picture

andypost
he/him
Primary language Russian
commented

Nice catch! Implementation in manager is ok

class CommentManager implements CommentManagerInterface {
  public function getAllFields() {
    $map = $this->fieldInfo->getFieldMap();
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.