Merge pull request #280 from bmic-development/kg-filter_spec_fixes

KG/JM - Protocol Filter Fixes / Spec Refactors

app/controllers/dashboard/base_controller.rb

@@ -78,7 +78,7 @@ def find_admin_for_protocol
     if @user.super_users.any? && @protocol.sub_service_requests.empty?
       @admin = true
     else
-      @admin = Protocol.for_admin(@user).include?(@protocol)
+      @admin = Protocol.for_admin(@user.id).include?(@protocol)
     end
   end
 end

app/controllers/dashboard/protocols_controller.rb

@@ -28,8 +28,8 @@ class Dashboard::ProtocolsController < Dashboard::BaseController
   before_action :protocol_authorizer_edit,                        only: [:edit, :update, :update_protocol_type, :archive]
 
   def index
-    admin_orgs   = @user.authorized_admin_organizations
-    @admin       = !admin_orgs.empty?
+    admin_orgs = @user.authorized_admin_organizations
+    @admin     = admin_orgs.any?
 
     @default_filter_params = { show_archived: 0, sorted_by: 'id desc' }
 
@@ -211,7 +211,7 @@ def display_requests
   private
 
   def filterrific_params
-    temp = params.require(:filterrific).permit(:identity_id,
+    params.require(:filterrific).permit(:identity_id,
       :search_name,
       :show_archived,
       :admin_filter,
@@ -222,12 +222,6 @@ def filterrific_params
       with_organization: [],
       with_status: [],
       with_owner: [])
-
-    unless @admin
-      temp[:admin_filter] = "for_identity #{@user.id}"
-    end
-
-    temp
   end
 
   def protocol_params

app/lib/dashboard/grouped_organizations.rb

@@ -44,8 +44,10 @@ def extract_name_and_id(orgs)
       org_options = []
       inactive = content_tag(:strong, I18n.t(:dashboard)[:protocol_filters][:inactive], class: 'text-danger filter-identifier')
       orgs.each do |org|
-        name = content_tag(:span, org.name)
-        name = name + inactive unless org.is_available
+        name = content_tag(
+                :span,
+                org.name + (org.is_available ? "" : inactive),
+                class: 'text')
         org_options << [name, org.id]
       end
       org_options

app/models/protocol.rb

@@ -132,12 +132,6 @@ def unique_rm_id_to_protocol
     end
   end
 
-  scope :for_identity, -> (identity) {
-    joins(:project_roles).
-    where(project_roles: { identity_id: identity.id }).
-    where.not(project_roles: { project_rights: 'none' })
-  }
-
   filterrific(
     default_filter_params: { show_archived: 0 },
     available_filters: [
@@ -170,22 +164,25 @@ def unique_rm_id_to_protocol
     title_query            = ["protocols.short_title LIKE #{like_search_term} escape '!'", "protocols.title LIKE #{like_search_term} escape '!'"]
     ### END SEARCH QUERIES ###
 
-    hr_pro_ids = HumanSubjectsInfo.where([hr_query, pro_num_query].join(' OR ')).map(&:protocol_id)
+    hr_pro_ids = HumanSubjectsInfo.where([hr_query, pro_num_query].join(' OR ')).pluck(:protocol_id)
     hr_protocol_id_query = hr_pro_ids.empty? ? nil : "protocols.id in (#{hr_pro_ids.join(', ')})"
 
     case search_attrs[:search_drop]
-
     when "Authorized User"
-      joins(:non_pi_authorized_users).
-        joins(:identities).
-        where(authorized_user_query).distinct
+      # To prevent overlap between the for_identity or for_admin scope, run the query unscoped
+      # and combine with the old scope's values
+      unscoped  = self.unscoped.joins(:non_pi_authorized_users).joins(:identities).where(authorized_user_query)
+      others    = self.current_scope
+
+      where(id: others & unscoped).distinct
     when "HR#"
       joins(:human_subjects_info).
         where(hr_query).distinct
     when "PI"
-      joins(:principal_investigators).
-        where(pi_query).
-        distinct
+      unscoped  = self.unscoped.joins(:principal_investigators).where(pi_query)
+      others    = self.current_scope
+
+      where(id: others & unscoped).distinct
     when "Protocol ID"
       where(protocol_id_query).distinct
     when "PRO#"
@@ -208,32 +205,32 @@ def unique_rm_id_to_protocol
     if filter == 'for_admin'
       for_admin(id)
     elsif filter == 'for_identity'
-      for_identity_id(id)
-    elsif filter == 'empty_protocols'
-      empty_protocols(id)
+      for_identity(id)
     end
   }
 
-  scope :for_admin, -> (identity_id) {
-    # returns protocols with ssrs in orgs authorized for identity_id
+  scope :for_identity, -> (identity_id) {
     return nil if identity_id == '0'
-    ssrs = SubServiceRequest.where.not(status: 'first_draft').where(organization_id: Organization.authorized_for_identity(identity_id))
-    joins(:sub_service_requests).merge(ssrs).distinct
-  }
 
-  scope :for_identity_id, -> (identity_id) {
-    # returns protocols user has a project role for
-    return nil if identity_id == '0'
-    joins(:project_roles).where(project_roles: { identity_id: identity_id }).where.not(project_roles: { project_rights: 'none' })
+    joins(:project_roles).
+    where(project_roles: { identity_id: identity_id }).
+    where.not(project_roles: { project_rights: 'none' })
   }
 
-  scope :empty_protocols, -> (identity_id) {
-    # returns protocols with no ssrs if you are a super user of any kind
+  scope :for_admin, -> (identity_id) {
+    # returns protocols with ssrs in orgs authorized for identity
     return nil if identity_id == '0'
-    if Identity.find(identity_id).super_users.any?
-      includes(:sub_service_requests).where(sub_service_requests: {id: nil})
+
+    ssrs = SubServiceRequest.where.not(status: 'first_draft').where(organization_id: Organization.authorized_for_identity(identity_id))
+
+    if SuperUser.where(identity_id: identity_id).any?
+      empty_protocol_ids  = includes(:sub_service_requests).where(sub_service_requests: { id: nil }).ids
+      protocol_ids        = ssrs.distinct.pluck(:protocol_id)
+      all_protocol_ids    = (protocol_ids + empty_protocol_ids).uniq
+
+      where(id: all_protocol_ids)
     else
-      return nil
+      joins(:sub_service_requests).merge(ssrs).distinct
     end
   }
 
@@ -276,6 +273,8 @@ def unique_rm_id_to_protocol
       order("TRIM(REPLACE(short_title, CHAR(9), ' ')) #{sort_order.upcase}")
     when 'pis'
       joins(primary_pi_role: :identity).order(".identities.first_name #{sort_order.upcase}")
+    when 'requests'
+      order("sub_service_requests_count #{sort_order.upcase}")
     end
   }
 

app/models/sub_service_request.rb

@@ -32,7 +32,7 @@ class SubServiceRequest < ApplicationRecord
   belongs_to :owner, :class_name => 'Identity', :foreign_key => "owner_id"
   belongs_to :service_request
   belongs_to :organization
-  belongs_to :protocol
+  belongs_to :protocol, counter_cache: true
   has_many :past_statuses, :dependent => :destroy
   has_many :line_items, :dependent => :destroy
   has_many :line_items_visits, through: :line_items

app/views/dashboard/protocol_filters/_filter_protocols_form.html.haml

@@ -75,12 +75,8 @@
             = form.label :admin_filter, t(:dashboard)[:protocol_filters][:my_admin_protocols], class: 'col-lg-10 control-label'
             .col-lg-2
               = form.radio_button :admin_filter, "for_admin #{current_user.id}"
-
-          - if current_user.super_users.any?
-            .form-group.row.empty-protocols
-              = form.label :admin_filter, t(:dashboard)[:protocol_filters][:empty_protocols], class: 'col-lg-10 control-label'
-              .col-lg-2
-                = form.radio_button :admin_filter, "empty_protocols #{current_user.id}"
+        - else
+          = form.hidden_field :admin_filter, value: "for_identity #{current_user.id}"
 
     .panel-footer
       .pull-right

app/views/dashboard/protocols/_protocols_list.html.haml

@@ -63,6 +63,9 @@
           %span.glyphicon.glyphicon-triangle-bottom.desc
       %th.col-sm-2.text-center
         = t(:dashboard)[:protocols][:table][:requests]
+        %span.protocol-sort{ name: 'requests', data: { sort_name: 'requests', sort_order: 'asc' } }
+          %span.glyphicon.glyphicon-triangle-top.asc
+          %span.glyphicon.glyphicon-triangle-bottom.desc
     - protocols.each do |protocol|
       %tr.protocols_index_row{ data: { protocol_id: protocol.id }}
         %td.id

db/migrate/20170517143845_protocol_sub_service_request_count.rb

@@ -0,0 +1,11 @@
+class ProtocolSubServiceRequestCount < ActiveRecord::Migration[5.0]
+  def change
+    add_column :protocols, :sub_service_requests_count, :integer, default: 0
+
+    Protocol.reset_column_information
+
+    Protocol.all.each do |protocol|
+      Protocol.update_counters protocol.id, :sub_service_requests_count => protocol.sub_service_requests.count
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170508172936) do
+ActiveRecord::Schema.define(version: 20170517143845) do
 
   create_table "admin_rates", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin" do |t|
     t.integer  "line_item_id"
@@ -401,7 +401,7 @@
     t.index ["sub_service_request_id"], name: "index_notifications_on_sub_service_request_id", using: :btree
   end
 
-  create_table "options", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
+  create_table "options", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin" do |t|
     t.integer  "question_id"
     t.text     "content",     limit: 65535, null: false
     t.datetime "created_at",                null: false
@@ -590,6 +590,7 @@
     t.boolean  "archived",                                                                    default: false
     t.integer  "study_type_question_group_id"
     t.integer  "research_master_id"
+    t.integer  "sub_service_requests_count",                                                  default: 0
     t.index ["next_ssr_id"], name: "index_protocols_on_next_ssr_id", using: :btree
   end
 
@@ -600,7 +601,7 @@
     t.index ["study_phase_id", "protocol_id"], name: "index_protocols_study_phases_on_study_phase_id_and_protocol_id", using: :btree
   end
 
-  create_table "question_responses", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
+  create_table "question_responses", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin" do |t|
     t.integer  "question_id"
     t.integer  "response_id"
     t.text     "content",     limit: 65535
@@ -631,7 +632,7 @@
     t.index ["service_id"], name: "index_questionnaires_on_service_id", using: :btree
   end
 
-  create_table "questions", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
+  create_table "questions", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin" do |t|
     t.integer  "section_id"
     t.boolean  "is_dependent",                null: false
     t.text     "content",       limit: 65535, null: false
@@ -676,7 +677,7 @@
     t.index ["protocol_id"], name: "index_research_types_info_on_protocol_id", using: :btree
   end
 
-  create_table "responses", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
+  create_table "responses", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin" do |t|
     t.integer  "survey_id"
     t.integer  "identity_id"
     t.integer  "sub_service_request_id"
@@ -698,7 +699,7 @@
     t.datetime "updated_at",                null: false
   end
 
-  create_table "sections", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
+  create_table "sections", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin" do |t|
     t.integer  "survey_id"
     t.string   "title"
     t.text     "description", limit: 65535
@@ -913,7 +914,7 @@
     t.index ["organization_id"], name: "index_super_users_on_organization_id", using: :btree
   end
 
-  create_table "surveys", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t|
+  create_table "surveys", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_bin" do |t|
     t.string   "title",                       null: false
     t.text     "description",   limit: 65535
     t.string   "access_code",                 null: false

spec/api/v1/protocols/get_protocol_spec.rb

@@ -66,7 +66,7 @@
         parsed_body         = JSON.parse(response.body)
         expected_attributes = build(:protocol).attributes.
                                 keys.
-                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id'].include?(key) }.
+                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id', 'sub_service_requests_count'].include?(key) }.
                                 push('callback_url', 'sparc_id').
                                 sort
 
@@ -82,7 +82,7 @@
         parsed_body         = JSON.parse(response.body)
         expected_attributes = build(:protocol).attributes.
                                 keys.
-                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id'].include?(key) }.
+                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id', 'sub_service_requests_count'].include?(key) }.
                                 push('callback_url', 'sparc_id', 'arms', 'service_requests', 'project_roles', 'human_subjects_info').
                                 sort
 

spec/api/v1/protocols/get_protocols_spec.rb

@@ -77,7 +77,7 @@
         parsed_body         = JSON.parse(response.body)
         expected_attributes = build(:protocol).attributes.
                                 keys.
-                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id'].include?(key) }.
+                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id', 'sub_service_requests_count'].include?(key) }.
                                 push('callback_url', 'sparc_id').
                                 sort
 
@@ -93,7 +93,7 @@
         parsed_body         = JSON.parse(response.body)
         expected_attributes = build(:protocol).attributes.
                                 keys.
-                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id'].include?(key) }.
+                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id', 'sub_service_requests_count'].include?(key) }.
                                 push('callback_url', 'sparc_id', 'arms', 'service_requests', 'project_roles', 'human_subjects_info').
                                 sort
 

spec/api/v1/protocols/get_protocols_with_ids_spec.rb

@@ -78,7 +78,7 @@
         parsed_body         = JSON.parse(response.body)
         expected_attributes = build(:protocol).attributes.
                                 keys.
-                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id'].include?(key) }.
+                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id', 'sub_service_requests_count'].include?(key) }.
                                 push('callback_url', 'sparc_id').
                                 sort
         expect(parsed_body['protocols'].map(&:keys).flatten.uniq.sort).to eq(expected_attributes)
@@ -93,7 +93,7 @@
         parsed_body         = JSON.parse(response.body)
         expected_attributes = build(:protocol).attributes.
                                 keys.
-                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id'].include?(key) }.
+                                reject { |key| ['study_phase', 'id', 'created_at', 'updated_at', 'deleted_at', 'research_master_id', 'sub_service_requests_count'].include?(key) }.
                                 push('callback_url', 'sparc_id', 'arms', 'service_requests', 'project_roles', 'human_subjects_info').
                                 sort
 

spec/controllers/dashboard/epic_queues/delete_destroy_spec.rb

@@ -23,6 +23,8 @@
 RSpec.describe Dashboard::EpicQueuesController do
   describe "DELETE #destroy" do
     before(:each) do
+      stub_const("EPIC_QUEUE_ACCESS", ['jug2'])
+
       @epic_queue = build_stubbed(:epic_queue)
       allow(@epic_queue).to receive(:destroy)
       allow(EpicQueue).to receive(:find).

spec/controllers/dashboard/epic_queues/get_index_spec.rb

@@ -24,6 +24,8 @@
   describe "GET #index" do
     describe "for overlord users" do
       before(:each) do
+        stub_const("EPIC_QUEUE_ACCESS", ['jug2'])
+
         protocol = create(:protocol,
                           :without_validations,
                           last_epic_push_status: 'failed'

spec/controllers/dashboard/sub_service_requests/put_update_spec.rb

@@ -78,6 +78,8 @@
     #####SURVEYS#####
     context 'ssr status is complete' do
       it 'should distribute surveys' do
+        stub_const("FINISHED_STATUSES", ['complete'])
+
         create(:super_user, identity: @logged_in_user, organization: @organization)
         @service         = create(:service_without_validations, organization_id:  @organization.id)
         @line_item      = create(:line_item_without_validations,

spec/controllers/service_requests/get_obtain_research_pricing_spec.rb

@@ -250,6 +250,8 @@
 
       context 'ssr status is set to "complete"' do
          before :each do
+          stub_const("FINISHED_STATUSES", ['complete'])
+
           @org      = create(:organization)
           service  = create(:service, organization: @org, one_time_fee: true)
           protocol = create(:protocol_federally_funded, primary_pi: logged_in_user, type: 'Study')
@@ -261,7 +263,6 @@
         end
 
         it 'should not update status to "get_a_cost_estimate"' do
-
           xhr :get, :obtain_research_pricing, {
             id: @sr.id
           }

spec/factories/protocol.rb

@@ -21,7 +21,7 @@
 FactoryGirl.define do
   factory :protocol, aliases: [:project] do
     next_ssr_id                  { Random.rand(10000) }
-    short_title                  { Faker::Lorem.word }
+    short_title                  { Faker::Lorem.sentence(2) }
     title                        { Faker::Lorem.sentence(3) }
     sponsor_name                 { Faker::Lorem.sentence(3) }
     brief_description            { Faker::Lorem.paragraph(2) }