Problem/Motivation

When an index includes a field like "body:processed", the rendering process uses a different theme when the item is saved and indexed individually or using the index UI to trigger a bulk index. Those processes use the admin theme but using Drush to index uses the default theme.

Steps to reproduce

  1. Create an index for a content type that uses the field's ":processed" value.
  2. Drop a breakpoint in web/core/lib/Drupal/Core/Render/Renderer.php, renderPlain() to see the active theme during the indexing process.
  3. Edit a node and note the active theme at the breakpoint.
  4. Run the full, bulk indexing process using the UI and note the active theme at the breakpoint.
  5. Run the full, bulk indexing process using drush and note the active theme at the breakpoint.

Each of these indexing process should be using the same theme for data integrity.

Proposed resolution

Remaining tasks

CommentFileSizeAuthor
#12 3320841-12--use_default_theme_for_processed_text.patch18.7 KBdrunken monkey
#10 search_api-textprocessed_theme_switch-3320841-10.patch9.43 KBweekbeforenext
#7 search_api-textprocessed_theme_switch-3320841-7.patch7.88 KBweekbeforenext
#4 search_api-textprocessed_theme_switch-3320841-4.patch6.64 KBweekbeforenext
#3 search_api-textprocessed_theme_switch-3320841-3.patch6.19 KBweekbeforenext

Comments

weekbeforenext created an issue. See original summary.

weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented
Issue summary: View changes
Related issues: +#3245214: Allow site builders to specify the theme used when indexing rendered data.
weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented
Version: 8.x-1.27 » 8.x-1.x-dev
Status: Active » Needs review
StatusFileSize
new search_api-textprocessed_theme_switch-3320841-3.patch6.19 KB

Attached is a patch. Not sure if it's the best solution, but it's a first attempt that seems to be working.

  • I copied theme switching code from RenderedItem.php, addFieldValues() into FieldHelper.php, extractField() and a new method, themeSwitched().
  • Updated the search_api.services.php to include theme and config services.
  • If TextProcessed is being used to render a field value, and admin is the active theme, the theme is switched before processing and back again when complete.
weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented
StatusFileSize
new search_api-textprocessed_theme_switch-3320841-4.patch6.64 KB

New patch that defines the themeSwitched() method in the FieldsHelperInterface.

The last submitted patch, 3: search_api-textprocessed_theme_switch-3320841-3.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 4: search_api-textprocessed_theme_switch-3320841-4.patch, failed testing. View results

weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented
Status: Needs work » Needs review
StatusFileSize
new search_api-textprocessed_theme_switch-3320841-7.patch7.88 KB

Added test fixes.

weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented

Status: Needs review » Needs work

The last submitted patch, 7: search_api-textprocessed_theme_switch-3320841-7.patch, failed testing. View results

weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented
StatusFileSize
new search_api-textprocessed_theme_switch-3320841-10.patch9.43 KB

Fixed more tests...

weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented
Status: Needs work » Needs review
drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
StatusFileSize
new 3320841-12--use_default_theme_for_processed_text.patch18.7 KB

Thanks for reporting this issue and already providing a patch for fixing it!
However, if the code is exactly the same in both places, then we should probably make it reusable in general. Unfortunately, this doesn’t really fit on any of the existing services, so I propose adding a new one. Patch attached, please test/review!

weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented

Thanks for the new patch! I like how the theme switching is now abstracted into it's own service.

The patch applies successfully. I verified the test instructions for a project ticket that kicked off this issue and they still pass.

Reviewing the patch itself, the code looks good to me.

Thanks again!

weekbeforenext’s picture

weekbeforenext
Primary language English
Location Asheville, NC
commented
Status: Needs review » Reviewed & tested by the community

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
commented
Status: Reviewed & tested by the community » Fixed

Great to hear, thanks a lot for testing and reviewing!
Merged.
Thanks again!

Status: Fixed » Closed (fixed)

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