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

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Active » Needs review
StatusFileSize
new 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
commented

Will review today

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
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
commented
StatusFileSize
new drupal8.comment-module.2101155-4.patch11.33 KB
new interdiff.txt4.52 KB

Let's add actual usage

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

Awesome RTBC +1

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

Thanks for cleaning up refs here, +1

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
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
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
commented
Priority: Major » Normal
Status: Active » Fixed
Issue tags: -Needs change record
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
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) commented
Issue summary: View changes

2097123