Clean up vestiges of 'protect_simple_description', which was removed.

Add tests and solidify models (add associations, etc.).

app/models/subgroup.rb

@@ -1,6 +1,9 @@
 class Subgroup < ApplicationRecord
   self.table_name = 'study_finder_subgroups'
+
   belongs_to :group, optional: true
+  has_many :trial_subgroups
+  has_many :trials, through: :trial_subgroups
 
   validates_presence_of :name
 

app/models/trial.rb

@@ -564,11 +564,10 @@ def self.filters(search)
       end
 
       if search.has_key?('category')
-        ret << { term: { category_ids: search[:category] } }
+        ret << { term: { category_ids: search['category'] } }
       end
-
       if search.has_key?('subcat')
-        ret << { term: { subcategory_ids: search[:subcat] } }
+        ret << { term: { subcategory_ids: search['subcat'] } }
       end
 
       if (search.has_key?('gender')) and (search[:gender] == 'Male' or search[:gender] == 'Female')

db/schema.rb

@@ -165,7 +165,6 @@
     t.boolean "trial_approval", default: false, null: false
     t.boolean "alert_on_empty_system_id", default: false, null: false
     t.string "study_contact_bcc"
-    t.boolean "protect_simple_description", default: false, null: false
     t.boolean "healthy_volunteers_filter", default: true, null: false
     t.boolean "gender_filter", default: true, null: false
     t.integer "google_analytics_version"

spec/controllers/admin/trials_controller_spec.rb

@@ -164,42 +164,6 @@
       trial.reload
       expect( trial.pi_name).to eq('Updated value')
     end
-
-    # context "given SystemInfo.protect_simple_description is false" do
-    #   it "should successfully update simple_description" do
-    #     create(:system_info, protect_simple_description: false)
-    #     trial = create(:trial, simple_description: "Original Description")
-    #     put :update, params: { id: trial.system_id, trial: { simple_description: "Updated value" }}
-    #     trial.reload
-    #     expect( trial.simple_description).to eq('Updated value')
-    #   end
-
-    #   it "should issue HTTP 200 but not update simple_description_override" do
-    #     create(:system_info, protect_simple_description: false)
-    #     trial = create(:trial, simple_description_override: "Original Description")
-    #     put :update, params: { id: trial.system_id, trial: { simple_description_override: "Updated value" }}
-    #     trial.reload
-    #     expect( trial.simple_description_override).to eq('Original Description')
-    #   end
-    # end
-
-    # context "given SystemInfo.protect_simple_description is true" do
-    #   it "should successfully update simple_description_override" do
-    #     create(:system_info, protect_simple_description: true)
-    #     trial = create(:trial, simple_description_override: "Original Description")
-    #     put :update, params: { id: trial.system_id, trial: { simple_description_override: "Updated value" }}
-    #     trial.reload
-    #     expect( trial.simple_description_override).to eq('Updated value')
-    #   end
-
-    #   it "should issue HTTP 200 but not update simple_description" do
-    #     create(:system_info, protect_simple_description: true)
-    #     trial = create(:trial, simple_description: "Original Description")
-    #     put :update, params: { id: trial.system_id, trial: { simple_description: "Updated value" }}
-    #     trial.reload
-    #     expect( trial.simple_description).to eq('Original Description')
-    #   end
-    # end
   end
 
   describe "GET #all_under_review" do

spec/controllers/api/studies_controller_spec.rb

@@ -91,33 +91,6 @@
       end
     end
 
-    it "can returns 400 if a system id is not alphanumeric" do
-        attributes = {
-        system_id: " bad id ",
-        contact_override: "blah@example.com",
-        contact_override_first_name: "Testy",
-        contact_override_last_name: "McTesterson",
-        irb_number: "1234567890",
-        pi_id: "somepi@example.com",
-        pi_name: "Some PI, M.D.",
-        recruiting: true,
-        simple_description: "This is a short description",
-        brief_title: "This is a brief title",
-        official_title: "This is an official title",
-        visible: true,
-        eligibility_criteria: "This is eligibility criteria",
-        min_age_unit: "16 years",
-        max_age_unit: "99 years",
-        healthy_volunteers_imported: true,
-        gender: "Male",
-        phase: "Phase I"
-      }
-
-      post :create, params: attributes
-
-      expect(response).to have_http_status(:bad_request)
-    end
-
     it "can create with conditions" do
       conditions = ["Something", "Something else"]
 

spec/controllers/researchers_controller_spec.rb

@@ -57,69 +57,32 @@
 
   describe "PUT #update" do
     it "successfully changes trial attribute" do
-      create(:system_info, protect_simple_description: false)
+      create(:system_info)
       trial = create(:trial, contact_override_last_name: "Test Name")
       put :update, params: { id: trial.system_id, trial: { contact_override_last_name: "Updated value" }, secret_key: SystemInfo.secret_key}
       trial.reload
       expect( trial.contact_override_last_name).to eq('Updated value')
     end
 
-
-    # context "given SystemInfo.protect_simple_description is false" do
-    #   it "should successfully update simple_description" do
-    #     create(:system_info, protect_simple_description: false)
-    #     trial = create(:trial, simple_description: "Original Description")
-    #     put :update, params: { id: trial.system_id, trial: { simple_description: "Updated value" }, secret_key: SystemInfo.secret_key}
-    #     trial.reload
-    #     expect( trial.simple_description).to eq('Updated value')
-    #   end
-
-    #   it "should issue HTTP 200 but not update simple_description_override" do
-    #     create(:system_info, protect_simple_description: false)
-    #     trial = create(:trial, simple_description_override: "Original Description")
-    #     put :update, params: { id: trial.system_id, trial: { simple_description_override: "Updated value" }, secret_key: SystemInfo.secret_key}
-    #     trial.reload
-    #     expect( trial.simple_description_override).to eq('Original Description')
-    #   end
-    # end
-
-    # context "given SystemInfo.protect_simple_description is true" do
-    #   it "should successfully update simple_description_override" do
-    #     create(:system_info, protect_simple_description: true)
-    #     trial = create(:trial, simple_description_override: "Original Description")
-    #     put :update, params: { id: trial.system_id, trial: { simple_description_override: "Updated value" }, secret_key: SystemInfo.secret_key}
-    #     trial.reload
-    #     expect( trial.simple_description_override).to eq('Updated value')
-    #   end
-
-    #   it "should issue HTTP 200 but not update simple_description" do
-    #     create(:system_info, protect_simple_description: true)
-    #     trial = create(:trial, simple_description: "Original Description")
-    #     put :update, params: { id: trial.system_id, trial: { simple_description: "Updated value" }, secret_key: SystemInfo.secret_key}
-    #     trial.reload
-    #     expect( trial.simple_description).to eq('Original Description')
-    #   end
-    # end
-
     it "trial with override information" do
-      create(:system_info, protect_simple_description: false)
+      create(:system_info)
       trial = create(:trial, brief_title: 'Testing a title', system_id: 'NCT000001' )
 
-      simple_description = 'Testing adding a simple_description'
+      simple_description_override = 'Testing adding a simple_description_override'
       contact_override = 'jim@aol.com'
       contact_override_first_name = 'Jim'
       contact_override_last_name = 'Smith'
 
       study_finder_trial = {
-        simple_description: simple_description, 
+        simple_description_override: simple_description_override, 
         contact_override: contact_override, 
         contact_override_first_name: contact_override_first_name, 
         contact_override_last_name: contact_override_last_name
       }
 
       put :update, params: { id: trial.system_id, trial: study_finder_trial, secret_key: SystemInfo.secret_key }
 
-      expect( assigns(:trial).simple_description ).to eq(simple_description)
+      expect( assigns(:trial).simple_description_override).to eq(simple_description_override)
       expect( assigns(:trial).contact_override ).to eq(contact_override)
       expect( assigns(:trial).contact_override_first_name ).to eq(contact_override_first_name)
       expect( assigns(:trial).contact_override_last_name ).to eq(contact_override_last_name)
@@ -129,7 +92,7 @@
     end
 
     it "fails when a secret_key is not provided" do
-      create(:system_info, protect_simple_description: false)
+      create(:system_info)
       trial = create(:trial, brief_title: 'Testing a title', system_id: 'NCT000001')
 
       simple_description = 'Testing adding a simple_description'
@@ -150,7 +113,7 @@
     end
 
     it "fails when an invalid secret_key is added" do
-      create(:system_info, protect_simple_description: false)
+      create(:system_info)
       trial = create(:trial, brief_title: 'Testing a title', system_id: 'NCT000001')
 
       simple_description = 'Testing adding a simple_description'

spec/factories/subgroup.rb

@@ -0,0 +1,6 @@
+FactoryBot.define do
+  factory :subgroup do
+    name { Faker::Adjective.positive}
+    association :group
+  end
+end

spec/factories/trial.rb

@@ -2,5 +2,8 @@
   sequence(:system_id, 10000) { |n| "STUDY#{n}" }
   factory :trial do
     system_id { generate(:system_id) }
+    brief_title { Faker::Lorem.sentence }
+    approved { true }
+    visible { true }
   end
 end

spec/factories/trial_subgroups.rb

@@ -1,6 +1,6 @@
 FactoryBot.define do
   factory :trial_subgroup do
-    subgroup { nil }
-    trial { nil }
+    association :subgroup
+    association :trial
   end
 end

spec/mailers/study_mailer_spec.rb

@@ -24,12 +24,15 @@
         contact_email_suffix: '@umn.edu'
       )
 
+      @to_address = Faker::Internet.email
+      @phone_number = Faker::PhoneNumber.phone_number
+
       @mail = StudyMailer.contact_team(
+        @to_address,
+        Faker::Name.name,
         Faker::Internet.email,
-        Faker::Name.full_name,
-        Faker::Internet.email,
-        Faker::PhoneNumber.phone_number,
-        Faker::HitchhikersGuideToTheGalaxy.marvin_quote,
+        @phone_number,
+        Faker::GreekPhilosophers.quote,
         @trial.system_id,
         @trial.brief_title,
         @system_info)
@@ -40,15 +43,15 @@
     end
 
     it 'renders the reciever email' do
-      expect( @mail.to[0] ).to eq('jim@aol.com')
+      expect( @mail.to[0] ).to eq(@to_address)
     end
 
     it 'assigns @title in the mail body' do
       expect( @mail.body.encoded ).to match(@trial.brief_title)
     end
 
     it "includes phone number" do
-      expect(@mail.body).to match("123-453-2345")
+      expect(@mail.body.raw_source).to match(@phone_number)
     end
 
     after {

spec/models/trial_spec.rb

@@ -36,6 +36,21 @@
       t = build(:trial, brief_title: "", acronym: "TT")
       expect(t.display_title).to eq(nil)
     end
+  end
+
+  context "when rendering simple_description" do
+    context "if simple_description is non-nil and simple_description_override is nil" do
+      it "returns simple_description_from_source" do
+        t = build(:trial, simple_description: "Test description", simple_description_override: nil)
+        expect(t.simple_description).to eq(t.simple_description_from_source)
+      end
+    end
+    context "if simple_description is non-nil and simple_description_override is non-nil" do
+      it "returns simple_description_override" do
+        t = build(:trial, simple_description: "Test description", simple_description_override: "Override description")
+        expect(t.simple_description).to eq(t.simple_description_override)
+      end
+    end
 
   end
 
@@ -109,16 +124,6 @@ def display_title
     expect(build(:trial, system_id: "nct123").has_nct_number?).to be true
   end
 
-  it "sorts search results by added_on date" do
-    create(:trial, added_on: 5.days.ago, visible: true)
-    create(:trial, added_on: 2.months.ago, visible: true)
-    create(:trial, added_on: 1.day.ago, visible: true)
-
-    result_dates = Trial.execute_search({q: ""}).results.map(&:added_on)
-
-    expect(result_dates).to eq(result_dates.sort.reverse)
-  end
-
   it "validates the presence of a system_id" do
     no_system_id = build(:trial, system_id: "")
     expect(no_system_id).not_to be_valid
@@ -196,5 +201,33 @@ def display_title
     expect(search_results).to include(1000.0, 1000.0, 18.0)
   end
 
+  context "Given trials and subgroups bridged by TrialSubgroups" do
+    it "will return correct search results with a subgroup param" do
+      # Create a set of trials, a set of groups, and then two subgroups for each group. 
+      # Then use Array.sample to pseudo-randomly assign 1 to 3 trials to each subgroup.
+      # Like real life, this also allows for a trial to belong to more than one subgroup.
+      Trial.__elasticsearch__.delete_index! 
+      trials = create_list(:trial, 100)
+      groups = create_list(:group, 3)
+      subgroups = Array.new
+      groups.map { |e| subgroups.push(*create_list(:subgroup, 2, group: e)) }
+
+      subgroups.each_with_index do |sg, idx| 
+        trials.sample([1,2,3].sample(1).first).map { |e| e.trial_subgroups.create(subgroup: sg) }
+      end
+
+      Trial.import(refresh: true, force: true)
+
+      subgroups.each do |sg| 
+        search_results = Trial.execute_search("subcat" => "#{sg.id}").results.map(&:subcategory_ids)
+        expect(search_results.count).to eq(TrialSubgroup.where(subgroup: sg).count)
+        # Each trial's subcategory_ids is an array of ids, so search results is an array of arrays.
+        # Ensure each component array includes the subgoup id we're searching on.
+        search_results.each do |r|
+          expect(r).to include(sg.id)
+        end
+      end
+    end
+  end
 
 end

spec/models/trial_subgroup_spec.rb

@@ -1,5 +1,6 @@
 require 'rails_helper'
 
 RSpec.describe TrialSubgroup, type: :model do
-  pending "add some examples to (or delete) #{__FILE__}"
+  # No tests here. We're only using basic ActiveRecord things with this, *except* elasticsearch
+  # and that functionality is tested via Trial.execute_search.
 end