Use Rails native AR patterns to control the user scope (#982)

* restore original approach to quickly looking up case/book/scorer membership

this saves an entrie query too!

* don't forget the distinct!

* add a test for the distinct filter

* We found some dead code!

app/controllers/api/v1/cases_controller.rb

@@ -25,18 +25,13 @@ def authenticate_api!
                status: :unauthorized
       end
 
-      # rubocop:disable Metrics/MethodLength
-      # rubocop:disable Metrics/AbcSize
       api :GET, '/api/cases',
           'List all cases to which the user has access.'
       error :code => 401, :desc => 'Unauthorized'
       param :archived, [ true, false ],
             :desc          => 'Whether or not to include archived cases in the response.',
             :required      => false,
             :default_value => false
-      param :sortBy, String,
-            :desc     => 'Sort the cases returned by any field on the case object, in ascending order.',
-            :required => false
       param :deep, [ true, false ],
             :desc          => '', # TODO: Unsure of what deep adds, it isn't used in the body below.
             :required      => false,
@@ -45,25 +40,19 @@ def index
         bool = ActiveRecord::Type::Boolean.new
 
         archived  = bool.deserialize(params[:archived]) || false
-        sort_by   = params[:sortBy]
         @deep     = bool.deserialize(params[:deep]) || false
 
         if archived
           @no_tries = true
           @no_teams = false
           @cases = Case.where(archived: archived, owner_id: current_user.id).all
         else
-          @cases = if 'last_viewed_at' == sort_by
-                     current_user.cases_involved_with.not_archived.includes(:metadata).references(:metadata)
-                       .order(Arel.sql('`case_metadata`.`last_viewed_at` DESC, `cases`.`id`')).limit(3)
-                   elsif sort_by
-                     current_user.cases_involved_with.not_archived.with_counts.preload( :tries).order(sort_by)
-                   else
-                     current_user.cases_involved_with.not_archived.with_counts.preload(:tries, :teams,
-                                                                                       :cases_teams)
-                       .left_outer_joins(:metadata)
-                       .order(Arel.sql('`case_metadata`.`last_viewed_at` DESC, `cases`.`updated_at` DESC'))
-                   end
+          @cases = current_user.cases_involved_with.not_archived.with_counts.preload(:tries, :teams,
+                                                                                     :cases_teams)
+            .left_outer_joins(:metadata)
+            .select('cases.*, case_metadata.last_viewed_at')
+            .order(Arel.sql('`case_metadata`.`last_viewed_at` DESC, `cases`.`updated_at` DESC'))
+
         end
 
         respond_with @cases
@@ -76,9 +65,6 @@ def index
       def show
         respond_with @case
       end
-      # rubocop:enable Metrics/MethodLength
-      # rubocop:enable Metrics/AbcSize
-
       api :POST, '/api/cases', 'Create a new case.'
       param_group :case_params
       def create

app/models/concerns/for_user_scope.rb

@@ -7,8 +7,7 @@ module ForUserScope
     scope :for_user, ->(user) do
       direct = where(owner: user)
       by_team = left_joins(teams: :members).where(teams_members: { member_id: user.id })
-      ids = by_team.or(direct).pluck(:id)
-      where(id: ids.uniq)
+      by_team.or(direct).distinct
     end
   end
 end

test/controllers/api/v1/cases_controller_test.rb

@@ -519,24 +519,13 @@ class CasesControllerTest < ActionController::TestCase
           assert_not_includes names, shared.case_name
         end
 
-        test 'limits list to 3 cases if sorting by last_viewed_at' do
-          get :index, params: { sortBy: 'last_viewed_at' }
-
-          assert_response :ok
-
-          body  = response.parsed_body
-          cases = body['all_cases']
-
-          assert cases.length <= 3
-        end
-
         test 'returns list of cases ordered by last viewed date' do
           date        = DateTime.current
           date_param  = date.strftime('%F %T')
           metadata    = second_case.metadata.find_or_create_by user_id: doug.id
           metadata.update last_viewed_at: date_param
 
-          get :index, params: { sortBy: 'last_viewed_at' }
+          get :index
 
           assert_response :ok
 
@@ -554,7 +543,7 @@ class CasesControllerTest < ActionController::TestCase
           metadata    = shared.metadata.find_or_create_by user_id: doug.id
           metadata.update last_viewed_at: date_param
 
-          get :index, params: { sortBy: 'last_viewed_at' }
+          get :index
 
           assert_response :ok
 

test/models/concerns/for_user_scope_test.rb

@@ -18,5 +18,10 @@ class ForUserScopeTest < ActiveSupport::TestCase
       cases = Case.for_user(users(:doug))
       assert cases.exclude?(cases(:matt_case))
     end
+
+    it 'returns distinct cases' do
+      case_ids = Case.for_user(users(:random)).pluck(:id)
+      assert_equal case_ids, case_ids.uniq
+    end
   end
 end