From 8e1edf26ac9b4d8079384d49850cf1de842910c9 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 13 Dec 2024 14:30:00 -0600 Subject: [PATCH 01/42] 291 | Add frontend for evaluators, score, & judging status checkboxes --- app/views/phases/_submissions_table.html.erb | 143 ++++++++++++------- 1 file changed, 91 insertions(+), 52 deletions(-) diff --git a/app/views/phases/_submissions_table.html.erb b/app/views/phases/_submissions_table.html.erb index 8d6e7640..3d68de5c 100644 --- a/app/views/phases/_submissions_table.html.erb +++ b/app/views/phases/_submissions_table.html.erb @@ -18,75 +18,114 @@ <% @submissions.each do |submission| %> - + Submission ID <%= submission.id %> -
- <% if submission.eligible_for_evaluation? %> - -
- <%= image_tag( - "images/usa-icons/verified.svg", - class: "usa-icon--size-3 margin-right-1", - alt: "" - )%> - Eligible for Evaluation -
- <% else %> - -
- <%= image_tag( - "images/usa-icons/highlight_off.svg", - class: "usa-icon--size-3 margin-right-1", - alt: "" - )%> - Not Eligible for Evaluation -
- <% end %> -
+
+
+ <%= form_with( + model: submission, + url: submission_path(submission), + method: :patch, + id: "eligibility_form_#{submission.id}", + data: { + submission_judging_status_target: "eligibilityForm" + }) do |f| %> +
+ Eligibility for evaluation + <%= f.check_box :judging_status, + id: "eligible_for_evaluation_#{submission.id}", + disabled: submission.eligibility_checkbox_disabled?, + checked: submission.eligible_for_evaluation?, + class: "usa-checkbox__input", + data: { + action: "change->submission-judging-status#toggleEligibility", + submission_judging_status_target: "eligibilityCheckbox" + } + %> + <%= f.label :judging_status, + "Toggle eligibility for evaluation", + for: "eligible_for_evaluation_#{submission.id}", + class: "usa-checkbox__label", + style: "font-size: 0;" %> +
+ <% end %> +
+
+ -
- <% if submission.selected_to_advance? %> - -
- <%= image_tag( - "images/usa-icons/star.svg", - class: "usa-icon--size-3 margin-right-1", - alt: "" - )%> - Selected to Advance -
- <% else %> - -
- <%= image_tag( - "images/usa-icons/star_outline.svg", - class: "usa-icon--size-3 margin-right-1", - alt: "" - )%> - Not Selected to Advance -
- <% end %> -
+
+
+ <%= form_with( + model: submission, + url: submission_path(submission), + method: :patch, + id: "advancement_form_#{submission.id}", + data: { + submission_judging_status_target: "advancementForm" + }) do |f| %> +
+ Selected to advance status + <%= f.check_box :judging_status, + id: "selected_to_advance_#{submission.id}", + disabled: submission.advancement_checkbox_disabled?, + checked: submission.selected_to_advance?, + class: "usa-checkbox__input", + data: { + action: "change->submission-judging-status#toggleAdvancement", + submission_judging_status_target: "advancementCheckbox" + } + %> + <%= f.label :judging_status, + "Toggle selected to advance status", + for: "selected_to_advance_#{submission.id}", + class: "usa-checkbox__label", + style: "font-size: 0;" %> +
+ <% end %> +
+
- No evaluators assigned to this submission + <% if submission.evaluators.any? %> +
+ <% submission.evaluator_submission_assignments.each do |assignment| %> +
+
+ + <%= "#{assignment.evaluator.first_name} #{assignment.evaluator.last_name}" %> + +
+ <% if ['assigned', 'recused'].include?(assignment.status) %> +
+ <%= evaluator_score(assignment).formatted_score %>% + + <%= assignment.evaluation_status.to_s.titleize %> + +
+ <% end %> +
+ <% end %> +
+ <% else %> + No evaluators assigned to this submission + <% end %> - - N/A + + <%= average_score(submission).formatted_score %>
<%= link_to submission_path(submission) do %> - <% end %>
- + <% end %> From 6e6c3b9a3d07ad5d661d023865edbb80b7173c8a Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 13 Dec 2024 14:31:01 -0600 Subject: [PATCH 02/42] 291 | Add average score --- app/controllers/phases_controller.rb | 5 ++++- app/helpers/evaluations_helper.rb | 29 ++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 app/helpers/evaluations_helper.rb diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index e9015b80..76b8e572 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -10,7 +10,10 @@ def index end def submissions - @submissions = @phase.submissions + @submissions = @phase.submissions.includes( + :evaluators, + evaluator_submission_assignments: [:evaluation] + ).order(:id) end private diff --git a/app/helpers/evaluations_helper.rb b/app/helpers/evaluations_helper.rb new file mode 100644 index 00000000..d960366d --- /dev/null +++ b/app/helpers/evaluations_helper.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +# View helpers for calculating evaluation & submission details. +module EvaluationsHelper + Score = Struct.new(:raw_score, :formatted_score, :display_score) + + # individual evaluator score + def evaluator_score(assignment) + score = assignment.evaluation&.total_score + + unless assignment.evaluation_status == :completed && score + return Score.new(0, "0", "N/A") + end + + Score.new(score, score.to_s, score) + end + + def average_score(submission) + completed_evaluations = submission.evaluations.where.not(completed_at: nil) + + unless completed_evaluations.any? + return Score.new(0, "0", "N/A") + end + + avg = completed_evaluations.average(:total_score) + score = avg ? avg.round : 0 + Score.new(score, "#{score}", "#{score}") + end +end From cc8c9a8427dd57abd62fcd10ac46c341ed6d937c Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 13 Dec 2024 14:32:04 -0600 Subject: [PATCH 03/42] 291 | Add evaluators and display status --- app/models/evaluator_submission_assignment.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index 48e300cc..32070c7c 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -43,9 +43,12 @@ def self.ordered_by_status end def evaluation_status - return status.to_sym unless assigned? + return :recused if recused? + return :unassigned if unassigned? + return :recused_unassigned if recused_unassigned? + return assigned_evaluation_status if assigned? - assigned_evaluation_status + status&.to_sym end private From 50eef08e41831db05428d876b2533935b801c594 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 13 Dec 2024 14:32:24 -0600 Subject: [PATCH 04/42] 292 | Add judging status checkbox logic --- app/controllers/submissions_controller.rb | 41 ++++++++- app/models/submission.rb | 19 ++++ spec/requests/submissions_spec.rb | 105 ++++++++++++++++++++++ 3 files changed, 163 insertions(+), 2 deletions(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index b0fa2af9..8e6ab613 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -8,6 +8,30 @@ class SubmissionsController < ApplicationController def show; end def update + if params[:submission][:judging_status].present? + handle_judging_status_update + else + handle_comments_update + end + end + + private + + def handle_judging_status_update + if valid_status_change? && @submission.update(submission_params) + respond_to do |format| + format.html { redirect_to submissions_phase_path(@submission.phase) } + format.json { render json: { status: :ok }, status: :ok } + end + else + respond_to do |format| + format.html { redirect_to submissions_phase_path(@submission.phase) } + format.json { render json: { status: :unprocessable_entity }, status: :unprocessable_entity } + end + end + end + + def handle_comments_update if @submission.update!(submission_params) flash.now[:success] = I18n.t("comments_saved") render :show, submission: @submission @@ -16,10 +40,23 @@ def update end end - private + def valid_status_change? + new_status = params.dig(:submission, :judging_status) + current_status = @submission.judging_status + + if new_status == 'selected' && current_status == 'not_selected' && @submission.eligibility_checkbox_disabled? + return false + end + + if new_status == 'winner' && (!@submission.eligible_for_evaluation? || @submission.advancement_checkbox_disabled?) + return false + end + + true + end def submission_params - params.require(:submission).permit(:comments) + params.require(:submission).permit(:comments, :judging_status) end # User access enforced by role diff --git a/app/models/submission.rb b/app/models/submission.rb index 6bd3c140..ccc8558e 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -36,6 +36,7 @@ class Submission < ApplicationRecord belongs_to :manager, class_name: 'User' has_many :evaluator_submission_assignments, dependent: :destroy has_many :evaluators, through: :evaluator_submission_assignments, class_name: "User" + has_many :evaluations, dependent: :destroy # Fields attribute :title, :string @@ -67,4 +68,22 @@ def eligible_for_evaluation? def selected_to_advance? winner? end + + def eligibility_checkbox_disabled? + evaluators.any? + end + + def advancement_checkbox_disabled? + !eligible_for_evaluation? || + !all_evaluations_completed? || + evaluators.empty? + end + + private + + def all_evaluations_completed? + evaluator_submission_assignments.includes(:evaluation).all? do |assignment| + assignment.evaluation_status == :completed + end + end end diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index 2038e403..d421a173 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -40,6 +40,12 @@ it "renders a list of submissions for a user's challenge" do submission = create(:submission, challenge: challenge, phase: phase) + evaluator = create(:user, role: 'evaluator') + create(:evaluator_submission_assignment, + submission: submission, + evaluator: evaluator, + status: :assigned, + evaluation: create(:evaluation)) get submissions_phase_path(phase) expect(response.body).to include("Boston Tea Party Cleanup") @@ -98,4 +104,103 @@ end end end + + describe "PATCH /submissions/:id" do + context "when logged in as a challenge manager" do + let(:user) { create_user(role: "challenge_manager") } + let(:submission) { create(:submission, challenge: challenge, phase: phase) } + let(:evaluator) { create(:user, role: 'evaluator') } + + before do + ChallengeManager.create(user: user, challenge: challenge) + end + + context "updating judging status" do + it "updates eligibility status to selected" do + patch submission_path(submission), params: { + submission: { judging_status: 'selected' } + }, as: :json + + expect(response).to have_http_status(:ok) + expect(submission.reload.judging_status).to eq('selected') + end + + it "updates advancement status to winner when evaluations are complete" do + submission.update!(judging_status: 'selected') + create(:evaluator_submission_assignment, + submission: submission, + evaluator: evaluator, + status: :assigned, + evaluation: create(:evaluation, completed_at: Time.current)) + + patch submission_path(submission), params: { + submission: { judging_status: 'winner' } + }, as: :json + + expect(response).to have_http_status(:ok) + expect(submission.reload.judging_status).to eq('winner') + end + + it "prevents advancement when evaluations are not complete" do + submission.update!(judging_status: 'selected') + create(:evaluator_submission_assignment, + submission: submission, + evaluator: evaluator, + status: :assigned) + + patch submission_path(submission), params: { + submission: { judging_status: 'winner' } + }, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(submission.reload.judging_status).to eq('selected') + end + + it "prevents eligibility changes when evaluators are assigned" do + create(:evaluator_submission_assignment, + submission: submission, + evaluator: evaluator, + status: :assigned) + + patch submission_path(submission), params: { + submission: { judging_status: 'selected' } + }, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(submission.reload.judging_status).to eq('not_selected') + end + + it "prevents changing judging_status from 'not_selected' to 'selected' when eligibility checkbox is disabled" do + submission.update!(judging_status: 'not_selected') + create(:evaluator_submission_assignment, + submission: submission, + evaluator: evaluator, + status: :assigned) + + patch submission_path(submission), params: { + submission: { judging_status: 'selected' } + }, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(submission.reload.judging_status).to eq('not_selected') + end + + it "prevents changing judging_status to 'winner' when not eligible" do + submission.update!(judging_status: 'not_selected') + create(:evaluator_submission_assignment, + submission: submission, + evaluator: evaluator, + status: :assigned, + evaluation: create(:evaluation, completed_at: Time.current)) + + patch submission_path(submission), params: { + submission: { judging_status: 'winner' } + }, as: :json + + expect(response).to have_http_status(:unprocessable_entity) + expect(submission.reload.judging_status).to eq('not_selected') + end + end + end + end end From 382894b2d01efb7be6fc5b7d069005fcadea1e55 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 13 Dec 2024 14:32:44 -0600 Subject: [PATCH 05/42] 292 | Add js to update judging status checkboxes --- app/javascript/controllers/index.js | 5 ++- .../submission_judging_status_controller.js | 45 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 app/javascript/controllers/submission_judging_status_controller.js diff --git a/app/javascript/controllers/index.js b/app/javascript/controllers/index.js index 82491138..11ad6de5 100644 --- a/app/javascript/controllers/index.js +++ b/app/javascript/controllers/index.js @@ -17,4 +17,7 @@ import EvaluationFormController from "./evaluation_form_controller"; application.register("evaluation-form", EvaluationFormController); import HotdogController from "./hotdog_controller"; -application.register("hotdog", HotdogController); \ No newline at end of file +application.register("hotdog", HotdogController); + +import SubmissionJudgingStatusController from "./submission_judging_status_controller"; +application.register("submission-judging-status", SubmissionJudgingStatusController); \ No newline at end of file diff --git a/app/javascript/controllers/submission_judging_status_controller.js b/app/javascript/controllers/submission_judging_status_controller.js new file mode 100644 index 00000000..566c8c55 --- /dev/null +++ b/app/javascript/controllers/submission_judging_status_controller.js @@ -0,0 +1,45 @@ +// app/javascript/controllers/submission_judging_status_controller.js +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["eligibilityForm", "advancementForm", "eligibilityCheckbox", "advancementCheckbox"] + + toggleEligibility(event) { + event.preventDefault() + const formData = new FormData(this.eligibilityFormTarget) + formData.set('submission[judging_status]', + event.target.checked ? 'selected' : 'not_selected' + ) + this.submitForm(formData, this.eligibilityFormTarget) + } + + toggleAdvancement(event) { + event.preventDefault() + const formData = new FormData(this.advancementFormTarget) + formData.set('submission[judging_status]', + event.target.checked ? 'winner' : 'selected' + ) + + if (event.target.checked) { + this.eligibilityCheckboxTarget.checked = true + } + this.submitForm(formData, this.advancementFormTarget) + } + + submitForm(formData, form) { + const csrfToken = document.querySelector('[name="csrf-token"]').content + + fetch(form.action, { + method: 'PATCH', + headers: { + 'X-CSRF-Token': csrfToken, + 'Accept': 'application/json' + }, + body: formData + }) + .catch(() => { + const checkbox = form.querySelector('input[type="checkbox"]') + checkbox.checked = !checkbox.checked + }) + } +} From bed653d91068897ff353ff3e932f50829b632d6a Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 13 Dec 2024 15:19:52 -0600 Subject: [PATCH 06/42] Small adjustments --- app/controllers/submissions_controller.rb | 18 ++++++++++-------- app/helpers/evaluations_helper.rb | 4 ++-- app/models/submission.rb | 4 +--- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 8e6ab613..d07789a8 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -17,17 +17,19 @@ def update private + def handle_judging_status_update if valid_status_change? && @submission.update(submission_params) - respond_to do |format| - format.html { redirect_to submissions_phase_path(@submission.phase) } - format.json { render json: { status: :ok }, status: :ok } - end + respond_with_status(:ok) else - respond_to do |format| - format.html { redirect_to submissions_phase_path(@submission.phase) } - format.json { render json: { status: :unprocessable_entity }, status: :unprocessable_entity } - end + respond_with_status(:unprocessable_entity) + end + end + + def respond_with_status(status) + respond_to do |format| + format.html { redirect_to submissions_phase_path(@submission.phase) } + format.json { render json: { status: status }, status: status } end end diff --git a/app/helpers/evaluations_helper.rb b/app/helpers/evaluations_helper.rb index d960366d..7e9795cc 100644 --- a/app/helpers/evaluations_helper.rb +++ b/app/helpers/evaluations_helper.rb @@ -12,7 +12,7 @@ def evaluator_score(assignment) return Score.new(0, "0", "N/A") end - Score.new(score, score.to_s, score) + Score.new(score, score.to_s, score.to_s) end def average_score(submission) @@ -24,6 +24,6 @@ def average_score(submission) avg = completed_evaluations.average(:total_score) score = avg ? avg.round : 0 - Score.new(score, "#{score}", "#{score}") + Score.new(score, score.to_s, score.to_s) end end diff --git a/app/models/submission.rb b/app/models/submission.rb index 66eb97fb..d491af9d 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -76,9 +76,7 @@ def eligibility_checkbox_disabled? end def advancement_checkbox_disabled? - !eligible_for_evaluation? || - !all_evaluations_completed? || - evaluators.empty? + !eligible_for_evaluation? || !all_evaluations_completed? || evaluators.empty? end private From 20ac36e3d4f553a14a686e7859a1f581cc2c2ad6 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 13 Dec 2024 17:23:32 -0600 Subject: [PATCH 07/42] 292 | Adjust submissions query --- app/controllers/phases_controller.rb | 19 +++++++++++--- app/controllers/submissions_controller.rb | 30 +++++++++++++++-------- app/models/submission.rb | 6 ++--- spec/requests/submissions_spec.rb | 5 ++++ 4 files changed, 43 insertions(+), 17 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 6edefff7..4e22425c 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -10,10 +10,10 @@ def index end def submissions - @submissions = @phase.submissions.includes( - :evaluators, - evaluator_submission_assignments: [:evaluation] - ).order(:id) + @submissions = @phase.submissions.order(:id) + + include_evaluator_associations + @submissions_count = @submissions.count not_started = @submissions.left_outer_joins(:evaluations). where({ "evaluations.id" => nil }).count @@ -29,4 +29,15 @@ def set_phase @phase = Phase.where(challenge: current_user.challenge_manager_challenges).find(params[:id]) @challenge = @phase.challenge end + + def include_evaluator_associations + return unless request.format.html? && @submissions.any? + return unless has_evaluator_assignments? + + @submissions = @submissions.includes(:evaluators, evaluator_submission_assignments: :evaluation) + end + + def has_evaluator_assignments? + EvaluatorSubmissionAssignment.where(submission_id: @submissions.select(:id)).exists? + end end diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index d07789a8..3f8dc62f 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -17,19 +17,15 @@ def update private - def handle_judging_status_update - if valid_status_change? && @submission.update(submission_params) - respond_with_status(:ok) - else - respond_with_status(:unprocessable_entity) + unless valid_status_change? + return respond_with_error end - end - def respond_with_status(status) - respond_to do |format| - format.html { redirect_to submissions_phase_path(@submission.phase) } - format.json { render json: { status: status }, status: status } + if @submission.update(submission_params) + respond_with_success + else + respond_with_error end end @@ -57,6 +53,20 @@ def valid_status_change? true end + def respond_with_success + respond_to do |format| + format.html { redirect_to submissions_phase_path(@submission.phase) } + format.json { render json: { success: true }, status: :ok } + end + end + + def respond_with_error + respond_to do |format| + format.html { redirect_to submissions_phase_path(@submission.phase) } + format.json { render json: { success: false }, status: :unprocessable_entity } + end + end + def submission_params params.require(:submission).permit(:comments, :judging_status) end diff --git a/app/models/submission.rb b/app/models/submission.rb index d491af9d..54eaadc8 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -82,8 +82,8 @@ def advancement_checkbox_disabled? private def all_evaluations_completed? - evaluator_submission_assignments.includes(:evaluation).all? do |assignment| - assignment.evaluation_status == :completed - end + evaluator_submission_assignments. + includes(:evaluation). + all? { |assignment| assignment.evaluation_status == :completed } end end diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index a9aed998..70a4c913 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -64,6 +64,11 @@ create(:submission, challenge: challenge, phase: phase) create(:submission, challenge: challenge, phase: phase, judging_status: "selected") + allow_any_instance_of(ActionView::Base).to receive(:render).and_call_original + allow_any_instance_of(ActionView::Base).to receive(:render). + with(hash_including(partial: "submissions_table")). + and_return("") + get submissions_phase_path(phase) expect(response.body).to include("Boston Tea Party Cleanup") # total submission count From 53b9ba6740f4d276091a8b499a40e13108755508 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 13 Dec 2024 17:29:05 -0600 Subject: [PATCH 08/42] Adjust query to include evaluators/assignments --- app/controllers/phases_controller.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 4e22425c..b612862b 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -32,12 +32,12 @@ def set_phase def include_evaluator_associations return unless request.format.html? && @submissions.any? - return unless has_evaluator_assignments? + return unless evaluator_assignments? @submissions = @submissions.includes(:evaluators, evaluator_submission_assignments: :evaluation) end - def has_evaluator_assignments? - EvaluatorSubmissionAssignment.where(submission_id: @submissions.select(:id)).exists? + def evaluator_assignments? + EvaluatorSubmissionAssignment.exists?(submission_id: @submissions.select(:id)) end end From fceba9141912f4ed2d16ad912c58406b6eb57a95 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 20 Dec 2024 11:46:37 -0600 Subject: [PATCH 09/42] 291 | Move judging status validation to submission model --- app/controllers/phases_controller.rb | 3 +- app/controllers/submissions_controller.rb | 65 ++++------------------- app/models/submission.rb | 25 ++++++++- 3 files changed, 35 insertions(+), 58 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index b612862b..b26f6732 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -31,8 +31,7 @@ def set_phase end def include_evaluator_associations - return unless request.format.html? && @submissions.any? - return unless evaluator_assignments? + return unless @submissions.any? && evaluator_assignments? @submissions = @submissions.includes(:evaluators, evaluator_submission_assignments: :evaluation) end diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 9f2ccd27..663df31d 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -8,64 +8,21 @@ class SubmissionsController < ApplicationController def show; end def update - if params[:submission][:judging_status].present? - handle_judging_status_update - else - handle_comments_update - end - end - - private - - def handle_judging_status_update - unless valid_status_change? - return respond_with_error - end - - if @submission.update(submission_params) - respond_with_success - else - respond_with_error - end - end - - def handle_comments_update - if @submission.update!(submission_params) - flash.now[:success] = I18n.t("submission_updated") - render :show, submission: @submission - else - render :show, status: :unprocessable_entity, submission: @submission - end - end - - def valid_status_change? - new_status = params.dig(:submission, :judging_status) - current_status = @submission.judging_status - - if new_status == 'selected' && current_status == 'not_selected' && @submission.eligibility_checkbox_disabled? - return false - end - - if new_status == 'winner' && (!@submission.eligible_for_evaluation? || @submission.advancement_checkbox_disabled?) - return false - end - - true - end - - def respond_with_success respond_to do |format| - format.html { redirect_to submissions_phase_path(@submission.phase) } - format.json { render json: { success: true }, status: :ok } + if @submission.update(submission_params) + format.html do + flash[:success] = I18n.t("comments_saved") + redirect_to submission_path(@submission.phase) + end + format.json { render json: { submission: @submission } } + else + format.html { render :show } + format.json { render json: { errors: @submission.errors }, status: :unprocessable_entity } + end end end - def respond_with_error - respond_to do |format| - format.html { redirect_to submissions_phase_path(@submission.phase) } - format.json { render json: { success: false }, status: :unprocessable_entity } - end - end + private def submission_params params.require(:submission).permit(:comments, :judging_status) diff --git a/app/models/submission.rb b/app/models/submission.rb index 54eaadc8..f4d072c9 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -48,6 +48,10 @@ class Submission < ApplicationRecord # Validations validates :title, presence: true + validate :can_be_selected_to_advance, + if: -> { judging_status_change == ['selected', 'winner'] } + validate :can_be_ineligible_for_evaluation, + if: -> { judging_status_change == ['selected', 'not_selected'] } scope :by_user, lambda { |user| case user.role @@ -71,8 +75,8 @@ def selected_to_advance? winner? end - def eligibility_checkbox_disabled? - evaluators.any? + def eligibility_deselection_disabled? + evaluator_submission_assignments.where(status: [:assigned, :recused]).exists? end def advancement_checkbox_disabled? @@ -86,4 +90,21 @@ def all_evaluations_completed? includes(:evaluation). all? { |assignment| assignment.evaluation_status == :completed } end + + def can_be_selected_to_advance + unless judging_status_was == 'selected' + errors.add(:judging_status, "can't be selected to advance when not eligible for evaluation") + return + end + + if advancement_checkbox_disabled? + errors.add(:judging_status, "can't be selected to advance if not all evaluations are complete") + end + end + + def can_be_ineligible_for_evaluation + if eligibility_deselection_disabled? + errors.add(:judging_status, "can't deselect evaluation eligibility when there are evaluators assigned") + end + end end From d7f0dbf46541dbcea42a3e103be20ee7fdb1437b Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 20 Dec 2024 11:47:41 -0600 Subject: [PATCH 10/42] 291 | Display active assignment evaluator names after unassign --- app/views/phases/_submissions_table.html.erb | 21 ++++++++++---------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/app/views/phases/_submissions_table.html.erb b/app/views/phases/_submissions_table.html.erb index 3d68de5c..33d3dfba 100644 --- a/app/views/phases/_submissions_table.html.erb +++ b/app/views/phases/_submissions_table.html.erb @@ -37,7 +37,7 @@ Eligibility for evaluation <%= f.check_box :judging_status, id: "eligible_for_evaluation_#{submission.id}", - disabled: submission.eligibility_checkbox_disabled?, + disabled: submission.eligibility_deselection_disabled?, checked: submission.eligible_for_evaluation?, class: "usa-checkbox__input", data: { @@ -90,23 +90,22 @@ - <% if submission.evaluators.any? %> + <% active_assignments = submission.evaluator_submission_assignments.select { |a| ['assigned', 'recused'].include?(a.status) } %> + <% if active_assignments.any? %>
- <% submission.evaluator_submission_assignments.each do |assignment| %> + <% active_assignments.each do |assignment| %>
<%= "#{assignment.evaluator.first_name} #{assignment.evaluator.last_name}" %>
- <% if ['assigned', 'recused'].include?(assignment.status) %> -
- <%= evaluator_score(assignment).formatted_score %>% - - <%= assignment.evaluation_status.to_s.titleize %> - -
- <% end %> +
+ <%= evaluator_score(assignment).formatted_score %> + + <%= assignment.evaluation_status.to_s.titleize %> + +
<% end %>
From 00289732b5c7fc30d31e861780474402c5494c0e Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 20 Dec 2024 11:48:06 -0600 Subject: [PATCH 11/42] 292 | Update tests to reflect judging status validation --- spec/requests/submissions_spec.rb | 65 +++++++++++++------------------ 1 file changed, 26 insertions(+), 39 deletions(-) diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index 70a4c913..eafa310c 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -143,80 +143,67 @@ expect(submission.reload.judging_status).to eq('selected') end - it "updates advancement status to winner when evaluations are complete" do + it "prevents deselecting evaluation eligibility when evaluators are assigned" do submission.update!(judging_status: 'selected') create(:evaluator_submission_assignment, submission: submission, evaluator: evaluator, - status: :assigned, - evaluation: create(:evaluation, completed_at: Time.current)) + status: :assigned) patch submission_path(submission), params: { - submission: { judging_status: 'winner' } + submission: { judging_status: 'not_selected' } }, as: :json - expect(response).to have_http_status(:ok) - expect(submission.reload.judging_status).to eq('winner') + expect(response).to have_http_status(:unprocessable_entity) + expect(submission.reload.judging_status).to eq('selected') + expect(JSON.parse(response.body)['errors']).to include( + "judging_status" => ["can't deselect evaluation eligibility when there are evaluators assigned"] + ) end - it "prevents advancement when evaluations are not complete" do + it "allows advancing to winner when eligible and evaluations complete" do submission.update!(judging_status: 'selected') create(:evaluator_submission_assignment, submission: submission, evaluator: evaluator, - status: :assigned) + status: :assigned, + evaluation: create(:evaluation, completed_at: Time.current)) patch submission_path(submission), params: { submission: { judging_status: 'winner' } }, as: :json - expect(response).to have_http_status(:unprocessable_entity) - expect(submission.reload.judging_status).to eq('selected') + expect(response).to have_http_status(:ok) + expect(submission.reload.judging_status).to eq('winner') end - it "prevents eligibility changes when evaluators are assigned" do + it "prevents selected to advance when evaluations are incomplete" do + submission.update!(judging_status: 'selected') create(:evaluator_submission_assignment, submission: submission, evaluator: evaluator, status: :assigned) patch submission_path(submission), params: { - submission: { judging_status: 'selected' } - }, as: :json - - expect(response).to have_http_status(:unprocessable_entity) - expect(submission.reload.judging_status).to eq('not_selected') - end - - it "prevents changing judging_status from 'not_selected' to 'selected' when eligibility checkbox is disabled" do - submission.update!(judging_status: 'not_selected') - create(:evaluator_submission_assignment, - submission: submission, - evaluator: evaluator, - status: :assigned) - - patch submission_path(submission), params: { - submission: { judging_status: 'selected' } + submission: { judging_status: 'winner' } }, as: :json expect(response).to have_http_status(:unprocessable_entity) - expect(submission.reload.judging_status).to eq('not_selected') + expect(submission.reload.judging_status).to eq('selected') + expect(JSON.parse(response.body)['errors']).to include( + "judging_status" => ["can't be selected to advance if not all evaluations are complete"] + ) end + end - it "prevents changing judging_status to 'winner' when not eligible" do - submission.update!(judging_status: 'not_selected') - create(:evaluator_submission_assignment, - submission: submission, - evaluator: evaluator, - status: :assigned, - evaluation: create(:evaluation, completed_at: Time.current)) - + context "updating comments" do + it "successfully updates comments" do patch submission_path(submission), params: { - submission: { judging_status: 'winner' } + submission: { comments: "Comment about submission here" } }, as: :json - expect(response).to have_http_status(:unprocessable_entity) - expect(submission.reload.judging_status).to eq('not_selected') + expect(response).to have_http_status(:ok) + expect(submission.reload.comments).to eq("Comment about submission here") end end end From fbdb6a25498a608e431229a59a45d8fddd02a98e Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 20 Dec 2024 11:55:13 -0600 Subject: [PATCH 12/42] 292 | format fixes --- app/controllers/submissions_controller.rb | 22 +++++++++++++++------- app/models/submission.rb | 16 +++++++--------- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index 663df31d..d4353250 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -10,14 +10,9 @@ def show; end def update respond_to do |format| if @submission.update(submission_params) - format.html do - flash[:success] = I18n.t("comments_saved") - redirect_to submission_path(@submission.phase) - end - format.json { render json: { submission: @submission } } + handle_successful_update(format) else - format.html { render :show } - format.json { render json: { errors: @submission.errors }, status: :unprocessable_entity } + handle_failed_update(format) end end end @@ -32,4 +27,17 @@ def submission_params def set_submission @submission = Submission.by_user(current_user).find(params[:id]) end + + def handle_successful_update(format) + format.html do + flash[:success] = I18n.t("comments_saved") + redirect_to submission_path(@submission.phase) + end + format.json { render json: { submission: @submission } } + end + + def handle_failed_update(format) + format.html { render :show } + format.json { render json: { errors: @submission.errors }, status: :unprocessable_entity } + end end diff --git a/app/models/submission.rb b/app/models/submission.rb index f4d072c9..20e75ee3 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -49,9 +49,9 @@ class Submission < ApplicationRecord # Validations validates :title, presence: true validate :can_be_selected_to_advance, - if: -> { judging_status_change == ['selected', 'winner'] } + if: -> { judging_status_change == %w[selected winner] } validate :can_be_ineligible_for_evaluation, - if: -> { judging_status_change == ['selected', 'not_selected'] } + if: -> { judging_status_change == %w[selected not_selected] } scope :by_user, lambda { |user| case user.role @@ -76,7 +76,7 @@ def selected_to_advance? end def eligibility_deselection_disabled? - evaluator_submission_assignments.where(status: [:assigned, :recused]).exists? + evaluator_submission_assignments.exists?(status: [:assigned, :recused]) end def advancement_checkbox_disabled? @@ -97,14 +97,12 @@ def can_be_selected_to_advance return end - if advancement_checkbox_disabled? - errors.add(:judging_status, "can't be selected to advance if not all evaluations are complete") - end + return unless advancement_checkbox_disabled? + errors.add(:judging_status, "can't be selected to advance if not all evaluations are complete") end def can_be_ineligible_for_evaluation - if eligibility_deselection_disabled? - errors.add(:judging_status, "can't deselect evaluation eligibility when there are evaluators assigned") - end + return unless eligibility_deselection_disabled? + errors.add(:judging_status, "can't deselect evaluation eligibility when there are evaluators assigned") end end From 304ad2d80b9445875be833f7f8945299bd5ba5fe Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 20 Dec 2024 12:03:33 -0600 Subject: [PATCH 13/42] 291 | add spacing --- app/controllers/phases_controller.rb | 2 +- app/models/submission.rb | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index b26f6732..56348521 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -33,7 +33,7 @@ def set_phase def include_evaluator_associations return unless @submissions.any? && evaluator_assignments? - @submissions = @submissions.includes(:evaluators, evaluator_submission_assignments: :evaluation) + @submissions = @submissions.includes(:evaluators, evaluator_submission_assignments: :evaluator) end def evaluator_assignments? diff --git a/app/models/submission.rb b/app/models/submission.rb index 20e75ee3..4f0e49c2 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -98,11 +98,13 @@ def can_be_selected_to_advance end return unless advancement_checkbox_disabled? + errors.add(:judging_status, "can't be selected to advance if not all evaluations are complete") end def can_be_ineligible_for_evaluation return unless eligibility_deselection_disabled? + errors.add(:judging_status, "can't deselect evaluation eligibility when there are evaluators assigned") end end From 38f5591aa18a25a1f54c7280310df824621f8e8a Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 27 Dec 2024 13:55:45 -0600 Subject: [PATCH 14/42] 292 | Remove eager loading of evaluators --- app/controllers/phases_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 56348521..97d91447 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -33,7 +33,7 @@ def set_phase def include_evaluator_associations return unless @submissions.any? && evaluator_assignments? - @submissions = @submissions.includes(:evaluators, evaluator_submission_assignments: :evaluator) + @submissions = @submissions.includes(evaluator_submission_assignments: :evaluator) end def evaluator_assignments? From bde972de60cc063c08db949de8f440b6f0d3e2b3 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 30 Dec 2024 09:29:47 -0600 Subject: [PATCH 15/42] 291 | Update success message --- app/controllers/submissions_controller.rb | 2 +- config/locales/en.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index d4353250..fd0e4846 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -30,7 +30,7 @@ def set_submission def handle_successful_update(format) format.html do - flash[:success] = I18n.t("comments_saved") + flash[:success] = I18n.t("submission_updated") redirect_to submission_path(@submission.phase) end format.json { render json: { submission: @submission } } diff --git a/config/locales/en.yml b/config/locales/en.yml index b3a6e2f8..a3839cf4 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -33,7 +33,7 @@ en: evaluation_form_destroyed: "Evaluation form was successfully destroyed." evaluation_form_saved: "Evaluation form was saved successfully." comments_saved: "Comments saved succesfully." - submission_updated: "Submission was updated succesfully." + submission_updated: "Submission was updated successfully." login_error: "There was an issue with logging in. Please try again." please_try_again: "Please try again." session_expired_alert: "Your session has expired. Please log in again." From 90ef9608cf0809aab6c04f9b2110dff13c032770 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 11:12:29 -0800 Subject: [PATCH 16/42] Update copy --- app/models/submission.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/submission.rb b/app/models/submission.rb index 4f0e49c2..6967ffd9 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -99,7 +99,7 @@ def can_be_selected_to_advance return unless advancement_checkbox_disabled? - errors.add(:judging_status, "can't be selected to advance if not all evaluations are complete") + errors.add(:judging_status, "can't be selected to advance until all evaluations are complete") end def can_be_ineligible_for_evaluation From 71b418284d7e0945691e6d7e13c142ee26566955 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 11:12:42 -0800 Subject: [PATCH 17/42] Update copy --- app/models/submission.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/submission.rb b/app/models/submission.rb index 6967ffd9..7a1e0c5b 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -105,6 +105,6 @@ def can_be_selected_to_advance def can_be_ineligible_for_evaluation return unless eligibility_deselection_disabled? - errors.add(:judging_status, "can't deselect evaluation eligibility when there are evaluators assigned") + errors.add(:judging_status, "must remain eligible for evaluation when evaluators are assigned") end end From 6e7f905f702dedc4ee078f48e6f4b4c1ccb5f0de Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 6 Jan 2025 14:50:38 -0600 Subject: [PATCH 18/42] 291 | Redirect to submission id --- app/controllers/submissions_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/submissions_controller.rb b/app/controllers/submissions_controller.rb index fd0e4846..b813d627 100644 --- a/app/controllers/submissions_controller.rb +++ b/app/controllers/submissions_controller.rb @@ -31,7 +31,7 @@ def set_submission def handle_successful_update(format) format.html do flash[:success] = I18n.t("submission_updated") - redirect_to submission_path(@submission.phase) + redirect_to submission_path(@submission) end format.json { render json: { submission: @submission } } end From 2a7ca670858a65404fab65f299dc3200fca30341 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 6 Jan 2025 14:51:25 -0600 Subject: [PATCH 19/42] 291 | Rename model methods and remove redundant judging status check --- app/models/submission.rb | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/app/models/submission.rb b/app/models/submission.rb index 7a1e0c5b..fac0a43d 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -75,12 +75,12 @@ def selected_to_advance? winner? end - def eligibility_deselection_disabled? + def evaluators_assigned? evaluator_submission_assignments.exists?(status: [:assigned, :recused]) end - def advancement_checkbox_disabled? - !eligible_for_evaluation? || !all_evaluations_completed? || evaluators.empty? + def evaluations_missing_or_incomplete? + !eligible_for_evaluation? || !all_evaluations_completed? || evaluator_submission_assignments.empty? end private @@ -92,18 +92,13 @@ def all_evaluations_completed? end def can_be_selected_to_advance - unless judging_status_was == 'selected' - errors.add(:judging_status, "can't be selected to advance when not eligible for evaluation") - return - end - - return unless advancement_checkbox_disabled? + return unless evaluations_missing_or_incomplete? errors.add(:judging_status, "can't be selected to advance until all evaluations are complete") end def can_be_ineligible_for_evaluation - return unless eligibility_deselection_disabled? + return unless evaluators_assigned? errors.add(:judging_status, "must remain eligible for evaluation when evaluators are assigned") end From e2fa456539ac69cb6bf1c696d5b5f54a2898a507 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 6 Jan 2025 14:51:49 -0600 Subject: [PATCH 20/42] 291 | Utilitze renamed model methods --- app/views/phases/_submissions_table.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/phases/_submissions_table.html.erb b/app/views/phases/_submissions_table.html.erb index 33d3dfba..25e6302e 100644 --- a/app/views/phases/_submissions_table.html.erb +++ b/app/views/phases/_submissions_table.html.erb @@ -37,7 +37,7 @@ Eligibility for evaluation <%= f.check_box :judging_status, id: "eligible_for_evaluation_#{submission.id}", - disabled: submission.eligibility_deselection_disabled?, + disabled: submission.evaluators_assigned?, checked: submission.eligible_for_evaluation?, class: "usa-checkbox__input", data: { @@ -71,7 +71,7 @@ Selected to advance status <%= f.check_box :judging_status, id: "selected_to_advance_#{submission.id}", - disabled: submission.advancement_checkbox_disabled?, + disabled: submission.evaluations_missing_or_incomplete?, checked: submission.selected_to_advance?, class: "usa-checkbox__input", data: { From a490677a4b4d3ee64dee8d9a09e75b1108176db4 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 14:58:33 -0800 Subject: [PATCH 21/42] fix syntax error --- app/models/submission.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/models/submission.rb b/app/models/submission.rb index 3f6f7aa1..36ca2cb6 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -101,6 +101,7 @@ def can_be_ineligible_for_evaluation return unless evaluators_assigned? errors.add(:judging_status, "must remain eligible for evaluation when evaluators are assigned") + end def average_score avg = evaluations.average(:total_score) From 2370d1f5e36d9e89a9e175b9e94af8561f5bdb13 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 15:21:17 -0800 Subject: [PATCH 22/42] convert #order_by_average_score class method to scope, make #average_score method public --- app/models/submission.rb | 42 ++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/app/models/submission.rb b/app/models/submission.rb index 36ca2cb6..d743f57c 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -67,10 +67,31 @@ class Submission < ApplicationRecord } scope :eligible_for_evaluation, -> { where(judging_status: [:selected, :winner]) } + scope :order_by_average_score, lambda { |direction| + direction_sql = direction == :desc ? 'DESC' : 'ASC' + + joins( + "LEFT JOIN evaluations ON evaluations.submission_id = submissions.id " \ + "AND evaluations.completed_at IS NOT NULL" + ). + group('submissions.id'). + order( + Arel.sql( + "COALESCE(ROUND(AVG(evaluations.total_score)), 0) #{direction_sql}, " \ + "submissions.id #{direction_sql}" + ) + ) + } + def eligible_for_evaluation? selected? or winner? end + def average_score + avg = evaluations.average(:total_score) + avg ? avg.round : 0 + end + def selected_to_advance? winner? end @@ -102,25 +123,4 @@ def can_be_ineligible_for_evaluation errors.add(:judging_status, "must remain eligible for evaluation when evaluators are assigned") end - - def average_score - avg = evaluations.average(:total_score) - avg ? avg.round : 0 - end - - def self.order_by_average_score(direction) - direction_sql = direction == :desc ? 'DESC' : 'ASC' - - joins( - "LEFT JOIN evaluations ON evaluations.submission_id = submissions.id " \ - "AND evaluations.completed_at IS NOT NULL" - ). - group('submissions.id'). - order( - Arel.sql( - "COALESCE(ROUND(AVG(evaluations.total_score)), 0) #{direction_sql}, " \ - "submissions.id #{direction_sql}" - ) - ) - end end From 06ad73e04a79eb691c529bcbf852beb9ba000b2e Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 15:33:59 -0800 Subject: [PATCH 23/42] update error message, fix linter --- spec/requests/submissions_spec.rb | 67 +++++++++++++++---------------- 1 file changed, 33 insertions(+), 34 deletions(-) diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index efdb9ae7..6d7af218 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -156,8 +156,8 @@ expect(response).to have_http_status(:unprocessable_entity) expect(submission.reload.judging_status).to eq('selected') - expect(JSON.parse(response.body)['errors']).to include( - "judging_status" => ["can't deselect evaluation eligibility when there are evaluators assigned"] + expect(response.parsed_body['errors']).to include( + "judging_status" => ["must remain eligible for evaluation when evaluators are assigned"] ) end @@ -190,13 +190,13 @@ expect(response).to have_http_status(:unprocessable_entity) expect(submission.reload.judging_status).to eq('selected') - expect(JSON.parse(response.body)['errors']).to include( - "judging_status" => ["can't be selected to advance if not all evaluations are complete"] + expect(response.parsed_body['errors']).to include( + "judging_status" => ["can't be selected to advance until all evaluations are complete"] ) end end - context "updating comments" do + context "when updating comments" do it "successfully updates comments" do patch submission_path(submission), params: { submission: { comments: "Comment about submission here" } @@ -260,9 +260,9 @@ expect(response.body).to have_css("[data-submission-id='#{not_started_submission.id}']") expect(response.body).to have_css("[data-submission-id='#{eligible_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{selected_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{in_progress_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{completed_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{selected_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") end it 'shows only completed submissions' do @@ -270,9 +270,9 @@ expect(response.body).to have_css("[data-submission-id='#{completed_submission.id}']") expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{not_started_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{in_progress_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{eligible_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{eligible_submission.id}']") end end @@ -282,50 +282,50 @@ expect(response.body).to have_css("[data-submission-id='#{eligible_submission.id}']") expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{not_started_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{in_progress_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{completed_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") end it 'displays only selected to advance submissions' do get submissions_phase_path(phase), params: { selected_to_advance: 'true' } expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{eligible_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{not_started_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{in_progress_submission.id}']") - expect(response.body).not_to have_css("[data-submission-id='#{completed_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{eligible_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") end end context 'when sorting submissions' do before do create(:evaluation, - evaluator_submission_assignment: create(:evaluator_submission_assignment, submission: in_progress_submission), - total_score: 80 - ) + evaluator_submission_assignment: create(:evaluator_submission_assignment, + submission: in_progress_submission), + total_score: 80) create(:evaluation, - evaluator_submission_assignment: create(:evaluator_submission_assignment, submission: completed_submission), - total_score: 90 - ) + evaluator_submission_assignment: create(:evaluator_submission_assignment, + submission: completed_submission), + total_score: 90) end it 'orders submissions by score high to low' do get submissions_phase_path(phase), params: { sort: 'average_score_high_to_low' } - expect(response.body).to have_selector( - "tr[data-submission-id='#{completed_submission.id}']" \ - " ~ tr[data-submission-id='#{in_progress_submission.id}']" + expect(response.body).to have_css( + "tr[data-submission-id='#{completed_submission.id}'] " \ + "~ tr[data-submission-id='#{in_progress_submission.id}']" ) end it 'orders submissions by score low to high' do get submissions_phase_path(phase), params: { sort: 'average_score_low_to_high' } - expect(response.body).to have_selector( - "tr[data-submission-id='#{in_progress_submission.id}']" \ - " ~ tr[data-submission-id='#{completed_submission.id}']" + expect(response.body).to have_css( + "tr[data-submission-id='#{in_progress_submission.id}'] " \ + "~ tr[data-submission-id='#{completed_submission.id}']" ) end end @@ -348,16 +348,15 @@ get submissions_phase_path(phase, page: 2, partial: true) expect(response).to have_http_status(:success) expect(response.body).to have_css('tr[data-submission-id]', count: 5) - expect(response.body).not_to have_button('Load more') + expect(response.body).to have_no_button('Load more') end context 'when sorting by average score' do let!(:scored_submissions) do submissions[0..24].each_with_index do |submission, index| create(:evaluation, - evaluator_submission_assignment: create(:evaluator_submission_assignment, submission: submission), - total_score: (index + 1) * 20 - ) + evaluator_submission_assignment: create(:evaluator_submission_assignment, submission: submission), + total_score: (index + 1) * 20) end end From f45288eb495a8d87d4a47421d9dc7b5e6d5921ff Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 15:41:44 -0800 Subject: [PATCH 24/42] linting --- spec/requests/submissions_spec.rb | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index 6d7af218..7cb1f24e 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -64,11 +64,6 @@ create(:submission, challenge: challenge, phase: phase) create(:submission, challenge: challenge, phase: phase, judging_status: "selected") - allow_any_instance_of(ActionView::Base).to receive(:render).and_call_original - allow_any_instance_of(ActionView::Base).to receive(:render). - with(hash_including(partial: "submissions_table")). - and_return("") - get submissions_phase_path(phase) expect(response.body).to include("Boston Tea Party Cleanup") # total submission count @@ -133,7 +128,7 @@ ChallengeManager.create(user: user, challenge: challenge) end - context "updating judging status" do + context "when updating judging status" do it "updates eligibility status to selected" do patch submission_path(submission), params: { submission: { judging_status: 'selected' } From b5a01233ce8d554b46940bd91353aac4a8d69f89 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 16:02:00 -0800 Subject: [PATCH 25/42] reformat to combine specs with the same name --- spec/requests/submissions_spec.rb | 442 +++++++++++++++--------------- 1 file changed, 214 insertions(+), 228 deletions(-) diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index 7cb1f24e..1aca11a9 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -2,99 +2,11 @@ RSpec.describe "Submissions" do let(:user) { nil } - let(:challenge) { create_challenge(user: user, title: "Boston Tea Party Cleanup") } - let(:phase) { create_phase(challenge_id: challenge.id) } + let(:challenge) { create(:challenge, user: user, title: "Boston Tea Party Cleanup") } + let(:phase) { create(:phase, challenge: challenge) } before { log_in_user(user) } - describe "GET /phases/:id/submissions" do - context "when logged in as a super admin" do - let(:user) { create_user(role: "super_admin") } - - it "redirects to the phoenix app" do - get phases_path - - expect(response).to redirect_to(ENV.fetch("PHOENIX_URI", nil)) - end - end - - context "when logged in as a admin" do - let(:user) { create_user(role: "admin") } - - it "redirects to the phoenix app" do - get phases_path - - expect(response).to redirect_to(ENV.fetch("PHOENIX_URI", nil)) - end - end - - context "when logged in as a challenge manager" do - let(:user) { create_user(role: "challenge_manager") } - - it "renders an empty list of submissions for a user's challenge" do - get submissions_phase_path(phase) - expect(response.body).to include("Boston Tea Party Cleanup") - - expect(response.body).to include("No submissions found.") - end - - it "renders a list of submissions for a user's challenge" do - submission = create(:submission, challenge: challenge, phase: phase) - evaluator = create(:user, role: 'evaluator') - create(:evaluator_submission_assignment, - submission: submission, - evaluator: evaluator, - status: :assigned, - evaluation: create(:evaluation)) - - get submissions_phase_path(phase) - expect(response.body).to include("Boston Tea Party Cleanup") - expect(response.body).to include(submission.id.to_s) - end - - it "does not render submissions for a challenge the user is not assigned to" do - challenge = create_challenge(title: "Star Spangled Banister") - phase = create_phase(challenge_id: challenge.id) - - get submissions_phase_path(phase) - expect(response).to have_http_status(:not_found) - end - - it "renders submission statistics" do - create(:submission, challenge: challenge, phase: phase) - create(:submission, challenge: challenge, phase: phase, judging_status: "selected") - - get submissions_phase_path(phase) - expect(response.body).to include("Boston Tea Party Cleanup") - # total submission count - expect(response.body).to have_css("h3.text-primary", text: "Total Submissions") - expect(response.body).to have_css("span.font-sans-3xl.text-primary.text-bold", text: "2") - # selected to advance - expect(response.body).to have_css("span.text-primary", text: "1 of 2") - end - end - - context "when logged in as an evaluator" do - let(:user) { create_user(role: "evaluator") } - - it "redirects to the dashboard" do - get submissions_phase_path(phase) - - expect(response).to redirect_to(dashboard_path) - end - end - - context "when logged in as a solver" do - let(:user) { create_user(role: "solver") } - - it "redirects to the phoenix app" do - get submissions_phase_path(phase) - - expect(response).to redirect_to(ENV.fetch("PHOENIX_URI", nil)) - end - end - end - describe "GET /submissions/:id" do context "when logged in as a challenge manager" do let(:user) { create_user(role: "challenge_manager") } @@ -108,8 +20,8 @@ end it "does not render submission details for a challenge the user is not assigned to" do - challenge = create_challenge - phase = create_phase(challenge_id: challenge.id) + challenge = create(:challenge) + phase = create(:phase, challenge:) submission = create(:submission, challenge: phase.challenge, brief_description: "This submission has teeth.") get submission_path(submission) @@ -204,187 +116,261 @@ end end - describe 'GET /phases/:id/submissions' do - let(:user) { create_user(role: "challenge_manager") } - let(:challenge) { create(:challenge, user: user) } - let(:phase) { create(:phase, challenge: challenge) } + describe "GET /phases/:id/submissions" do + context "when logged in as a super admin" do + let(:user) { create_user(role: "super_admin") } - before do - ChallengeManager.create!(user: user, challenge: challenge) - log_in_user(user) - end + it "redirects to the phoenix app" do + get phases_path - context 'when viewing submissions' do - let!(:not_started_submission) { create(:submission, challenge: challenge, phase: phase) } - let!(:in_progress_submission) do - submission = create(:submission, challenge: challenge, phase: phase) - assignment = create(:evaluator_submission_assignment, submission: submission, status: :assigned) - create(:evaluation, evaluator_submission_assignment: assignment, completed_at: nil) - submission - end - let!(:completed_submission) do - submission = create(:submission, challenge: challenge, phase: phase) - assignment = create(:evaluator_submission_assignment, submission: submission) - create(:evaluation, evaluator_submission_assignment: assignment, completed_at: Time.current) - submission + expect(response).to redirect_to(ENV.fetch("PHOENIX_URI", nil)) end - let!(:eligible_submission) { create(:submission, challenge: challenge, phase: phase, judging_status: 'selected') } - let!(:selected_submission) do - submission = create(:submission, challenge: challenge, phase: phase, judging_status: 'winner') - assignment = create(:evaluator_submission_assignment, submission: submission) - create(:evaluation, evaluator_submission_assignment: assignment, completed_at: Time.current) - submission + end + + context "when logged in as a admin" do + let(:user) { create_user(role: "admin") } + + it "redirects to the phoenix app" do + get phases_path + + expect(response).to redirect_to(ENV.fetch("PHOENIX_URI", nil)) end + end - it 'displays all submissions and their status counts' do - get submissions_phase_path(phase) + context "when logged in as an evaluator" do + let(:user) { create_user(role: "evaluator") } - [not_started_submission, in_progress_submission, completed_submission, - eligible_submission, selected_submission].each do |submission| - expect(response.body).to have_css("[data-submission-id='#{submission.id}']") - end + it "redirects to the dashboard" do + get submissions_phase_path(phase) - expect(response.body).to have_css('.text-secondary-dark.text-bold', text: '2') # not_started, eligible - expect(response.body).to have_css('.text-orange.text-bold', text: '1') # in_progress - expect(response.body).to have_css('.text-green.text-bold', text: '2') # completed, selected + expect(response).to redirect_to(dashboard_path) end + end - context 'when filtering submissions' do - it 'shows only submissions matching the selected status' do - get submissions_phase_path(phase), params: { status: 'not_started' } + context "when logged in as a solver" do + let(:user) { create_user(role: "solver") } - expect(response.body).to have_css("[data-submission-id='#{not_started_submission.id}']") - expect(response.body).to have_css("[data-submission-id='#{eligible_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{selected_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") - end + it "redirects to the phoenix app" do + get submissions_phase_path(phase) + + expect(response).to redirect_to(ENV.fetch("PHOENIX_URI", nil)) + end + end + + context "when logged in as a challenge manager" do + let(:user) { create_user(role: "challenge_manager") } - it 'shows only completed submissions' do - get submissions_phase_path(phase), params: { status: 'completed' } + context "when the challenge manager is not assigned to the challenge phase" do + it "renders :not_found" do + challenge = create(:challenge, title: "Star Spangled Banister") + phase = create(:phase, challenge: challenge) - expect(response.body).to have_css("[data-submission-id='#{completed_submission.id}']") - expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{eligible_submission.id}']") + get submissions_phase_path(phase) + expect(response).to have_http_status(:not_found) end end - context 'when filtering by eligibility' do - it 'displays only eligible for evaluation submissions' do - get submissions_phase_path(phase), params: { eligible_for_evaluation: 'true' } - - expect(response.body).to have_css("[data-submission-id='#{eligible_submission.id}']") - expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") + context "when there are no submissions for the challenge phase" do + it "renders an empty list for the challenge phase" do + get submissions_phase_path(phase) + expect(response.body).to include("Boston Tea Party Cleanup") + expect(response.body).to include("No submissions found.") end + end - it 'displays only selected to advance submissions' do - get submissions_phase_path(phase), params: { selected_to_advance: 'true' } + context "when there are submissions for the challenge phase" do + before do + ChallengeManager.create!(user: user, challenge: challenge) + end - expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{eligible_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") - expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") + it "renders submission statistics" do + create(:submission, challenge: challenge, phase: phase) + create(:submission, challenge: challenge, phase: phase, judging_status: "selected") + + get submissions_phase_path(phase) + expect(response.body).to include("Boston Tea Party Cleanup") + # total submission count + expect(response.body).to have_css("h3.text-primary", text: "Total Submissions") + expect(response.body).to have_css("span.font-sans-3xl.text-primary.text-bold", text: "2") + # selected to advance + expect(response.body).to have_css("span.text-primary", text: "1 of 2") end end - context 'when sorting submissions' do - before do - create(:evaluation, - evaluator_submission_assignment: create(:evaluator_submission_assignment, - submission: in_progress_submission), - total_score: 80) - - create(:evaluation, - evaluator_submission_assignment: create(:evaluator_submission_assignment, - submission: completed_submission), - total_score: 90) + context 'when viewing submissions' do + let!(:not_started_submission) { create(:submission, challenge: challenge, phase: phase) } + let!(:in_progress_submission) do + submission = create(:submission, challenge: challenge, phase: phase) + assignment = create(:evaluator_submission_assignment, submission: submission, status: :assigned) + create(:evaluation, evaluator_submission_assignment: assignment, completed_at: nil) + submission + end + let!(:completed_submission) do + submission = create(:submission, challenge: challenge, phase: phase) + assignment = create(:evaluator_submission_assignment, submission: submission) + create(:evaluation, evaluator_submission_assignment: assignment, completed_at: Time.current) + submission + end + let!(:eligible_submission) do + create(:submission, challenge: challenge, phase: phase, judging_status: 'selected') + end + let!(:selected_submission) do + submission = create(:submission, challenge: challenge, phase: phase, judging_status: 'winner') + assignment = create(:evaluator_submission_assignment, submission: submission) + create(:evaluation, evaluator_submission_assignment: assignment, completed_at: Time.current) + submission end - it 'orders submissions by score high to low' do - get submissions_phase_path(phase), params: { sort: 'average_score_high_to_low' } + it 'displays all submissions and their status counts' do + get submissions_phase_path(phase) - expect(response.body).to have_css( - "tr[data-submission-id='#{completed_submission.id}'] " \ - "~ tr[data-submission-id='#{in_progress_submission.id}']" - ) + [not_started_submission, in_progress_submission, completed_submission, + eligible_submission, selected_submission].each do |submission| + expect(response.body).to have_css("[data-submission-id='#{submission.id}']") + end + + expect(response.body).to have_css('.text-secondary-dark.text-bold', text: '2') # not_started, eligible + expect(response.body).to have_css('.text-orange.text-bold', text: '1') # in_progress + expect(response.body).to have_css('.text-green.text-bold', text: '2') # completed, selected end - it 'orders submissions by score low to high' do - get submissions_phase_path(phase), params: { sort: 'average_score_low_to_high' } + context 'when filtering submissions' do + it 'shows only submissions matching the selected status' do + get submissions_phase_path(phase), params: { status: 'not_started' } - expect(response.body).to have_css( - "tr[data-submission-id='#{in_progress_submission.id}'] " \ - "~ tr[data-submission-id='#{completed_submission.id}']" - ) + expect(response.body).to have_css("[data-submission-id='#{not_started_submission.id}']") + expect(response.body).to have_css("[data-submission-id='#{eligible_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{selected_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") + end + + it 'shows only completed submissions' do + get submissions_phase_path(phase), params: { status: 'completed' } + + expect(response.body).to have_css("[data-submission-id='#{completed_submission.id}']") + expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{eligible_submission.id}']") + end end - end - end - context 'when paginating submissions' do - let!(:submissions) do - (1..25).map do |n| - create(:submission, challenge: challenge, phase: phase) + context 'when filtering by eligibility' do + it 'displays only eligible for evaluation submissions' do + get submissions_phase_path(phase), params: { eligible_for_evaluation: 'true' } + + expect(response.body).to have_css("[data-submission-id='#{eligible_submission.id}']") + expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") + end + + it 'displays only selected to advance submissions' do + get submissions_phase_path(phase), params: { selected_to_advance: 'true' } + + expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{eligible_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{not_started_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{in_progress_submission.id}']") + expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") + end end - end - it 'returns first page of submissions' do - get submissions_phase_path(phase) - expect(response.body).to have_css('tr[data-submission-id]', count: 20) - expect(response.body).to have_button('Load more') - end + context 'when sorting submissions' do + before do + create(:evaluation, + evaluator_submission_assignment: create(:evaluator_submission_assignment, + submission: in_progress_submission), + total_score: 80) + + create(:evaluation, + evaluator_submission_assignment: create(:evaluator_submission_assignment, + submission: completed_submission), + total_score: 90) + end - it 'returns next page of submissions via partial' do - get submissions_phase_path(phase, page: 2, partial: true) - expect(response).to have_http_status(:success) - expect(response.body).to have_css('tr[data-submission-id]', count: 5) - expect(response.body).to have_no_button('Load more') + it 'orders submissions by score high to low' do + get submissions_phase_path(phase), params: { sort: 'average_score_high_to_low' } + + expect(response.body).to have_css( + "tr[data-submission-id='#{completed_submission.id}'] " \ + "~ tr[data-submission-id='#{in_progress_submission.id}']" + ) + end + + it 'orders submissions by score low to high' do + get submissions_phase_path(phase), params: { sort: 'average_score_low_to_high' } + + expect(response.body).to have_css( + "tr[data-submission-id='#{in_progress_submission.id}'] " \ + "~ tr[data-submission-id='#{completed_submission.id}']" + ) + end + end end - context 'when sorting by average score' do - let!(:scored_submissions) do - submissions[0..24].each_with_index do |submission, index| - create(:evaluation, - evaluator_submission_assignment: create(:evaluator_submission_assignment, submission: submission), - total_score: (index + 1) * 20) + context 'when paginating submissions' do + let!(:submissions) do + (1..25).map do |n| + create(:submission, challenge: challenge, phase: phase) end end - it 'paginates correctly when sorted by score' do - get submissions_phase_path(phase, page: 1, sort: 'average_score_high_to_low') - expect(response).to have_http_status(:success) - first_page_scores = response.body.scan(/data-score="(\d+)"/).flatten - expect(first_page_scores.count).to eq(20) - expect(first_page_scores.map(&:to_i)).to eq(first_page_scores.map(&:to_i).sort.reverse) + it 'returns first page of submissions' do + get submissions_phase_path(phase) + expect(response.body).to have_css('tr[data-submission-id]', count: 20) expect(response.body).to have_button('Load more') + end - get submissions_phase_path(phase, page: 2, partial: true, sort: 'average_score_high_to_low') + it 'returns next page of submissions via partial' do + get submissions_phase_path(phase, page: 2, partial: true) expect(response).to have_http_status(:success) - second_page_scores = response.body.scan(/data-score="(\d+)"/).flatten - expect(second_page_scores.count).to eq(5) - expect(second_page_scores.map(&:to_i)).to eq(second_page_scores.map(&:to_i).sort.reverse) + expect(response.body).to have_css('tr[data-submission-id]', count: 5) + expect(response.body).to have_no_button('Load more') end - end - context 'when filtering submissions' do - let!(:eligible_submissions) do - submissions[0..22].each { |s| s.update!(judging_status: 'selected') } + context 'when sorting by average score' do + let!(:scored_submissions) do + submissions[0..24].each_with_index do |submission, index| + create(:evaluation, + evaluator_submission_assignment: create(:evaluator_submission_assignment, submission: submission), + total_score: (index + 1) * 20) + end + end + + it 'paginates correctly when sorted by score' do + get submissions_phase_path(phase, page: 1, sort: 'average_score_high_to_low') + expect(response).to have_http_status(:success) + first_page_scores = response.body.scan(/data-score="(\d+)"/).flatten + expect(first_page_scores.count).to eq(20) + expect(first_page_scores.map(&:to_i)).to eq(first_page_scores.map(&:to_i).sort.reverse) + expect(response.body).to have_button('Load more') + + get submissions_phase_path(phase, page: 2, partial: true, sort: 'average_score_high_to_low') + expect(response).to have_http_status(:success) + second_page_scores = response.body.scan(/data-score="(\d+)"/).flatten + expect(second_page_scores.count).to eq(5) + expect(second_page_scores.map(&:to_i)).to eq(second_page_scores.map(&:to_i).sort.reverse) + end end - it 'paginates correctly with eligible filter' do - get submissions_phase_path(phase, page: 1, eligible_for_evaluation: 'true') - expect(response).to have_http_status(:success) - expect(response.body.scan(/data-submission-id="(\d+)"/).flatten.count).to eq(20) - expect(response.body).to have_button('Load more') + context 'when filtering submissions' do + let!(:eligible_submissions) do + submissions[0..22].each { |s| s.update!(judging_status: 'selected') } + end - get submissions_phase_path(phase, page: 2, partial: true, eligible_for_evaluation: 'true') - expect(response).to have_http_status(:success) - expect(response.body.scan(/data-submission-id="(\d+)"/).flatten.count).to eq(3) + it 'paginates correctly with eligible filter' do + get submissions_phase_path(phase, page: 1, eligible_for_evaluation: 'true') + expect(response).to have_http_status(:success) + expect(response.body.scan(/data-submission-id="(\d+)"/).flatten.count).to eq(20) + expect(response.body).to have_button('Load more') + + get submissions_phase_path(phase, page: 2, partial: true, eligible_for_evaluation: 'true') + expect(response).to have_http_status(:success) + expect(response.body.scan(/data-submission-id="(\d+)"/).flatten.count).to eq(3) + end end end end From 1c377b85575cf7a6c48333fccb899a4e56429643 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 16:11:00 -0800 Subject: [PATCH 26/42] linting --- spec/requests/submissions_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index 1aca11a9..2b35c506 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -314,7 +314,7 @@ context 'when paginating submissions' do let!(:submissions) do (1..25).map do |n| - create(:submission, challenge: challenge, phase: phase) + create(:submission, challenge: challenge, phase: phase, title: "submission #{n}") end end @@ -332,7 +332,7 @@ end context 'when sorting by average score' do - let!(:scored_submissions) do + before do submissions[0..24].each_with_index do |submission, index| create(:evaluation, evaluator_submission_assignment: create(:evaluator_submission_assignment, submission: submission), @@ -357,7 +357,7 @@ end context 'when filtering submissions' do - let!(:eligible_submissions) do + before do submissions[0..22].each { |s| s.update!(judging_status: 'selected') } end From 52ed1e365e4d4ea2c527b7eccc9f9b1890357832 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 16:14:11 -0800 Subject: [PATCH 27/42] remove unused helper methods --- app/helpers/evaluations_helper.rb | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/app/helpers/evaluations_helper.rb b/app/helpers/evaluations_helper.rb index 7e9795cc..d1fe2bb1 100644 --- a/app/helpers/evaluations_helper.rb +++ b/app/helpers/evaluations_helper.rb @@ -2,28 +2,4 @@ # View helpers for calculating evaluation & submission details. module EvaluationsHelper - Score = Struct.new(:raw_score, :formatted_score, :display_score) - - # individual evaluator score - def evaluator_score(assignment) - score = assignment.evaluation&.total_score - - unless assignment.evaluation_status == :completed && score - return Score.new(0, "0", "N/A") - end - - Score.new(score, score.to_s, score.to_s) - end - - def average_score(submission) - completed_evaluations = submission.evaluations.where.not(completed_at: nil) - - unless completed_evaluations.any? - return Score.new(0, "0", "N/A") - end - - avg = completed_evaluations.average(:total_score) - score = avg ? avg.round : 0 - Score.new(score, score.to_s, score.to_s) - end end From b27170a851d088cdf2d7dfed04490d6eda3eef41 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 6 Jan 2025 15:17:15 -0600 Subject: [PATCH 28/42] 291 | Fix test by mocking and adding an evaluation --- spec/system/submission_details_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/system/submission_details_spec.rb b/spec/system/submission_details_spec.rb index 0b06b6c9..0966b1c6 100644 --- a/spec/system/submission_details_spec.rb +++ b/spec/system/submission_details_spec.rb @@ -20,12 +20,21 @@ it "allows manipulation of judging status" do visit submission_path(submission) + evaluator = create(:user, role: 'evaluator') find_by_id('eligible-for-evaluation').click click_on('Save') updated_submission = Submission.find(submission.id) expect(updated_submission.judging_status).to eq('selected') + assigned_submission = create(:evaluator_submission_assignment, + submission: updated_submission, + evaluator: evaluator, + status: :assigned) + create(:evaluation, + evaluator_submission_assignment: assigned_submission, + completed_at: Time.current) + find_by_id('selected-to-advance').click click_on('Save') updated_submission = Submission.find(submission.id) From a32c5e1de949927430beb38517c1b7ff54aab36a Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 6 Jan 2025 19:37:47 -0600 Subject: [PATCH 29/42] 291 | Fix merge conflict, combine submission index --- app/controllers/phases_controller.rb | 2 +- .../phases/_submissions_table_rows.html.erb | 215 ++++++++++++------ 2 files changed, 149 insertions(+), 68 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index c8b1eb39..a9068c8f 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -11,7 +11,7 @@ def index def submissions @submissions = @phase.submissions.order(:id) - + include_evaluator_associations set_submission_counts diff --git a/app/views/phases/_submissions_table_rows.html.erb b/app/views/phases/_submissions_table_rows.html.erb index f6c8b800..0cdde0cd 100644 --- a/app/views/phases/_submissions_table_rows.html.erb +++ b/app/views/phases/_submissions_table_rows.html.erb @@ -1,72 +1,153 @@ <% @submissions.each do |submission| %> - - - Submission ID <%= submission.id %> - - -
- <% if submission.eligible_for_evaluation? %> - -
- <%= image_tag( - "images/usa-icons/verified.svg", - class: "usa-icon--size-3 margin-right-1", - alt: "" - )%> - Eligible for Evaluation -
- <% else %> - -
- <%= image_tag( - "images/usa-icons/highlight_off.svg", - class: "usa-icon--size-3 margin-right-1", - alt: "" - )%> - Not Eligible for Evaluation -
+ + + Submission ID <%= submission.id %> + + + + + <% if submission.eligible_for_evaluation? %> +
+ <%= image_tag( + "images/usa-icons/verified.svg", + class: "usa-icon--size-3 margin-right-1", + alt: "" + )%> + Eligible for Evaluation +
+ <% else %> +
+ <%= image_tag( + "images/usa-icons/highlight_off.svg", + class: "usa-icon--size-3 margin-right-1", + alt: "" + )%> + Not Eligible for Evaluation +
+ <% end %> + + + + + <% else %> + No evaluators assigned to this submission + <% end %> + + + + <%= average_score(submission).formatted_score %> + + + +
+ <%= link_to submission_path(submission) do %> + + <% end %> +
+ + <% end %> From 3c795636b903b4bcf742f3fd4bb5a95fd534d476 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 20:01:53 -0800 Subject: [PATCH 30/42] fix submission_details_spec --- spec/system/submission_details_spec.rb | 46 +++++++++++++++++--------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/spec/system/submission_details_spec.rb b/spec/system/submission_details_spec.rb index 0b06b6c9..e931c3cd 100644 --- a/spec/system/submission_details_spec.rb +++ b/spec/system/submission_details_spec.rb @@ -6,10 +6,14 @@ describe "Logged-in as a Challenge Manager" do let(:user) { create_user(role: "challenge_manager") } let(:challenge) { create_challenge(user: user, title: "Boston Tea Party Cleanup") } - let(:submission) { create(:submission, manager: user, challenge: challenge) } + let(:phase) { create(:phase, challenge: challenge) } + let(:submission) { create(:submission, manager: user, challenge:, phase:) } + let(:fake_comments) { Faker::Lorem.sentence } - - before { system_login_user(user) } + before do + create(:challenge_manager, challenge:, user:) + system_login_user(user) + end it "submission details page is accessible" do visit submission_path(submission) @@ -18,28 +22,38 @@ expect(page).to(be_axe_clean) end - it "allows manipulation of judging status" do + it "allows marking judging status eligible for evaluation" do visit submission_path(submission) + eligible_input = page.find_by_id('eligible-for-evaluation').find('input.usa-checkbox__input', visible: :hidden) + expect(eligible_input).not_to be_checked find_by_id('eligible-for-evaluation').click - click_on('Save') - updated_submission = Submission.find(submission.id) - expect(updated_submission.judging_status).to eq('selected') + click_on "Save" + expect(page).to have_css("p.usa-alert__text", text: "Submission was updated successfully.") + eligible_input = page.find_by_id('eligible-for-evaluation').find('input.usa-checkbox__input', visible: :hidden) + expect(eligible_input).to be_checked + expect(submission.reload.judging_status).to eq("selected") + end + it "allows marking judging status selected to advance" do + # evaluations must exist and all be completed before selecting the submission to advance + evaluator = create_user(role: "evaluator") + submission.update(judging_status: :selected) + assignment = create(:evaluator_submission_assignment, status: :assigned, evaluator:, submission:) + create(:evaluation, evaluator_submission_assignment: assignment, completed_at: Time.current) + + visit submission_path(submission) find_by_id('selected-to-advance').click - click_on('Save') - updated_submission = Submission.find(submission.id) - expect(updated_submission.judging_status).to eq('winner') + click_on "Save" + expect(page).to have_css("p.usa-alert__text", text: "Submission was updated successfully.") + expect(submission.reload.judging_status).to eq("winner") end it "saves comments" do visit submission_path(submission) - comments = Faker::Lorem.sentence - - fill_in "Comments and notes:", with: comments - click_on('Save') - updated_submission = Submission.find(submission.id) - expect(updated_submission.comments).to eq(comments) + fill_in "Comments and notes:", with: fake_comments + click_on "Save" + assert_text(fake_comments) end end end From 07c0e5bf04ab36161651b02983c8c08588a6f969 Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 20:11:13 -0800 Subject: [PATCH 31/42] expect UI state --- spec/system/submission_details_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/system/submission_details_spec.rb b/spec/system/submission_details_spec.rb index e931c3cd..119057a1 100644 --- a/spec/system/submission_details_spec.rb +++ b/spec/system/submission_details_spec.rb @@ -43,9 +43,13 @@ create(:evaluation, evaluator_submission_assignment: assignment, completed_at: Time.current) visit submission_path(submission) + selected_input = page.find_by_id('selected-to-advance').find('input.usa-checkbox__input', visible: :hidden) + expect(selected_input).not_to be_checked find_by_id('selected-to-advance').click click_on "Save" expect(page).to have_css("p.usa-alert__text", text: "Submission was updated successfully.") + selected_input = page.find_by_id('selected-to-advance').find('input.usa-checkbox__input', visible: :hidden) + expect(selected_input).to be_checked expect(submission.reload.judging_status).to eq("winner") end From 3ab2cd6349286ed1e90d2f2ebd4b2c0fbefe531b Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Mon, 6 Jan 2025 20:55:45 -0800 Subject: [PATCH 32/42] fix merge conflict --- app/views/phases/_submissions_table_rows.html.erb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/phases/_submissions_table_rows.html.erb b/app/views/phases/_submissions_table_rows.html.erb index 0cdde0cd..1fdfd08a 100644 --- a/app/views/phases/_submissions_table_rows.html.erb +++ b/app/views/phases/_submissions_table_rows.html.erb @@ -136,8 +136,8 @@ <% end %> - - <%= average_score(submission).formatted_score %> + + <%= submission.average_score %> From e220288e25f4d78fadf05183dea1fb65699fd478 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 13:46:39 -0600 Subject: [PATCH 33/42] 291 | Make submission table rows consistent in size --- app/views/phases/_submissions_table.html.erb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/views/phases/_submissions_table.html.erb b/app/views/phases/_submissions_table.html.erb index 67303e2e..3e6d6ce9 100644 --- a/app/views/phases/_submissions_table.html.erb +++ b/app/views/phases/_submissions_table.html.erb @@ -14,12 +14,12 @@ - - - - - - + + + + + + From 0be31fa8981c5ff12852d2b5802b8dd5eadf7f00 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 13:47:17 -0600 Subject: [PATCH 34/42] 291 | Add format scoring --- app/helpers/evaluations_helper.rb | 15 +++++++++++++++ app/views/phases/_submissions_table_rows.html.erb | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/app/helpers/evaluations_helper.rb b/app/helpers/evaluations_helper.rb index d1fe2bb1..c18ed068 100644 --- a/app/helpers/evaluations_helper.rb +++ b/app/helpers/evaluations_helper.rb @@ -2,4 +2,19 @@ # View helpers for calculating evaluation & submission details. module EvaluationsHelper + Score = Struct.new(:raw_score, :formatted_score, :display_score) + + def evaluator_score(assignment) # individual evaluator score + score = display_score(assignment) + return Score.new(0, "0", "N/A") if score == 'N/A' + + Score.new(score, score.to_s, score.to_s) + end + + def average_score(submission) + score = submission.average_score + return Score.new(0, "0", "N/A") if score.zero? + + Score.new(score, score.to_s, score.to_s) + end end diff --git a/app/views/phases/_submissions_table_rows.html.erb b/app/views/phases/_submissions_table_rows.html.erb index 1fdfd08a..2283c56a 100644 --- a/app/views/phases/_submissions_table_rows.html.erb +++ b/app/views/phases/_submissions_table_rows.html.erb @@ -136,8 +136,8 @@ <% end %> -
Submission IDEligible for EvaluationSelected to AdvanceAssigned EvaluatorsAverage ScoreView SubmissionSubmission IDEligible for EvaluationSelected to AdvanceAssigned EvaluatorsAverage ScoreView Submission
- <%= submission.average_score %> + + <%= average_score(submission).formatted_score %> From 4477db370e5dce7395657bb733b3bd1925a30b60 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 13:47:58 -0600 Subject: [PATCH 35/42] 291 | Remove redundant evaluation inclusion --- app/models/submission.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/app/models/submission.rb b/app/models/submission.rb index d743f57c..24ef658f 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -108,7 +108,6 @@ def evaluations_missing_or_incomplete? def all_evaluations_completed? evaluator_submission_assignments. - includes(:evaluation). all? { |assignment| assignment.evaluation_status == :completed } end From f1445734bd14d402edba3baaa0480c9569b9d173 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 13:48:14 -0600 Subject: [PATCH 36/42] 291 | Move sort and filter logic to service --- app/controllers/phases_controller.rb | 101 ++++++------------------ app/services/sort_and_filter_service.rb | 76 ++++++++++++++++++ 2 files changed, 99 insertions(+), 78 deletions(-) create mode 100644 app/services/sort_and_filter_service.rb diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index a9068c8f..1bd78aa4 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -10,30 +10,23 @@ def index end def submissions - @submissions = @phase.submissions.order(:id) - - include_evaluator_associations + @submissions = @phase.submissions.includes(evaluator_submission_assignments: [:evaluator, :evaluation]) set_submission_counts set_submission_statuses - apply_filters - apply_sorting + @submissions = SortAndFilterService.new( + @submissions, + params, + @not_started, + @in_progress, + @completed + ).sort_and_filter @filtered_count = @submissions.unscope(:group).distinct.count(:id) @submissions = paginate_submissions(@submissions) - respond_to do |format| - format.html do - if params[:partial] - render partial: 'submissions_table_rows', - locals: { submissions: @submissions }, - formats: [:html] - else - render :submissions - end - end - end + render_response end private @@ -43,12 +36,6 @@ def set_phase @challenge = @phase.challenge end - def include_evaluator_associations - return unless @submissions.any? && evaluator_assignments? - - @submissions = @submissions.includes(evaluator_submission_assignments: :evaluator) - end - def evaluator_assignments? EvaluatorSubmissionAssignment.exists?(submission_id: @submissions.select(:id)) end @@ -74,65 +61,23 @@ def set_submission_statuses } end - def apply_filters - filter_by_eligibility - filter_by_status - end - - def filter_by_eligibility - return unless params[:eligible_for_evaluation] == 'true' || - params[:selected_to_advance] == 'true' - - @submissions = apply_eligibility_filter(@submissions) - end - - def filter_by_status - return unless params[:status] - - @submissions = apply_status_filter(@submissions) - end - - def apply_status_filter(submissions) - case params[:status] - when 'not_started' then @not_started - when 'in_progress' then @in_progress - when 'completed' then @completed - when 'recused' then filter_recused_submissions - else submissions - end - end - - def filter_recused_submissions - @submissions.joins(:evaluator_submission_assignments). - where(evaluator_submission_assignments: { status: :recused }) - end - - def apply_eligibility_filter(submissions) - if params[:selected_to_advance] == 'true' - submissions.where(judging_status: %w[winner]) - elsif params[:eligible_for_evaluation] == 'true' - submissions.where(judging_status: %w[selected winner]) - else - submissions - end - end - - def apply_sorting - case params[:sort] - when 'average_score_high_to_low' - @submissions = @submissions.order_by_average_score(:desc) - when 'average_score_low_to_high' - @submissions = @submissions.order_by_average_score(:asc) - when 'submission_id_high_to_low' - @submissions = @submissions.order(id: :desc) - when 'submission_id_low_to_high' - @submissions = @submissions.order(id: :asc) - end - end - def paginate_submissions(submissions) page = (params[:page] || 1).to_i per_page = 20 submissions.offset((page - 1) * per_page).limit(per_page) end + + def render_response + respond_to do |format| + format.html do + if params[:partial] + render partial: 'submissions_table_rows', + locals: { submissions: @submissions }, + formats: [:html] + else + render :submissions + end + end + end + end end diff --git a/app/services/sort_and_filter_service.rb b/app/services/sort_and_filter_service.rb new file mode 100644 index 00000000..b97c60c6 --- /dev/null +++ b/app/services/sort_and_filter_service.rb @@ -0,0 +1,76 @@ +# frozen_string_literal: true + +# This service handles sort and filtering submissions. +class SortAndFilterService + def initialize(submissions, params, not_started, in_progress, completed) + @submissions = submissions + @params = params + @not_started = not_started + @in_progress = in_progress + @completed = completed + end + + def sort_and_filter + apply_filters + apply_sorting + @submissions + end + + private + + def apply_filters + filter_by_eligibility + filter_by_status + end + + def filter_by_eligibility + return unless @params[:eligible_for_evaluation] == 'true' || + @params[:selected_to_advance] == 'true' + + @submissions = apply_eligibility_filter(@submissions) + end + + def filter_by_status + return unless @params[:status] + + @submissions = apply_status_filter(@submissions) + end + + def apply_status_filter(submissions) + case @params[:status] + when 'not_started' then @not_started + when 'in_progress' then @in_progress + when 'completed' then @completed + when 'recused' then filter_recused_submissions + else submissions + end + end + + def filter_recused_submissions + @submissions.joins(:evaluator_submission_assignments). + where(evaluator_submission_assignments: { status: :recused }) + end + + def apply_eligibility_filter(submissions) + if @params[:selected_to_advance] == 'true' + submissions.where(judging_status: %w[winner]) + elsif @params[:eligible_for_evaluation] == 'true' + submissions.where(judging_status: %w[selected winner]) + else + submissions + end + end + + def apply_sorting + case @params[:sort] + when 'average_score_high_to_low' + @submissions = @submissions.order_by_average_score(:desc) + when 'average_score_low_to_high' + @submissions = @submissions.order_by_average_score(:asc) + when 'submission_id_high_to_low' + @submissions = @submissions.order(id: :desc) + when 'submission_id_low_to_high' + @submissions = @submissions.order(id: :asc) + end + end +end From b4a06eef76780dea001c3ace1361345364ccaaf0 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 13:49:25 -0600 Subject: [PATCH 37/42] 291 | Add option to skip bullet warnings on RSpec tests --- spec/rails_helper.rb | 8 ++++++++ spec/requests/submissions_spec.rb | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 5dadca69..4f9a7d5c 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -84,4 +84,12 @@ config.before(:suite) do FactoryBot.reload end + + config.before(:each, bullet: :skip) do + Bullet.enable = false + end + + config.after(:each, bullet: :skip) do + Bullet.enable = true + end end diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index 2b35c506..ec95605e 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -245,7 +245,7 @@ expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") end - it 'shows only completed submissions' do + it 'shows only completed submissions', bullet: :skip do get submissions_phase_path(phase), params: { status: 'completed' } expect(response.body).to have_css("[data-submission-id='#{completed_submission.id}']") @@ -257,7 +257,7 @@ end context 'when filtering by eligibility' do - it 'displays only eligible for evaluation submissions' do + it 'displays only eligible for evaluation submissions', bullet: :skip do get submissions_phase_path(phase), params: { eligible_for_evaluation: 'true' } expect(response.body).to have_css("[data-submission-id='#{eligible_submission.id}']") @@ -267,7 +267,7 @@ expect(response.body).to have_no_css("[data-submission-id='#{completed_submission.id}']") end - it 'displays only selected to advance submissions' do + it 'displays only selected to advance submissions', bullet: :skip do get submissions_phase_path(phase), params: { selected_to_advance: 'true' } expect(response.body).to have_css("[data-submission-id='#{selected_submission.id}']") From 8515494ecb9c52165f8e5234e18719f39eb7b9f4 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 13:57:15 -0600 Subject: [PATCH 38/42] 291 | Reduce arguments initialized in sort and filter service --- app/controllers/phases_controller.rb | 8 +++++--- app/helpers/evaluations_helper.rb | 3 ++- app/services/sort_and_filter_service.rb | 8 ++++---- 3 files changed, 11 insertions(+), 8 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 1bd78aa4..b30f5214 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -18,9 +18,11 @@ def submissions @submissions = SortAndFilterService.new( @submissions, params, - @not_started, - @in_progress, - @completed + { + not_started: @not_started, + in_progress: @in_progress, + completed: @completed + } ).sort_and_filter @filtered_count = @submissions.unscope(:group).distinct.count(:id) diff --git a/app/helpers/evaluations_helper.rb b/app/helpers/evaluations_helper.rb index c18ed068..48ad2989 100644 --- a/app/helpers/evaluations_helper.rb +++ b/app/helpers/evaluations_helper.rb @@ -4,7 +4,8 @@ module EvaluationsHelper Score = Struct.new(:raw_score, :formatted_score, :display_score) - def evaluator_score(assignment) # individual evaluator score + # individual evaluator score + def evaluator_score(assignment) score = display_score(assignment) return Score.new(0, "0", "N/A") if score == 'N/A' diff --git a/app/services/sort_and_filter_service.rb b/app/services/sort_and_filter_service.rb index b97c60c6..a585fcc3 100644 --- a/app/services/sort_and_filter_service.rb +++ b/app/services/sort_and_filter_service.rb @@ -2,12 +2,12 @@ # This service handles sort and filtering submissions. class SortAndFilterService - def initialize(submissions, params, not_started, in_progress, completed) + def initialize(submissions, params, submission_statuses = {}) @submissions = submissions @params = params - @not_started = not_started - @in_progress = in_progress - @completed = completed + @not_started = submission_statuses[:not_started] + @in_progress = submission_statuses[:in_progress] + @completed = submission_statuses[:completed] end def sort_and_filter From eded377f8a22468311b9a74ddd8511e828d1d310 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 17:17:10 -0600 Subject: [PATCH 39/42] 291 | Add more specific name for submissions sort and filter service --- app/controllers/phases_controller.rb | 2 +- ...filter_service.rb => submissions_sort_and_filter_service.rb} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename app/services/{sort_and_filter_service.rb => submissions_sort_and_filter_service.rb} (98%) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index b30f5214..368d30f4 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -15,7 +15,7 @@ def submissions set_submission_counts set_submission_statuses - @submissions = SortAndFilterService.new( + @submissions = SubmissionsSortAndFilterService.new( @submissions, params, { diff --git a/app/services/sort_and_filter_service.rb b/app/services/submissions_sort_and_filter_service.rb similarity index 98% rename from app/services/sort_and_filter_service.rb rename to app/services/submissions_sort_and_filter_service.rb index a585fcc3..2b970204 100644 --- a/app/services/sort_and_filter_service.rb +++ b/app/services/submissions_sort_and_filter_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # This service handles sort and filtering submissions. -class SortAndFilterService +class SubmissionsSortAndFilterService def initialize(submissions, params, submission_statuses = {}) @submissions = submissions @params = params From cc430c26b73d12b98ef34bbcec03d9de3beb2cab Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 17:30:53 -0600 Subject: [PATCH 40/42] 291 | Only calculate avg score with assigned and completed evaluation scores --- app/models/submission.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/models/submission.rb b/app/models/submission.rb index 24ef658f..9259d042 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -88,7 +88,11 @@ def eligible_for_evaluation? end def average_score - avg = evaluations.average(:total_score) + avg = evaluations.joins(:evaluator_submission_assignment). + where(evaluator_submission_assignments: { status: :assigned }). + where.not(completed_at: nil). + average(:total_score) + avg ? avg.round : 0 end From c80157fae71b6b4c6f5bb8d5a50611428f4b4120 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 7 Jan 2025 17:41:33 -0600 Subject: [PATCH 41/42] 291 | Skip bullet warning on rspec avg score sort --- spec/requests/submissions_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index ec95605e..96a6ee85 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -340,7 +340,7 @@ end end - it 'paginates correctly when sorted by score' do + it 'paginates correctly when sorted by score', bullet: :skip do get submissions_phase_path(phase, page: 1, sort: 'average_score_high_to_low') expect(response).to have_http_status(:success) first_page_scores = response.body.scan(/data-score="(\d+)"/).flatten From 3fbe1d0498224957ff42216ec1e99f56bc06bbdb Mon Sep 17 00:00:00 2001 From: Stephen Chudleigh Date: Tue, 7 Jan 2025 15:57:49 -0800 Subject: [PATCH 42/42] add alert text expectation --- spec/system/submission_details_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/system/submission_details_spec.rb b/spec/system/submission_details_spec.rb index 119057a1..c58a9e16 100644 --- a/spec/system/submission_details_spec.rb +++ b/spec/system/submission_details_spec.rb @@ -57,6 +57,7 @@ visit submission_path(submission) fill_in "Comments and notes:", with: fake_comments click_on "Save" + expect(page).to have_css("p.usa-alert__text", text: "Submission was updated successfully.") assert_text(fake_comments) end end