fix issue with scopes being applied twice in paginated queries on BelongsToManyThrough

Author: alexweissman

app/sprinkles/account/src/Database/Migrations/v400/UsersTable.php

@@ -13,7 +13,7 @@
 
 /**
  * Users table migration
- * Removed the 'display_name', 'title', 'secret_token', and 'flag_password_reset' fields, and added first and last name and 'last_activity_at'.
+ * Removed the 'display_name', 'title', 'secret_token', and 'flag_password_reset' fields, and added first and last name and 'last_activity_id'.
  * Version 4.0.0
  *
  * See https://laravel.com/docs/5.4/migrations#tables

app/sprinkles/core/src/Database/Relations/Concerns/Unique.php

@@ -274,25 +274,30 @@ public function getPaginatedQuery(Builder $query, $limit = null, $offset = null)
         // the desired set of unique model _ids_, and then constrain our final query
         // to these models with a whereIn clause.
         $relatedKeyName = $this->related->getQualifiedKeyName();
-        $constrainedBuilder = $constrainedBuilder
-                                ->select($relatedKeyName)
-                                ->groupBy($relatedKeyName);
+
+        // Apply an additional scope to override any selected columns in other global scopes
+        $uniqueIdScope = function ($subQuery) use ($relatedKeyName) {
+            $subQuery->select($relatedKeyName)
+                     ->groupBy($relatedKeyName);
+        };
+
+        $identifier = spl_object_hash($uniqueIdScope);
+
+        $constrainedBuilder->withGlobalScope($identifier, $uniqueIdScope);
 
         if ($limit) {
-            $constrainedBuilder = $constrainedBuilder->limit($limit);
+            $constrainedBuilder->limit($limit);
         }
 
         if ($offset) {
-            $constrainedBuilder = $constrainedBuilder->offset($offset);
+            $constrainedBuilder->offset($offset);
         }
 
         $primaryKeyName = $this->getParent()->getKeyName();
         $modelIds = $constrainedBuilder->get()->pluck($primaryKeyName)->toArray();
 
         // Modify the unconstrained query to limit to these models
-        $query = $query->whereIn($relatedKeyName, $modelIds);
-
-        return $query;
+        return $query->whereIn($relatedKeyName, $modelIds);
     }
 
     /**
@@ -323,18 +328,18 @@ public function getModels($columns = ['*'], $condenseModels = true)
         // models with the result of those columns as a separate model relation.
         $columns = $this->query->getQuery()->columns ? [] : $columns;
 
+        // Add any necessary pagination on the related models
+        if ($this->limit || $this->offset) {
+            $this->getPaginatedQuery($this->query, $this->limit, $this->offset);
+        }
+
         // Apply scopes to the Eloquent\Builder instance.
         $builder = $this->query->applyScopes();
 
         $builder = $builder->addSelect(
             $this->shouldSelect($columns)
         );
 
-        // Add any necessary pagination on the related models
-        if ($this->limit || $this->offset) {
-            $builder = $this->getPaginatedQuery($builder, $this->limit, $this->offset);
-        }
-
         $models = $builder->getModels();
 
         // Hydrate the pivot models so we can load the via models

app/sprinkles/core/tests/Unit/BelongsToManyThroughTest.php

@@ -8,6 +8,7 @@
 namespace UserFrosting\Tests\Unit;
 
 use Illuminate\Database\Capsule\Manager as DB;
+use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
 use Illuminate\Database\Eloquent\Relations\BelongsToMany;
 use Mockery as m;
 use ReflectionClass;
@@ -31,17 +32,19 @@ public function tearDown()
 
     function testPaginatedQuery()
     {
+        // Creates a real BelongsToManyThrough object
         $relation = $this->getRelation();
 
         // We need to define a mock base query, because Eloquent\Builder will pass through many calls
         // to this underlying Query\Builder object.
         $baseQuery = m::mock(QueryBuilder::class);
-        $builder = m::mock('Illuminate\Database\Eloquent\Builder', [$baseQuery])->makePartial();
+        $builder = m::mock(EloquentBuilder::class, [$baseQuery])->makePartial();
 
         $related = $relation->getRelated();
         $related->shouldReceive('getQualifiedKeyName')->once()->andReturn('users.id');
-        $builder->shouldReceive('select')->once()->with('users.id')->andReturnSelf();
-        $builder->shouldReceive('groupBy')->once()->with('users.id')->andReturnSelf();
+
+        $builder->shouldReceive('withGlobalScope')->once()->andReturnSelf();
+
         $builder->shouldReceive('limit')->once()->with(2)->andReturnSelf();
         $builder->shouldReceive('offset')->once()->with(1)->andReturnSelf();
 
@@ -63,14 +66,14 @@ function testPaginatedQuery()
     protected function getRelation()
     {
         // We simulate a BelongsToManyThrough relationship that gets all related users for a specified permission(s).
-        $builder = m::mock('Illuminate\Database\Eloquent\Builder');
+        $builder = m::mock(EloquentBuilder::class);
         $related = m::mock('Illuminate\Database\Eloquent\Model');
         $related->shouldReceive('getKey')->andReturn(1);
         $related->shouldReceive('getTable')->andReturn('users');
         $related->shouldReceive('getKeyName')->andReturn('id');
         // Tie the mocked builder to the mocked related model
         $builder->shouldReceive('getModel')->andReturn($related);
-        
+
         // Mock the intermediate role->permission BelongsToMany relation
         $intermediateRelationship = m::mock(BelongsToMany::class);
         $intermediateRelationship->shouldReceive('getTable')->once()->andReturn('permission_roles');