Merge pull request #312 from QutBioacoustics/fix-empty-project-return…

…s-no-sites

ensures admin filter api also respects the project filter for site permissions

lib/modules/access/by_permission.rb

@@ -146,15 +146,15 @@ def analysis_jobs_items(analysis_job, user, system_mode = false, levels = Access
 
         if system_mode
           query = AnalysisJobsItem.system_query
-              .joins(audio_recording: :site)
-              .order(audio_recording_id: :asc)
+                      .joins(audio_recording: :site)
+                      .order(audio_recording_id: :asc)
         else
           analysis_job = Access::Core.validate_analysis_job(analysis_job)
           query = AnalysisJobsItem
-              .joins(audio_recording: :site)
-              .order(audio_recording_id: :asc)
-              .joins(:analysis_job) # this join ensures only non-deleted results are returned
-              .where(analysis_jobs: {id: analysis_job.id})
+                      .joins(audio_recording: :site)
+                      .order(audio_recording_id: :asc)
+                      .joins(:analysis_job) # this join ensures only non-deleted results are returned
+                      .where(analysis_jobs: {id: analysis_job.id})
         end
 
         permission_sites(user, levels, query)
@@ -237,7 +237,6 @@ def permission_projects(user, levels)
       def permission_sites(user, levels, query, project_ids = nil)
 
         is_admin, query = permission_admin(user, levels, query)
-        return query if is_admin
 
 =begin
   EXISTS
@@ -252,14 +251,11 @@ def permission_sites(user, levels, query, project_ids = nil)
         pt = Project.arel_table
         ps = Arel::Table.new(:projects_sites)
         st = Site.arel_table
+
         # levels_modified is not used because permission_projects also
         # calls calculate_levels, which would end up with the inverse results
         levels_modified, exists = calculate_levels(levels)
 
-        # project permission will never be nil, which is the way it should work
-        # when being used as a part of a subquery rather than the whole subquery
-        permissions = permission_projects(user, levels)
-
         permissions_by_site =
             ps
                 .join(pt).on(ps[:project_id].eq(pt[:id]))
@@ -272,11 +268,15 @@ def permission_sites(user, levels, query, project_ids = nil)
                   .where(pt[:id].in(project_ids))
         end
 
-        permissions_by_site =
-            permissions_by_site
-                .where(permissions)
-                .project(1)
-                .exists
+        unless is_admin
+          # project permission will never be nil, which is the way it should work
+          # when being used as a part of a subquery rather than the whole subquery
+          permissions = permission_projects(user, levels)
+          permissions_by_site = permissions_by_site.where(permissions)
+        end
+
+        permissions_by_site = permissions_by_site.project(1).exists
+
 =begin
   EXISTS
     (SELECT 1

spec/lib/filter/query_spec.rb

@@ -1413,9 +1413,12 @@ def create_filter(params)
       expect(ids_actual).to match_array(ids_expected)
     end
 
-    it 'overrides filter parameters that match text partial match field' do
+    it 'overrides filter parameters that match text partial match field for admin' do
+      # audio_recording needs a site, otherwise it won't be found
+      # in by_permission#permission_sites
       audio_recording = FactoryGirl.create(
           :audio_recording,
+          site: Site.first,
           media_type: 'audio/mp3',
           recorded_date: '2012-03-26 07:06:59',
           duration_seconds: 120)
@@ -1440,6 +1443,36 @@ def create_filter(params)
       expect(ids_actual).to match_array(ids_expected)
     end
 
+    it 'overrides filter parameters that match text partial match field for writer' do
+      # audio_recording needs a site, otherwise it won't be found
+      # in by_permission#permission_sites
+      audio_recording = FactoryGirl.create(
+          :audio_recording,
+          site: Site.first,
+          media_type: 'audio/mp3',
+          recorded_date: '2012-03-26 07:06:59',
+          duration_seconds: 120)
+
+      filter = Filter::Query.new(
+          {filter: {
+              duration_seconds: {eq: 100},
+              or: {media_type: {contains: 'wav'}, status: {contains: 'wav'}}
+          },
+           filter_duration_seconds: 120,
+           filter_partial_match: 'mp3'
+          },
+          Access::ByPermission.audio_recordings(writer_user),
+          AudioRecording,
+          AudioRecording.filter_settings
+      )
+
+      expect(filter.filter).to eq({duration_seconds: {eq: 120}, or: {media_type: {contains: 'mp3'}, status: {contains: 'mp3'}}})
+
+      ids_actual = filter.query_full.pluck(:id)
+      ids_expected = [audio_recording.id]
+      expect(ids_actual).to match_array(ids_expected)
+    end
+
   end
 
   context 'available filter items' do
@@ -1519,4 +1552,40 @@ def create_filter(params)
     end
   end
 
+  context 'project with no sites' do
+    it 'returns no sites for admin' do
+      filter_hash = { filter: { } }
+      project_id = FactoryGirl.create(:project, creator: admin_user).id
+      filter_query = Access::ByPermission.sites(admin_user, Access::Core.levels, project_id)
+      filter = Filter::Query.new(
+          filter_hash,
+          filter_query,
+          Site,
+          Site.filter_settings
+      )
+
+      expect(filter.filter).to eq(filter_hash[:filter])
+
+      query = filter.query_full
+      expect(query.pluck(:id)).to match_array([])
+    end
+
+    it 'returns no sites for regular user' do
+      filter_hash = { filter: { } }
+      project_id = FactoryGirl.create(:project, creator: writer_user).id
+      filter_query = Access::ByPermission.sites(writer_user, Access::Core.levels, project_id)
+      filter = Filter::Query.new(
+          filter_hash,
+          filter_query,
+          Site,
+          Site.filter_settings
+      )
+
+      expect(filter.filter).to eq(filter_hash[:filter])
+
+      query = filter.query_full
+      expect(query.pluck(:id)).to match_array([])
+    end
+  end
+
 end