Part of #2096923: [meta] Clean-up CommentManager service and deprecate procedural helper functions this will allow for comment module to be more flexible and decoupled from node eventually.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

marcingy’s picture

Status: Active » Needs review
FileSize
9.01 KB
larowlan’s picture

It should just be removed.
There's an issue to allow render cache callbacks to be services which converts the static call to a service. That leaves only one call to comment add in the form. It can be inlined.

larowlan’s picture

BTW it is decoupled from node

larowlan’s picture

Actually that issue is fixed #2247779: Allow #pre_render, #post_render and #post_render_cache callbacks to be service methods so comment add can go inline for the remaining call(s) (IMO). My motivation in https://www.drupal.org/node/2247779 was to kill comment add

marcingy’s picture

FileSize
5.47 KB

Re-rolled inline

marcingy’s picture

Title: Move comment_add to comment manager » Remove comment_add and inline final occurance
larowlan’s picture

+++ b/core/modules/comment/src/Plugin/Field/FieldFormatter/CommentDefaultFormatter.php
@@ -174,7 +187,17 @@ public function viewElements(FieldItemListInterface $items) {
+            $field = Fieldconfig::loadByName($entity_type, $field_name);

We should have the field in scope here, no need to use the static. $this->getFieldDefinition() from memory

Status: Needs review » Needs work

The last submitted patch, 5: issue-2289989-5.patch, failed testing.

andypost’s picture

Status: Needs work » Needs review
FileSize
3.52 KB
5.16 KB

A couple of fixes:
1) there's comment_type not a "bundle" - take this directly from field
2) use entity form builder variable to prevent collisions with form builder

Suppose this one rtbc

marcingy’s picture

Status: Needs review » Reviewed & tested by the community

Changes make sense

alexpott’s picture

Title: Remove comment_add and inline final occurance » Remove comment_add and inline final occurrence
Status: Reviewed & tested by the community » Fixed

Committed 4b91f16 and pushed to 8.x. Thanks!

  • alexpott committed 4b91f16 on 8.x
    Issue #2289989 by marcingy, andypost: Remove comment_add and inline...

  • alexpott committed cfe3e15 on 8.x
    Revert "Issue #2289989 by marcingy, andypost: Remove comment_add and...
  • alexpott committed e7f8c6d on 8.x
    Issue #2289989 by marcingy, andypost: Remove comment_add and inline...
alexpott’s picture

The original commit included #2292197: The StringTranslationTrait trait doxygen could use a little love - sorry.

Committed e7f8c6d and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

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