Fix: typesense missing scoutmetadata

[arminwinkt]

This Pull Request fixes the missing scoutMetadata for typesense search results.

As written in the Typesense documentation, there are additional data that gets returned by typesense.

For example highlights, which shows what to highlight in your search results.

For me specifically the geo_distance_meters value was very intresting if you query for a geo search:

This PR adds all this additional data to the model via scouts scoutMetadata

[taylorotwell]

Drafting pending response to feedback.

[arminwinkt]

Thank you @tharropoulos for your feedback.
Added some more tests as you mentioned.
Looking forward to your second review.

[tharropoulos]

Thank you @tharropoulos for your feedback. Added some more tests as you mentioned. Looking forward to your second review.

All looks well, but could you lastly add some integration tests as well? As we are mocking the Typesense Client itself, we could miss some details in the process. Otherwise, this covers every branch inside the function.

[jonjakoblich]

Thank you @tharropoulos for your feedback. Added some more tests as you mentioned. Looking forward to your second review.

All looks well, but could you lastly add some integration tests as well? As we are mocking the Typesense Client itself, we could miss some details in the process. Otherwise, this covers every branch inside the function.

@tharropoulos I have need of this functionality for Typesense and was ready to write the tests myself, but I see that by default the codebase excludes the integration tests for external services in phpunit.xml.dist.

What is your recommendation to write and test one to meet your requirements?

[tharropoulos]

Thank you @tharropoulos for your feedback. Added some more tests as you mentioned. Looking forward to your second review.

All looks well, but could you lastly add some integration tests as well? As we are mocking the Typesense Client itself, we could miss some details in the process. Otherwise, this covers every branch inside the function.

@tharropoulos I have need of this functionality for Typesense and was ready to write the tests myself, but I see that by default the codebase excludes the integration tests for external services in phpunit.xml.dist.

What is your recommendation to write and test one to meet your requirements?

One thing I did is run the integration tests locally, to ensure that it works for an actual Typesense server instance. As soon as you add the tests to SC, we'll be able to test them against a local Typesense instance

[jonjakoblich]

One thing I did is run the integration tests locally, to ensure that it works for an actual Typesense server instance. As soon as you add the tests to SC, we'll be able to test them against a local Typesense instance

@tharropoulos Please forgive me, I am new to contributing to Laravel packages. I played around with the integration tests last night. It seems like I would need to install this package on a functioning Laravel app to run the external-network group integration tests.

Does that sound about right? If not, how are you guys running them?

Otherwise the current integration tests fail as they are for the group external-network since it cannot grab a working config even after making the needed adjustments to phpunit.xml.dist to add the TYPESENSE_API_KEY env variable.

[tharropoulos]

One thing I did is run the integration tests locally, to ensure that it works for an actual Typesense server instance. As soon as you add the tests to SC, we'll be able to test them against a local Typesense instance

@tharropoulos Please forgive me, I am new to contributing to Laravel packages. I played around with the integration tests last night. It seems like I would need to install this package on a functioning Laravel app to run the external-network group integration tests.

Does that sound about right? If not, how are you guys running them?

Otherwise the current integration tests fail as they are for the group external-network since it cannot grab a working config even after making the needed adjustments to phpunit.xml.dist to add the TYPESENSE_API_KEY env variable.

On my end I just remove the skip group from the phpunit config and run:

❯ ./vendor/bin/phpunit tests/Integration/TypesenseSearchableTest.php

[jonjakoblich]

On my end I just remove the skip group from the phpunit config and run:

❯ ./vendor/bin/phpunit tests/Integration/TypesenseSearchableTest.php

For me all 8 current integration tests fail with the following error.

Typesense\Exceptions\RequestMalformed: Parameter fields is required.

I know that error can be rectified by setting up the scout config file, but that doesn't appear to make a difference in the integration tests. Just wondering what I need to do to get it up and running or what is different about your testing environment from mine.

[tharropoulos]

On my end I just remove the skip group from the phpunit config and run: ❯ ./vendor/bin/phpunit tests/Integration/TypesenseSearchableTest.php

For me all 8 current integration tests fail with the following error.

Typesense\Exceptions\RequestMalformed: Parameter fields is required.

I know that error can be rectified by setting up the scout config file, but that doesn't appear to make a difference in the integration tests. Just wondering what I need to do to get it up and running or what is different about your testing environment from mine.

In the config/scout.php file, you need to un-comment the User configuration for Typesense:

             User::class => [
                 'collection-schema' => [
                     'fields' => [
                         [
                             'name' => 'id',
                             'type' => 'string',
                         ],
                         [
                             'name' => 'name',
                             'type' => 'string',
                         ],
                         [
                             'name' => 'created_at',
                             'type' => 'int64',
                         ],
                     ],
                     'default_sorting_field' => 'created_at',
                 ],
                 'search-parameters' => [
                     'query_by' => 'name'
                 ],
             ],

while also importing the fixture itself:

use Laravel\Scout\Tests\Fixtures\User;

[jonjakoblich]

On my end I just remove the skip group from the phpunit config and run: ❯ ./vendor/bin/phpunit tests/Integration/TypesenseSearchableTest.php

For me all 8 current integration tests fail with the following error. Typesense\Exceptions\RequestMalformed: Parameter fields is required. I know that error can be rectified by setting up the scout config file, but that doesn't appear to make a difference in the integration tests. Just wondering what I need to do to get it up and running or what is different about your testing environment from mine.

In the config/scout.php file, you need to un-comment the User configuration for Typesense:

             User::class => [
                 'collection-schema' => [
                     'fields' => [
                         [
                             'name' => 'id',
                             'type' => 'string',
                         ],
                         [
                             'name' => 'name',
                             'type' => 'string',
                         ],
                         [
                             'name' => 'created_at',
                             'type' => 'int64',
                         ],
                     ],
                     'default_sorting_field' => 'created_at',
                 ],
                 'search-parameters' => [
                     'query_by' => 'name'
                 ],
             ],

while also importing the fixture itself:

use Laravel\Scout\Tests\Fixtures\User;

Yes, I can confirm I have tried and I am doing all of those things, yet still getting the aforementioned error messages for each Typesense integration test. Thank you for taking the time to troubleshoot.

[jonjakoblich]

Hi @tharropoulos. I am giving this another go. In addition to your instructions above I added the following line to phpunit.xml.dist

<env name="TYPESENSE_API_KEY" value="xyz"/>

I imported the user fixture into the Scout config file, and it seems all the Typesense integration tests (except one) return an empty array instead of the expected data, resulting in 7 failures out of 8 assertions and 8 tests. This is also true with a fresh clone of the latest version of Scout.

Are you getting this too?

[tharropoulos]

Hi @tharropoulos. I am giving this another go. In addition to your instructions above I added the following line to phpunit.xml.dist

<env name="TYPESENSE_API_KEY" value="xyz"/>

I imported the user fixture into the Scout config file, and it seems all the Typesense integration tests (except one) return an empty array instead of the expected data, resulting in 7 failures out of 8 assertions and 8 tests. This is also true with a fresh clone of the latest version of Scout.

Are you getting this too?

Hey there, yeah, seems so. I got around it by first removing the default_sorting_field in the Typesense collection config casting the id of the User fixture to string :

-            'id' => (int) $this->id,
+            'id' => (string) $this->id,

and adding this to the TestCase abc:

 use Orchestra\Testbench\Concerns\WithWorkbench;
 
 use function Orchestra\Testbench\artisan;
+use Illuminate\Support\Facades\DB;
+
 use function Orchestra\Testbench\remote;
 
 abstract class TestCase extends \Orchestra\Testbench\TestCase
 {
     use WithWorkbench;
 
+    protected function getEnvironmentSetUp($app)
+    {
+        // Setup SQLite in-memory database
+        $app['config']->set('database.default', 'testbench');
+        $app['config']->set('database.connections.testbench', [
+            'driver' => 'sqlite',
+            'database' => ':memory:',
+            'prefix' => '',
+        ]);
+    }
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+
+        $this->artisan('migrate:fresh');
+
+        DB::reconnect();
+
+        if (method_exists($this, 'defineDatabaseMigrations')) {
+            $this->defineDatabaseMigrations();
+        }
+
+        if (method_exists($this, 'afterRefreshingDatabase')) {
+            $this->afterRefreshingDatabase();
+        }
+    }

This way it actually indexes the collection to Typesense. The tests may not match the actual sort order (depends on your typesense version), so just confirm the length / existence of the metadata itself in the test