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

[5.6] Fix for HasManyThrough returning incorrect results with chunk()… #24096

Merged
merged 1 commit into from
May 3, 2018
Merged

[5.6] Fix for HasManyThrough returning incorrect results with chunk()… #24096

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
60 changes: 50 additions & 10 deletions src/Illuminate/Database/Eloquent/Relations/HasManyThrough.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,36 @@ public function updateOrCreate(array $attributes, array $values = [])

return $instance;
}
/**
* Chunk the results of the query.
*
* @param int $count
* @param callable $callback
* @return bool
*/
public function chunk($count, callable $callback)
{
$builder = $this->getBuilder();
return $builder->chunk($count,$callback);
}

/**
* Execute a callback over each item while chunking.
*
* @param callable $callback
* @param int $count
* @return bool
*/
public function each(callable $callback, $count = 1000)
{
return $this->chunk($count, function ($results) use ($callback) {
foreach ($results as $key => $value) {
if ($callback($value, $key) === false) {
return false;
}
}
});
}

/**
* Execute the query and get the first related model.
Expand Down Expand Up @@ -354,16 +384,8 @@ public function getResults()
*/
public function get($columns = ['*'])
{
// First we'll add the proper select columns onto the query so it is run with
// the proper columns. Then, we will get the results and hydrate out pivot
// models with the result of those columns as a separate model relation.
$columns = $this->query->getQuery()->columns ? [] : $columns;

$builder = $this->query->applyScopes();

$models = $builder->addSelect(
$this->shouldSelect($columns)
)->getModels();
$builder = $this->getBuilder($columns);
$models = $builder->getModels();

// If we actually found models we will also eager load any relationships that
// have been specified as needing to be eager loaded. This will solve the
Expand Down Expand Up @@ -517,4 +539,22 @@ public function getQualifiedLocalKeyName()
{
return $this->farParent->qualifyColumn($this->localKey);
}

/**
* Add the proper select columns onto the query so it is run with the proper
* columns. and return a builder instance with the correct columns.
*
* @param array $columns
* @return \Illuminate\Database\Eloquent\Builder
*/
private function getBuilder($columns = ['*']){

$columns = $this->query->getQuery()->columns ? [] : $columns;

$builder = $this->query->applyScopes();

return $builder->addSelect(
$this->shouldSelect($columns)
);
}
}
58 changes: 57 additions & 1 deletion tests/Database/DatabaseEloquentHasManyThroughIntegrationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,6 @@ public function testAllColumnsAreRetrievedByDefault()
{
$this->seedData();
$post = HasManyThroughTestCountry::first()->posts()->first();

$this->assertEquals([
'id',
'user_id',
Expand All @@ -181,6 +180,43 @@ public function testOnlyProperColumnsAreSelectedIfProvided()
], array_keys($post->getAttributes()));
}

public function testChunkReturnsCorrectModels(){
$this->seedData();
$this->seedDataExtended();
$country = HasManyThroughTestCountry::find(2);

$country->posts()->chunk(10,function($postsChunk){
$post = $postsChunk->first();
$this->assertEquals([
'id',
'user_id',
'title',
'body',
'email',
'created_at',
'updated_at',
'country_id'],array_keys($post->getAttributes()));

});
}
public function testEachReturnsCorrectModels(){
$this->seedData();
$this->seedDataExtended();
$country = HasManyThroughTestCountry::find(2);

$country->posts()->each(function($post){
$this->assertEquals([
'id',
'user_id',
'title',
'body',
'email',
'created_at',
'updated_at',
'country_id'],array_keys($post->getAttributes()));

});
}
public function testIntermediateSoftDeletesAreIgnored()
{
$this->seedData();
Expand Down Expand Up @@ -214,6 +250,26 @@ protected function seedData()
['title' => 'Another title', 'body' => 'Another body', 'email' => 'taylorotwell@gmail.com'],
]);
}
protected function seedDataExtended(){
$country = HasManyThroughTestCountry::create(['id' => 2, 'name' => 'United Kingdom', 'shortname' => 'uk']);
$country->users()->create(['id' => 2, 'email' => 'example1@gmail.com', 'country_short' => 'uk'])
->posts()->createMany([
['title' => 'Example1 title1', 'body' => 'Example1 body1', 'email' => 'example1post1@gmail.com'],
['title' => 'Example1 title2', 'body' => 'Example1 body2', 'email' => 'example1post2@gmail.com']
]);
$country->users()->create(['id' => 3, 'email' => 'example2@gmail.com', 'country_short' => 'uk'])
->posts()->createMany([
['title' => 'Example2 title1', 'body' => 'Example2 body1', 'email' => 'example2post1@gmail.com'],
['title' => 'Example2 title2', 'body' => 'Example2 body2', 'email' => 'example2post2@gmail.com']
]);
$country->users()->create(['id' => 4, 'email' => 'example3@gmail.com', 'country_short' => 'uk'])
->posts()->createMany([
['title' => 'Example3 title1', 'body' => 'Example3 body1', 'email' => 'example3post1@gmail.com'],
['title' => 'Example3 title2', 'body' => 'Example3 body2', 'email' => 'example3post2@gmail.com']
]);


}

/**
* Seed data for a default HasManyThrough setup.
Expand Down