Skip to content

Commit

Permalink
bug #44 Fix sorting grids by nullable path (pamil)
Browse files Browse the repository at this point in the history
This PR was merged into the 1.7 branch.

Discussion
----------

#### Background

- Author can have a nationality assigned, but does not have to
- Nationality have a name
- There are three authors with nationality assigned and two without

#### Actual behaviour

When I sort by nationality, it shows only the three authors that have a nationality assigned

#### Expected behaviour

When I sort by nationality, I would expect it to include all five authors, treating the ones without the nationality as it would be empty

Commits
-------

ca1b3e8 Reproduce: when sorted by nullable path, the grid does not include rows that does not have that path
f720e11 Fix sorting grids by nullable path
  • Loading branch information
pamil authored Jan 14, 2020
2 parents ba6deed + f720e11 commit 78bddd5
Show file tree
Hide file tree
Showing 6 changed files with 27 additions and 2 deletions.
2 changes: 1 addition & 1 deletion src/Bundle/Doctrine/ORM/ExpressionBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ private function resolveFieldByAddingJoins(string $field): string
}
}

$this->queryBuilder->innerJoin($rootAndAssociationField, $associationAlias);
$this->queryBuilder->leftJoin($rootAndAssociationField, $associationAlias);
$resolvedField = sprintf('%s.%s', $associationAlias, $remainder);
}

Expand Down
2 changes: 2 additions & 0 deletions src/Bundle/Tests/DataFixtures/ORM/fixtures.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ AppBundle\Entity\Author:
author_john_watson:
name: "John Watson"
nationality: "@nationality_english"
author_random:
name: "Random Author Without Nationality"
author_{1..19}:
name: "<firstName()> <lastName()>"
nationality: "@nationality_english"
Expand Down
17 changes: 17 additions & 0 deletions src/Bundle/Tests/Functional/GridApiTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,23 @@ public function it_filters_books_by_author_when_an_author_is_used_in_join_in_que
$this->assertSame('A Study in Scarlet', $this->getFirstItemFromCurrentResponse()['title']);
}

/** @test */
public function it_includes_all_rows_even_when_sorting_by_a_nullable_path(): void
{
$this->client->request('GET', '/authors/');
$totalItemsCountBeforeSorting = $this->getTotalItemsCountFromCurrentResponse();

$this->client->request('GET', '/authors/?sorting[nationality]=desc');
$totalItemsCountAfterSorting = $this->getTotalItemsCountFromCurrentResponse();

$this->assertSame($totalItemsCountBeforeSorting, $totalItemsCountAfterSorting);
}

private function getTotalItemsCountFromCurrentResponse(): int
{
return json_decode($this->client->getResponse()->getContent(), true)['total'];
}

private function getItemsFromCurrentResponse(): array
{
return json_decode($this->client->getResponse()->getContent(), true)['_embedded']['items'];
Expand Down
2 changes: 1 addition & 1 deletion src/Bundle/Tests/Responses/Expected/authors_grid.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"page": 1,
"limit": 10,
"pages": 3,
"total": 21,
"total": 22,
"_links": {
"self": {
"href": "\/authors\/?page=1&limit=10"
Expand Down
5 changes: 5 additions & 0 deletions src/Bundle/test/app/config/grids.yml
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,11 @@ sylius_grid:
type: string
label: Name
sortable: ~
nationality:
type: string
label: Name
sortable: nationality.name
path: nationality.name
limits: [10, 5, 15]

app_book_by_american_authors:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,5 @@ AppBundle\Entity\Author:
joinColumn:
name: nationality_id
referencedColumnName: id
nullable: true
cascade: ["all"]

0 comments on commit 78bddd5

Please sign in to comment.