Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Combining two different elements into one index #69

Closed
furioursus opened this issue May 4, 2019 · 51 comments
Closed

Combining two different elements into one index #69

furioursus opened this issue May 4, 2019 · 51 comments
Assignees

Comments

@furioursus
Copy link

Odd question, but we have all of our sections in one large index, but our Solspace Calendar events are a different element type. Since I can't add to an index more than once apparently, what are my options, if any for merging craft\elements\Entry and Solspace\Calendar\Elements\Event into one index?

@riasvdv
Copy link
Collaborator

riasvdv commented May 17, 2019

I haven't tested this, but I think you could use craft\base\Element as the element type, you'll have to experiment a bit with the criteria then to get everything in there.

With the new feature in 1.2.1 you can also just do your checks in the transformer and if you don't want to index them return an empty array.

@furioursus
Copy link
Author

Okay, I'll give it a look and see if anything can come of it!

@riasvdv riasvdv closed this as completed Jun 14, 2019
@besimhu
Copy link

besimhu commented Oct 1, 2019

I am a bit late to this. There was a way to map different elementTypes to the same index. You would just map to the same index name. It worked for me.

I recently started upgrading to the new version and have noticed that the unintended feature actually does not work in v2.

@riasvdv there is no way one could use the same index name, but use two different element types? Say one for categories and another for entries?

@riasvdv
Copy link
Collaborator

riasvdv commented Oct 1, 2019

You should be able to set craft\base\Element as the ElementType, you'll then get an ElementQuery in your criteria on which you can set some extra constraints, then in your transformer you should receive a craft\base\Element object, where you can check which element you got back and how to transform it.

I haven't tested any of this yet, but I don't see a reason why it couldn't work

@besimhu
Copy link

besimhu commented Oct 3, 2019

@riasvdv thank you for the reply.

I saw your comment earlier on about using craft\base\Element but unfortunately that doesn't make a whole lot of sense to me.

Do you have any resources you could share? Not resources to Craft, but how to write it inside of the index configuration? I am not familiar with the -> writing where you chain functions.

My understanding is I define the elementType and then the criteria is based off of that elementType. But if I wanted to do say a category and entry elementType, that is where I am confused.

@accalton
Copy link

@riasvdv I just wanted to let you know that I tested using the craft\base\Element class, however it's an abstract class and therefore fails when the transformer tries to instantiate it.

@riasvdv riasvdv reopened this Oct 16, 2019
@riasvdv
Copy link
Collaborator

riasvdv commented Oct 16, 2019

Didn't realise you couldn't start a query from the base Element class, I'll reopen this, but I don't know if I'll be able to find a solution for this.

@riasvdv
Copy link
Collaborator

riasvdv commented Oct 16, 2019

As a (temporary) solution, Algolia does allow searching multipe indices at the same time: https://www.algolia.com/doc/api-reference/api-methods/multiple-queries/

@alexmglover
Copy link

@riasvdv Is there a good reason for validating the index names are unique? I have config for each section based on my criteria that I am passing over that configures the values going into the index. I don't really think the unique check needs to happen, and I can't think of any negative side effects if it were to be removed.

@riasvdv
Copy link
Collaborator

riasvdv commented Oct 31, 2019

If you think removing it will have no negative side effects, and you can add some tests that prove multiple index configurations do the expected things, I'd gladly accept a PR that implements this.

@verbeeksteven
Copy link
Contributor

It would be nice if you could use CraftCMS manually and pass in an array or something, rather then using elementType, criteria, etc. So something like

$getDataFromCraftCMSYourself = \craft...

$indices[] = \rias\scout\ScoutIndex::create('index')->data($getDataFromCraftCMSYourself)->transformer(function ($data) {} );

Or something?

Obviously this would be a high/advanced user not the default, but this would allow us to basically do anything.

@nikolowry
Copy link

Found myself in the same scenario after needing to implement a global search that included both craft\elements\Entry and Solspace\Calendar\Elements\Event.

I got around Scout's validateConfig method by installing cweagans/composer-patches and using this patch:

diff -rupN a/src/Scout.php b/src/Scout.php
--- a/src/Scout.php	2020-01-27 12:13:35.148078307 -0500
+++ b/src/Scout.php	2020-01-27 12:14:38.294046990 -0500
@@ -133,6 +133,7 @@ class Scout extends Plugin
 
     private function validateConfig()
     {
+        return;
         $indices = $this->getSettings()->getIndices();
 
         if ($indices->unique('indexName')->count() !== $indices->count()) {

@riasvdv maybe you could add a validate_config boolean property that defaults to true in Scout's conifg?

@besimhu
Copy link

besimhu commented Feb 18, 2020

@nikolowry

Are you able to provide a full example of how you patched and or what needs to be done? It looks like you copied some git diff in your messagfe.

@nikolowry
Copy link

@besimhu a patch is produced by diffing. To replicate what I did:

  • composer require cweagans/composer-patches
  • Save the patch contents I posted above in your project's root at patches/Scout-Indices-No-Validation.patch
  • In your composer.json add the following to extra:
{
  "extra": {
    "patches": {
      "rias/craft-scout": {
        "No Scout Indices Validation": "patches/Scout-Indices-No-Validation.patch"
      }
    }
  }
}
  • Now any time you run composer install or composer update, cweagans/composer-patches will automatically apply the patch to Scout. You can run composer update nothing to manually trigger it for the first time

@besimhu
Copy link

besimhu commented Feb 21, 2020

@nikolowry thank you, it worked as you described and I was able to finally upgrade to the latest version.

@nikolowry
Copy link

nikolowry commented Feb 22, 2020

@riasvdv what type of tests would you need to convince you multiple elements on one index works as expected?

I've been indexing two elements -- Entries and Solspace\Calendar\Event -- on the same indices in two separate environments (dev/stage) for over a month without an issue.

Or thoughts on adding a boolean config setting to allow users to index multiple elements on one index? You could explicitly state it's an unsupported/experimental feature and devs should use at their own risk.

I'm advocating for this because full-site search is such a core functionality of most CMS's and Craft is refusing to implement, craftcms/cms#878.

Using Scout with the patch above, along with https://github.com/trendyminds/algolia, was the only way I could pull off Craft's traditional pagination in Twig.

@besimhu
Copy link

besimhu commented Feb 24, 2020

I agree with @nikolowry ... I am relying on the instantsearch.js and that does not allow for combining multiple indecies. I have some content that falls under categories, and while they are categories, they do contain website content. So when I am wanting to do a website search, I need to include those as part of the website index.

@nikolowry
Copy link

@timkelty thoughts?

@timkelty
Copy link
Collaborator

timkelty commented Apr 20, 2020

Reading through this, my inclination is to keep indexName unique, but allow users to manually tap into a getElements method, like @verbeeksteven #69 (comment) suggested.

So for @onebrightlight it might look something like this:

ScoutIndex::create()
    ->indexName('myIndex')
    ->getElements(function() {
      $entries = craft\elements\Entry::find(…);
      $events = Solspace\Calendar\Elements\Event::find(…);

      return array_merge($entries, $events);
    })
    ->transform(function (craft\base\Element $element) {
            'lorem' => $element->lorem,
            'ipsum' => $element->ipsum,
        ];
    });

It seems like this would fit people's use-cases, while maintaining the 1-1 relationship of ScoutIndex to Algolia index.

@martinhellwagner
Copy link

@timkelty

I'm confused: is this already implemented or will this be implemented in the future?

@timkelty
Copy link
Collaborator

@martinhellwagner not yet implemented, but thinking it should be a quick addition.

@bummzack
Copy link

We're running into the same issue. A solution as described in #69 (comment) would be a very welcome feature! ❤️

@elivz
Copy link

elivz commented Jul 27, 2020

I also would love this functionality. Any progress or update on it?

@timkelty
Copy link
Collaborator

@elivz No progress, but I'm really hoping to carve out some time for some long standing Scout issues in the coming week.

Does what I outline here fit your use-case? #69 (comment)

@elivz
Copy link

elivz commented Jul 27, 2020

@timkelty Yes! I think being able to pass in an array of items in place of the ElementType/Criteria query rules would open up a lot of possibilities. Not just combining different element types, but potentially even indexing non-Element data from a raw SQL query or pulling from an external API or whatever (not sure if that would cause other complications & I currently have no need for it...just thinking).

@timkelty
Copy link
Collaborator

@jornwildenbeest Sadly no, other than in my thoughts.

I started a new position, so been quite busy.
That said, they use Scout quite heavily, so I still expect I will be able to carve out time, just been tight right now!

@ademers
Copy link

ademers commented Dec 11, 2020

@timkelty Thanks for the update!

@denisyilmaz
Copy link

any chances this feature will be realized sometime soon?

@janhenckens janhenckens added this to the 2.5.0 milestone Aug 13, 2021
@gregkohn
Copy link
Contributor

@janhenckens, any progress on this? if not, would you accept a PR that went with the getElements() approach described above?

@timkelty
Copy link
Collaborator

@gregkohn I know lots of people are waiting for this, so it seems like a PR would be valuable, unless @janhenckens has already started something.

@janhenckens
Copy link
Member

@gregkohn Sure would! Feel free to submit it and/or hit me up on discord if there's anything you'd like to discuss !

@janhenckens janhenckens removed this from the 2.5.0 milestone Nov 30, 2021
@mdunbavan
Copy link
Collaborator

Hello @timkelty @gregkohn @janhenckens

Is there anyway that I can help move this issue forward at all? If it is a case of things getting in the way and time not being on your side could we all collaborate in finding a solution to this issue?

Thanks,
Mark

@gregkohn
Copy link
Contributor

gregkohn commented Feb 9, 2022

👋 For my part, I've moved companies and switched focuses, and unfortunately won't be able to assist in moving forward here. Good luck!

@janhenckens
Copy link
Member

@mdunbavan Finding time is certainly a problem for me in this case. Happy to chat or have a call about how we could tackle if you'd like to give it a go! (feel free to ping me on discord, same username)

@mdunbavan
Copy link
Collaborator

mdunbavan commented Feb 10, 2022

It would be really interesting to hear why this feature of multiple elements/types/categories being saved to one index was removed from this version? I get that we need validation rules but one of the key killer features of Scout v1.1 was the ability to do this because algolia js worked so well with it.

private function validateConfig()
    {
        $indices = $this->getSettings()->getIndices();

        if ($indices->unique('indexName')->count() !== $indices->count()) {
            throw new Exception('Index names must be unique in the Scout config.');
        }
    }

This is what checks the unique names which is fine.

Can we do something like create a new function such as \rias\scout\ScoutIndex::saveTo('indexName') which first checks that the indexName exists by loping through indices. If that is correct, we allow exactly the same functionality as create but we pass the indexes to an existing indice?

@twitcher07
Copy link

Reading through this, my inclination is to keep indexName unique, but allow users to manually tap into a getElements method, like @verbeeksteven #69 (comment) suggested.

So for @onebrightlight it might look something like this:

ScoutIndex::create()
    ->indexName('myIndex')
    ->getElements(function() {
      $entries = craft\elements\Entry::find(…);
      $events = Solspace\Calendar\Elements\Event::find(…);

      return array_merge($entries, $events);
    })
    ->transform(function (craft\base\Element $element) {
            'lorem' => $element->lorem,
            'ipsum' => $element->ipsum,
        ];
    });

It seems like this would fit people's use-cases, while maintaining the 1-1 relationship of ScoutIndex to Algolia index.

I am looking to have a global index across multiple element types (in my case combining entries and assets). So a solution like this would be very crucial since it seems like there is currently not a way to do it with this plugin.

@Stalex89
Copy link

Stalex89 commented Apr 9, 2024

Also looking forward for this feature, I would also like to combine Craft entries and Craft Commerce Products into same index. Any change of getting this feature soon?

@simonkuran
Copy link

Also really needing this feature. We have multiple sites that need a site-wide search that includes entries and Calendar events. An ETA for this feature would be ideal so we know if we need to move forward with an alternative solution.

@janhenckens
Copy link
Member

Hey @Stalex89 @simonkuran, there's an open PR for this (#267) that I need to have a look at.

I'll probably release it as a beta first so we can test it with a bunch of people, since this could impact performance.

Would you need this is Craft 4 or 5? Just so I have a reference where it would need to be merged first?

@simonkuran
Copy link

@janhenckens Thanks for your attention on this. Craft 4 support would be ideal, but if the feature needs to be Craft 5+ that would be ok.

@janhenckens
Copy link
Member

Craft 5 is so new that it makes sense to release it for both, but then I'll see about making a beta release for 4 first. I'll report back here on the progress.

@Stalex89
Copy link

@janhenckens Thank you for your response! Would be nice to have it still on Craft 4

@janhenckens
Copy link
Member

Making good progress on this! I'd like to add some more tests to cover this feature, hoping to tag a beta by the end of the week.

@janhenckens janhenckens assigned janhenckens and unassigned timkelty Apr 29, 2024
@janhenckens
Copy link
Member

Okay folks, calling everyone interested in this feature to test it out!

To install:

composer require studioespresso/craft-scout 4.1.0-beta.1

Docs for the new syntax can be found here https://github.com/studioespresso/craft-scout/tree/feature/multi-elementtypes?tab=readme-ov-file#-getelementscallable-queries

Please create a new issue for anything you run into while testing 🙏

@janhenckens
Copy link
Member

I'll give this another 2 weeks to be tested (we're on beta.2 now) before releasing this.

To install the latest beta:

composer require studioespresso/craft-scout 4.1.0-beta.2

Docs for the new syntax can be found here https://github.com/studioespresso/craft-scout/tree/feature/multi-elementtypes?tab=readme-ov-file#-getelementscallable-queries

Please create a new issue for anything you run into while testing 🙏

@janhenckens
Copy link
Member

Hey folks, I wanted to let you all know that this feature is now available in the latest releases for Craft 4 and Craft 5. Documentation can be found here: https://studioespresso.github.io/craft-scout/multiple-element-types.html

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests