Updated: Comment #0

Problem/Motivation

There's a CommentManager that could be overriden in contrib but without Interface definition
Also we need to inject the service to other services and controllers and it better to make via interface

Proposed resolution

Add interface

Related issues
#2097123: Deprecate comment_num_new() in favour of method on CommentManager

CommentFileSizeAuthor
#4 interdiff.txt4.52 KBandypost
#4 drupal8.comment-module.2101155-4.patch11.33 KBandypost
#1 2101155-commentManagerInterface-1.patch6.8 KBandypost
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
Status: Active » Needs review
FileSize
2101155-commentManagerInterface-1.patch6.8 KB

maybe change doc block of implementation to 'Provides comment manager service'?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

Will review today

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Status: Needs review » Reviewed & tested by the community 
+++ b/core/modules/comment/lib/Drupal/comment/CommentManagerInterface.php
@@ -0,0 +1,87 @@
+  public function getParentEntityUri(CommentInterface $comment);

I still think this belongs on CommentInterface but thats for another issue.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented
FileSize
drupal8.comment-module.2101155-4.patch11.33 KB
interdiff.txt4.52 KB

Let's add actual usage

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented

Awesome RTBC +1

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

Thanks for cleaning up refs here, +1

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: Add CommentManagerInterface » Change notice: Add CommentManagerInterface
Priority: Normal » Major
Status: Reviewed & tested by the community » Active
Issue tags: +Needs change record

Committed and pushed to 8.x. Thanks!

This needs a change notice.

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost commented

Filed initial change notice https://drupal.org/node/2112417 suppose we should extend it latter when more functions would be approved to be converted to swappable implementation, for example #2111357: Get rid of comment_count_unpublished() in favor of CommentStorage method

PS: #3 addressed in #2111419: Remove CommentManager::getParentEntityUri() in favor of Comment::permalink()

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Priority: Major » Normal
Status: Active » Fixed
Issue tags: -Needs change record
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Title: Change notice: Add CommentManagerInterface » Add CommentManagerInterface

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

(not verified) CreditAttribution: commented
Issue summary: View changes

2097123