Use case

node module has article and page as bundles. for the bundle article you create a view mode called search result, but for the bundle page you don't. Now, since you haven't enabled this on the entity level, but on a per bundle level it doesn't show in the views configuration. See https://www.evernote.com/shard/s44/sh/cbbcaf72-623c-4cb3-98ba-d668483746...(Index-Search-API-Database-Index)---Site-Install.png

We need to fix this so we can mimic core search easily.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Nick_vh’s picture

Assigned: Unassigned » Nick_vh
Nick_vh’s picture

Status: Active » Needs work

Making good progress in the core issue. Going to set this to needs work as we can almost rely on that patch

Nick_vh’s picture

FileSize
9.32 KB
Nick_vh’s picture

Status: Needs work » Needs review
FileSize
9.13 KB
Nick_vh’s picture

FileSize
9.03 KB

The last submitted patch, 5: 2471669-5.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 6: 2471669-6.patch, failed testing.

Nick_vh’s picture

FileSize
11.55 KB
Nick_vh’s picture

Status: Needs work » Needs review

The last submitted patch, 4: 2471669-4.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 9: 2471669-9.patch, failed testing.

Nick_vh’s picture

Status: Needs work » Needs review
FileSize
13.14 KB
drunken monkey’s picture

Status: Needs review » Needs work

Thanks, looks like a good start!
However, a there are still some problems with your patch:

  1. +++ b/config/schema/search_api.views.schema.yml
    @@ -13,11 +13,19 @@ views.row.search_api:
    +      mapping:
    +        datasource:
    +          type: sequence
    +          sequence:
    +            type: string
    +            label: View mode
    +        bundle:
    +          type: sequence
    +          sequence:
    +            type: string
    +            label: View mode
    

    With that structure, won't you run into problems if two entity types have bundles with the same name?
    I think the structure should be: $options['view_modes'][$datasource_id][$bundle] = $view_mode (where $bundle might be NULL).

  2. +++ b/src/Datasource/DatasourceInterface.php
    @@ -104,11 +115,26 @@ interface DatasourceInterface extends IndexPluginInterface {
    +   * @param string $bundle
    +   *   The bundle for which we want to know the view modes. Sometimes bundles
    +   *   can have view modes that are only activated on a per bundle basis.
    

    This should have a leading "(optional) " in the description, and also explain what should be done for NULL (since I'm not sure myself). Is it only called that way when a datasource has no bundles? Or should it return the bundle-independent view modes?

  3. +++ b/src/Datasource/DatasourceInterface.php
    @@ -104,11 +115,26 @@ interface DatasourceInterface extends IndexPluginInterface {
    +   * Retrieves the bundles which will be indexed for this datasource.
    +   *
    +   * If a datasource does not contain bundles, it should return an empty array.
    +   *
    +   * @return string[]
    +   *   The IDs of all indexed bundles for this datasource.
    +   */
    +  public function getBundles();
    

    This looks like it's just been copied from the content entity datasource? It should be rephrased to reflect the general approach that the datasource "provides" or "contains" bundles, not indicate that the datasource might only contain a subset of available bundles.

  4. +++ b/src/Datasource/DatasourceInterface.php
    @@ -117,6 +143,7 @@ interface DatasourceInterface extends IndexPluginInterface {
        *   (optional) The view mode that should be used to render the item.
    +   *
        * @param string|null $langcode
    

    Seems to have slipped in?

  5. +++ b/src/Plugin/search_api/datasource/ContentEntity.php
    @@ -355,6 +355,17 @@ class ContentEntity extends DatasourcePluginBase {
    +    if ($item instanceof EntityAdapter) {
    +      return $item->getValue()->bundle();
    +    }
    

    Does this always work? Also for entity types without bundles or config entities?

  6. +++ b/src/Plugin/search_api/datasource/ContentEntity.php
    @@ -489,14 +497,21 @@ class ContentEntity extends DatasourcePluginBase {
    -    return array_keys($bundles);
    +    return $bundles;
    

    This doesn't seem to make sense? According to the documentation, this should return just the IDs. (Though returning IDs mapped to labels would make much more sense, I think it should do that.)
    But then, of course, all the wrapping array_keys() calls don't make sense. It seems like you stopped half-way in implementing my suggestion of returning an ID-label map.

  7. +++ b/src/Plugin/search_api/processor/RenderedItem.php
    @@ -179,8 +179,6 @@ class RenderedItem extends ProcessorPluginBase {
    -    // Annoyingly, this doc comment is needed for PHPStorm. See
    -    // http://youtrack.jetbrains.com/issue/WI-23586
    

    It's debatable whether we want to keep that comment, but in any case it shouldn't be part of this issue.

  8. +++ b/src/Plugin/views/row/SearchApiRow.php
    @@ -143,24 +143,49 @@ class SearchApiRow extends RowPluginBase {
    +    /**
    +     * @var \Drupal\search_api\Datasource\DatasourceInterface $datasource
    +     */
    

    This should be in one line, as anywhere else.

Nick_vh’s picture

Status: Needs work » Needs review
FileSize
11.86 KB

Updated. Please review :)

re: #5 - this is always true. Addressed all other comments I think.

Nick_vh’s picture

FileSize
1.28 KB

drunkenmonkey - please add this to the patch while you are reviewing or changing it. Otherwise I'll reroll it later

Status: Needs review » Needs work

The last submitted patch, 16: 2471669-additional.patch, failed testing.

drunken monkey’s picture

Status: Needs work » Needs review
FileSize
14.71 KB
8.7 KB

All reviewing and no coding makes Thomas a dull boy.

Nick_vh’s picture

Status: Needs review » Reviewed & tested by the community

Go for it!

drunken monkey’s picture

Status: Reviewed & tested by the community » Fixed

Committed.
Thanks again!

Status: Fixed » Closed (fixed)

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

tstoeckler’s picture

tstoeckler’s picture