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

[Instantsearch] Scout indexing products & categories #734

Merged
merged 10 commits into from
Feb 18, 2025

Conversation

Jade-GG
Copy link
Collaborator

@Jade-GG Jade-GG commented Feb 14, 2025

Requires this patch for the products to index properly. Without it, only the first 500 products get indexed.

This PR introduces a generic version of the Searchable trait that we can put onto any Eloquent model to make it indexable by Scout, as well as some generic functions that can be hooked into to filter/query the data to your liking. This also replaces the indexer commands with one generic IndexCommand.

A few notes:

  • The base Rapidez models will automatically get detected as a Scout model---any others need to be added into the indexer.php config file under extra_models. Is this the best way to go?
  • I've kept function names as is, however it might be useful to change them. For example, makeAllSearchableUsing() might be better if we call a new function inside it called something like searchableQuery()
  • We don't have some sort of initialization function before indexing, so I've put the maxPositions and categories variables for the Product model into Cache with the array driver. There may be a cleaner way to do this.
  • I didn't touch any of the actual code that was already inside of the IndexProductsCommand for now to make sure we keep feature parity, but the withCategories function can probably be refactored?
  • The Category model had a weird custom primary key name set which broke Scout. I couldn't find anything that this was actually used for, so I restored it back to what it should be.
  • I've put the index prefix inside of the ConfigComposer's getConfig() function. I'm not sure if this is the best way to do this.

$this->line('Store: ' . $store['name']);

foreach ($types as $type => $model) {
config()->set('rapidez.index', $type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to infer this from the model itself? It would save us from setting this value which thanks to your changes doesn't seem needed anymore.

Perhaps a function that reads from an empty model variable/constant, which if it's not set Str::snake() on the class name

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's also an option; currently we do this differently on multiple places, for example:

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to infer this from the model itself? It would save us from setting this value which thanks to your changes doesn't seem needed anymore.

Perhaps a function that reads from an empty model variable/constant, which if it's not set Str::snake() on the class name

I had done this originally, but figured we might want to keep flexibility up for now, and it might not work properly if you overwrite a model with a different class name (e.g. if your code standards require you to call your extended version of a product model ProductExt or whatever).

There's also the thing where it checks if you tried to use Scout directly. Although we'd probably be better off using a different solution for that eventually.

@royduin royduin merged commit e97e3ed into instantsearch Feb 18, 2025
11 of 12 checks passed
@royduin royduin deleted the feature/products-index branch February 18, 2025 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants