diff --git a/app/assets/stylesheets/application.sass.scss b/app/assets/stylesheets/application.sass.scss index 9d0fd363..28db437b 100644 --- a/app/assets/stylesheets/application.sass.scss +++ b/app/assets/stylesheets/application.sass.scss @@ -21,3 +21,7 @@ dialog::backdrop { #add-criteria-button.usa-button { background-color: #4d8055; } + +.usa-combo-box .border-secondary + input { + border: 1px solid red; +} \ No newline at end of file diff --git a/app/controllers/evaluation_forms_controller.rb b/app/controllers/evaluation_forms_controller.rb index dcfa29b4..5980adce 100644 --- a/app/controllers/evaluation_forms_controller.rb +++ b/app/controllers/evaluation_forms_controller.rb @@ -2,6 +2,8 @@ # Controller for evaluation forms CRUD actions. class EvaluationFormsController < ApplicationController + helper FormHelper + 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] @@ -99,7 +101,7 @@ def set_available_phases def evaluation_form_params permitted = params.require(:evaluation_form). permit(:title, :instructions, :phase_id, :status, :comments_required, - :weighted_scoring, :publication_date, :closing_date, :challenge_id, + :scale_type, :publication_date, :closing_date, :challenge_id, evaluation_criteria_attributes: [ :id, :title, :description, :points_or_weight, :scoring_type, :option_range_start, :option_range_end, :_destroy, diff --git a/app/helpers/evaluation_forms_helper.rb b/app/helpers/evaluation_forms_helper.rb index a73f8a6f..ce39b6aa 100644 --- a/app/helpers/evaluation_forms_helper.rb +++ b/app/helpers/evaluation_forms_helper.rb @@ -29,11 +29,6 @@ def evaluation_period(evaluation_form) "#{start_date} - #{end_date}" end - def inline_error(evaluation_form, field) - error = evaluation_form.errors[field].present? ? evaluation_form.errors[field].first : "" - tag.span(error, class: "text-secondary font-body-2xs", id: "evaluation_form_#{field}_error") - end - def criteria_field_id(form, attribute, is_template) prefix = "evaluation_form_evaluation_criteria_attributes" diff --git a/app/helpers/form_helper.rb b/app/helpers/form_helper.rb new file mode 100644 index 00000000..6154023a --- /dev/null +++ b/app/helpers/form_helper.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +# Helpers for rendering various form elements and errors +module FormHelper + def label_error_class(form, field) + object = form.object + object.errors[field].present? ? "text-secondary" : "" + end + + def input_error_class(form, field) + object = form.object + object.errors[field].present? ? "border-secondary" : "" + end + + def inline_error(form, field) + object = form.object + field_id = (form.object_name + "_#{field}_error").gsub(/[\[\]]/, "_").squeeze('_') + error = object.errors[field].present? ? object.errors[field].join(", ") : "" + + tag.span(error, class: "text-secondary font-body-2xs", id: field_id) + end +end diff --git a/app/javascript/controllers/evaluation_criteria_controller.js b/app/javascript/controllers/evaluation_criteria_controller.js index 97873799..2412069a 100644 --- a/app/javascript/controllers/evaluation_criteria_controller.js +++ b/app/javascript/controllers/evaluation_criteria_controller.js @@ -16,6 +16,9 @@ export default class extends Controller { this.replacePlaceholders(newCriteria); this.enableInputs(newCriteria); + this.collapseAllCriteria(); + this.expandCriterion(newCriteria); + this.criteriaListTarget.appendChild(newCriteria); this.updateCriteriaTitles(); @@ -40,6 +43,26 @@ export default class extends Controller { this.updateCriteriaTitles(); } + collapseAllCriteria() { + const accordionButtons = this.element.querySelectorAll( + ".usa-accordion__button" + ); + const accordions = this.element.querySelectorAll(".usa-accordion__content"); + + accordionButtons.forEach((button) => + button.setAttribute("aria-expanded", false) + ); + accordions.forEach((content) => content.setAttribute("hidden", "")); + } + + expandCriterion(criterion) { + const accordionButton = criterion.querySelector(".usa-accordion__button"); + const accordionContent = criterion.querySelector(".usa-accordion__content"); + + if (accordionButton) accordionButton.setAttribute("aria-expanded", true); + if (accordionContent) accordionContent.removeAttribute("hidden"); + } + toggleScoringType(event) { const row = event.target.closest(".criteria-row"); const scoringType = row.querySelector(".scoring-type-radio:checked").value; @@ -171,7 +194,7 @@ export default class extends Controller { .querySelectorAll(".criteria-option-label-row") .forEach((labelRow, index) => { labelRow.style.display = - index >= start && index <= end ? "flex" : "none"; + index >= start && index <= end ? "block" : "none"; const input = labelRow.querySelector("input"); input.disabled = index < start || index > end; }); diff --git a/app/javascript/controllers/evaluation_form_controller.js b/app/javascript/controllers/evaluation_form_controller.js index 104c6dfd..0113cfef 100644 --- a/app/javascript/controllers/evaluation_form_controller.js +++ b/app/javascript/controllers/evaluation_form_controller.js @@ -21,11 +21,10 @@ export default class extends Controller { "data-min-date", `${year}-${month}-${day}` ); - - this.updateErrorMessage("evaluation_form_challenge_id", ""); - this.updateErrorMessage("evaluation_form_phase_id", ""); } else { - this.updateErrorMessage("evaluation_form_challenge_id", "can't be blank"); + this.challengeIDTarget.value = null; + this.phaseIDTarget.value = null; + this.startDateTarget.innerHTML = "mm/dd/yyyy"; } } @@ -72,15 +71,43 @@ export default class extends Controller { } validatePresence(e) { - if (!e.target.value) { - e.target.classList.add("border-secondary"); - this.updateErrorMessage(e.target.id, "can't be blank"); + const target = e.target; + const formGroup = target.closest(".usa-form-group"); + const fieldName = target.dataset.fieldName || target.id; + + const label = this.findLabel(target, formGroup); + + if (!target.value) { + this.addErrorClasses(target, label); + this.updateErrorMessage(fieldName, "can't be blank"); } else { - e.target.classList.remove("border-secondary"); - this.updateErrorMessage(e.target.id, ""); + this.removeErrorClasses(target, label); + this.updateErrorMessage(fieldName, ""); } } + findLabel(target, formGroup) { + const isSelect = + target.tagName === "SELECT" || + target.classList.contains("usa-combo-box__input"); + const isRadio = target.type === "radio"; + + const labelId = isSelect ? target.name : target.id; + const labelQuery = isRadio ? "legend" : `label[for="${labelId}"]`; + + return formGroup.querySelector(labelQuery); + } + + addErrorClasses(target, label) { + target.classList.add("border-secondary"); + if (label) label.classList.add("text-secondary"); + } + + removeErrorClasses(target, label) { + target.classList.remove("border-secondary"); + if (label) label.classList.remove("text-secondary"); + } + updateErrorMessage(field, message) { document.getElementById(field + "_error").innerHTML = message; } diff --git a/app/models/evaluation_criterion.rb b/app/models/evaluation_criterion.rb index c0f83b49..a0b39db9 100644 --- a/app/models/evaluation_criterion.rb +++ b/app/models/evaluation_criterion.rb @@ -37,4 +37,19 @@ class EvaluationCriterion < ApplicationRecord validates :title, length: { maximum: 150 } validates :description, length: { maximum: 1000 } validates :points_or_weight, numericality: { only_integer: true } + validates :scoring_type, presence: true + + validate :validate_option_labels_not_blank, if: -> { rating? || binary? } + + private + + def validate_option_labels_not_blank + return unless option_labels.is_a?(Hash) + + option_labels.each do |key, value| + if value.blank? + errors.add("option_labels_#{key}", "can't be blank") + end + end + end end diff --git a/app/models/evaluation_form.rb b/app/models/evaluation_form.rb index 8645bea4..1b3edb06 100644 --- a/app/models/evaluation_form.rb +++ b/app/models/evaluation_form.rb @@ -8,7 +8,7 @@ # title :string not null # instructions :string not null # comments_required :boolean default(FALSE) -# weighted_scoring :boolean default(FALSE) +# scale_type :string not null # closing_date :date not null # challenge_id :bigint not null # created_at :datetime not null @@ -17,12 +17,14 @@ # class EvaluationForm < ApplicationRecord belongs_to :challenge - belongs_to :phase + belongs_to :phase, optional: true # Disables default must exist error message has_many :evaluation_criteria, lambda { order(:created_at) }, class_name: 'EvaluationCriterion', dependent: :destroy, inverse_of: :evaluation_form accepts_nested_attributes_for :evaluation_criteria, allow_destroy: true + enum :scale_type, { point: "point", weight: "weight" }, type: "string" + scope :by_user, lambda { |user| joins(challenge: :challenge_manager_users). where(challenge_manager_users: { id: user.id }) @@ -30,28 +32,61 @@ class EvaluationForm < ApplicationRecord validates :title, presence: true, length: { maximum: 150 } validates :instructions, presence: true + validates :scale_type, presence: true validates :closing_date, presence: true + # Adds custom error message for phase presence failure instead of default from above + validates :phase, presence: { message: I18n.t("evaluation_form.phase.presence_error") } validates :phase_id, uniqueness: true validate :criteria_weights_must_sum_to_one_hundred validate :validate_unique_criteria_titles + def weighted_scoring? + scale_type == "weight" + end + + def point_scoring? + scale_type == "point" + end + private def validate_unique_criteria_titles - titles = evaluation_criteria.reject(&:marked_for_destruction?).map(&:title) + duplicate_titles = find_duplicate_titles + + add_criteria_title_errors(duplicate_titles) unless duplicate_titles.empty? + end + + def find_duplicate_titles + current_criteria_titles = evaluation_criteria.reject(&:marked_for_destruction?).map(&:title) + current_criteria_titles.select { |title| current_criteria_titles.count(title) > 1 }.uniq + end - return unless titles.uniq.length != titles.length + def add_criteria_title_errors(duplicate_titles) + criteria = evaluation_criteria. + reject(&:marked_for_destruction?). + select { |c| duplicate_titles.include?(c.title) }. + reject { |c| c.errors.added?(:title, I18n.t("evaluation_criteria.duplicate_title_error")) } + criteria.each { |c| c.errors.add(:title, I18n.t("evaluation_criteria.duplicate_title_error")) } errors.add(:base, I18n.t("evaluation_criterion_unique_title_in_form_error")) end def criteria_weights_must_sum_to_one_hundred - total_weight = evaluation_criteria.reject(&:marked_for_destruction?).sum(&:points_or_weight) + return unless weighted_scoring? && total_criteria_weight != 100 - return unless weighted_scoring? && total_weight != 100 + add_weight_errors + end + + def total_criteria_weight + evaluation_criteria.reject(&:marked_for_destruction?).sum { |criteria| criteria.points_or_weight.to_i } + end + def add_weight_errors + evaluation_criteria.reject(&:marked_for_destruction?).each do |criteria| + criteria.errors.add("points_or_weight", I18n.t("evaluation_criteria.must_sum_to_100_error")) + end errors.add(:base, I18n.t("evaluation_form_criteria_weight_total_error")) end end diff --git a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb index 5a375040..406c6a8e 100644 --- a/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb +++ b/app/views/evaluation_forms/_evaluation_criterion_fields.html.erb @@ -2,7 +2,7 @@