Problem/Motivation

When the NodeSearch plugin renders nodes for indexing, in NodeSearch::indexNode(), and again where it renders the results as part of the execute() method, it is currently invoking hook_node_update_index() in order to render the comments.

Since comments are now a field whose display can be determined by a view mode, this should not be necessary.

Proposed resolution

Get rid of hook_node_update_index() and its one implementation in comment.module, and instead obey the settings on the search_index and search_result display modes.

Also, there is a kludge in CommentDefaultFormatter::viewElements() that is making sure to not render comments in those two view modes, in favor of using the hook, which would be removed.

Remaining tasks

Make a patch.

User interface changes

Users will be able to control whether comments are added to node indexes and displayed for search result highlighting, by using display modes.

API changes

hook_node_update_index() will go away.

CommentFileSizeAuthor
#17 2324719-17.patch26.79 KBclaudiu.cristea

Comments

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Issue summary: View changes
andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#1920044: Move comment field settings that relate to rendering to formatter options

The problem here that http://api.drupal.org/api/search/8/collectRenderDisplays does not load disabled entity view displays so I have no idea how to make it without #1920044: Move comment field settings that relate to rendering to formatter options

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented

This makes sense +1, will try and get a re-roll on #1920044: Move comment field settings that relate to rendering to formatter options today

catch’s picture

catch
he/him
Primary language English
commented
Issue tags: -beta deadline +beta target

Don't think there's anything beta deadline about this, it's just removing cruft.

Hook probably go past RC so adding beta target, but we could do the comment clean-up even if the hook stays and mark it deprecated if we have to.

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Status: Active » Postponed

See #2, needs to be postponed for now.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Postponed » Active
Issue tags: +Entity Field API

Pointed issue does not solve #2
Suppose it should be possible to use disabled entity displays for search.
Also that's would be great issue to implement this ability

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented

I don't see how we can implement this without having the comment rendering options on the formatter though?

jhodgdon’s picture

jhodgdon
she/her
Primary language English
commented
Related issues: +#1113832: [PP-1] Comment module renders "reply" and other links in search index/results

Talked to pwolanin about this in DDD today. We think the right solution to this (and would also solve #1113832: [PP-1] Comment module renders "reply" and other links in search index/results) is to make a new comment field formatter for search indexing and results snippet generation that would do the right thing:
- Display all the comments without threading or page limits
- Omit links (reply, permalink, etc.)
- Use this formatter as the default in the search index and search results view modes

xjm’s picture

xjm
she/her
Primary language English
commented
Version: 8.0.x-dev » 8.2.x-dev
Issue tags: -beta target

This issue was marked as a beta target for the 8.0.x beta, but is not applicable as an 8.1.x beta target, so untagging.

This could potentially be implemented in a minor with BC, so moving to 8.2.x.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Related issues: +#1577902: [META] Remove all usages of drupal_static() & drupal_static_reset()

+1 for this as it helps getting rid of drupal_static(). See #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset() .

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

claudiu.cristea’s picture

claudiu.cristea
Primary language Romanian
Location Arad 🇷🇴
commented
Status: Active » Needs review
Parent issue: » #1577902: [META] Remove all usages of drupal_static() & drupal_static_reset()
Related issues: -#1577902: [META] Remove all usages of drupal_static() & drupal_static_reset()
StatusFileSize
new 2324719-17.patch26.79 KB

A first raw try with the idea from #8. For now, covers only the indexing.

Status: Needs review » Needs work

The last submitted patch, 17: 2324719-17.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

andypost’s picture

andypost
he/him
Primary language Russian
commented 
+++ b/core/modules/comment/comment.module
@@ -805,3 +753,42 @@ function comment_entity_view_display_presave(EntityViewDisplayInterface $display
+function comment_entity_view_display_alter(EntityViewDisplayInterface $display, array $context) {
+  if (!in_array($context['view_mode'], ['search_result', 'search_index'])) {

I still not sure we should add so many things instead of allowing comments to be rendered as field in search view modes
Then no need in templates and everything will work like supposed as comment field

Meantime gonna resurrect patch in #1920044: Move comment field settings that relate to rendering to formatter options and check the way in #1113832: [PP-1] Comment module renders "reply" and other links in search index/results

andypost’s picture

andypost
he/him
Primary language Russian
commented

On other hand search within comments should be separate task which needs follow-up and research
For example comments maybe queued for indexing when their node(entity) indexed, it affects accuracy of whole search
Also this indexing must order comments from newer to older to increase search relevancy, then how to deal with paging?

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

quietone’s picture

quietone commented
Status: Needs work » Postponed

The Search Module was approved for removal in #3476883: [Policy, no patch] Move Search module to contrib .

This is Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

The deprecation work is in #3565780: [meta] Tasks to deprecate the Search module and the removal work in #3565783: [meta] Tasks to remove the Search module.

Search will be moved to a contributed project before Drupal 12.0.0 is released.