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

Nested sort #245

Open
joelatgrayv opened this issue Jul 16, 2024 · 7 comments
Open

Nested sort #245

joelatgrayv opened this issue Jul 16, 2024 · 7 comments

Comments

@joelatgrayv
Copy link

Hi,

Is there a way to sort on nested documents?

    'sort' => [
            'nested_field.attribute' => [
                'order' => 'asc',
                'nested' => [
                    'path' => 'nested_field'
                ]
            ]
        ]
@Jeroen-G
Copy link
Owner

Possibly, I never tried it 😅

@joelatgrayv
Copy link
Author

joelatgrayv commented Jul 16, 2024

@Jeroen-G I added support for it in a fork: https://github.com/SauceIndustries/Explorer. However, I'm getting
1 covered mutants were not detected. I'm not sure how to resolve it...

Infection is trying to modify a line but it doesn't actually change anything so the tests continue to pass:

--- Original
+++ New
@@ @@
         if (!empty($this->nestedPath)) {
             $result[$this->field]['nested_path'] = $this->nestedPath;
         }
-        return $result;
+        return count($result) > 1 ? array_slice($result, 0, 1, true) : $result;
     }
 }
Screenshot 2024-07-16 at 2 27 12 PM

@Jeroen-G
Copy link
Owner

Jeroen-G commented Jul 17, 2024

If all test pass after infection changes something then you are either missing a test, or a test is doing not enough.

In this case I think there is no test where build() is called while $result is empty.

I think infection.json outputs also the name of the mutator, which is then explained on https://infection.github.io/guide/mutators.html

@joelatgrayv
Copy link
Author

Thanks for looking. How could that ever happen?

$field can't be null
$order asserts that it's asc or desc
$result is initialized with those values.

@Jeroen-G
Copy link
Owner

Looking at it again, if you write a test for this function both with the if outcome true and false and you assert the result in both tests then you should be covered.

@Jeroen-G
Copy link
Owner

Let me know when you need more help!

@joelatgrayv
Copy link
Author

joelatgrayv commented Jul 18, 2024

I have all lines covered. Here is the coverage:

Screenshot 2024-07-18 at 7 14 18 AM

Here are the tests from SortOrderTests

    public function test_it_builds_sorting(): void
    {
        $sort = Sorting::for(
            new Sort(':fld:', SortOrder::DESCENDING),
        );

        self::assertSame([ 'sort' => [ [ ':fld:' => ['order' => 'desc'] ]]],$sort->build());
    }

    public function test_it_builds_sorting_with_nested(): void
    {
        $sort = Sorting::for(
            new Sort('foreign_model.name', SortOrder::DESCENDING, 'foreign_model'),
        );

        $expected = [ 'sort' => [ [ 'foreign_model.name' => ['order' => 'desc', 'nested_path' => 'foreign_model'] ]]];
        $result = $sort->build();
        self::assertSame($expected,$result);
    }

And another from FinderTest:

public function test_it_accepts_a_sortable_query_with_nested_sort(): void
    {
        $client = Mockery::mock(Client::class);
        $client->expects('search')
            ->with([
                'index' => self::TEST_INDEX,
                'body' => [
                    'query' => [
                        'bool' => [
                            'must' => [],
                            'should' => [],
                            'filter' => [],
                        ],
                    ],
                    'sort' => [
                        ['foreign_model.name' => [
                            'order' => 'desc',
                            'nested_path' => 'foreign_model',
                        ]],
                    ],
                ],
            ])
            ->andReturn([
                'hits' => [
                    'total' => ['value' => 1],
                    'hits' => [$this->hit()],
                ],
            ]);

        $query = Query::with(new BoolQuery());
        $builder = new SearchCommand(self::TEST_INDEX, $query);
        $query->setSort([new Sort('foreign_model.name', SortOrder::DESCENDING, 'foreign_model')]);

        $subject = new Finder($client, $builder);
        $results = $subject->find();

        self::assertCount(1, $results);
    }

The problem is that the mutation doesn't actually change anything. It does this with the result:

return count($result) > 1 ? array_slice($result, 0, 1, true) : $result;

But that doesn't change anything so the results are the same and it's expecting it to be different and fail.

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

No branches or pull requests

2 participants