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.
Updated: Comment #0
Problem/Motivation
CommentManager/CommentManagerInterface is getting too large, tell-tale sign is the number of dependencies.
Proposed resolution
Split the viewmode/output/links/render stuff from the entity/field-ui stuff
Remaining tasks
Patch
User interface changes
None
API changes
CommentManagerInterface split into two interfaces.
Comment | File | Size | Author |
---|---|---|---|
#44 | interdiff-41-44.txt | 80.47 KB | kishor_kolekar |
#44 | 2188287-cm-split-44.patch | 96.64 KB | kishor_kolekar |
#41 | interdiff-37-41.txt | 11.01 KB | kishor_kolekar |
#41 | 2188287-cm-split-41.patch | 24.46 KB | kishor_kolekar |
#37 | interdiff_34-37.txt | 11.52 KB | swatichouhan012 |
Comments
Comment #1
andypostComment #2
andypostPatch moves
forbiddenMessage()
method toCommentHelperInterface
Also removed dependency on current user service according #2182055: Comment module causes Circular reference error on a REST request
PS:
getParentEntityUri()
should be removed in #2245001: Remove unneeded CommentManagerInterface::getParentEntityUri()Comment #3
andypostre-roll
Comment #4
larowlanUnless bot disagrees
Comment #6
andypost3: 2188287-cm-split-3.patch queued for re-testing.
Comment #7
andypostComment #8
xjmCan we pretty please come up with a more meaningful name than CommentHelper?
Also, can you check whether there are any existing change records that would need updates for this?
Comment #9
dawehnerMaybe simply CommentRenderHelper or CommentRender
Comment #10
andypostOTOH this helper service just renders
t(some text)
so having a service for that looks overkill.Previously there was a theme function for that but we move this to manager service...
The only reason to have the service for that piece of render is a caching of access rights, but I have no idea how to implement that properly
this class variable a kind of static cache that is not cleared
Comment #11
andypostIn #2247779: Allow #pre_render, #post_render and #post_render_cache callbacks to be service methods proposed
CommentPostRenderCache
withcomment.post_render_cache
definitionComment #12
andypostComment #13
andypostLet's rename to
CommentRenderHelper
, that's precisely describes the purposeComment #14
mrjmd CreditAttribution: mrjmd commentedGoing to give this a try.
Comment #17
mrjmd CreditAttribution: mrjmd commentedThis need a reroll that's more complex than I can take on at the moment... several conflicts need to be resolved.
Comment #18
andypostAlso it would be great to move
comment_preview()
to the serviceComment #19
tim.plunkettComment #20
andypostrtbc #2357199: Consider CommentManagerInterface::addDefaultField() as deprecated and remove in favour of CommentTestTrait
new related #2338319: Clean-up CommentManager::getFields() function signature/invocations
Comment #21
andypostAdded related issues
#2458817: Creating new user entities for anonymous users is very slow - trying to remove
comment_prepare_author
Comment #26
Caralin CreditAttribution: Caralin as a volunteer and at ETECTURE GmbH commentedchanging the version to 8.5.x-dev
re-rolled the patch from #3 against 8.5.x
adding patch and interdiff
this needs more cleanup work!
Comment #27
jofitz CreditAttribution: jofitz at ComputerMinds commentedSetting to Needs Review to run the testbot to help review the progress of this patch.
Removed Needs Reroll tag.
Comment #33
andypostComment #34
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commentedAdded Reroll.
Comment #36
vaibhavjainRemoving tag Reroll, as this patch applies cleanly to 9.0.x branch.
Added tag Needs tests, as it is failing tests.
Comment #37
swatichouhan012 CreditAttribution: swatichouhan012 at Valuebound for Valuebound commentedupdated patch to fix "non-existent service "entity.manager" Test fail, kindly review new patch file.
Comment #39
hash6 CreditAttribution: hash6 at QED42 for Drupal India Association commentedComment #40
kishor_kolekar CreditAttribution: kishor_kolekar as a volunteer and at QED42 for Drupal India Association commentedComment #41
kishor_kolekar CreditAttribution: kishor_kolekar as a volunteer and at QED42 for Drupal India Association commentedkindly review new patch.
Comment #42
kishor_kolekar CreditAttribution: kishor_kolekar as a volunteer and at QED42 for Drupal India Association commentedComment #44
kishor_kolekar CreditAttribution: kishor_kolekar as a volunteer and at QED42 for Drupal India Association commentedhi @andypost I have updated the following changes
please review the new patch .
Comment #46
xjmThis would be a minor-only change. Since 8.9.x and 9.0.x are now in beta, I'm moving this to 9.1.x. Thanks!