Revert #320 and use fetch() instead of load() again (#323)

**Related Issue(s)**

Adds test for #319 
Reverts misguided WIP #320 

laravel/framework#16217

**PR Type**

Test/Fix

**Changes**

- Add test to ensure the global scope is called when using the @hasmany directive
- Add TODO for removing custom fetch logic once Laravel resolves their issue with eager loading

**Breaking changes**

Nope

src/Providers/LighthouseServiceProvider.php

@@ -96,6 +96,8 @@ function () {
      */
     protected function registerCollectionMacros()
     {
+        // TODO remove and just use load() as soon as Laravel fixes https://github.com/laravel/framework/issues/16217
+        // This fixes the behaviour of how eager loading queries are built
         Collection::macro('fetch', function ($eagerLoadRelations = null) {
             if (count($this->items) > 0) {
                 if (is_string($eagerLoadRelations)) {

src/Support/DataLoader/Loaders/BelongsToLoader.php

@@ -28,16 +28,19 @@ public function __construct(string $relation, array $resolveArgs)
      */
     public function resolve(): array
     {
-        $parents = \Illuminate\Database\Eloquent\Collection::make(
-            collect($this->keys)->pluck('parent')
-        );
-
-        return $parents->load([$this->relation => function ($query) {
-            $query->when(isset($args['query.filter']), function ($query) {
-                return QueryFilter::build($query, $this->resolveArgs);
-            });
-        }])->mapWithKeys(function (Model $model) {
-            return [$model->getKey() => $model->getRelation($this->relation)];
-        })->all();
+        return collect($this->keys)
+            ->pluck('parent')
+            // Using our own Collection macro
+            ->fetch([$this->relation =>
+                function ($query) {
+                    $query->when(isset($args['query.filter']), function ($query) {
+                        return QueryFilter::build($query, $this->resolveArgs);
+                    });
+                }
+            ])
+            ->mapWithKeys(function (Model $model) {
+                return [$model->getKey() => $model->getRelation($this->relation)];
+            })
+            ->all();
     }
 }

src/Support/DataLoader/Loaders/HasManyLoader.php

@@ -48,18 +48,22 @@ public function __construct(string $relation, array $resolveArgs, array $scopes,
      */
     public function resolve(): array
     {
-        $eagerLoadRelationWithConstraints = [$this->relation => function ($query) {
-            foreach ($this->scopes as $scope) {
-                call_user_func_array([$query, $scope], [$this->resolveArgs]);
-            }
+        $eagerLoadRelationWithConstraints = [$this->relation =>
+            function ($query) {
+                foreach ($this->scopes as $scope) {
+                    call_user_func_array([$query, $scope], [$this->resolveArgs]);
+                }
 
-            $query->when(isset($args['query.filter']), function ($q) {
-                return QueryFilter::build($q, $this->resolveArgs);
-            });
-        }];
+                $query->when(isset($args['query.filter']), function ($q) {
+                    return QueryFilter::build($q, $this->resolveArgs);
+                });
+            }
+        ];
 
         /** @var Collection $parents */
-        $parents = collect($this->keys)->pluck('parent');
+        $parents = collect($this->keys)
+            ->pluck('parent');
+
         switch ($this->paginationType) {
             case PaginationManipulator::PAGINATION_TYPE_CONNECTION:
             case PaginationManipulator::PAGINATION_ALIAS_RELAY:
@@ -78,8 +82,8 @@ public function resolve(): array
                 $parents->fetchForPage($count, $page, $eagerLoadRelationWithConstraints);
                 break;
             default:
-                $parents = new \Illuminate\Database\Eloquent\Collection($parents);
-                $parents->load($eagerLoadRelationWithConstraints);
+                // Using our own Collection macro
+                $parents->fetch($eagerLoadRelationWithConstraints);
                 break;
         }
 

src/Support/DataLoader/Loaders/HasOneLoader.php

@@ -39,23 +39,23 @@ public function __construct(string $relation, array $resolveArgs, array $scopes)
      */
     public function resolve(): array
     {
-        $eagerLoadRelationWithConstraints = [$this->relation => function ($query) {
-            foreach ($this->scopes as $scope) {
-                call_user_func_array([$query, $scope], [$this->resolveArgs]);
-            }
-
-            $query->when(isset($args['query.filter']), function ($q) {
-                return QueryFilter::build($q, $this->resolveArgs);
-            });
-        }];
-
-        /** @var Collection $parents */
-        $parents = collect($this->keys)->pluck('parent');
-        $parents = new \Illuminate\Database\Eloquent\Collection($parents);
-        $parents->load($eagerLoadRelationWithConstraints);
-
-        return $parents->mapWithKeys(function (Model $model) {
-            return [$model->getKey() => $model->getRelation($this->relation)];
-        })->all();
+        return collect($this->keys)
+            ->pluck('parent')
+            // Using our own Collection macro
+            ->fetch([$this->relation =>
+                function ($query) {
+                    foreach ($this->scopes as $scope) {
+                        call_user_func_array([$query, $scope], [$this->resolveArgs]);
+                    }
+
+                    $query->when(isset($args['query.filter']), function ($q) {
+                        return QueryFilter::build($q, $this->resolveArgs);
+                    });
+                }
+            ])
+            ->mapWithKeys(function (Model $model) {
+                return [$model->getKey() => $model->getRelation($this->relation)];
+            })
+            ->all();
     }
 }

tests/Integration/Schema/Directives/Fields/HasManyDirectiveTest.php

@@ -38,6 +38,11 @@ protected function setUp()
         $this->tasks = factory(Task::class, 3)->create([
             'user_id' => $this->user->getKey(),
         ]);
+        factory(Task::class)->create([
+            'user_id' => $this->user->getKey(),
+            // This task should be ignored via global scope on the Task model
+            'name' => 'cleaning'
+        ]);
 
         $this->be($this->user);
     }
@@ -72,6 +77,10 @@ public function itCanQueryHasManyRelationship()
         }
         ');
 
+        $tasksWithoutGlobalScope = auth()->user()->tasks()->withoutGlobalScope('no_cleaning')->count();
+        $this->assertSame(4, $tasksWithoutGlobalScope);
+
+        // Ensure global scopes are respected here
         $this->assertCount(3, array_get($result->data, 'user.tasks'));
     }
 
@@ -82,8 +91,8 @@ public function itCanQueryHasManyPaginator()
     {
         $schema = '
         type User {
-            tasks: [Task!]! @hasMany(type:"paginator")
-            posts: [Post!]! @hasMany(type:"paginator")
+            tasks: [Task!]! @hasMany(type: "paginator")
+            posts: [Post!]! @hasMany(type: "paginator")
         }
         
         type Task {
@@ -129,7 +138,7 @@ public function itCanQueryHasManyRelayConnection()
     {
         $schema = '
         type User {
-            tasks: [Task!]! @hasMany(type:"relay")
+            tasks: [Task!]! @hasMany(type: "relay")
         }
         
         type Task {
@@ -169,7 +178,7 @@ public function itCanQueryHasManyNestedRelationships()
     {
         $schema = '
         type User {
-            tasks: [Task!]! @hasMany(type:"relay")
+            tasks: [Task!]! @hasMany(type: "relay")
         }
         
         type Task {

tests/Integration/Support/DataLoader/QueryBuilderTest.php

@@ -34,6 +34,7 @@ public function setUp()
     public function itCanLoadRelationshipsWithLimitsOnCollection()
     {
         $users = User::all();
+        // TODO remove this as soon as Laravel fixes https://github.com/laravel/framework/issues/16217
         $users->fetch(['tasks' => function ($query) {
             $query->take(3);
         }]);

tests/Utils/Models/Task.php

@@ -3,6 +3,7 @@
 namespace Tests\Utils\Models;
 
 use Illuminate\Database\Eloquent\Model;
+use Illuminate\Database\Eloquent\Builder;
 use Illuminate\Database\Eloquent\SoftDeletes;
 use Illuminate\Database\Eloquent\Relations\HasOne;
 use Illuminate\Database\Eloquent\Relations\BelongsTo;
@@ -14,6 +15,16 @@ class Task extends Model
 
     protected $guarded = [];
 
+    protected static function boot()
+    {
+        parent::boot();
+
+        // This is used to test that this scope works in all kinds of queries
+        static::addGlobalScope('no_cleaning', function (Builder $builder) {
+            $builder->where('name', '!=', 'cleaning');
+        });
+    }
+
     public function user(): BelongsTo
     {
         return $this->belongsTo(User::class);