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.

Sign up for GitHub

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

[9.0] Add support for self join relationships. #2051

Merged
merged 10 commits into from
Apr 24, 2019
Merged

[9.0] Add support for self join relationships. #2051

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
6c7f997
Alias for joined table in BelongsTo
Morinohtar Apr 10, 2019
db33565
fix(QueryDataTableTest): replace deprecated function assertContains w…
Morinohtar Apr 10, 2019
8ba66b3
fix(EloquentDataTable): proper alias syntax according to database dri…
Morinohtar Apr 11, 2019
e7d6800
fix(EloquentDataTable): proper alias syntax according to database dri…
Morinohtar Apr 11, 2019
55a15fe
Merge branch '9.0' of github.com:Morinohtar/laravel-datatables into 9.0
Morinohtar Apr 11, 2019
2b637c4
Merge branch '9.0' of github.com:Morinohtar/laravel-datatables into 9.0
Morinohtar Apr 11, 2019
00cbfad
Merge branch '9.0' of github.com:Morinohtar/laravel-datatables into 9.0
Morinohtar Apr 11, 2019
1fbe5eb
chore(EloquentDataTable): proper code style.
Morinohtar Apr 11, 2019
5daf655
chore: reverting code.
Morinohtar Apr 16, 2019
b953eac
chore: reverting code.
Morinohtar Apr 16, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/EloquentDataTable.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ protected function joinEagerLoadedColumn($relation, $relationColumn)
$table = '';
$deletedAt = false;
$lastQuery = $this->query;
foreach (explode('.', $relation) as $eachRelation) {
foreach (explode('.', $relation) as $index => $eachRelation) {
$model = $lastQuery->getRelation($eachRelation);
switch (true) {
case $model instanceof BelongsToMany:
Expand Down Expand Up @@ -169,19 +169,23 @@ protected function joinEagerLoadedColumn($relation, $relationColumn)

case $model instanceof BelongsTo:
$table = $model->getRelated()->getTable();
$alias = "alias_{$index}_{$table}";
$tableAs = "{$table} AS $alias";
Copy link
Owner

Choose a reason for hiding this comment

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

This will break some installations with Oracle database. Please use "{$table} $alias" removing the AS from query.

Copy link
Contributor Author

@Morinohtar Morinohtar Apr 11, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

But if i remove the AS then MySQL won't work... ok, i managed something, update coming soon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you run your tests again now?

Copy link
Owner

@yajra yajra Apr 12, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry but your previous PR version is much better. AFAIK, it should work on MySQL since that is one of the reasons why I created this package version to work on Oracle.

On the other hand, I think I already created a patch that properly translates Oracle AS query. Will review with my other package https://github.com/yajra/laravel-oci8 too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So, what now? You want the first one again?

$foreign = $model->getQualifiedForeignKeyName();
$other = $model->getQualifiedOwnerKeyName();
$owner = "{$alias}.{$model->getOwnerKeyName()}";
$deletedAt = $this->checkSoftDeletesOnModel($model->getRelated());
break;

default:
throw new Exception('Relation ' . get_class($model) . ' is not yet supported.');
}
$this->performJoin($table, $foreign, $other, $deletedAt);
$this->performJoin($tableAs ?? $table, $foreign, $owner ?? $other, $deletedAt);
$lastQuery = $model->getQuery();
}

return $table . '.' . $relationColumn;
$table = $alias ?? $table;

return "{$table}.{$relationColumn}";
}

protected function checkSoftDeletesOnModel($model)
Expand Down
2 changes: 1 addition & 1 deletion tests/Integration/QueryDataTableTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ public function it_allows_search_on_added_column_with_custom_filter_handler()
]);

$queries = $crawler->json()['queries'];
$this->assertContains('"1" = ?', $queries[1]['query']);
$this->assertStringContainsString('"1" = ?', $queries[1]['query']);
}

protected function setUp(): void
Expand Down