-
Notifications
You must be signed in to change notification settings - Fork 244
Conversation
No "cross-installation" possible due to changed method signature Pass around new $builder var Also: Use getScoutModelsByIds/getScoutKey Inspired by laravel/scout@4.0...5.0#diff-f2faa85f27580a9b67d73a8a273bfef1 / laravel/scout@9f06b9c
Looks like we need to drop PHP5 from the Build Matrix, as Scout5 / Laravel5.6 also require PHP7+ now. Again, the user needed to deal with this because of Laravel already (Installation conflicts otherwise), not this package. |
Hi, Great job but why don't make it compatible with Elastic 6+ at the same time with the problems of mapping ?(https://www.elastic.co/guide/en/elasticsearch/reference/6.x/removal-of-types.html) |
I favor to make one PR per feature rather than confusing big PRs with a lot of stuff. We can make a seperate PR for that and maybe also backport it to the tree with Scout4 then. |
@ErickTamayo - Any chance you can merge this today? The checks passed, and it's just waiting on a merge. |
Merge pleeeeeeeeeeease! <3 |
For now, until the author merges this pull request it's possible to use the forked package by pulling it in using a custom composer repository:
|
* Updating 0.0.1 (ErickTamayo#8) * fixing config key + updating the Usage on README (ErickTamayo#6) * Rename ElasticsearchEngineTets.php to ElasticsearchEngineTest.php (ErickTamayo#7) * Updating 0.0.1 (ErickTamayo#11) * fixing config key + updating the Usage on README (ErickTamayo#6) * Rename ElasticsearchEngineTets.php to ElasticsearchEngineTest.php (ErickTamayo#7) * Update ElasticsearchEngine.php (ErickTamayo#10) * Updating 0.0.1 (ErickTamayo#14) * fixing config key + updating the Usage on README (ErickTamayo#6) * Rename ElasticsearchEngineTets.php to ElasticsearchEngineTest.php (ErickTamayo#7) * Update ElasticsearchEngine.php (ErickTamayo#10) * optimized creation of params array for better performance when dealing with huge amount of models (ErickTamayo#13) * Make stable (ErickTamayo#18) (ErickTamayo#19) * Bumped Scout version to v2.0.x Bumped elasticsearch/elasticsearch to v5.0 * Bumped Scout version to 3.0 * Fix config name (ErickTamayo#35) * v3.0.2 bumping to php 5.6.6 (Earliest version that can support php elasticsearch) (ErickTamayo#48) `within` now is being taken into account when specified ErickTamayo#31 Now if the record is not found on the db will not throw undefined offset ErickTamayo#36 Implemented orderBy ErickTamayo#46 * Update ElasticsearchProvider.php (ErickTamayo#54) use `app` instead of `resolve` to make lumen 5.4 compatible. `app` and `resolve` are identical since 5.4, see https://github.com/laravel/framework/blob/5.4/src/Illuminate/Foundation/helpers.php#L690 * chain `values` method in `map` function (ErickTamayo#66) In ```php collect($results['hits']['hits'])->map(function ($hit) use ($model, $models) { return isset($models[$hit['_id']]) ? $models[$hit['_id']] : null; })->filter(); ``` would generate some holds which would cause `paginate` results objects rather than list. * Support for "WHERE IN" searches if array is passed to `where` (ErickTamayo#55) Right now passing an array results in: "illegal_state_exception: Can't get text on a START_ARRAY" Solution based on http://stackoverflow.com/a/40737488/7362396 * Execute the callback that can be passed to the Builder in performSearch (ErickTamayo#56) Context: laravel/scout#111 Implementation analogical to the official AlgoliaEngine implementation: https://github.com/laravel/scout/blob/v3.0.3/src/Engines/AlgoliaEngine.php#L112-L119 * Update ElasticsearchEngine.php (ErickTamayo#51) Predefine elastic variable. * Enable Laravel5.5 package Auto-Discovery (ErickTamayo#81) * Removed count function. Fixes ErickTamayo#80 (ErickTamayo#84) Since php 7.2, count a number will generate a warning. Because $results['hits']['total'] is a number we can just compare it to zero. `if ($results['hits']['total'] === 0)` * Allow installation with Scout 4 (ErickTamayo#99) * Compatibility with Scout 5 (ErickTamayo#106) * Compatibility with Scout 5 No "cross-installation" possible due to changed method signature Pass around new $builder var Also: Use getScoutModelsByIds/getScoutKey Inspired by laravel/scout@4.0...5.0#diff-f2faa85f27580a9b67d73a8a273bfef1 / laravel/scout@9f06b9c * Remove PHP5 as laravel/scout^5 / laravel5.6 also removed it * Added support for Laravel Scout 6.0 and 7.0, Laravel 5.8 (ErickTamayo#119) * Elasticsearch downgrade Co-authored-by: Erick Tamayo <ErickTamayo@users.noreply.github.com> Co-authored-by: Till Krüss <tillkruss@users.noreply.github.com> Co-authored-by: Joey <joey.xf@gmail.com> Co-authored-by: Tobias Kronthaler <kronthto@users.noreply.github.com> Co-authored-by: Nirav <nirav.t.prajapati21@gmail.com> Co-authored-by: Camilo <camilo.castro.cabrera@gmail.com> Co-authored-by: Semyon Chetvertnyh <semyon.chetvertnyh@gmail.com>
In #99 it was possible to support both, v3 & v4 of Scout. Unfortunately, this is no longer possible for v5 due to a changed method signature.
I reviewed the changes in Scout 5 and figured the only thing that changed was that we now pass around another callback. This also made it necessary to use Scouts
getScoutModelsByIds
for something that was coded in this package until now (I think this is better, one less function we need to care about/test).I adjusted the unit-tests to pass again. Also, I checked out this branch in one of my projects where I use this package, and tested querying, indexing, and deleting, with success.
I think this should be tagged MINOR (
3.1.0
), to leave room in3.0.x
to backport fixes, but also allow Composer to automatically update it when Scout 5 is installed. Composer will not auto-update aslong as the user has a constraint onlaravel/scout:^4
. And if he upgrades Scout to 5 he has to deal with their breaking changes. I don't think there are any in this PR then.I tried to keep it minimal, we can make other upgrades like the PHP-ES-Client in seperate PRs.