From b4638a4e873132624e485ee5eeec59008a83c226 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 5 Dec 2024 17:22:33 -0600 Subject: [PATCH 01/23] 207 | Add logic to apply filters for statuses and apply sorting for avg score and id --- app/controllers/phases_controller.rb | 50 +++++++++++++++++++ app/models/evaluator_submission_assignment.rb | 9 +++- app/models/submission.rb | 20 ++++++++ 3 files changed, 78 insertions(+), 1 deletion(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index e9015b80..9246e57e 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -11,6 +11,24 @@ def index def submissions @submissions = @phase.submissions + + @not_started = @submissions.left_outer_joins(:evaluations) + .where(evaluations: { id: nil }) + + @in_progress = @submissions.joins(:evaluations) + .where(evaluations: { completed_at: nil }) + + @completed = @submissions.joins(:evaluations) + .where.not(evaluations: { completed_at: nil }) + + @submissions_by_status = { + not_started: @not_started.count, + in_progress: @in_progress.count, + completed: @completed.count + } + + apply_filters + apply_sorting end private @@ -19,4 +37,36 @@ def set_phase @phase = Phase.where(challenge: current_user.challenge_manager_challenges).find(params[:id]) @challenge = @phase.challenge end + + private + + def apply_filters + case params[:status] + when 'not_started' + @submissions = @not_started + when 'in_progress' + @submissions = @in_progress + when 'completed' + @submissions = @completed + when 'recused' + @submissions = @submissions.joins(:evaluator_submission_assignments). + where(evaluator_submission_assignments: { status: :recused }) + end + + @submissions = @submissions.select(&:eligible_for_evaluation?) if params[:eligible_for_evaluation] == 'true' + @submissions = @submissions.select(&:selected_to_advance?) if params[:selected_to_advance] == 'true' + 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 diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index 3f246292..d8a2f9da 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -16,5 +16,12 @@ class EvaluatorSubmissionAssignment < ApplicationRecord belongs_to :evaluator, class_name: "User", foreign_key: :user_id, inverse_of: :assigned_submissions has_one :evaluation, dependent: :destroy - enum :status, { assigned: 0, unassigned: 1, recused: 2 } + has_one :phase, through: :submission + + enum :status, { + assigned: 0, + unassigned: 1, + recused: 2, + recused_unassigned: 3 + } end diff --git a/app/models/submission.rb b/app/models/submission.rb index 6bd3c140..d0050510 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, through: :evaluator_submission_assignments # Fields attribute :title, :string @@ -67,4 +68,23 @@ def eligible_for_evaluation? def selected_to_advance? winner? end + + def average_score + avg = evaluations.average(:total_score) + score = avg ? avg.round : 0 + [score, "#{score}%"] + end + + def self.order_by_average_score(direction) + direction_sql = direction == :desc ? 'DESC' : 'ASC' + + joins("LEFT JOIN evaluations ON evaluations.submission_id = submissions.id") + .group('submissions.id') + .order( + Arel.sql( + "COALESCE(ROUND(AVG(evaluations.total_score)), 0) #{direction_sql}, " \ + "submissions.id #{direction_sql}" + ) + ) + end end From 558a93ff61d1e249e07ba348a530878f5d7cda79 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 5 Dec 2024 17:22:47 -0600 Subject: [PATCH 02/23] 207 | Add js for sorting and filtering --- app/javascript/controllers/index.js | 8 ++- .../sort_filter_menu_controller.js | 60 +++++++++++++++++++ 2 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 app/javascript/controllers/sort_filter_menu_controller.js diff --git a/app/javascript/controllers/index.js b/app/javascript/controllers/index.js index 076ccf60..574cdbdc 100644 --- a/app/javascript/controllers/index.js +++ b/app/javascript/controllers/index.js @@ -6,8 +6,10 @@ import { application } from "./application"; import EvaluationFormController from "./evaluation_form_controller"; import EvaluationCriteriaController from "./evaluation_criteria_controller"; +import SortFilterMenuController from "./sort_filter_menu_controller"; +import DeleteEvaluatorModalController from "./delete_evaluator_modal_controller" + application.register("evaluation-form", EvaluationFormController); application.register("evaluation-criteria", EvaluationCriteriaController); - -import DeleteEvaluatorModalController from "./delete_evaluator_modal_controller" -application.register("delete-evaluator-modal", DeleteEvaluatorModalController) +application.register("delete-evaluator-modal", DeleteEvaluatorModalController); +application.register("sort-filter-menu", SortFilterMenuController); diff --git a/app/javascript/controllers/sort_filter_menu_controller.js b/app/javascript/controllers/sort_filter_menu_controller.js new file mode 100644 index 00000000..84194587 --- /dev/null +++ b/app/javascript/controllers/sort_filter_menu_controller.js @@ -0,0 +1,60 @@ +// app/javascript/controllers/sort_filter_menu_controller.js +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["menuItem"] + + connect() { + this.updateCheckmarks() + } + + toggle(event) { + event.preventDefault() + const clickedItem = event.currentTarget + const filterValue = clickedItem.dataset.filterValue + + const currentUrl = new URL(window.location.href) + const [param, value] = filterValue.split('=') + + if (currentUrl.searchParams.get(param) === value) { + this.clearAllFilters() + } else { + this.applyFilter(filterValue) + } + } + + applyFilter(filterValue) { + const currentUrl = new URL(window.location.href) + const [param, value] = filterValue.split('=') + + this.removeExistingParams(currentUrl) + currentUrl.searchParams.set(param, value) + window.location.href = currentUrl.toString() + } + + clearAllFilters() { + const currentUrl = new URL(window.location.href) + this.removeExistingParams(currentUrl) + window.location.href = currentUrl.toString() + } + + removeExistingParams(url) { + const paramsToRemove = ['status', 'eligible_for_evaluation', 'selected_to_advance', 'sort'] + paramsToRemove.forEach(key => { + url.searchParams.delete(key) + }) + } + + updateCheckmarks() { + const currentUrl = new URL(window.location.href) + this.menuItemTargets.forEach(item => { + const checkmark = item.querySelector('.checkmark') + const [param, value] = item.dataset.filterValue.split('=') + if (currentUrl.searchParams.get(param) === value) { + checkmark.style.display = 'inline' + } else { + checkmark.style.display = 'none' + } + }) + } +} From eba1bde92519641ece66e196df51c3c90f835ca1 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 5 Dec 2024 17:23:00 -0600 Subject: [PATCH 03/23] 207 | Sort & Filter menu UI --- app/views/phases/_sort_and_filter.html.erb | 59 ++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 app/views/phases/_sort_and_filter.html.erb diff --git a/app/views/phases/_sort_and_filter.html.erb b/app/views/phases/_sort_and_filter.html.erb new file mode 100644 index 00000000..c3a63e4d --- /dev/null +++ b/app/views/phases/_sort_and_filter.html.erb @@ -0,0 +1,59 @@ +
+
+
+ +
+
+
\ No newline at end of file From bb23ee392e84023b9c6161a8020e73b459c61b09 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 5 Dec 2024 17:23:41 -0600 Subject: [PATCH 04/23] 207 | Add call to sort and filter menu --- app/views/phases/_submissions_table.html.erb | 30 +++++++++++--------- app/views/phases/submissions.html.erb | 10 +++---- app/views/shared/_alert_error.html.erb | 2 +- 3 files changed, 23 insertions(+), 19 deletions(-) diff --git a/app/views/phases/_submissions_table.html.erb b/app/views/phases/_submissions_table.html.erb index 8d6e7640..ad005c1f 100644 --- a/app/views/phases/_submissions_table.html.erb +++ b/app/views/phases/_submissions_table.html.erb @@ -6,7 +6,9 @@ <%= render 'shared/alert_error', alert_heading: t('alerts.recused_evaluator.heading'), alert_text: t('alerts.recused_evaluator.text') %> <% end %> - +<%= render partial: "phases/sort_and_filter" %> + +
@@ -14,26 +16,27 @@ + <% @submissions.each do |submission| %> - <% @submissions.each do |submission| %> - + diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index b00af395..052f9fd1 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -149,33 +149,33 @@ [not_started_submission, in_progress_submission, completed_submission, eligible_submission, selected_submission].each do |submission| - expect(response.body).to include(submission.id.to_s) + expect(response.body).to have_css("[data-submission-id='#{submission.id}']") end - expect(response.body).to include('text-secondary-dark text-bold">2<') # not_started, eligible - expect(response.body).to include('text-orange text-bold">1<') # in_progress - expect(response.body).to include('text-green text-bold">2<') # completed, selected + 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 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 include(not_started_submission.id.to_s) - expect(response.body).to include(eligible_submission.id.to_s) - expect(response.body).not_to include(selected_submission.id.to_s) - expect(response.body).not_to include(in_progress_submission.id.to_s) - expect(response.body).not_to include(completed_submission.id.to_s) + 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}']") end it 'shows only completed submissions' do get submissions_phase_path(phase), params: { status: 'completed' } - expect(response.body).to include(completed_submission.id.to_s) - expect(response.body).to include(selected_submission.id.to_s) - expect(response.body).not_to include(not_started_submission.id.to_s) - expect(response.body).not_to include(in_progress_submission.id.to_s) - expect(response.body).not_to include(eligible_submission.id.to_s) + 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}']") end end @@ -183,21 +183,21 @@ it 'displays only eligible for evaluation submissions' do get submissions_phase_path(phase), params: { eligible_for_evaluation: 'true' } - expect(response.body).to include(eligible_submission.id.to_s) - expect(response.body).to include(selected_submission.id.to_s) - expect(response.body).not_to include(not_started_submission.id.to_s) - expect(response.body).not_to include(in_progress_submission.id.to_s) - expect(response.body).not_to include(completed_submission.id.to_s) + 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}']") end it 'displays only selected to advance submissions' do get submissions_phase_path(phase), params: { selected_to_advance: 'true' } - expect(response.body).to include(selected_submission.id.to_s) - expect(response.body).not_to include(eligible_submission.id.to_s) - expect(response.body).not_to include(not_started_submission.id.to_s) - expect(response.body).not_to include(in_progress_submission.id.to_s) - expect(response.body).not_to include(completed_submission.id.to_s) + 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}']") end end @@ -217,21 +217,19 @@ it 'orders submissions by score high to low' do get submissions_phase_path(phase), params: { sort: 'average_score_high_to_low' } - response_body = response.body - high_score_index = response_body.index(completed_submission.id.to_s) - low_score_index = response_body.index(in_progress_submission.id.to_s) - - expect(high_score_index).to be < low_score_index + expect(response.body).to have_selector( + "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' } - response_body = response.body - high_score_index = response_body.index(completed_submission.id.to_s) - low_score_index = response_body.index(in_progress_submission.id.to_s) - - expect(low_score_index).to be < high_score_index + expect(response.body).to have_selector( + "tr[data-submission-id='#{in_progress_submission.id}']" \ + " ~ tr[data-submission-id='#{completed_submission.id}']" + ) end end end From a6e6642d85a57655167987d9ad85713ef746fef1 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 26 Dec 2024 13:11:16 -0600 Subject: [PATCH 18/23] 207 | Fix mobile UI --- app/assets/stylesheets/application.sass.scss | 54 ++++++++++++-------- app/views/phases/_sort_and_filter.html.erb | 22 +++++--- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/app/assets/stylesheets/application.sass.scss b/app/assets/stylesheets/application.sass.scss index 40e943b6..a7e383df 100644 --- a/app/assets/stylesheets/application.sass.scss +++ b/app/assets/stylesheets/application.sass.scss @@ -22,30 +22,40 @@ dialog::backdrop { background-color: #4d8055; } -.usa-accordion__button.usa-nav__link.usa-current { - &:hover { - color: inherit; - outline: none; - } +.usa-nav__primary { + button.usa-accordion__button { + &::before, + &::after, + span::after { + display: none !important; + content: none !important; + -webkit-mask: none !important; + mask: none !important; + } - &::after { - display: none; - } -} + .usa-icon { + width: 1.5rem; + height: 1.5rem; + fill: #005ea2; + vertical-align: middle; + position: relative; + top: -2px; -.usa-nav__primary { - button[aria-expanded="false"], - button[aria-expanded="true"] { - span { - padding-right: 2rem; - padding-bottom: 4px; - font-weight: 700; - - &::after { - right: 12px; - background-color: #005ea2 !important; - display: block !important; - position: absolute; + @media (min-width: 64em) { + margin-left: -16px; + } + } + + .icon-up { + display: none; + } + + &[aria-expanded="true"] { + .icon-down { + display: none; + } + .icon-up { + display: inline-block; } } } diff --git a/app/views/phases/_sort_and_filter.html.erb b/app/views/phases/_sort_and_filter.html.erb index d1f790a0..781d2cb1 100644 --- a/app/views/phases/_sort_and_filter.html.erb +++ b/app/views/phases/_sort_and_filter.html.erb @@ -6,14 +6,22 @@
  • -
      <% menu_items = [ { label: "Evaluation progress: Completed", value: "status=completed" }, @@ -29,7 +37,7 @@ ] %> <% menu_items.each do |item| %>
    • - @@ -37,7 +45,7 @@ width: 18, height: 18, alt: "Selected", - class: "icon-white checkmark margin-left-neg-1 padding-top-05", + class: "icon-white checkmark margin-left-neg-3 padding-top-2px tablet:padding-top-05 tablet:margin-left-neg-1", style: "display: none;" %> <%= item[:label] %> From 21cb976c33a53f4063bcdf6daa8572b04fe34595 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 3 Jan 2025 17:10:22 -0600 Subject: [PATCH 19/23] 207 | Override low contrast on hover in mobile --- app/assets/stylesheets/application.sass.scss | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/app/assets/stylesheets/application.sass.scss b/app/assets/stylesheets/application.sass.scss index a7e383df..ec3dc4db 100644 --- a/app/assets/stylesheets/application.sass.scss +++ b/app/assets/stylesheets/application.sass.scss @@ -59,6 +59,13 @@ dialog::backdrop { } } } + + @media (max-width: 64em) { + a:not(.usa-button):hover { + background-color: #005ea2; + color: white; + } + } } .usa-combo-box .border-secondary + input { From 38b98603bef4e341f83b20f274426b3a0341b84c Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 3 Jan 2025 17:12:59 -0600 Subject: [PATCH 20/23] 207 | Remove :through association and scope complete submissions --- app/controllers/phases_controller.rb | 8 ++++---- app/models/submission.rb | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 881c98a5..c0a950af 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -34,11 +34,11 @@ def set_submission_counts def set_submission_statuses @not_started = @submissions.where.missing(:evaluations) - @in_progress = @submissions.joins(:evaluations). + @in_progress = @submissions.joins(evaluator_submission_assignments: :evaluation). where(evaluations: { completed_at: nil }).distinct - @completed = @submissions.joins(:evaluations). - where.not(evaluations: { completed_at: nil }).distinct - + @completed = @submissions.joins(evaluator_submission_assignments: :evaluation). + where.not(evaluations: { completed_at: nil }). + where.not(id: @in_progress.select(:id)).distinct @submissions_by_status = { not_started: @not_started.count, in_progress: @in_progress.count, diff --git a/app/models/submission.rb b/app/models/submission.rb index 01d3ef94..1dcdaf94 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -36,7 +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, through: :evaluator_submission_assignments, dependent: :destroy + has_many :evaluations, dependent: :destroy # Fields attribute :title, :string @@ -73,8 +73,7 @@ def selected_to_advance? def average_score avg = evaluations.average(:total_score) - score = avg ? avg.round : 0 - [score, score.to_s] + avg ? avg.round : 0 end def self.order_by_average_score(direction) From 5cb3650b11dbdaa92561ffc58f34555e14e8fa48 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 3 Jan 2025 17:15:33 -0600 Subject: [PATCH 21/23] 207 | Simplify scoring format --- app/views/phases/_submissions_table.html.erb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/views/phases/_submissions_table.html.erb b/app/views/phases/_submissions_table.html.erb index 312bde4c..471d36ea 100644 --- a/app/views/phases/_submissions_table.html.erb +++ b/app/views/phases/_submissions_table.html.erb @@ -78,9 +78,8 @@
    • - <% score, formatted_score = submission.average_score %> -
    Submission IDSelected to Advance Assigned Evaluators Average ScoreView Submission
    - Submission ID <%= submission.id %> + 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 -
    + +
    + <%= image_tag( + "images/usa-icons/verified.svg", + class: "usa-icon--size-3 margin-right-1", + alt: "" + )%> + Eligible for Evaluation +
    <% else %>
    @@ -75,8 +78,9 @@
    		No evaluators assigned to this submission - N/A + <% score, formatted_score = submission.average_score %> + + <%= formatted_score %>
    diff --git a/app/views/phases/submissions.html.erb b/app/views/phases/submissions.html.erb index 16ab4729..84f07320 100644 --- a/app/views/phases/submissions.html.erb +++ b/app/views/phases/submissions.html.erb @@ -1,10 +1,10 @@

    <%= challenge_phase_title(@phase.challenge, @phase) %>

    View challenge submissions and manage evaluation progress.

    +<%= render partial: "submissions_table", locals: { submissions: @submissions } %> + <% if @submissions.empty? %> -
    -

    This challenge phase does not currently have any submissions.

    +
    +

    No submissions found.

    -<% else %> - <%= render partial: "submissions_table", locals: { submissions: @submissions } %> -<% end %> \ No newline at end of file +<% end %> diff --git a/app/views/shared/_alert_error.html.erb b/app/views/shared/_alert_error.html.erb index dce010d2..79653aea 100644 --- a/app/views/shared/_alert_error.html.erb +++ b/app/views/shared/_alert_error.html.erb @@ -2,7 +2,7 @@
    - Submission ID<%= submission.id %> + Submission ID <%= submission.id %>
    From 61b8f97a8fdffa810561b489d8660dea2e0f5f50 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 19 Dec 2024 15:12:09 -0600 Subject: [PATCH 11/23] 207 | Exclude incomplete evaluations from average scoring --- app/models/submission.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/submission.rb b/app/models/submission.rb index c437bd07..67c5c286 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -74,13 +74,13 @@ def selected_to_advance? def average_score avg = evaluations.average(:total_score) score = avg ? avg.round : 0 - [score, "#{score}%"] + [score, "#{score}"] end def self.order_by_average_score(direction) direction_sql = direction == :desc ? 'DESC' : 'ASC' - joins("LEFT JOIN evaluations ON evaluations.submission_id = submissions.id"). + joins("LEFT JOIN evaluations ON evaluations.submission_id = submissions.id AND evaluations.completed_at IS NOT NULL"). group('submissions.id'). order( Arel.sql( From cb0fc62cb6a5db08f5398d239e2b8c5887ed8263 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 19 Dec 2024 15:12:44 -0600 Subject: [PATCH 12/23] 207 | Update sort and filter tests --- spec/requests/phases_spec.rb | 107 ------------------------- spec/requests/submissions_spec.rb | 125 ++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 107 deletions(-) diff --git a/spec/requests/phases_spec.rb b/spec/requests/phases_spec.rb index 248c9693..0ad59618 100644 --- a/spec/requests/phases_spec.rb +++ b/spec/requests/phases_spec.rb @@ -81,111 +81,4 @@ end end end - - describe 'GET /phases/:id/submissions' do - let(:challenge_user) { create_user(role: "challenge_manager") } - let(:challenge) { create(:challenge, user: challenge_user) } - let(:phase) { create(:phase, challenge: challenge) } - let!(:submission1) { create(:submission, challenge: challenge, phase: phase, status: 'submitted') } - let!(:submission2) { create(:submission, challenge: challenge, phase: phase, status: 'submitted') } - let!(:submission3) { create(:submission, challenge: challenge, phase: phase, status: 'submitted', judging_status: 'selected') } - let!(:submission4) { create(:submission, challenge: challenge, phase: phase, status: 'submitted', judging_status: 'winner') } - - before { log_in_user(challenge_user) } - - it 'assigns submissions and calculates status counts' do - get submissions_phase_path(phase) - - expect(assigns(:submissions)).to match_array([submission1, submission2, submission3, submission4]) - expect(assigns(:not_started)).to match_array([submission1, submission2, submission3, submission4]) - expect(assigns(:in_progress)).to be_empty - expect(assigns(:completed)).to be_empty - expect(assigns(:submissions_by_status)).to eq({ - not_started: 4, - in_progress: 0, - completed: 0 - }) - end - - context 'when filtering by status' do - let(:assignment1) { create(:evaluator_submission_assignment, submission: submission1) } - let(:assignment2) { create(:evaluator_submission_assignment, submission: submission2) } - - before(:each) do - # Create evaluations - create(:evaluation, evaluator_submission_assignment: assignment1, completed_at: Time.current) - create(:evaluation, evaluator_submission_assignment: assignment2, completed_at: nil) - - # Ensure assignment2 is NOT recused - assignment2.update!(status: :assigned) - # Explicitly set assignment1 as recused - assignment1.update!(status: :recused) - end - - it 'filters not_started submissions' do - get submissions_phase_path(phase), params: { status: 'not_started' } - expect(assigns(:submissions)).to match_array([submission3, submission4]) - end - - it 'filters in_progress submissions' do - get submissions_phase_path(phase), params: { status: 'in_progress' } - expect(assigns(:submissions)).to match_array([submission2]) - end - - it 'filters completed submissions' do - get submissions_phase_path(phase), params: { status: 'completed' } - expect(assigns(:submissions)).to match_array([submission1]) - end - - it 'filters recused submissions' do - expect(assignment1.reload.status).to eq('recused') - expect(assignment2.reload.status).to eq('assigned') - - get submissions_phase_path(phase), params: { status: 'recused' } - expect(assigns(:submissions)).to match_array([submission1]) - end - end - - context 'when filtering by eligibility' do - it 'filters eligible for evaluation submissions' do - get submissions_phase_path(phase), params: { eligible_for_evaluation: 'true' } - expect(assigns(:submissions)).to match_array([submission3, submission4]) - end - - it 'filters selected to advance submissions' do - get submissions_phase_path(phase), params: { selected_to_advance: 'true' } - expect(assigns(:submissions)).to match_array([submission4]) - end - end - - context 'when sorting submissions' do - let(:assignment1) { create(:evaluator_submission_assignment, submission: submission1) } - let(:assignment2) { create(:evaluator_submission_assignment, submission: submission2) } - - before(:each) do - create(:evaluation, evaluator_submission_assignment: assignment1, total_score: 90) - create(:evaluation, evaluator_submission_assignment: assignment2, total_score: 85) - end - - it 'sorts by average score high to low' do - get submissions_phase_path(phase), params: { sort: 'average_score_high_to_low' } - expect(assigns(:submissions).to_a).to eq([submission4, submission3, submission2, submission1]) - end - - it 'sorts by average score low to high' do - get submissions_phase_path(phase), params: { sort: 'average_score_low_to_high' } - expect(assigns(:submissions).to_a).to eq([submission1, submission2, submission3, submission4]) - end - - it 'sorts by submission id high to low' do - get submissions_phase_path(phase), params: { sort: 'submission_id_high_to_low' } - expect(assigns(:submissions).to_a).to eq([submission4, submission3, submission2, submission1]) - end - - it 'sorts by submission id low to high' do - get submissions_phase_path(phase), params: { sort: 'submission_id_low_to_high' } - expect(assigns(:submissions).to_a).to eq([submission1, submission2, submission3, submission4]) - end - end - end end diff --git a/spec/requests/submissions_spec.rb b/spec/requests/submissions_spec.rb index ec3fdf60..b00af395 100644 --- a/spec/requests/submissions_spec.rb +++ b/spec/requests/submissions_spec.rb @@ -111,4 +111,129 @@ end 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) } + + before do + ChallengeManager.create!(user: user, challenge: challenge) + log_in_user(user) + end + + 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) { 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 + + it 'displays all submissions and their status counts' do + get submissions_phase_path(phase) + + [not_started_submission, in_progress_submission, completed_submission, + eligible_submission, selected_submission].each do |submission| + expect(response.body).to include(submission.id.to_s) + end + + expect(response.body).to include('text-secondary-dark text-bold">2<') # not_started, eligible + expect(response.body).to include('text-orange text-bold">1<') # in_progress + expect(response.body).to include('text-green text-bold">2<') # completed, selected + end + + 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 include(not_started_submission.id.to_s) + expect(response.body).to include(eligible_submission.id.to_s) + expect(response.body).not_to include(selected_submission.id.to_s) + expect(response.body).not_to include(in_progress_submission.id.to_s) + expect(response.body).not_to include(completed_submission.id.to_s) + end + + it 'shows only completed submissions' do + get submissions_phase_path(phase), params: { status: 'completed' } + + expect(response.body).to include(completed_submission.id.to_s) + expect(response.body).to include(selected_submission.id.to_s) + expect(response.body).not_to include(not_started_submission.id.to_s) + expect(response.body).not_to include(in_progress_submission.id.to_s) + expect(response.body).not_to include(eligible_submission.id.to_s) + 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 include(eligible_submission.id.to_s) + expect(response.body).to include(selected_submission.id.to_s) + expect(response.body).not_to include(not_started_submission.id.to_s) + expect(response.body).not_to include(in_progress_submission.id.to_s) + expect(response.body).not_to include(completed_submission.id.to_s) + end + + it 'displays only selected to advance submissions' do + get submissions_phase_path(phase), params: { selected_to_advance: 'true' } + + expect(response.body).to include(selected_submission.id.to_s) + expect(response.body).not_to include(eligible_submission.id.to_s) + expect(response.body).not_to include(not_started_submission.id.to_s) + expect(response.body).not_to include(in_progress_submission.id.to_s) + expect(response.body).not_to include(completed_submission.id.to_s) + 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 + ) + end + + it 'orders submissions by score high to low' do + get submissions_phase_path(phase), params: { sort: 'average_score_high_to_low' } + + response_body = response.body + high_score_index = response_body.index(completed_submission.id.to_s) + low_score_index = response_body.index(in_progress_submission.id.to_s) + + expect(high_score_index).to be < low_score_index + end + + it 'orders submissions by score low to high' do + get submissions_phase_path(phase), params: { sort: 'average_score_low_to_high' } + + response_body = response.body + high_score_index = response_body.index(completed_submission.id.to_s) + low_score_index = response_body.index(in_progress_submission.id.to_s) + + expect(low_score_index).to be < high_score_index + end + end + end + end end From fcda9ab133c4cef1f9b72b83146d16d5f3eed94d Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 19 Dec 2024 15:34:47 -0600 Subject: [PATCH 13/23] 207 | Split out assignments --- app/controllers/phases_controller.rb | 36 ++++++++++++++++++---------- app/models/submission.rb | 7 ++++-- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 49849620..4860a3d6 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -12,10 +12,27 @@ def index def submissions @submissions = @phase.submissions + set_submission_counts + set_submission_statuses + + apply_filters + apply_sorting + end + + private + + def set_phase + @phase = Phase.where(challenge: current_user.challenge_manager_challenges).find(params[:id]) + @challenge = @phase.challenge + end + + def set_submission_counts @submissions_count = @submissions.count @eligible_count = @submissions.eligible_for_evaluation.count @selected_count = @submissions.winner.count + end + def set_submission_statuses @not_started = @submissions.where.missing(:evaluations) @in_progress = @submissions.joins(:evaluations). where(evaluations: { completed_at: nil }).distinct @@ -27,16 +44,6 @@ def submissions in_progress: @in_progress.count, completed: @completed.count } - - apply_filters - apply_sorting - end - - private - - def set_phase - @phase = Phase.where(challenge: current_user.challenge_manager_challenges).find(params[:id]) - @challenge = @phase.challenge end def apply_filters @@ -60,8 +67,13 @@ def filter_recused_submissions end def apply_eligibility_filters - @submissions = @submissions.where(judging_status: [:selected, :winner]) if params[:eligible_for_evaluation] == 'true' - @submissions = @submissions.where(judging_status: :winner) if params[:selected_to_advance] == 'true' + if params[:eligible_for_evaluation] == 'true' + @submissions = @submissions.where(judging_status: [:selected, :winner]) + end + + if params[:selected_to_advance] == 'true' + @submissions = @submissions.where(judging_status: :winner) + end end def apply_sorting diff --git a/app/models/submission.rb b/app/models/submission.rb index 67c5c286..01d3ef94 100644 --- a/app/models/submission.rb +++ b/app/models/submission.rb @@ -74,13 +74,16 @@ def selected_to_advance? def average_score avg = evaluations.average(:total_score) score = avg ? avg.round : 0 - [score, "#{score}"] + [score, score.to_s] 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"). + joins( + "LEFT JOIN evaluations ON evaluations.submission_id = submissions.id " \ + "AND evaluations.completed_at IS NOT NULL" + ). group('submissions.id'). order( Arel.sql( From 7f9e590cc8a0eae07b02558d1c3cd0a4c5f0fbdb Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 19 Dec 2024 16:04:23 -0600 Subject: [PATCH 14/23] 207 | Adjust eligibility filters --- app/controllers/phases_controller.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index 4860a3d6..e69d9a49 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -67,13 +67,13 @@ def filter_recused_submissions end def apply_eligibility_filters - if params[:eligible_for_evaluation] == 'true' - @submissions = @submissions.where(judging_status: [:selected, :winner]) - end + return unless params[:eligible_for_evaluation] == 'true' || params[:selected_to_advance] == 'true' - if params[:selected_to_advance] == 'true' - @submissions = @submissions.where(judging_status: :winner) - end + @submissions = if params[:selected_to_advance] == 'true' + @submissions.where(judging_status: 'winner') + else + @submissions.where(judging_status: ['selected', 'winner']) + end end def apply_sorting From 1fd1b7bfca8387eade88ac58ec3bc2dee72d09bf Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 19 Dec 2024 16:14:45 -0600 Subject: [PATCH 15/23] 207 | Combine status and eligibility to prevent error with order of operations --- app/controllers/phases_controller.rb | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index e69d9a49..c4e15212 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -47,18 +47,19 @@ def set_submission_statuses end def apply_filters - apply_status_filter if params[:status] - apply_eligibility_filters - end + if params[:eligible_for_evaluation] == 'true' || params[:selected_to_advance] == 'true' + apply_eligibility_filters + 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 + if params[:status] + @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 end def filter_recused_submissions @@ -67,8 +68,6 @@ def filter_recused_submissions end def apply_eligibility_filters - return unless params[:eligible_for_evaluation] == 'true' || params[:selected_to_advance] == 'true' - @submissions = if params[:selected_to_advance] == 'true' @submissions.where(judging_status: 'winner') else From 513f6fc6541c8c8a187467678a87603d5a8d228d Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 19 Dec 2024 16:45:53 -0600 Subject: [PATCH 16/23] 207 | Add guards --- app/controllers/phases_controller.rb | 46 ++++++++++++++++++---------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index c4e15212..e575dbaf 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -47,18 +47,30 @@ def set_submission_statuses end def apply_filters - if params[:eligible_for_evaluation] == 'true' || params[:selected_to_advance] == 'true' - apply_eligibility_filters - end + 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 - if params[:status] - @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 + 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 @@ -67,12 +79,12 @@ def filter_recused_submissions where(evaluator_submission_assignments: { status: :recused }) end - def apply_eligibility_filters - @submissions = if params[:selected_to_advance] == 'true' - @submissions.where(judging_status: 'winner') - else - @submissions.where(judging_status: ['selected', 'winner']) - end + def apply_eligibility_filter(submissions) + if params[:selected_to_advance] == 'true' + submissions.where(judging_status: %w[winner]) + else + submissions.where(judging_status: %w[selected winner]) + end end def apply_sorting From 8b49758373f45710e630a76ce2b9110f8d966e27 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 19 Dec 2024 17:26:13 -0600 Subject: [PATCH 17/23] 207 | Update tests to use data-submission-id --- app/controllers/phases_controller.rb | 4 +- app/views/phases/_submissions_table.html.erb | 2 +- spec/requests/submissions_spec.rb | 66 ++++++++++---------- 3 files changed, 36 insertions(+), 36 deletions(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index e575dbaf..881c98a5 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -82,8 +82,10 @@ def filter_recused_submissions def apply_eligibility_filter(submissions) if params[:selected_to_advance] == 'true' submissions.where(judging_status: %w[winner]) - else + elsif params[:eligible_for_evaluation] == 'true' submissions.where(judging_status: %w[selected winner]) + else + submissions end end diff --git a/app/views/phases/_submissions_table.html.erb b/app/views/phases/_submissions_table.html.erb index 71246078..312bde4c 100644 --- a/app/views/phases/_submissions_table.html.erb +++ b/app/views/phases/_submissions_table.html.erb @@ -21,7 +21,7 @@
    Submission ID <%= submission.id %> No evaluators assigned to this submission - <%= formatted_score %> + + <%= score %>
    From f330e8b51290bcf2b88c2ea2f442ca39c47fbe42 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 6 Jan 2025 12:06:54 -0600 Subject: [PATCH 22/23] 207 | Include submissions with not_started evaluations that are partially complete --- app/controllers/phases_controller.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/app/controllers/phases_controller.rb b/app/controllers/phases_controller.rb index c0a950af..36963339 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -33,7 +33,8 @@ def set_submission_counts end def set_submission_statuses - @not_started = @submissions.where.missing(:evaluations) + @not_started = @submissions.left_joins(evaluator_submission_assignments: :evaluation). + where('evaluations.id IS NULL').distinct @in_progress = @submissions.joins(evaluator_submission_assignments: :evaluation). where(evaluations: { completed_at: nil }).distinct @completed = @submissions.joins(evaluator_submission_assignments: :evaluation). From 81195b42feafa215261c8c4861e4e1a4b3406c32 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 6 Jan 2025 12:11:33 -0600 Subject: [PATCH 23/23] 207 | Adjust query --- 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 36963339..f1477d59 100644 --- a/app/controllers/phases_controller.rb +++ b/app/controllers/phases_controller.rb @@ -34,7 +34,7 @@ def set_submission_counts def set_submission_statuses @not_started = @submissions.left_joins(evaluator_submission_assignments: :evaluation). - where('evaluations.id IS NULL').distinct + where(evaluations: { id: nil }).distinct @in_progress = @submissions.joins(evaluator_submission_assignments: :evaluation). where(evaluations: { completed_at: nil }).distinct @completed = @submissions.joins(evaluator_submission_assignments: :evaluation).