diff --git a/app/controllers/evaluation_forms_controller.rb b/app/controllers/evaluation_forms_controller.rb index 8305ec56..dcfa29b4 100644 --- a/app/controllers/evaluation_forms_controller.rb +++ b/app/controllers/evaluation_forms_controller.rb @@ -5,6 +5,7 @@ class EvaluationFormsController < ApplicationController before_action -> { authorize_user('challenge_manager') } before_action :set_evaluation_form, only: %i[show edit update destroy] before_action :set_evaluation_forms, only: %i[index] + before_action :set_available_phases, only: %i[new create edit update] # GET /evaluation_forms or /evaluation_forms.json def index; end @@ -80,6 +81,20 @@ def set_evaluation_forms includes([:challenge, :phase]) end + def set_available_phases + current_phase_id = @evaluation_form&.phase_id + + @available_phases = + current_user.challenge_manager_challenges.includes(:phases).map do |challenge| + { + challenge:, + phases: challenge.phases.reject do |phase| + current_phase_id != phase.id && EvaluationForm.exists?(phase_id: phase.id) + end + } + end + end + # Only allow a list of trusted parameters through. def evaluation_form_params permitted = params.require(:evaluation_form). diff --git a/app/helpers/evaluation_forms_helper.rb b/app/helpers/evaluation_forms_helper.rb index 7aad7cda..a73f8a6f 100644 --- a/app/helpers/evaluation_forms_helper.rb +++ b/app/helpers/evaluation_forms_helper.rb @@ -10,6 +10,18 @@ def challenge_phase_title(challenge, phase) "#{challenge.title} - Phase #{phase_number(challenge, phase)}" end + def options_for_available_phases(available_phases) + available_phases.flat_map do |entry| + challenge = entry[:challenge] + entry[:phases].map do |phase| + [ + challenge_phase_title(challenge, phase).to_s, + "#{challenge.id}.#{phase.id}.#{phase.end_date.strftime('%m/%d/%Y')}" + ] + end + end + end + def evaluation_period(evaluation_form) start_date = evaluation_form.phase.end_date.strftime("%m/%d/%Y") end_date = evaluation_form.closing_date.strftime("%m/%d/%Y") diff --git a/app/models/evaluation_form.rb b/app/models/evaluation_form.rb index 5892021c..8645bea4 100644 --- a/app/models/evaluation_form.rb +++ b/app/models/evaluation_form.rb @@ -32,9 +32,13 @@ class EvaluationForm < ApplicationRecord validates :instructions, presence: true validates :closing_date, presence: true + validates :phase_id, uniqueness: true + validate :criteria_weights_must_sum_to_one_hundred validate :validate_unique_criteria_titles + private + def validate_unique_criteria_titles titles = evaluation_criteria.reject(&:marked_for_destruction?).map(&:title) diff --git a/app/views/evaluation_forms/_form.html.erb b/app/views/evaluation_forms/_form.html.erb index ce1708a3..6c327f2a 100644 --- a/app/views/evaluation_forms/_form.html.erb +++ b/app/views/evaluation_forms/_form.html.erb @@ -44,13 +44,15 @@
Choose the challenge this form will evaluate.
- + <%= select_tag( + 'challenge-combo', + options_for_select(options_for_available_phases(@available_phases)), + class: 'usa-select', + title: 'challenge-combo', + data: { action: 'evaluation-form#handleChallengeSelect' }, + required: true, + disabled: disabled + ) %>
<%= inline_error(evaluation_form, :challenge_id) %> diff --git a/db/migrate/20241125060011_add_unique_phase_constraint_to_evaluation_forms.rb b/db/migrate/20241125060011_add_unique_phase_constraint_to_evaluation_forms.rb new file mode 100644 index 00000000..1a137a1b --- /dev/null +++ b/db/migrate/20241125060011_add_unique_phase_constraint_to_evaluation_forms.rb @@ -0,0 +1,13 @@ +class AddUniquePhaseConstraintToEvaluationForms < ActiveRecord::Migration[7.2] + def change + ActiveRecord::Base.connection.truncate_tables( + EvaluationForm.table_name, + EvaluationCriterion.table_name, + Evaluation.table_name, + EvaluationScore.table_name + ) + + remove_index :evaluation_forms, :phase_id, if_exists: true + add_index :evaluation_forms, :phase_id, unique: true + end +end diff --git a/db/structure.sql b/db/structure.sql index cd01a527..a636fffc 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -1897,7 +1897,7 @@ CREATE INDEX index_evaluation_forms_on_challenge_id ON public.evaluation_forms U -- Name: index_evaluation_forms_on_phase_id; Type: INDEX; Schema: public; Owner: - -- -CREATE INDEX index_evaluation_forms_on_phase_id ON public.evaluation_forms USING btree (phase_id); +CREATE UNIQUE INDEX index_evaluation_forms_on_phase_id ON public.evaluation_forms USING btree (phase_id); -- @@ -2465,6 +2465,7 @@ ALTER TABLE ONLY public.winners SET search_path TO "$user", public; INSERT INTO "schema_migrations" (version) VALUES +(20241125060011), (20241120024946), (20241120024939), (20241115193801), diff --git a/spec/factories/evaluation_form.rb b/spec/factories/evaluation_form.rb index 7e239ffe..fdd1cdc8 100644 --- a/spec/factories/evaluation_form.rb +++ b/spec/factories/evaluation_form.rb @@ -23,7 +23,11 @@ # Assures proper points sum of 100 when weighted_scoring = 100 # Skips initial validation on eval form create because of dependency before(:create) do - EvaluationForm.skip_callback(:validate, :before, :criteria_weights_must_sum_to_one_hundred) + if EvaluationForm._validate_callbacks.any? do |cb| + cb.kind == :before && cb.filter == :criteria_weights_must_sum_to_one_hundred + end + EvaluationForm.skip_callback(:validate, :before, :criteria_weights_must_sum_to_one_hundred) + end end after(:build) do |evaluation_form| @@ -52,6 +56,7 @@ else create_list(:evaluation_criterion, num_criteria, evaluation_form:) end + evaluation_form.reload ensure EvaluationForm.set_callback(:validate, :before, :criteria_weights_must_sum_to_one_hundred) diff --git a/spec/models/evaluation_form_spec.rb b/spec/models/evaluation_form_spec.rb index e4cdc1f6..12e292cf 100644 --- a/spec/models/evaluation_form_spec.rb +++ b/spec/models/evaluation_form_spec.rb @@ -34,6 +34,21 @@ expect(evaluation_form).not_to be_valid expect(evaluation_form.errors[:closing_date]).to include("can't be blank") end + + it 'validates the presence of a phase' do + phase = create(:phase) + evaluation_form = create(:evaluation_form, phase:) + expect(evaluation_form.phase).to eq(phase) + end + + it 'requires a phase to be unique' do + phase = create(:phase) + create(:evaluation_form, phase:) + + expect do + create(:evaluation_form, phase:) + end.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: Phase has already been taken") + end end describe "scope" do diff --git a/spec/system/evaluation_form_spec.rb b/spec/system/evaluation_form_spec.rb index 54bb002c..03ec9b67 100644 --- a/spec/system/evaluation_form_spec.rb +++ b/spec/system/evaluation_form_spec.rb @@ -4,7 +4,7 @@ let(:user) { create_user(role: "challenge_manager", status: "active") } describe "new evaluation form page" do - let!(:challenge) { create(:challenge, user:) } + let!(:challenge) { create(:challenge, user:, is_multi_phase: true) } before do system_login_user(user) @@ -49,6 +49,36 @@ expect(page).to(be_axe_clean) end + it "only shows phases without existing evaluation forms as options for phase select" do + create(:evaluation_form, phase: challenge.phases[0]) + new_challenge = create(:challenge, user:) + + # Generate taken and non taken challenge_phase_title values + invalid_phase = challenge.phases[0] + invalid_challenge_phase_title = challenge_phase_title(challenge, invalid_phase) + valid_phase = challenge.phases[1] + valid_challenge_phase_title = challenge_phase_title(challenge, valid_phase) + new_challenge.phases[0] + new_challenge_phase_title = challenge_phase_title(challenge, valid_phase) + + visit new_evaluation_form_path + + expect_form_phase_select_to_not_contain(invalid_challenge_phase_title) + expect_form_phase_select_to_contain(valid_challenge_phase_title) + expect_form_phase_select_to_contain(new_challenge_phase_title) + end + + it "shows no results found if all phases are taken" do + # Make an evaluation form for all 3 initial factory challenge phases + create(:evaluation_form, phase: challenge.phases[0]) + create(:evaluation_form, phase: challenge.phases[1]) + create(:evaluation_form, phase: challenge.phases[2]) + + visit new_evaluation_form_path + + expect_form_phase_select_to_be_empty + end + it "prevents form submission and focuses first missing required field" do title = "Prevents form submission #{Faker::Lorem.sentence(word_count: 3)}" visit new_evaluation_form_path @@ -675,6 +705,30 @@ def expect_form_phase_to_equal(value) expect(find_by_id('challenge-combo').value).to eq(value) end +def expect_form_phase_select_to_not_contain(value) + expect(page).to have_no_select( + class: "usa-combo-box__select", + with_options: [value], + visible: :all + ) +end + +def expect_form_phase_select_to_contain(value) + expect(page).to have_select( + class: "usa-combo-box__select", + with_options: [value], + visible: :all + ) +end + +def expect_form_phase_select_to_be_empty + expect(page).to have_select( + class: "usa-combo-box__select", + options: [], + visible: :all + ) +end + def expect_form_instructions_to_equal(value) expect(find_by_id('evaluation_form_instructions').value).to eq(value) end