diff --git a/app/models/subgroup.rb b/app/models/subgroup.rb index 9fc4faa..89a5d6a 100644 --- a/app/models/subgroup.rb +++ b/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 diff --git a/app/models/trial.rb b/app/models/trial.rb index 3af2815..dd0feab 100644 --- a/app/models/trial.rb +++ b/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') diff --git a/db/migrate/20230531220402_add_protect_simple_description_to_system_infos.rb b/db/migrate/20230531220402_add_protect_simple_description_to_system_infos.rb deleted file mode 100644 index 326239b..0000000 --- a/db/migrate/20230531220402_add_protect_simple_description_to_system_infos.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddProtectSimpleDescriptionToSystemInfos < ActiveRecord::Migration[7.0] - def change - add_column :study_finder_system_infos, :protect_simple_description, :boolean, null: false, default: false - end -end diff --git a/db/schema.rb b/db/schema.rb index 7daa65f..8309d22 100644 --- a/db/schema.rb +++ b/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" diff --git a/spec/controllers/admin/trials_controller_spec.rb b/spec/controllers/admin/trials_controller_spec.rb index ce71cc4..fdf6e48 100644 --- a/spec/controllers/admin/trials_controller_spec.rb +++ b/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 diff --git a/spec/controllers/api/studies_controller_spec.rb b/spec/controllers/api/studies_controller_spec.rb index b290b45..5666fbf 100644 --- a/spec/controllers/api/studies_controller_spec.rb +++ b/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"] diff --git a/spec/controllers/researchers_controller_spec.rb b/spec/controllers/researchers_controller_spec.rb index 2eac5a5..93233f1 100644 --- a/spec/controllers/researchers_controller_spec.rb +++ b/spec/controllers/researchers_controller_spec.rb @@ -57,61 +57,24 @@ 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 @@ -119,7 +82,7 @@ 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' diff --git a/spec/factories/subgroup.rb b/spec/factories/subgroup.rb new file mode 100644 index 0000000..42b5649 --- /dev/null +++ b/spec/factories/subgroup.rb @@ -0,0 +1,6 @@ +FactoryBot.define do + factory :subgroup do + name { Faker::Adjective.positive} + association :group + end +end diff --git a/spec/factories/trial.rb b/spec/factories/trial.rb index a6685b6..e8a1e6c 100644 --- a/spec/factories/trial.rb +++ b/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 diff --git a/spec/factories/trial_subgroups.rb b/spec/factories/trial_subgroups.rb index bd3aab2..c6dd407 100644 --- a/spec/factories/trial_subgroups.rb +++ b/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 diff --git a/spec/mailers/study_mailer_spec.rb b/spec/mailers/study_mailer_spec.rb index a38da4a..db30ff0 100644 --- a/spec/mailers/study_mailer_spec.rb +++ b/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,7 +43,7 @@ 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 @@ -48,7 +51,7 @@ 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 { diff --git a/spec/models/trial_spec.rb b/spec/models/trial_spec.rb index 51a97d8..6a8b781 100644 --- a/spec/models/trial_spec.rb +++ b/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 diff --git a/spec/models/trial_subgroup_spec.rb b/spec/models/trial_subgroup_spec.rb index 4615c63..b3fdb57 100644 --- a/spec/models/trial_subgroup_spec.rb +++ b/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