From 7abaa150f5d9754610a082edab45974f846f8595 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 4 Nov 2024 13:54:27 -0600 Subject: [PATCH 01/38] 245 | Update invited evaluator user setup --- .../manage_evaluators_controller.rb | 19 +++-- app/models/user.rb | 2 +- .../manage_evaluators_controller_spec.rb | 69 ++++++++++++++----- spec/models/user_spec.rb | 6 +- 4 files changed, 69 insertions(+), 27 deletions(-) diff --git a/app/controllers/manage_evaluators_controller.rb b/app/controllers/manage_evaluators_controller.rb index 57d13a90..48254be7 100644 --- a/app/controllers/manage_evaluators_controller.rb +++ b/app/controllers/manage_evaluators_controller.rb @@ -78,13 +78,15 @@ def fetch_existing_evaluators # Create action helpers def process_evaluator_invitation(email) - existing_invitation = @challenge.evaluator_invitations.find_by(email:, phase: @phase) - user = User.find_by(email:) - - if existing_invitation + user = User.find_by(email: email) + existing_invitation = @challenge.evaluator_invitations.find_by(email: email, phase: @phase) + + if user + result = add_user_as_evaluator(user) + existing_invitation&.destroy if result[:success] + result + elsif existing_invitation resend_invitation(existing_invitation) - elsif user && valid_evaluator_role?(user) - add_user_as_evaluator(user) else create_new_invitation(email) end @@ -105,8 +107,11 @@ def valid_evaluator_role?(user) end def add_user_as_evaluator(user) - cpe = ChallengePhasesEvaluator.find_or_create_by(challenge: @challenge, phase: @phase, user:) + cpe = ChallengePhasesEvaluator.find_or_create_by(challenge: @challenge, phase: @phase, user: user) + invitation = @challenge.evaluator_invitations.find_by(email: user.email, phase: @phase) + if cpe.persisted? + invitation&.destroy { success: true, message: "#{user.email} has been added as an evaluator for this phase." } else { success: false, message: "Failed to add #{user.email} as an evaluator." } diff --git a/app/models/user.rb b/app/models/user.rb index db1b074e..37f1fa89 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -133,7 +133,7 @@ def self.default_role_and_status_for_email(email) if default_challenge_manager?(email) %w[challenge_manager pending] else - %w[solver active] + %w[evaluator pending] end end diff --git a/spec/controllers/manage_evaluators_controller_spec.rb b/spec/controllers/manage_evaluators_controller_spec.rb index 7f54578f..43855a7a 100644 --- a/spec/controllers/manage_evaluators_controller_spec.rb +++ b/spec/controllers/manage_evaluators_controller_spec.rb @@ -66,7 +66,7 @@ end context 'when inviting an existing user' do - let(:existing_user) { create(:user, role: 'evaluator') } + let(:existing_user) { create(:user, role: 'evaluator', status: 'pending') } it 'adds the user as an evaluator without creating a new invitation' do expect { @@ -84,27 +84,64 @@ end end - context 'when inviting a new user' do - let(:new_user_params) do - { - evaluator_invitation: { - email: 'new_evaluator@example.com', - phase_id: phase.id, - first_name: 'New', - last_name: 'Evaluator', - last_invite_sent: Time.current + context 'when inviting an existing user with an invitation' do + let(:existing_user) { create(:user, role: 'evaluator', status: 'pending') } + let!(:invitation) { create(:evaluator_invitation, challenge: challenge, phase: phase, email: existing_user.email) } + + it 'adds the user as an evaluator and deletes only the specific invitation' do + expect { + post challenge_manage_evaluators_path(challenge), params: { + evaluator_invitation: { + email: existing_user.email, + phase_id: phase.id + } } - } + }.to change(ChallengePhasesEvaluator, :count).by(1) + .and change(EvaluatorInvitation, :count).by(-1) + + expect(EvaluatorInvitation.find_by(id: invitation.id)).to be_nil + expect(response).to redirect_to(challenge_manage_evaluators_path(challenge, phase_id: phase.id)) + expect(flash[:notice]).to include("has been added as an evaluator for this phase") end + end - it 'creates a new evaluator invitation' do + context 'when adding an existing user with a non-evaluator role' do + let(:existing_user) { create(:user, role: 'solver', status: 'pending') } + + it 'adds the user as an evaluator without changing their role' do expect { - post challenge_manage_evaluators_path(challenge), params: new_user_params - }.to change(EvaluatorInvitation, :count).by(1) + post challenge_manage_evaluators_path(challenge), params: { + evaluator_invitation: { + email: existing_user.email, + phase_id: phase.id + } + } + }.to change(ChallengePhasesEvaluator, :count).by(1) - expect(ChallengePhasesEvaluator.count).to eq(0) + existing_user.reload + expect(existing_user.role).to eq('solver') expect(response).to redirect_to(challenge_manage_evaluators_path(challenge, phase_id: phase.id)) - expect(flash[:notice]).to include("Invitation sent to") + expect(flash[:notice]).to include("has been added as an evaluator for this phase") + end + end + + context 'when adding a new user with default evaluator role' do + let(:existing_evaluator) { create(:user, role: 'evaluator', status: 'pending') } + + it 'adds the user without changing their role' do + expect { + post challenge_manage_evaluators_path(challenge), params: { + evaluator_invitation: { + email: existing_evaluator.email, + phase_id: phase.id + } + } + }.to change(ChallengePhasesEvaluator, :count).by(1) + + existing_evaluator.reload + expect(existing_evaluator.role).to eq('evaluator') + expect(response).to redirect_to(challenge_manage_evaluators_path(challenge, phase_id: phase.id)) + expect(flash[:notice]).to include("has been added as an evaluator for this phase") end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 39923ad1..e61c6414 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -147,7 +147,7 @@ expect(created_user.status).to eq("pending") end - it 'creates active solver user if no matching token or email and non .gov email' do + it 'creates pending evaluator user if no matching token or email and non .gov email' do email = non_gov_userinfo[0]["email"] token = non_gov_userinfo[0]["sub"] @@ -155,8 +155,8 @@ expect(created_user.email).to eq(email) expect(created_user.token).to eq(token) - expect(created_user.role).to eq("solver") - expect(created_user.status).to eq("active") + expect(created_user.role).to eq("evaluator") + expect(created_user.status).to eq("pending") end it 'update user with token if matching email but no token set (from admin creation)' do From 20931198b0c90029a975fdf1da94b917b1466605 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 5 Nov 2024 11:15:43 -0600 Subject: [PATCH 02/38] 245 | Move evaluator invitation to evaluator user logic --- app/models/user.rb | 19 +++++++++- spec/models/user_spec.rb | 82 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 96 insertions(+), 5 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 37f1fa89..dfffde1e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -33,6 +33,10 @@ # recertification_expired_at :datetime # class User < ApplicationRecord + after_create :accept_evaluator_invitation + + VALID_EVALUATOR_ROLES = %w[evaluator solver challenge_manager].freeze + belongs_to :agency, optional: true has_many :challenges, dependent: :destroy @@ -130,14 +134,25 @@ def self.create_user_from_userinfo(userinfo) end def self.default_role_and_status_for_email(email) - if default_challenge_manager?(email) + if EvaluatorInvitation.exists?(email: email) + %w[evaluator pending] + elsif default_challenge_manager?(email) %w[challenge_manager pending] else - %w[evaluator pending] + %w[solver active] end end def self.default_challenge_manager?(email) /\.(gov|mil)$/.match?(email) end + + private + + def accept_evaluator_invitation + EvaluatorInvitation.where(email: self.email).each do |invite| + ChallengePhasesEvaluator.create(challenge: invite.challenge, phase: invite.phase, user: self) + invite.destroy + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e61c6414..49efe045 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -147,7 +147,7 @@ expect(created_user.status).to eq("pending") end - it 'creates pending evaluator user if no matching token or email and non .gov email' do + it 'creates active solver user if no matching token or email and non .gov email' do email = non_gov_userinfo[0]["email"] token = non_gov_userinfo[0]["sub"] @@ -155,8 +155,8 @@ expect(created_user.email).to eq(email) expect(created_user.token).to eq(token) - expect(created_user.role).to eq("evaluator") - expect(created_user.status).to eq("pending") + expect(created_user.role).to eq("solver") + expect(created_user.status).to eq("active") end it 'update user with token if matching email but no token set (from admin creation)' do @@ -174,4 +174,80 @@ expect(updated_user.token).to eq(token) end end + + describe '#accept_evaluator_invitation' do + let(:challenge1) { create(:challenge) } + let(:challenge2) { create(:challenge) } + let(:phase1) { create(:phase, challenge: challenge1) } + let(:phase2) { create(:phase, challenge: challenge1) } + let(:phase3) { create(:phase, challenge: challenge2) } + let(:user_email) { 'test@example.com' } + + context 'when there are existing evaluator invitations' do + before do + create(:evaluator_invitation, challenge: challenge1, phase: phase1, email: user_email) + create(:evaluator_invitation, challenge: challenge1, phase: phase2, email: user_email) + create(:evaluator_invitation, challenge: challenge2, phase: phase3, email: user_email) + end + + it 'creates ChallengePhasesEvaluator records for all invitations when user is created' do + expect { + create(:user, email: user_email, role: 'evaluator') + }.to change(ChallengePhasesEvaluator, :count).by(3) + end + + it 'destroys all EvaluatorInvitation records when user is created' do + expect { + create(:user, email: user_email, role: 'evaluator') + }.to change(EvaluatorInvitation, :count).by(-3) + end + + it 'associates the new user with the correct challenges and phases' do + user = create(:user, email: user_email, role: 'evaluator') + expect(user.challenge_phases_evaluators.count).to eq(3) + expect(user.challenge_phases_evaluators.map(&:challenge)).to contain_exactly(challenge1, challenge1, challenge2) + expect(user.challenge_phases_evaluators.map(&:phase)).to contain_exactly(phase1, phase2, phase3) + end + end + + context 'when there are no existing evaluator invitations' do + it 'does not create any ChallengePhasesEvaluator records' do + expect { + create(:user, email: user_email) + }.not_to change(ChallengePhasesEvaluator, :count) + end + + it 'does not destroy any EvaluatorInvitation records' do + expect { + create(:user, email: user_email) + }.not_to change(EvaluatorInvitation, :count) + end + end + + context 'when there are invitations for multiple challenges and phases' do + before do + create(:evaluator_invitation, challenge: challenge1, phase: phase1, email: user_email) + create(:evaluator_invitation, challenge: challenge2, phase: phase3, email: user_email) + end + + it 'creates ChallengePhasesEvaluator records for all invitations' do + expect { + create(:user, email: user_email, role: 'evaluator') + }.to change(ChallengePhasesEvaluator, :count).by(2) + end + + it 'destroys all EvaluatorInvitation records' do + expect { + create(:user, email: user_email, role: 'evaluator') + }.to change(EvaluatorInvitation, :count).by(-2) + end + + it 'associates the user with the correct challenges and phases' do + user = create(:user, email: user_email, role: 'evaluator') + expect(user.challenge_phases_evaluators.count).to eq(2) + expect(user.challenge_phases_evaluators.map(&:challenge)).to contain_exactly(challenge1, challenge2) + expect(user.challenge_phases_evaluators.map(&:phase)).to contain_exactly(phase1, phase3) + end + end + end end From 6771cf4a5f6276b8cc01a3c71dc696f69c3abaa6 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 5 Nov 2024 11:17:05 -0600 Subject: [PATCH 03/38] 245 | Add check for evaluator's role --- app/models/challenge_phases_evaluator.rb | 10 ++++++++ .../models/challenge_phases_evaluator_spec.rb | 25 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/app/models/challenge_phases_evaluator.rb b/app/models/challenge_phases_evaluator.rb index d2389a27..300f4d2f 100644 --- a/app/models/challenge_phases_evaluator.rb +++ b/app/models/challenge_phases_evaluator.rb @@ -15,4 +15,14 @@ class ChallengePhasesEvaluator < ApplicationRecord belongs_to :challenge belongs_to :phase belongs_to :user + + validate :user_has_valid_role, if: -> { user.present? } + + private + + def user_has_valid_role + unless User::VALID_EVALUATOR_ROLES.include?(user.role) + errors.add(:user, "must have a valid evaluator role") + end + end end diff --git a/spec/models/challenge_phases_evaluator_spec.rb b/spec/models/challenge_phases_evaluator_spec.rb index df993ed0..630870ae 100644 --- a/spec/models/challenge_phases_evaluator_spec.rb +++ b/spec/models/challenge_phases_evaluator_spec.rb @@ -62,4 +62,29 @@ expect(phase.evaluators).to include(user) expect(user.evaluated_phases).to include(phase) end + + context "with invalid user role" do + let(:invalid_user) { create(:user, role: User::VALID_EVALUATOR_ROLES.first) } + + before do + invalid_user.update_column(:role, 'admin') + end + + it "is invalid" do + evaluator = build(:challenge_phases_evaluator, challenge:, phase:, user: invalid_user) + expect(evaluator).not_to be_valid + expect(evaluator.errors[:user]).to include("must have a valid evaluator role") + end + end + + User::VALID_EVALUATOR_ROLES.each do |role| + context "with #{role} role" do + let(:valid_user) { create(:user, role: role) } + + it "is valid" do + evaluator = build(:challenge_phases_evaluator, challenge:, phase:, user: valid_user) + expect(evaluator).to be_valid + end + end + end end From ef02ade07bd1c479863ac8578ad8dbd5ffa35867 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 5 Nov 2024 11:18:57 -0600 Subject: [PATCH 04/38] 245 | Remove duplicate logic and handle in/valid evaluator roles --- .../manage_evaluators_controller.rb | 91 +++++++++---------- .../manage_evaluators_controller_spec.rb | 37 +++++++- 2 files changed, 77 insertions(+), 51 deletions(-) diff --git a/app/controllers/manage_evaluators_controller.rb b/app/controllers/manage_evaluators_controller.rb index 48254be7..3a546463 100644 --- a/app/controllers/manage_evaluators_controller.rb +++ b/app/controllers/manage_evaluators_controller.rb @@ -6,8 +6,6 @@ class ManageEvaluatorsController < ApplicationController before_action :set_challenge before_action :set_phases, only: [:index] - VALID_EVALUATOR_ROLES = %w[evaluator solver challenge_manager].freeze - def index if @phases.empty? handle_empty_phases @@ -18,12 +16,12 @@ def index def create @phase = @challenge.phases.find(evaluator_invitation_params[:phase_id]) - result = process_evaluator_invitation(evaluator_invitation_params[:email]) + user = User.find_by(email: evaluator_invitation_params[:email]) - if result[:success] - handle_successful_creation(result) + if existing_evaluator?(user) + handle_existing_evaluator(user) else - handle_failed_creation + process_new_evaluator end end @@ -45,6 +43,10 @@ def set_phases @phases = @challenge.phases.order(:start_date) end + def existing_evaluator?(user) + user && ChallengePhasesEvaluator.exists?(challenge: @challenge, phase: @phase, user: user) + end + def evaluator_invitation_params params.require(:evaluator_invitation).permit( :first_name, :last_name, :email, :challenge_id, :phase_id, :last_invite_sent @@ -60,61 +62,44 @@ def handle_empty_phases def handle_existing_phases @phase = select_phase - @evaluator_invitations = fetch_evaluator_invitations - @existing_evaluators = fetch_existing_evaluators + fetch_evaluators_and_invitations end def select_phase params[:phase_id] ? @phases.find(params[:phase_id]) : @phases.first end - def fetch_evaluator_invitations - @phase.evaluator_invitations + def fetch_evaluators_and_invitations + @evaluator_invitations = @phase.evaluator_invitations + @existing_evaluators = @phase.evaluators end - def fetch_existing_evaluators - @phase.evaluators + # Create action helpers + def process_new_evaluator + result = process_evaluator_invitation(evaluator_invitation_params[:email]) + result[:success] ? handle_successful_creation(result) : handle_failed_creation(result[:message]) end - # Create action helpers def process_evaluator_invitation(email) user = User.find_by(email: email) - existing_invitation = @challenge.evaluator_invitations.find_by(email: email, phase: @phase) - if user - result = add_user_as_evaluator(user) - existing_invitation&.destroy if result[:success] - result - elsif existing_invitation - resend_invitation(existing_invitation) + add_user_as_evaluator(user) else - create_new_invitation(email) + existing_invitation = @challenge.evaluator_invitations.find_by(email: email, phase: @phase) + existing_invitation ? resend_invitation(existing_invitation) : create_new_invitation(email) end end - # prevent duplicate evaluator invitations - def resend_invitation(invitation) - invitation.update(last_invite_sent: Time.current) # only update last_invite_sent for now - { - success: true, - message: "An invitation to this challenge has already been sent to " \ - "#{invitation.email}. Invitation has been resent." - } - end - - def valid_evaluator_role?(user) - VALID_EVALUATOR_ROLES.include?(user.role) - end - def add_user_as_evaluator(user) - cpe = ChallengePhasesEvaluator.find_or_create_by(challenge: @challenge, phase: @phase, user: user) - invitation = @challenge.evaluator_invitations.find_by(email: user.email, phase: @phase) - - if cpe.persisted? - invitation&.destroy - { success: true, message: "#{user.email} has been added as an evaluator for this phase." } + if User::VALID_EVALUATOR_ROLES.include?(user.role) + cpe = ChallengePhasesEvaluator.find_or_create_by(challenge: @challenge, phase: @phase, user: user) + if cpe.persisted? + { success: true, message: "#{user.email} has been added as an evaluator for this phase." } + else + { success: false, message: "Failed to add #{user.email} as an evaluator." } + end else - { success: false, message: "Failed to add #{user.email} as an evaluator." } + { success: false, message: "#{user.email} does not have a valid evaluator role." } end end @@ -132,12 +117,27 @@ def handle_successful_creation(result) notice: result[:message] end - def handle_failed_creation - @evaluator_invitations = fetch_evaluator_invitations - @existing_evaluators = fetch_existing_evaluators + def handle_failed_creation(error_message) + flash.now[:alert] = error_message + fetch_evaluators_and_invitations render :index end + # prevent duplicate evaluators or evauator invitations + def handle_existing_evaluator(user) + flash[:notice] = "#{user.email} has already been added as an evaluator for this phase." + redirect_to challenge_manage_evaluators_path(@challenge, phase_id: @phase.id) + end + + def resend_invitation(invitation) + invitation.update(last_invite_sent: Time.current) # only update last_invite_sent for now + { + success: true, + message: "An invitation to this challenge has already been sent to " \ + "#{invitation.email}. Invitation has been resent." + } + end + # Destroy action helpers def process_evaluator_removal(evaluator_type, evaluator_id) case evaluator_type @@ -179,7 +179,6 @@ def remove_evaluator_invitation(invitation_id) def render_json_response(result) if result[:success] - flash[:notice] = result[:message] render json: { success: true, message: result[:message] } else render json: { success: false, message: result[:message] }, status: :unprocessable_entity diff --git a/spec/controllers/manage_evaluators_controller_spec.rb b/spec/controllers/manage_evaluators_controller_spec.rb index 43855a7a..4946b7a6 100644 --- a/spec/controllers/manage_evaluators_controller_spec.rb +++ b/spec/controllers/manage_evaluators_controller_spec.rb @@ -88,7 +88,11 @@ let(:existing_user) { create(:user, role: 'evaluator', status: 'pending') } let!(:invitation) { create(:evaluator_invitation, challenge: challenge, phase: phase, email: existing_user.email) } - it 'adds the user as an evaluator and deletes only the specific invitation' do + before do + ChallengePhasesEvaluator.create!(challenge: challenge, phase: phase, user: existing_user) + end + + it 'adds the user as an evaluator without creating a new ChallengePhasesEvaluator' do expect { post challenge_manage_evaluators_path(challenge), params: { evaluator_invitation: { @@ -96,12 +100,12 @@ phase_id: phase.id } } - }.to change(ChallengePhasesEvaluator, :count).by(1) - .and change(EvaluatorInvitation, :count).by(-1) + }.not_to change(ChallengePhasesEvaluator, :count) - expect(EvaluatorInvitation.find_by(id: invitation.id)).to be_nil + expect(ChallengePhasesEvaluator.where(challenge: challenge, phase: phase, user: existing_user).count).to eq(1) + expect(EvaluatorInvitation.find_by(id: invitation.id)).to be_present expect(response).to redirect_to(challenge_manage_evaluators_path(challenge, phase_id: phase.id)) - expect(flash[:notice]).to include("has been added as an evaluator for this phase") + expect(flash[:notice]).to include("has already been added as an evaluator for this phase") end end @@ -144,6 +148,29 @@ expect(flash[:notice]).to include("has been added as an evaluator for this phase") end end + + context 'when adding an existing user with an invalid role' do + let(:existing_user) { create(:user, role: 'evaluator', status: 'pending') } + + before do + existing_user.update_column(:role, 'admin') # invalid evaluator role + end + + it 'does not add the user as an evaluator and returns an error' do + initial_count = ChallengePhasesEvaluator.count + + post challenge_manage_evaluators_path(challenge), params: { + evaluator_invitation: { + email: existing_user.email, + phase_id: phase.id + } + } + + expect(ChallengePhasesEvaluator.count).to eq(initial_count) + expect(response).to render_template(:index) + expect(flash[:alert]).to include("does not have a valid evaluator role") + end + end end describe 'DELETE #destroy' do From 23e5701c2b99e7e53b65b41971a9cf59f15e06fb Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 5 Nov 2024 11:25:00 -0600 Subject: [PATCH 05/38] 245 | Add successfully removed flash notice back in --- app/controllers/manage_evaluators_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/manage_evaluators_controller.rb b/app/controllers/manage_evaluators_controller.rb index 3a546463..4917b390 100644 --- a/app/controllers/manage_evaluators_controller.rb +++ b/app/controllers/manage_evaluators_controller.rb @@ -179,6 +179,7 @@ def remove_evaluator_invitation(invitation_id) def render_json_response(result) if result[:success] + flash[:notice] = result[:message] render json: { success: true, message: result[:message] } else render json: { success: false, message: result[:message] }, status: :unprocessable_entity From 27f35d0f5f816a567096f62e96e7cf77362ddf3a Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Tue, 5 Nov 2024 19:55:47 -0600 Subject: [PATCH 06/38] 245 | Update tests --- .../manage_evaluators_controller.rb | 2 +- app/models/challenge_phases_evaluator.rb | 2 +- .../manage_evaluators_controller_spec.rb | 34 +++++++++++++------ .../models/challenge_phases_evaluator_spec.rb | 24 +------------ spec/models/user_spec.rb | 25 -------------- 5 files changed, 26 insertions(+), 61 deletions(-) diff --git a/app/controllers/manage_evaluators_controller.rb b/app/controllers/manage_evaluators_controller.rb index 4917b390..82931161 100644 --- a/app/controllers/manage_evaluators_controller.rb +++ b/app/controllers/manage_evaluators_controller.rb @@ -123,7 +123,7 @@ def handle_failed_creation(error_message) render :index end - # prevent duplicate evaluators or evauator invitations + # prevent duplicate evaluators or evaluator invitations def handle_existing_evaluator(user) flash[:notice] = "#{user.email} has already been added as an evaluator for this phase." redirect_to challenge_manage_evaluators_path(@challenge, phase_id: @phase.id) diff --git a/app/models/challenge_phases_evaluator.rb b/app/models/challenge_phases_evaluator.rb index 300f4d2f..8c836bc2 100644 --- a/app/models/challenge_phases_evaluator.rb +++ b/app/models/challenge_phases_evaluator.rb @@ -16,7 +16,7 @@ class ChallengePhasesEvaluator < ApplicationRecord belongs_to :phase belongs_to :user - validate :user_has_valid_role, if: -> { user.present? } + validate :user_has_valid_role private diff --git a/spec/controllers/manage_evaluators_controller_spec.rb b/spec/controllers/manage_evaluators_controller_spec.rb index 4946b7a6..cbe3523d 100644 --- a/spec/controllers/manage_evaluators_controller_spec.rb +++ b/spec/controllers/manage_evaluators_controller_spec.rb @@ -5,7 +5,7 @@ let(:challenge) { create(:challenge) } let(:phase) { create(:phase, challenge: challenge) } let(:evaluator) { create(:user, role: 'evaluator') } - let(:invitation) { create(:evaluator_invitation, challenge: challenge, phase: phase) } + let(:invitation) { create(:evaluator_invitation, challenge: challenge, phase: phase, email: 'invitation@example.com') } before do ChallengeManager.create(user: user, challenge: challenge) @@ -84,15 +84,14 @@ end end - context 'when inviting an existing user with an invitation' do + context 'when inviting a user who is already an evaluator for the challenge phase' do let(:existing_user) { create(:user, role: 'evaluator', status: 'pending') } - let!(:invitation) { create(:evaluator_invitation, challenge: challenge, phase: phase, email: existing_user.email) } before do ChallengePhasesEvaluator.create!(challenge: challenge, phase: phase, user: existing_user) end - it 'adds the user as an evaluator without creating a new ChallengePhasesEvaluator' do + it 'it does not create an additional ChallengePhaseEvaluator' do expect { post challenge_manage_evaluators_path(challenge), params: { evaluator_invitation: { @@ -100,15 +99,32 @@ phase_id: phase.id } } - }.not_to change(ChallengePhasesEvaluator, :count) + }.to_not change(ChallengePhasesEvaluator, :count) expect(ChallengePhasesEvaluator.where(challenge: challenge, phase: phase, user: existing_user).count).to eq(1) - expect(EvaluatorInvitation.find_by(id: invitation.id)).to be_present expect(response).to redirect_to(challenge_manage_evaluators_path(challenge, phase_id: phase.id)) expect(flash[:notice]).to include("has already been added as an evaluator for this phase") end end + context 'when inviting a user who already has an invitation for the challenge phase' do + it 'does not create an additional EvaluatorInvitation' do + invitation # create existing invitation + + expect { + post challenge_manage_evaluators_path(challenge), params: { + evaluator_invitation: { + email: invitation.email, + phase_id: phase.id + } + } + }.not_to change(EvaluatorInvitation, :count) + + expect(response).to redirect_to(challenge_manage_evaluators_path(challenge, phase_id: phase.id)) + expect(flash[:notice]).to include("An invitation to this challenge has already been sent to #{invitation.email}. Invitation has been resent.") + end + end + context 'when adding an existing user with a non-evaluator role' do let(:existing_user) { create(:user, role: 'solver', status: 'pending') } @@ -150,11 +166,7 @@ end context 'when adding an existing user with an invalid role' do - let(:existing_user) { create(:user, role: 'evaluator', status: 'pending') } - - before do - existing_user.update_column(:role, 'admin') # invalid evaluator role - end + let(:existing_user) { create(:user, role: 'admin') } it 'does not add the user as an evaluator and returns an error' do initial_count = ChallengePhasesEvaluator.count diff --git a/spec/models/challenge_phases_evaluator_spec.rb b/spec/models/challenge_phases_evaluator_spec.rb index 630870ae..b7d1eec9 100644 --- a/spec/models/challenge_phases_evaluator_spec.rb +++ b/spec/models/challenge_phases_evaluator_spec.rb @@ -21,24 +21,6 @@ expect(challenge.evaluators).to include(user) end - it "requires a challenge" do - evaluator = build(:challenge_phases_evaluator, challenge: nil) - expect(evaluator).not_to be_valid - expect(evaluator.errors[:challenge]).to include("must exist") - end - - it "requires a phase" do - evaluator = build(:challenge_phases_evaluator, phase: nil) - expect(evaluator).not_to be_valid - expect(evaluator.errors[:phase]).to include("must exist") - end - - it "requires a user" do - evaluator = build(:challenge_phases_evaluator, user: nil) - expect(evaluator).not_to be_valid - expect(evaluator.errors[:user]).to include("must exist") - end - it "allows multiple evaluators for the same challenge and phase" do challenge = create(:challenge) phase = create(:phase, challenge:) @@ -64,11 +46,7 @@ end context "with invalid user role" do - let(:invalid_user) { create(:user, role: User::VALID_EVALUATOR_ROLES.first) } - - before do - invalid_user.update_column(:role, 'admin') - end + let(:invalid_user) { create(:user, role: 'admin') } it "is invalid" do evaluator = build(:challenge_phases_evaluator, challenge:, phase:, user: invalid_user) diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 49efe045..9aefc071 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -224,30 +224,5 @@ end end - context 'when there are invitations for multiple challenges and phases' do - before do - create(:evaluator_invitation, challenge: challenge1, phase: phase1, email: user_email) - create(:evaluator_invitation, challenge: challenge2, phase: phase3, email: user_email) - end - - it 'creates ChallengePhasesEvaluator records for all invitations' do - expect { - create(:user, email: user_email, role: 'evaluator') - }.to change(ChallengePhasesEvaluator, :count).by(2) - end - - it 'destroys all EvaluatorInvitation records' do - expect { - create(:user, email: user_email, role: 'evaluator') - }.to change(EvaluatorInvitation, :count).by(-2) - end - - it 'associates the user with the correct challenges and phases' do - user = create(:user, email: user_email, role: 'evaluator') - expect(user.challenge_phases_evaluators.count).to eq(2) - expect(user.challenge_phases_evaluators.map(&:challenge)).to contain_exactly(challenge1, challenge2) - expect(user.challenge_phases_evaluators.map(&:phase)).to contain_exactly(phase1, phase3) - end - end end end From e64fa2316e3355cdfa22962558d4fe82b9bd0ff9 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Wed, 6 Nov 2024 10:25:45 -0600 Subject: [PATCH 07/38] 245 | CPE created automatically, adjust redudant conditional --- app/helpers/manage_evaluators_helper.rb | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/app/helpers/manage_evaluators_helper.rb b/app/helpers/manage_evaluators_helper.rb index a20441f6..08b8854a 100644 --- a/app/helpers/manage_evaluators_helper.rb +++ b/app/helpers/manage_evaluators_helper.rb @@ -3,7 +3,7 @@ module ManageEvaluatorsHelper def user_status(evaluator, challenge) if evaluator.is_a?(User) - user_status_for_existing_user(evaluator, challenge) + evaluator.status == 'active' ? "Available" : "Awaiting Approval" else "Invite Sent" end @@ -19,14 +19,4 @@ def assigned_submissions_count(evaluator, challenge, phase) 0 end end - - private - - def user_status_for_existing_user(user, challenge) - if challenge.challenge_phases_evaluators.exists?(user_id: user.id) - user.status == 'active' ? "Available" : "Awaiting Approval" - else - "Invite Sent" - end - end end From 1d36e0d94b29c27501873c3b6ccf69ea171f6b40 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 7 Nov 2024 12:19:09 -0600 Subject: [PATCH 08/38] 179 | UI/Add view for evaluator submissions --- .../evaluator_submissions/index.html.erb | 145 ++++++++++++++++++ app/views/manage_evaluators/index.html.erb | 2 +- 2 files changed, 146 insertions(+), 1 deletion(-) create mode 100644 app/views/evaluator_submissions/index.html.erb diff --git a/app/views/evaluator_submissions/index.html.erb b/app/views/evaluator_submissions/index.html.erb new file mode 100644 index 00000000..144dc576 --- /dev/null +++ b/app/views/evaluator_submissions/index.html.erb @@ -0,0 +1,145 @@ +
+ +
+ <%= link_to challenge_manage_evaluators_path, class: "usa-link display-inline-flex flex-align-center" do %> + <%= image_tag('images/usa-icons/arrow_back.svg', class: "usa-icon--size-3", alt: "Back to previous page") %> + Back + <% end %> +
+ +

<%= @challenge.title %> - <%= @phase.title %>

+

View submissions assigned to an evalutor.

+ +

Evaluator: <%= "#{@evaluator.first_name} #{@evaluator.last_name}" %>

+ +

Assigned Submissions

+

A list of submissions assigned to the user.

+ +
+
+
+
+
+
+ <%= @assigned_submissions.count %> +
+
+

+ Assigned Submissions +

+

+ Evaluations due by <%= @phase.end_date.strftime('%m/%d/%Y') %> +

+
+
+
+ +
+ +
+
+ <%= @submissions_count["completed"] || 0 %>
+ Completed +
+
+ <%= @submissions_count["in_progress"] || 0 %>
+ In Progress +
+
+ <%= @submissions_count["not_started"] || 0 %>
+ Not Started +
+
+
+
+
+ + <% if @assigned_submissions.any? %> +
+ + + + + + + + + + + <% @assigned_submissions.each do |assignment| %> + + + + + + + <% end %> + +
Submission IDEvaluation statusScore
<%= assignment.submission.id %> + <%= assignment.status.titleize %> + 80<%# assignment.score || 'N/A' %> +
+ <% if assignment.status == "completed" %> + <%= link_to manage_submissions_path(@challenge), class: 'usa-button font-body-3xs margin-right-1', style: 'white-space: nowrap;' do %> + View Evaluation + <% end %> + <% end %> + <%= button_tag "Unassign", type: 'button', class: 'usa-button usa-button--outline font-body-3xs', data: { + action: "click->unassign-evaluator-submission-modal#open", + submission_id: assignment.submission.id, + evaluator_id: @evaluator.id, + challenge_id: @challenge.id, + phase_id: @phase.id + } %> +
+
+
+ <% else %> +
+

There currently are no assigned submissions to this evaluator. Please assigned submissions to this evaluator to view.

+
+ <% end %> + + <% if @unassigned_submissions.any? %> +

Unassigned Submissions

+

A list of recused and unassigned submissions. Reassigning a user to a submission will make the submission available for the user to evaluate.

+ +
+ + + + + + + + + + + <% @unassigned_submissions.each do |assignment| %> + + + + + + + <% end %> + +
Submission IDEvaluation statusScore
<%= assignment.submission.id %> + <%= assignment.status.titleize %> + 80<%# assignment.score || 'N/A' %> +
+ <%= button_to "Reassign", reassign_challenge_phase_evaluator_submission_path(@challenge, @phase, assignment.submission, evaluator_id: @evaluator.id), method: :post, class: 'usa-button usa-button--outline font-body-3xs', data: { turbo: false } %> +
+
+
+ <% end %> + + <%= render 'unassign_evaluator_submission_modal' %> +
diff --git a/app/views/manage_evaluators/index.html.erb b/app/views/manage_evaluators/index.html.erb index d313c11d..120c7e2a 100644 --- a/app/views/manage_evaluators/index.html.erb +++ b/app/views/manage_evaluators/index.html.erb @@ -75,7 +75,7 @@ <%= assigned_submissions_count(evaluator, @challenge, @phase) %>
- <%= link_to manage_submissions_path(@challenge), class: 'usa-button font-body-3xs', style: 'white-space: nowrap;' do %> + <%= link_to challenge_phase_evaluator_submissions_path(@challenge, @phase, evaluator_id: evaluator.id), class: 'usa-button font-body-3xs', style: 'white-space: nowrap;' do %> View Submissions <% end %> <%= button_tag "Delete", type: 'button', class: 'usa-button usa-button--outline font-body-3xs', data: { From d576fa087da8278a4d3c15f096eafe3a16f1d38d Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 7 Nov 2024 12:20:40 -0600 Subject: [PATCH 09/38] 179 | Add UI for the unassign modal and js --- .../delete_evaluator_modal_controller.js | 3 +- app/javascript/controllers/index.js | 3 + ...n_evaluator_submission_modal_controller.js | 75 +++++++++++++++++++ ...assign_evaluator_submission_modal.html.erb | 48 ++++++++++++ 4 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 app/javascript/controllers/unassign_evaluator_submission_modal_controller.js create mode 100644 app/views/evaluator_submissions/_unassign_evaluator_submission_modal.html.erb diff --git a/app/javascript/controllers/delete_evaluator_modal_controller.js b/app/javascript/controllers/delete_evaluator_modal_controller.js index badb8b2b..a6c0714f 100644 --- a/app/javascript/controllers/delete_evaluator_modal_controller.js +++ b/app/javascript/controllers/delete_evaluator_modal_controller.js @@ -44,14 +44,13 @@ export default class extends Controller { deleteEvaluator(forceDelete = false) { const csrfToken = document.querySelector('meta[name="csrf-token"]').content - fetch(`/challenges/${this.challengeIdValue}/manage_evaluators`, { + fetch(`/challenges/${this.challengeIdValue}/manage_evaluators/${this.evaluatorIdValue}`, { method: 'DELETE', headers: { 'Content-Type': 'application/json', 'X-CSRF-Token': csrfToken }, body: JSON.stringify({ - evaluator_id: this.evaluatorIdValue, evaluator_type: this.evaluatorTypeValue, phase_id: this.phaseIdValue, force_delete: forceDelete diff --git a/app/javascript/controllers/index.js b/app/javascript/controllers/index.js index 076ccf60..a5369909 100644 --- a/app/javascript/controllers/index.js +++ b/app/javascript/controllers/index.js @@ -11,3 +11,6 @@ application.register("evaluation-criteria", EvaluationCriteriaController); import DeleteEvaluatorModalController from "./delete_evaluator_modal_controller" application.register("delete-evaluator-modal", DeleteEvaluatorModalController) + +import UnassignEvaluatorSubmissionModalController from "./unassign_evaluator_submission_modal_controller" +application.register("unassign-evaluator-submission-modal", UnassignEvaluatorSubmissionModalController) diff --git a/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js b/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js new file mode 100644 index 00000000..9429427e --- /dev/null +++ b/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js @@ -0,0 +1,75 @@ +import { Controller } from "@hotwired/stimulus" + +export default class extends Controller { + static targets = ["modal", "confirmButton"] + static values = { + challengeId: String, + phaseId: String, + submissionId: String, + evaluatorId: String + } + + connect() { + this.modalTarget.addEventListener('click', this.handleOutsideClick.bind(this)) + } + + disconnect() { + this.modalTarget.removeEventListener('click', this.handleOutsideClick.bind(this)) + } + + open(event) { + event.preventDefault() + this.submissionIdValue = event.currentTarget.dataset.submissionId + this.evaluatorIdValue = event.currentTarget.dataset.evaluatorId + this.challengeIdValue = event.currentTarget.dataset.challengeId + this.phaseIdValue = event.currentTarget.dataset.phaseId + this.modalTarget.showModal() + } + + close() { + this.modalTarget.close() + } + + handleOutsideClick(event) { + if (event.target === this.modalTarget) { + this.close() + } + } + + confirm() { + this.unassignEvaluatorSubmission() + } + + unassignEvaluatorSubmission() { + const csrfToken = document.querySelector('meta[name="csrf-token"]').content + + fetch(`/challenges/${this.challengeIdValue}/phases/${this.phaseIdValue}/evaluator_submissions/${this.submissionIdValue}/unassign`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'X-CSRF-Token': csrfToken + }, + body: JSON.stringify({ + evaluator_id: this.evaluatorIdValue + }) + }) + .then(response => { + if (!response.ok) { + throw new Error('Network response was not ok') + } + return response.json() + }) + .then(data => { + if (data.success) { + this.close() + window.location.reload() + } else { + throw new Error(data.message || 'Failed to unassign evaluator from submission') + } + }) + .catch(error => { + console.error('Error:', error) + alert(error.message || 'An error occurred while unassigning the evaluator from the submission') + }) + } +} diff --git a/app/views/evaluator_submissions/_unassign_evaluator_submission_modal.html.erb b/app/views/evaluator_submissions/_unassign_evaluator_submission_modal.html.erb new file mode 100644 index 00000000..17320c1b --- /dev/null +++ b/app/views/evaluator_submissions/_unassign_evaluator_submission_modal.html.erb @@ -0,0 +1,48 @@ + +
+
+ +
+ +
+ +
+ +
+
From a91e88a9facb2264d702d4e2cfd7edec3a106987 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 7 Nov 2024 12:23:06 -0600 Subject: [PATCH 10/38] 179 | Add routes --- config/routes.rb | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 5dd4002f..b82f7624 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -15,6 +15,24 @@ resources :evaluation_forms post '/evaluation_forms/clone', to: 'evaluation_forms#create_from_existing' resources :manage_submissions, only: [:index] + resources :challenges, only: [] do + resources :manage_submissions, only: [:show] + resources :manage_evaluators, only: [:index, :create, :destroy] + resources :evaluator_invitations, only: [] do + member do + post 'resend' + end + end + resources :phases, only: [] do + resources :evaluator_submissions, only: [:index] do + member do + post 'unassign' + post 'reassign' + end + end + get 'evaluator_submissions/:evaluator_id', to: 'evaluator_submissions#index', as: :evaluator_submissions_for_evaluator + end + end # Reveal health status on /up that returns 200 if the app boots with no exceptions, otherwise 500. # Can be used by load balancers and uptime monitors to verify that the app is live. @@ -29,17 +47,4 @@ post "/login", to: "accounts#login" end end - - resources :challenges do - resources :manage_evaluators, only: [:index, :create] do - collection do - delete :destroy - end - end - resources :evaluator_invitations, only: [] do - member do - post 'resend' - end - end - end end From a4a9298393630a53b95ec1098ae1ed1ba0f93fed Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 7 Nov 2024 12:24:06 -0600 Subject: [PATCH 11/38] 179 | Add evaluator_submissions_assignments association to phase through submissions --- app/models/phase.rb | 5 +++++ app/views/manage_submissions/_table.html.erb | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/app/models/phase.rb b/app/models/phase.rb index bb4193be..24c8ca27 100644 --- a/app/models/phase.rb +++ b/app/models/phase.rb @@ -25,6 +25,7 @@ class Phase < ApplicationRecord belongs_to :challenge # More relations from phoenix app has_many :submissions, dependent: :destroy + has_many :evaluator_submission_assignments, through: :submissions has_one :evaluation_form, dependent: :destroy # has_one :winner, class_name: 'PhaseWinner' has_many :evaluator_invitations, dependent: :destroy @@ -48,4 +49,8 @@ class Phase < ApplicationRecord # Validations validates :title, :start_date, :end_date, presence: true + + def submissions_count + evaluator_submission_assignments.select(:submission_id).distinct.count + end end diff --git a/app/views/manage_submissions/_table.html.erb b/app/views/manage_submissions/_table.html.erb index 27d9014b..cc04ffcd 100644 --- a/app/views/manage_submissions/_table.html.erb +++ b/app/views/manage_submissions/_table.html.erb @@ -17,7 +17,7 @@ <%= challenge.status.capitalize %> - <%= phase.submissions.length %> + <%= phase.submissions_count %> <% if phase.evaluation_form %> From 742af61cc834e6e5cf4f32a29d40c053312895df Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 7 Nov 2024 12:24:49 -0600 Subject: [PATCH 12/38] 179 | Add evaluation submissions controller --- .../evaluator_submissions_controller.rb | 60 +++++++++++++ .../manage_evaluators_controller.rb | 2 +- .../evaluator_submissions_controller_spec.rb | 90 +++++++++++++++++++ .../manage_evaluators_controller_spec.rb | 6 +- 4 files changed, 154 insertions(+), 4 deletions(-) create mode 100644 app/controllers/evaluator_submissions_controller.rb create mode 100644 spec/controllers/evaluator_submissions_controller_spec.rb diff --git a/app/controllers/evaluator_submissions_controller.rb b/app/controllers/evaluator_submissions_controller.rb new file mode 100644 index 00000000..3e13b7ac --- /dev/null +++ b/app/controllers/evaluator_submissions_controller.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +class EvaluatorSubmissionsController < ApplicationController + before_action :set_challenge_and_phase + before_action :set_evaluator, only: [:index] + before_action :set_assignment, only: [:unassign, :reassign] + + def index + @evaluator_assignments = @phase.evaluator_submission_assignments + .includes(:submission) + .where(user_id: @evaluator.id) + + @assigned_submissions = @evaluator_assignments.where.not(status: :unassigned) # includes completed, in_progress, not_started, and recused + .ordered_by_status + @unassigned_submissions = @evaluator_assignments.where(status: :unassigned) + + @submissions_count = @assigned_submissions.group(:status).count + end + + def unassign + if @assignment.update(status: :unassigned) + flash[:success] = t('evaluator_submissions.unassign.success') + render json: { success: true, message: flash[:success] } + else + flash[:error] = t('evaluator_submissions.unassign.failure') + render json: { success: false, message: flash[:error] }, status: :unprocessable_entity + end + end + + def reassign + if @assignment.update(status: :not_started) + flash[:success] = t('evaluator_submissions.reassign.success') + else + flash[:error] = t('evaluator_submissions.reassign.failure') + ": #{@assignment.errors.full_messages.join(', ')}" + end + + redirect_to challenge_phase_evaluator_submissions_path(@challenge, @phase, evaluator_id: params[:evaluator_id]) + end + + private + + def set_challenge_and_phase + @challenge = Challenge.find(params[:challenge_id]) + @phase = @challenge.phases.find(params[:phase_id]) + end + + def set_evaluator + @evaluator = if params[:evaluator_id] + User.find(params[:evaluator_id]) + else + current_user + end + end + + def set_assignment + @assignment = EvaluatorSubmissionAssignment.joins(:submission) + .where(submissions: { phase_id: @phase.id }) + .find_by!(submission_id: params[:id], user_id: params[:evaluator_id]) + end +end diff --git a/app/controllers/manage_evaluators_controller.rb b/app/controllers/manage_evaluators_controller.rb index 82931161..be7b532b 100644 --- a/app/controllers/manage_evaluators_controller.rb +++ b/app/controllers/manage_evaluators_controller.rb @@ -27,7 +27,7 @@ def create def destroy @phase = @challenge.phases.find(params[:phase_id]) - result = process_evaluator_removal(params[:evaluator_type], params[:evaluator_id]) + result = process_evaluator_removal(params[:evaluator_type], params[:id]) render_json_response(result) end diff --git a/spec/controllers/evaluator_submissions_controller_spec.rb b/spec/controllers/evaluator_submissions_controller_spec.rb new file mode 100644 index 00000000..5e951b6f --- /dev/null +++ b/spec/controllers/evaluator_submissions_controller_spec.rb @@ -0,0 +1,90 @@ +require 'rails_helper' + +RSpec.describe EvaluatorSubmissionsController, type: :request do + let(:challenge_manager) { create_and_log_in_user(role: 'challenge_manager') } + let(:challenge) { create(:challenge) } + let(:phase) { create(:phase, challenge: challenge) } + let(:evaluator) { create(:user, role: 'evaluator') } + let(:submission) { create(:submission, challenge: challenge, phase: phase) } + let!(:assignment) { create(:evaluator_submission_assignment, submission: submission, evaluator: evaluator, status: :not_started) } + + before do + ChallengeManager.create(user: challenge_manager, challenge: challenge) + end + + describe 'GET #index' do + before do + get challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id) + end + + it 'assigns @evaluator_assignments' do + expect(assigns(:evaluator_assignments)).to include(assignment) + end + + it 'assigns @assigned_submissions' do + expect(assigns(:assigned_submissions)).to include(assignment) + end + + it 'assigns @unassigned_submissions' do + expect(assigns(:unassigned_submissions)).to be_empty + end + + it 'assigns @submissions_count' do + expect(assigns(:submissions_count)).to eq({ "not_started" => 1 }) + end + + it 'renders the index template' do + expect(response).to render_template(:index) + end + end + + describe 'POST #unassign' do + it 'unassigns the evaluator successfully and updates counts' do + expect { + post unassign_challenge_phase_evaluator_submission_path(challenge, phase, submission, evaluator_id: evaluator.id) + }.to change { assignment.reload.status }.from('not_started').to('unassigned') + + expect(response).to have_http_status(:success) + expect(JSON.parse(response.body)['success']).to be true + + get challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id) + expect(assigns(:assigned_submissions)).to be_empty + expect(assigns(:unassigned_submissions)).to include(assignment) + expect(assigns(:submissions_count)).to eq({}) + end + + it 'handles failure to unassign' do + allow_any_instance_of(EvaluatorSubmissionAssignment).to receive(:update).and_return(false) + post unassign_challenge_phase_evaluator_submission_path(challenge, phase, submission, evaluator_id: evaluator.id) + expect(response).to have_http_status(:unprocessable_entity) + expect(JSON.parse(response.body)['success']).to be false + end + end + + describe 'POST #reassign' do + before do + assignment.update(status: :unassigned) + end + + it 'reassigns the evaluator successfully and updates counts' do + expect { + post reassign_challenge_phase_evaluator_submission_path(challenge, phase, submission, evaluator_id: evaluator.id) + }.to change { assignment.reload.status }.from('unassigned').to('not_started') + + expect(flash[:success]).to eq('Evaluator reassigned successfully') + expect(response).to redirect_to(challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id)) + + get challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id) + expect(assigns(:assigned_submissions)).to include(assignment) + expect(assigns(:unassigned_submissions)).to be_empty + expect(assigns(:submissions_count)).to eq({ "not_started" => 1 }) + end + + it 'handles failure to reassign' do + allow_any_instance_of(EvaluatorSubmissionAssignment).to receive(:update).and_return(false) + post reassign_challenge_phase_evaluator_submission_path(challenge, phase, submission, evaluator_id: evaluator.id) + expect(flash[:error]).to include('Failed to reassign evaluator') + expect(response).to redirect_to(challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id)) + end + end +end diff --git a/spec/controllers/manage_evaluators_controller_spec.rb b/spec/controllers/manage_evaluators_controller_spec.rb index cbe3523d..016d7157 100644 --- a/spec/controllers/manage_evaluators_controller_spec.rb +++ b/spec/controllers/manage_evaluators_controller_spec.rb @@ -198,7 +198,7 @@ it 'removes the evaluator from their associated phase' do expect { - delete challenge_manage_evaluators_path(challenge), params: { evaluator_id: evaluator.id, evaluator_type: 'user', phase_id: phase1.id } + delete challenge_manage_evaluator_path(challenge, evaluator), params: { evaluator_type: 'user', phase_id: phase1.id } }.to change { challenge.challenge_phases_evaluators.where(phase: phase1).count }.by(-1) expect(response).to have_http_status(:success) @@ -212,7 +212,7 @@ it 'removes the evaluator invitation' do expect { - delete challenge_manage_evaluators_path(challenge), params: { evaluator_id: invitation.id, evaluator_type: 'invitation', phase_id: phase1.id } + delete challenge_manage_evaluator_path(challenge, invitation), params: { evaluator_type: 'invitation', phase_id: phase1.id } }.to change(EvaluatorInvitation, :count).by(-1) expect(response).to have_http_status(:success) @@ -222,7 +222,7 @@ context 'with invalid evaluator type' do it 'returns an error JSON response' do - delete challenge_manage_evaluators_path(challenge), params: { evaluator_id: 1, evaluator_type: 'invalid', phase_id: phase.id } + delete challenge_manage_evaluator_path(challenge, 1), params: { evaluator_type: 'invalid', phase_id: phase1.id } expect(response).to have_http_status(:unprocessable_entity) expect(JSON.parse(response.body)).to eq({ From 44c2db55c2f7f762d8a1d08f0d24fd75378e3113 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 7 Nov 2024 12:25:06 -0600 Subject: [PATCH 13/38] 179 | Add evaluation status text colors --- app/helpers/manage_evaluators_helper.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/app/helpers/manage_evaluators_helper.rb b/app/helpers/manage_evaluators_helper.rb index 08b8854a..3064bf40 100644 --- a/app/helpers/manage_evaluators_helper.rb +++ b/app/helpers/manage_evaluators_helper.rb @@ -19,4 +19,21 @@ def assigned_submissions_count(evaluator, challenge, phase) 0 end end + + def evaluation_status(status) + case status.to_sym + when :recused + 'text-accent-warm-dark' + when :not_started + 'text-secondary-dark' + when :in_progress + 'text-orange' + when :completed + 'text-green' + when :unassigned + 'text-accent-cool-darker' + else + 'text-base' + end + end end From 145886aad05ed530dc67e15dfda8ac63a3e84e1f Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 7 Nov 2024 12:25:24 -0600 Subject: [PATCH 14/38] 179 | Add evaluation status order --- app/models/evaluator_submission_assignment.rb | 23 ++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index 4c13f2c9..3d1ec982 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -4,5 +4,26 @@ class EvaluatorSubmissionAssignment < ApplicationRecord belongs_to :submission belongs_to :evaluator, class_name: "User", foreign_key: :user_id, inverse_of: :assigned_submissions - enum :status, { assigned: 0, unassigned: 1, recused: 2 } + has_one :phase, through: :submission + + enum :status, { + assigned: 0, + unassigned: 1, + recused: 2, + not_started: 3, + in_progress: 4, + completed: 5 + } + + STATUS_ORDER = [:recused, :not_started, :in_progress, :completed] + + scope :ordered_by_status, -> { + order(Arel.sql( + "CASE " + + STATUS_ORDER.map.with_index { |status, index| + "WHEN evaluator_submission_assignments.status = #{statuses[status]} THEN #{index + 1}" + }.join(" ") + + " ELSE #{STATUS_ORDER.length + 1} END" + )) + } end From 4295f4d745365422770a2ccc10c78ff7b1d58b97 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Thu, 7 Nov 2024 12:42:17 -0600 Subject: [PATCH 15/38] 179 | Add evaluator submissions message translations --- config/locales/en.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/config/locales/en.yml b/config/locales/en.yml index b47db12e..16ee61a9 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -53,3 +53,10 @@ en: evaluation_criterion_unique_title_in_form_error: "Evaluation criteria title must be unique within the same form." evaluation_form_criteria_weight_total_error: "The total weight of all evaluation criteria must add up to 100 when weighted scoring is enabled." evaluation_criteria_form_title_placeholder: "Add criteria title here" + evaluator_submissions: + unassign: + success: "Evaluator unassigned successfully" + failure: "Failed to unassign evaluator" + reassign: + success: "Evaluator reassigned successfully" + failure: "Failed to reassign evaluator" From 8775c822c25d2e2080c72315153397505c7e224f Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:13:00 -0600 Subject: [PATCH 16/38] 179 | Update routes --- config/routes.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/config/routes.rb b/config/routes.rb index 402a6d7a..b6d71289 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -27,14 +27,10 @@ post 'resend_invite' end end - resources :phases, only: [] do - resources :evaluator_submissions, only: [:index] do - member do - post 'unassign' - post 'reassign' - end + resources :evaluator_submission_assignments, only: [:index, :update] do + collection do + patch '', to: 'evaluator_submission_assignments#update' end - get 'evaluator_submissions/:evaluator_id', to: 'evaluator_submissions#index', as: :evaluator_submissions_for_evaluator end end resources :submissions, only: [:index, :show, :update] From a68e0cb9c85fa487dbb3a29cccf45b66ac838b14 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:15:24 -0600 Subject: [PATCH 17/38] 179 | Update view and use partials --- .../_submission_row.html.erb | 21 +++ .../_submission_summary.html.erb | 42 +++++ .../_submission_table.html.erb | 15 ++ ...assign_evaluator_submission_modal.html.erb | 10 -- .../_unassigned_submission_row.html.erb | 12 ++ .../index.html.erb | 44 ++++++ .../evaluator_submissions/index.html.erb | 145 ------------------ 7 files changed, 134 insertions(+), 155 deletions(-) create mode 100644 app/views/evaluator_submission_assignments/_submission_row.html.erb create mode 100644 app/views/evaluator_submission_assignments/_submission_summary.html.erb create mode 100644 app/views/evaluator_submission_assignments/_submission_table.html.erb rename app/views/{evaluator_submissions => evaluator_submission_assignments}/_unassign_evaluator_submission_modal.html.erb (80%) create mode 100644 app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb create mode 100644 app/views/evaluator_submission_assignments/index.html.erb delete mode 100644 app/views/evaluator_submissions/index.html.erb diff --git a/app/views/evaluator_submission_assignments/_submission_row.html.erb b/app/views/evaluator_submission_assignments/_submission_row.html.erb new file mode 100644 index 00000000..48658c2c --- /dev/null +++ b/app/views/evaluator_submission_assignments/_submission_row.html.erb @@ -0,0 +1,21 @@ + + <%= assignment.submission.id %> + + <%= assignment.status.titleize %> + + N/A<%# display_score(assignment) %> + +
+ <% if assignment.status == "completed" %> + <%= link_to "View Evaluation", submissions_path(@challenge), class: 'usa-button font-body-3xs margin-right-1' %> + <% end %> + <%= button_tag "Unassign", type: 'button', class: 'usa-button usa-button--outline font-body-3xs', data: { + action: "click->unassign-evaluator-submission-modal#open", + submission_id: assignment.submission.id, + evaluator_id: evaluator.id, + phase_id: @phase.id, + status: :unassigned + } %> +
+ + diff --git a/app/views/evaluator_submission_assignments/_submission_summary.html.erb b/app/views/evaluator_submission_assignments/_submission_summary.html.erb new file mode 100644 index 00000000..50fb90c9 --- /dev/null +++ b/app/views/evaluator_submission_assignments/_submission_summary.html.erb @@ -0,0 +1,42 @@ +
+
+
+
+
+
+ <%= @assigned_submissions.count %> +
+
+

+ Assigned Submissions +

+

+ Evaluations due by <%= @phase.end_date.strftime('%m/%d/%Y') %> +

+
+
+
+ +
+ +
+
+ <%= @submissions_count["completed"] || 0 %>
+ Completed +
+
+ <%= @submissions_count["in_progress"] || 0 %>
+ In Progress +
+
+ <%= @submissions_count["not_started"] || 0 %>
+ Not Started +
+
+
+
+
diff --git a/app/views/evaluator_submission_assignments/_submission_table.html.erb b/app/views/evaluator_submission_assignments/_submission_table.html.erb new file mode 100644 index 00000000..4ac1d454 --- /dev/null +++ b/app/views/evaluator_submission_assignments/_submission_table.html.erb @@ -0,0 +1,15 @@ +
+ + + + + + + + + + + <%= yield %> + +
Submission IDEvaluation statusScore
+
diff --git a/app/views/evaluator_submissions/_unassign_evaluator_submission_modal.html.erb b/app/views/evaluator_submission_assignments/_unassign_evaluator_submission_modal.html.erb similarity index 80% rename from app/views/evaluator_submissions/_unassign_evaluator_submission_modal.html.erb rename to app/views/evaluator_submission_assignments/_unassign_evaluator_submission_modal.html.erb index 17320c1b..2700509f 100644 --- a/app/views/evaluator_submissions/_unassign_evaluator_submission_modal.html.erb +++ b/app/views/evaluator_submission_assignments/_unassign_evaluator_submission_modal.html.erb @@ -34,15 +34,5 @@
- diff --git a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb new file mode 100644 index 00000000..92dde7c3 --- /dev/null +++ b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb @@ -0,0 +1,12 @@ + + <%= assignment.submission.id %> + + <%= assignment.status.titleize %> + + N/A<%# display_score(assignment) %> + +
+ <%= button_to "Reassign", phase_evaluator_submission_assignment_path(phase, assignment.submission, evaluator_id: evaluator.id, submission_id: assignment.submission.id, status: :not_started), method: :patch, class: 'usa-button usa-button--outline font-body-3xs' %> +
+ + diff --git a/app/views/evaluator_submission_assignments/index.html.erb b/app/views/evaluator_submission_assignments/index.html.erb new file mode 100644 index 00000000..4b202055 --- /dev/null +++ b/app/views/evaluator_submission_assignments/index.html.erb @@ -0,0 +1,44 @@ +
+ + <%= render 'shared/back_link', path: phase_evaluators_path(@phase) %> + +

<%= @challenge.title %> - <%= @phase.title %>

+

View submissions assigned to an evalutor.

+ +

Evaluator: <%= "#{@evaluator.first_name} #{@evaluator.last_name}" %>

+ +

Assigned Submissions

+

<%= t('evaluator_submission_assignments.assigned_submissions') %>

+ + <%= render 'submission_summary', assigned_count: @assigned_submissions.count, + submissions_count: @submissions_count, + phase_end_date: @phase.end_date %> + + <% if @assigned_submissions.any? %> + <%= render layout: 'submission_table' do %> + <% @assigned_submissions.each do |assignment| %> + <%= render 'submission_row', assignment: assignment, evaluator: @evaluator %> + <% end %> + <% end %> + <% else %> +

+ <%= t('evaluator_submission_assignments.empty_state') %> +

+ <% end %> + + <% if @unassigned_submissions.any? %> +

Unassigned Submissions

+

<%= t('evaluator_submission_assignments.unassigned_submissions') %>

+ + <%= render layout: 'submission_table' do %> + <% @unassigned_submissions.each do |assignment| %> + <%= render 'unassigned_submission_row', assignment: assignment, evaluator: @evaluator, phase: @phase %> + <% end %> + <% end %> + <% end %> + + <%= render 'unassign_evaluator_submission_modal' %> +
diff --git a/app/views/evaluator_submissions/index.html.erb b/app/views/evaluator_submissions/index.html.erb deleted file mode 100644 index 144dc576..00000000 --- a/app/views/evaluator_submissions/index.html.erb +++ /dev/null @@ -1,145 +0,0 @@ -
- -
- <%= link_to challenge_manage_evaluators_path, class: "usa-link display-inline-flex flex-align-center" do %> - <%= image_tag('images/usa-icons/arrow_back.svg', class: "usa-icon--size-3", alt: "Back to previous page") %> - Back - <% end %> -
- -

<%= @challenge.title %> - <%= @phase.title %>

-

View submissions assigned to an evalutor.

- -

Evaluator: <%= "#{@evaluator.first_name} #{@evaluator.last_name}" %>

- -

Assigned Submissions

-

A list of submissions assigned to the user.

- -
-
-
-
-
-
- <%= @assigned_submissions.count %> -
-
-

- Assigned Submissions -

-

- Evaluations due by <%= @phase.end_date.strftime('%m/%d/%Y') %> -

-
-
-
- -
- -
-
- <%= @submissions_count["completed"] || 0 %>
- Completed -
-
- <%= @submissions_count["in_progress"] || 0 %>
- In Progress -
-
- <%= @submissions_count["not_started"] || 0 %>
- Not Started -
-
-
-
-
- - <% if @assigned_submissions.any? %> -
- - - - - - - - - - - <% @assigned_submissions.each do |assignment| %> - - - - - - - <% end %> - -
Submission IDEvaluation statusScore
<%= assignment.submission.id %> - <%= assignment.status.titleize %> - 80<%# assignment.score || 'N/A' %> -
- <% if assignment.status == "completed" %> - <%= link_to manage_submissions_path(@challenge), class: 'usa-button font-body-3xs margin-right-1', style: 'white-space: nowrap;' do %> - View Evaluation - <% end %> - <% end %> - <%= button_tag "Unassign", type: 'button', class: 'usa-button usa-button--outline font-body-3xs', data: { - action: "click->unassign-evaluator-submission-modal#open", - submission_id: assignment.submission.id, - evaluator_id: @evaluator.id, - challenge_id: @challenge.id, - phase_id: @phase.id - } %> -
-
-
- <% else %> -
-

There currently are no assigned submissions to this evaluator. Please assigned submissions to this evaluator to view.

-
- <% end %> - - <% if @unassigned_submissions.any? %> -

Unassigned Submissions

-

A list of recused and unassigned submissions. Reassigning a user to a submission will make the submission available for the user to evaluate.

- -
- - - - - - - - - - - <% @unassigned_submissions.each do |assignment| %> - - - - - - - <% end %> - -
Submission IDEvaluation statusScore
<%= assignment.submission.id %> - <%= assignment.status.titleize %> - 80<%# assignment.score || 'N/A' %> -
- <%= button_to "Reassign", reassign_challenge_phase_evaluator_submission_path(@challenge, @phase, assignment.submission, evaluator_id: @evaluator.id), method: :post, class: 'usa-button usa-button--outline font-body-3xs', data: { turbo: false } %> -
-
-
- <% end %> - - <%= render 'unassign_evaluator_submission_modal' %> -
From 5c5ad7e16918d51cee19564870bef48f64f315e4 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:16:11 -0600 Subject: [PATCH 18/38] 179 | Use update for reassign & unassign --- ...uator_submission_assignments_controller.rb | 52 +++++++++ .../evaluator_submissions_controller.rb | 60 ---------- .../evaluator_submissions_controller_spec.rb | 90 --------------- .../evaluator_submission_assignments_spec.rb | 104 ++++++++++++++++++ 4 files changed, 156 insertions(+), 150 deletions(-) create mode 100644 app/controllers/evaluator_submission_assignments_controller.rb delete mode 100644 app/controllers/evaluator_submissions_controller.rb delete mode 100644 spec/controllers/evaluator_submissions_controller_spec.rb create mode 100644 spec/requests/evaluator_submission_assignments_spec.rb diff --git a/app/controllers/evaluator_submission_assignments_controller.rb b/app/controllers/evaluator_submission_assignments_controller.rb new file mode 100644 index 00000000..e0bcfc80 --- /dev/null +++ b/app/controllers/evaluator_submission_assignments_controller.rb @@ -0,0 +1,52 @@ +# frozen_string_literal: true + +class EvaluatorSubmissionAssignmentsController < ApplicationController + before_action :set_challenge_and_phase + before_action :set_evaluator, only: [:index] + before_action :set_assignment, only: [:update] + + def index + @evaluator_assignments = @phase.evaluator_submission_assignments + .includes(:submission) + .where(user_id: @evaluator.id) + @assigned_submissions = @evaluator_assignments.where(status: [:completed, :in_progress, :not_started, :recused]) + .ordered_by_status + @unassigned_submissions = @evaluator_assignments.where(status: [:unassigned, :recused_unassigned]) + .ordered_by_status + @submissions_count = @assigned_submissions.group(:status).count + end + + # update only the status of the evaluation submission assignment to unassign or reassign an evaluator + def update + new_status = params[:status]&.to_sym + + if @assignment.update(status: EvaluatorSubmissionAssignment.statuses[new_status]) + flash[:success] = t("evaluator_submission_assignments.#{new_status}.success") + respond_to do |format| + format.html { redirect_to phase_evaluator_submission_assignments_path(@phase, evaluator_id: params[:evaluator_id]) } + format.json { render json: { success: true, message: flash[:success] } } + end + else + flash[:error] = t("evaluator_submission_assignments.#{new_status}.failure") + respond_to do |format| + format.html { redirect_to phase_evaluator_submission_assignments_path(@phase, evaluator_id: params[:evaluator_id]) } + format.json { render json: { success: false, message: flash[:error] }, status: :unprocessable_entity } + end + end + end + + private + + def set_challenge_and_phase + @phase = Phase.where(challenge: current_user.challenge_manager_challenges).find(params[:phase_id]) + @challenge = @phase.challenge + end + + def set_evaluator + @evaluator = params[:evaluator_id] ? User.find(params[:evaluator_id]) : current_user + end + + def set_assignment + @assignment = EvaluatorSubmissionAssignment.find_by!(user_id: params[:evaluator_id], submission_id: params[:submission_id]) + end +end diff --git a/app/controllers/evaluator_submissions_controller.rb b/app/controllers/evaluator_submissions_controller.rb deleted file mode 100644 index 3e13b7ac..00000000 --- a/app/controllers/evaluator_submissions_controller.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -class EvaluatorSubmissionsController < ApplicationController - before_action :set_challenge_and_phase - before_action :set_evaluator, only: [:index] - before_action :set_assignment, only: [:unassign, :reassign] - - def index - @evaluator_assignments = @phase.evaluator_submission_assignments - .includes(:submission) - .where(user_id: @evaluator.id) - - @assigned_submissions = @evaluator_assignments.where.not(status: :unassigned) # includes completed, in_progress, not_started, and recused - .ordered_by_status - @unassigned_submissions = @evaluator_assignments.where(status: :unassigned) - - @submissions_count = @assigned_submissions.group(:status).count - end - - def unassign - if @assignment.update(status: :unassigned) - flash[:success] = t('evaluator_submissions.unassign.success') - render json: { success: true, message: flash[:success] } - else - flash[:error] = t('evaluator_submissions.unassign.failure') - render json: { success: false, message: flash[:error] }, status: :unprocessable_entity - end - end - - def reassign - if @assignment.update(status: :not_started) - flash[:success] = t('evaluator_submissions.reassign.success') - else - flash[:error] = t('evaluator_submissions.reassign.failure') + ": #{@assignment.errors.full_messages.join(', ')}" - end - - redirect_to challenge_phase_evaluator_submissions_path(@challenge, @phase, evaluator_id: params[:evaluator_id]) - end - - private - - def set_challenge_and_phase - @challenge = Challenge.find(params[:challenge_id]) - @phase = @challenge.phases.find(params[:phase_id]) - end - - def set_evaluator - @evaluator = if params[:evaluator_id] - User.find(params[:evaluator_id]) - else - current_user - end - end - - def set_assignment - @assignment = EvaluatorSubmissionAssignment.joins(:submission) - .where(submissions: { phase_id: @phase.id }) - .find_by!(submission_id: params[:id], user_id: params[:evaluator_id]) - end -end diff --git a/spec/controllers/evaluator_submissions_controller_spec.rb b/spec/controllers/evaluator_submissions_controller_spec.rb deleted file mode 100644 index 5e951b6f..00000000 --- a/spec/controllers/evaluator_submissions_controller_spec.rb +++ /dev/null @@ -1,90 +0,0 @@ -require 'rails_helper' - -RSpec.describe EvaluatorSubmissionsController, type: :request do - let(:challenge_manager) { create_and_log_in_user(role: 'challenge_manager') } - let(:challenge) { create(:challenge) } - let(:phase) { create(:phase, challenge: challenge) } - let(:evaluator) { create(:user, role: 'evaluator') } - let(:submission) { create(:submission, challenge: challenge, phase: phase) } - let!(:assignment) { create(:evaluator_submission_assignment, submission: submission, evaluator: evaluator, status: :not_started) } - - before do - ChallengeManager.create(user: challenge_manager, challenge: challenge) - end - - describe 'GET #index' do - before do - get challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id) - end - - it 'assigns @evaluator_assignments' do - expect(assigns(:evaluator_assignments)).to include(assignment) - end - - it 'assigns @assigned_submissions' do - expect(assigns(:assigned_submissions)).to include(assignment) - end - - it 'assigns @unassigned_submissions' do - expect(assigns(:unassigned_submissions)).to be_empty - end - - it 'assigns @submissions_count' do - expect(assigns(:submissions_count)).to eq({ "not_started" => 1 }) - end - - it 'renders the index template' do - expect(response).to render_template(:index) - end - end - - describe 'POST #unassign' do - it 'unassigns the evaluator successfully and updates counts' do - expect { - post unassign_challenge_phase_evaluator_submission_path(challenge, phase, submission, evaluator_id: evaluator.id) - }.to change { assignment.reload.status }.from('not_started').to('unassigned') - - expect(response).to have_http_status(:success) - expect(JSON.parse(response.body)['success']).to be true - - get challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id) - expect(assigns(:assigned_submissions)).to be_empty - expect(assigns(:unassigned_submissions)).to include(assignment) - expect(assigns(:submissions_count)).to eq({}) - end - - it 'handles failure to unassign' do - allow_any_instance_of(EvaluatorSubmissionAssignment).to receive(:update).and_return(false) - post unassign_challenge_phase_evaluator_submission_path(challenge, phase, submission, evaluator_id: evaluator.id) - expect(response).to have_http_status(:unprocessable_entity) - expect(JSON.parse(response.body)['success']).to be false - end - end - - describe 'POST #reassign' do - before do - assignment.update(status: :unassigned) - end - - it 'reassigns the evaluator successfully and updates counts' do - expect { - post reassign_challenge_phase_evaluator_submission_path(challenge, phase, submission, evaluator_id: evaluator.id) - }.to change { assignment.reload.status }.from('unassigned').to('not_started') - - expect(flash[:success]).to eq('Evaluator reassigned successfully') - expect(response).to redirect_to(challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id)) - - get challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id) - expect(assigns(:assigned_submissions)).to include(assignment) - expect(assigns(:unassigned_submissions)).to be_empty - expect(assigns(:submissions_count)).to eq({ "not_started" => 1 }) - end - - it 'handles failure to reassign' do - allow_any_instance_of(EvaluatorSubmissionAssignment).to receive(:update).and_return(false) - post reassign_challenge_phase_evaluator_submission_path(challenge, phase, submission, evaluator_id: evaluator.id) - expect(flash[:error]).to include('Failed to reassign evaluator') - expect(response).to redirect_to(challenge_phase_evaluator_submissions_path(challenge, phase, evaluator_id: evaluator.id)) - end - end -end diff --git a/spec/requests/evaluator_submission_assignments_spec.rb b/spec/requests/evaluator_submission_assignments_spec.rb new file mode 100644 index 00000000..794f6f10 --- /dev/null +++ b/spec/requests/evaluator_submission_assignments_spec.rb @@ -0,0 +1,104 @@ +require 'rails_helper' + +RSpec.describe EvaluatorSubmissionAssignmentsController, type: :request do + let(:challenge_manager) { create_and_log_in_user(role: 'challenge_manager') } + let(:challenge) { create(:challenge) } + let(:phase) { create(:phase, challenge: challenge) } + let(:evaluator) { create(:user, role: 'evaluator') } + let(:submission) { create(:submission, challenge: challenge, phase: phase) } + let!(:not_started_assignment) do + create(:evaluator_submission_assignment, + submission: submission, + evaluator: evaluator, + status: :not_started) + end + let!(:unassigned_assignment) do + create(:evaluator_submission_assignment, + submission: create(:submission, challenge: challenge, phase: phase), + evaluator: evaluator, + status: :unassigned) + end + + before do + ChallengeManager.create(user: challenge_manager, challenge: challenge) + end + + describe 'GET #index' do + before do + get phase_evaluator_submission_assignments_path(phase, evaluator_id: evaluator.id) + end + + it 'assigns @evaluator_assignments' do + expect(assigns(:evaluator_assignments)).to include(not_started_assignment, unassigned_assignment) + end + + it 'assigns @assigned_submissions' do + expect(assigns(:assigned_submissions)).to include(not_started_assignment) + expect(assigns(:assigned_submissions)).not_to include(unassigned_assignment) + end + + it 'assigns @unassigned_submissions' do + expect(assigns(:unassigned_submissions)).to include(unassigned_assignment) + expect(assigns(:unassigned_submissions)).not_to include(not_started_assignment) + end + + it 'assigns @submissions_count' do + expect(assigns(:submissions_count)).to eq({ "not_started" => 1 }) + end + end + + describe 'PATCH #update' do + context 'when reassigning' do + it 'reassigns the evaluator successfully and updates counts' do + + patch phase_evaluator_submission_assignments_path(phase, + evaluator_id: evaluator.id, + submission_id: unassigned_assignment.submission_id, + status: :not_started) + + expect(unassigned_assignment.reload.status).to eq('not_started') + expect(flash[:success]).to eq(I18n.t('evaluator_submission_assignments.not_started.success')) + expect(response).to redirect_to(phase_evaluator_submission_assignments_path(phase, evaluator_id: evaluator.id)) + end + + it 'fails to reassign when the assignment is invalid' do + allow_any_instance_of(EvaluatorSubmissionAssignment).to receive(:update).and_return(false) + + patch phase_evaluator_submission_assignments_path(phase, + evaluator_id: evaluator.id, + submission_id: unassigned_assignment.submission_id, + status: :not_started) + + expect(unassigned_assignment.reload.status).to eq('unassigned') + expect(flash[:error]).to eq(I18n.t('evaluator_submission_assignments.not_started.failure')) + expect(response).to redirect_to(phase_evaluator_submission_assignments_path(phase, evaluator_id: evaluator.id)) + end + end + + context 'when unassigning' do + it 'unassigns the evaluator successfully' do + patch phase_evaluator_submission_assignments_path(phase, + evaluator_id: evaluator.id, + submission_id: not_started_assignment.submission_id, + status: :unassigned) + + expect(not_started_assignment.reload.status).to eq('unassigned') + expect(flash[:success]).to eq(I18n.t('evaluator_submission_assignments.unassigned.success')) + expect(response).to redirect_to(phase_evaluator_submission_assignments_path(phase, evaluator_id: evaluator.id)) + end + + it 'fails to unassign when the assignment is invalid' do + allow_any_instance_of(EvaluatorSubmissionAssignment).to receive(:update).and_return(false) + + patch phase_evaluator_submission_assignments_path(phase, + evaluator_id: evaluator.id, + submission_id: not_started_assignment.submission_id, + status: :unassigned) + + expect(not_started_assignment.reload.status).to eq('not_started') + expect(flash[:error]).to eq(I18n.t('evaluator_submission_assignments.unassigned.failure')) + expect(response).to redirect_to(phase_evaluator_submission_assignments_path(phase, evaluator_id: evaluator.id)) + end + end + end +end From e86b23ed8137aee820ed810455b3b8885fcb4a65 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:16:48 -0600 Subject: [PATCH 19/38] 179 | Add recused_unassigned status --- app/helpers/evaluators_helper.rb | 11 +++++++++++ app/models/evaluator_submission_assignment.rb | 5 +++-- app/models/phase.rb | 4 ---- spec/helpers/evaluators_helper_spec.rb | 12 ++++++------ 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/app/helpers/evaluators_helper.rb b/app/helpers/evaluators_helper.rb index 6b941f42..bb16dd9f 100644 --- a/app/helpers/evaluators_helper.rb +++ b/app/helpers/evaluators_helper.rb @@ -14,6 +14,7 @@ def assigned_submissions_count(evaluator, challenge, phase) evaluator.evaluator_submission_assignments. joins(:submission). where(submissions: { challenge:, phase: }). + where.not(status: [:unassigned, :recused_unassigned]). count else 0 @@ -32,8 +33,18 @@ def evaluation_status(status) 'text-green' when :unassigned 'text-accent-cool-darker' + when :recused_unassigned + 'text-secondary' else 'text-base' end end + + # TODO: Display score for the evaluation submission assignment after EvaluationScore is added + # def display_score(assignment, evaluator_id) + # evaluation = Evaluation.find_by(evaluator_submission_assignment: assignment, user_id: evaluator_id) + # score = evaluation&.evaluation_scores&.effective_score + + # evaluation&.completed? && score ? score : 'N/A' + # end end diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index 3d1ec982..297a1171 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -12,10 +12,11 @@ class EvaluatorSubmissionAssignment < ApplicationRecord recused: 2, not_started: 3, in_progress: 4, - completed: 5 + completed: 5, + recused_unassigned: 6 } - STATUS_ORDER = [:recused, :not_started, :in_progress, :completed] + STATUS_ORDER = [:recused, :unassigned, :recused_unassigned, :not_started, :in_progress, :completed] scope :ordered_by_status, -> { order(Arel.sql( diff --git a/app/models/phase.rb b/app/models/phase.rb index 24c8ca27..601ea989 100644 --- a/app/models/phase.rb +++ b/app/models/phase.rb @@ -49,8 +49,4 @@ class Phase < ApplicationRecord # Validations validates :title, :start_date, :end_date, presence: true - - def submissions_count - evaluator_submission_assignments.select(:submission_id).distinct.count - end end diff --git a/spec/helpers/evaluators_helper_spec.rb b/spec/helpers/evaluators_helper_spec.rb index cd95919b..bc08793b 100644 --- a/spec/helpers/evaluators_helper_spec.rb +++ b/spec/helpers/evaluators_helper_spec.rb @@ -10,10 +10,9 @@ let(:submission) { create(:submission, challenge: challenge, phase: phase) } it 'returns the correct count of assigned submissions' do - create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission) - create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission) - create(:evaluator_submission_assignment, evaluator: evaluator, - submission: create(:submission, challenge: challenge, phase: phase)) + create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission, status: :not_started) + create(:evaluator_submission_assignment, evaluator: evaluator, submission: create(:submission, challenge: challenge, phase: phase), status: :in_progress) + create(:evaluator_submission_assignment, evaluator: evaluator, submission: create(:submission, challenge: challenge, phase: phase), status: :completed) expect(helper.assigned_submissions_count(evaluator, challenge, phase)).to eq(3) end @@ -27,9 +26,10 @@ end it 'only counts submissions for the specified challenge and phase' do - create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission) + create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission, status: :not_started) create(:evaluator_submission_assignment, evaluator: evaluator, - submission: create(:submission, challenge: create(:challenge), phase: create(:phase))) + submission: create(:submission, challenge: create(:challenge), phase: create(:phase)), + status: :not_started) expect(helper.assigned_submissions_count(evaluator, challenge, phase)).to eq(1) end From fbe433a0b57abc42d523f42a3541a80f9550725f Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:17:20 -0600 Subject: [PATCH 20/38] 179 | Back link partial --- app/views/evaluators/index.html.erb | 9 ++------- app/views/shared/_back_link.html.erb | 6 ++++++ 2 files changed, 8 insertions(+), 7 deletions(-) create mode 100644 app/views/shared/_back_link.html.erb diff --git a/app/views/evaluators/index.html.erb b/app/views/evaluators/index.html.erb index 365f6576..db59a1a3 100644 --- a/app/views/evaluators/index.html.erb +++ b/app/views/evaluators/index.html.erb @@ -1,11 +1,6 @@
-
- <%= link_to phases_path, class: "usa-link display-inline-flex flex-align-center" do %> - <%= image_tag('images/usa-icons/arrow_back.svg', class: "usa-icon--size-3", alt: "Back to previous page") %> - Back - <% end %> -
+ <%= render 'shared/back_link', path: phases_path(@phase) %>

<%= @challenge.title %> - <%= @phase.title %>

Create and manage a list of evaluators for the challenge.

@@ -75,7 +70,7 @@ <%= assigned_submissions_count(evaluator, @challenge, @phase) %>
- <%= link_to phases_path(@challenge, @phase, evaluator_id: evaluator.id), class: 'usa-button font-body-3xs', style: 'white-space: nowrap;' do %> + <%= link_to phase_evaluator_submission_assignments_path(@phase, evaluator_id: evaluator.id), class: 'usa-button font-body-3xs', style: 'white-space: nowrap;' do %> View Submissions <% end %> <%= button_tag "Delete", type: 'button', class: 'usa-button usa-button--outline font-body-3xs', data: { diff --git a/app/views/shared/_back_link.html.erb b/app/views/shared/_back_link.html.erb new file mode 100644 index 00000000..ebd39677 --- /dev/null +++ b/app/views/shared/_back_link.html.erb @@ -0,0 +1,6 @@ +
+ <%= link_to path, class: "usa-link display-inline-flex flex-align-center" do %> + <%= image_tag('images/usa-icons/arrow_back.svg', class: "usa-icon--size-3", alt: "Back to previous page") %> + <%= 'Back' %> + <% end %> +
\ No newline at end of file From 3ff00893ac2b1389790413f5609d222fd1a63ceb Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:17:57 -0600 Subject: [PATCH 21/38] 179 | Adjust js to use patch update on status reassign/unassign --- ...n_evaluator_submission_modal_controller.js | 44 ++++++++----------- 1 file changed, 19 insertions(+), 25 deletions(-) diff --git a/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js b/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js index 9429427e..c7ac2fc2 100644 --- a/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js +++ b/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js @@ -3,7 +3,6 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { static targets = ["modal", "confirmButton"] static values = { - challengeId: String, phaseId: String, submissionId: String, evaluatorId: String @@ -18,12 +17,11 @@ export default class extends Controller { } open(event) { - event.preventDefault() - this.submissionIdValue = event.currentTarget.dataset.submissionId - this.evaluatorIdValue = event.currentTarget.dataset.evaluatorId - this.challengeIdValue = event.currentTarget.dataset.challengeId - this.phaseIdValue = event.currentTarget.dataset.phaseId - this.modalTarget.showModal() + event.preventDefault(); + this.submissionIdValue = event.currentTarget.dataset.submissionId; + this.evaluatorIdValue = event.currentTarget.dataset.evaluatorId; + this.phaseIdValue = event.currentTarget.dataset.phaseId; + this.modalTarget.showModal(); } close() { @@ -41,35 +39,31 @@ export default class extends Controller { } unassignEvaluatorSubmission() { - const csrfToken = document.querySelector('meta[name="csrf-token"]').content - - fetch(`/challenges/${this.challengeIdValue}/phases/${this.phaseIdValue}/evaluator_submissions/${this.submissionIdValue}/unassign`, { - method: 'POST', + const csrfToken = document.querySelector('meta[name="csrf-token"]').content; + + fetch(`/phases/${this.phaseIdValue}/evaluator_submission_assignments?evaluator_id=${this.evaluatorIdValue}`, { + method: 'PATCH', headers: { 'Content-Type': 'application/json', - 'X-CSRF-Token': csrfToken + 'X-CSRF-Token': csrfToken, + 'Accept': 'application/json' }, body: JSON.stringify({ - evaluator_id: this.evaluatorIdValue + submission_id: this.submissionIdValue, + status: 'unassigned' }) }) - .then(response => { - if (!response.ok) { - throw new Error('Network response was not ok') - } - return response.json() - }) + .then(response => response.json()) .then(data => { if (data.success) { - this.close() - window.location.reload() + window.location.href = `/phases/${this.phaseIdValue}/evaluator_submission_assignments?evaluator_id=${this.evaluatorIdValue}`; } else { - throw new Error(data.message || 'Failed to unassign evaluator from submission') + throw new Error(data.message || 'Failed to unassign evaluator from submission'); } }) .catch(error => { - console.error('Error:', error) - alert(error.message || 'An error occurred while unassigning the evaluator from the submission') - }) + console.error('Error:', error); + alert(error.message || 'An error occurred while unassigning the evaluator from the submission'); + }); } } From e7882bf290b82e156149750a2384e72c7b0efcc1 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:18:10 -0600 Subject: [PATCH 22/38] 179 | Add to translations --- config/locales/en.yml | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/config/locales/en.yml b/config/locales/en.yml index 6d4bbcd4..524a7a3c 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -63,13 +63,16 @@ en: evaluation_criterion_unique_title_in_form_error: "Evaluation criteria title must be unique within the same form." evaluation_form_criteria_weight_total_error: "The total weight of all evaluation criteria must add up to 100 when weighted scoring is enabled." evaluation_criteria_form_title_placeholder: "Add criteria title here" - evaluator_submissions: - unassign: - success: "Evaluator unassigned successfully" - failure: "Failed to unassign evaluator" - reassign: + evaluator_submission_assignments: + assigned_submissions: "A list of submissions assigned to the user." + unassigned_submissions: "A list of recused and unassigned submissions. Reassigning a user to a submission will make the submission available for the user to evaluate." + empty_state: "There currently are no assigned submissions to this evaluator. Please assign submissions to this evaluator to view." + not_started: success: "Evaluator reassigned successfully" - failure: "Failed to reassign evaluator" + failure: "Failed to reassign evaluator" + unassigned: + success: "Evaluator unassigned successfully" + failure: "Failed to unassign evaluator" alerts: recused_evaluator: heading: "Recused Evaluator" From 9c89936f51968e3d45f285554529019f04edb21f Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:38:45 -0600 Subject: [PATCH 23/38] Small updates --- ...uator_submission_assignments_controller.rb | 64 +++++++++++++------ ...n_evaluator_submission_modal_controller.js | 11 +++- app/models/evaluator_submission_assignment.rb | 20 +++--- 3 files changed, 63 insertions(+), 32 deletions(-) diff --git a/app/controllers/evaluator_submission_assignments_controller.rb b/app/controllers/evaluator_submission_assignments_controller.rb index e0bcfc80..ab89070b 100644 --- a/app/controllers/evaluator_submission_assignments_controller.rb +++ b/app/controllers/evaluator_submission_assignments_controller.rb @@ -6,32 +6,26 @@ class EvaluatorSubmissionAssignmentsController < ApplicationController before_action :set_assignment, only: [:update] def index - @evaluator_assignments = @phase.evaluator_submission_assignments - .includes(:submission) - .where(user_id: @evaluator.id) - @assigned_submissions = @evaluator_assignments.where(status: [:completed, :in_progress, :not_started, :recused]) - .ordered_by_status - @unassigned_submissions = @evaluator_assignments.where(status: [:unassigned, :recused_unassigned]) - .ordered_by_status - @submissions_count = @assigned_submissions.group(:status).count + @evaluator_assignments = @phase.evaluator_submission_assignments. + includes(:submission). + where(user_id: @evaluator.id) + @assigned_submissions = @evaluator_assignments. + where(status: %i[completed in_progress not_started recused]). + ordered_by_status + @unassigned_submissions = @evaluator_assignments. + where(status: %i[:unassigned, :recused_unassigned]). + ordered_by_status + @submissions_count = @assigned_submissions.group('evaluator_submission_assignments.status').count end # update only the status of the evaluation submission assignment to unassign or reassign an evaluator def update new_status = params[:status]&.to_sym - if @assignment.update(status: EvaluatorSubmissionAssignment.statuses[new_status]) - flash[:success] = t("evaluator_submission_assignments.#{new_status}.success") - respond_to do |format| - format.html { redirect_to phase_evaluator_submission_assignments_path(@phase, evaluator_id: params[:evaluator_id]) } - format.json { render json: { success: true, message: flash[:success] } } - end + if update_assignment_status(new_status) + handle_successful_update(new_status) else - flash[:error] = t("evaluator_submission_assignments.#{new_status}.failure") - respond_to do |format| - format.html { redirect_to phase_evaluator_submission_assignments_path(@phase, evaluator_id: params[:evaluator_id]) } - format.json { render json: { success: false, message: flash[:error] }, status: :unprocessable_entity } - end + handle_failed_update(new_status) end end @@ -47,6 +41,36 @@ def set_evaluator end def set_assignment - @assignment = EvaluatorSubmissionAssignment.find_by!(user_id: params[:evaluator_id], submission_id: params[:submission_id]) + @assignment = EvaluatorSubmissionAssignment.find_by!( + user_id: params[:evaluator_id], + submission_id: params[:submission_id] + ) + end + + def update_assignment_status(new_status) + @assignment.update(status: EvaluatorSubmissionAssignment.statuses[new_status]) + end + + def handle_successful_update(new_status) + flash[:success] = t("evaluator_submission_assignments.#{new_status}.success") + respond_to do |format| + format.html { redirect_to_assignment_path } + format.json { render json: { success: true, message: flash[:success] } } + end + end + + def handle_failed_update(new_status) + flash[:error] = t("evaluator_submission_assignments.#{new_status}.failure") + respond_to do |format| + format.html { redirect_to_assignment_path } + format.json { render json: { success: false, message: flash[:error] }, status: :unprocessable_entity } + end + end + + def redirect_to_assignment_path + redirect_to phase_evaluator_submission_assignments_path( + @phase, + evaluator_id: params[:evaluator_id] + ) end end diff --git a/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js b/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js index c7ac2fc2..526db01d 100644 --- a/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js +++ b/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js @@ -18,12 +18,11 @@ export default class extends Controller { open(event) { event.preventDefault(); - this.submissionIdValue = event.currentTarget.dataset.submissionId; - this.evaluatorIdValue = event.currentTarget.dataset.evaluatorId; - this.phaseIdValue = event.currentTarget.dataset.phaseId; + this.setValues(event.currentTarget.dataset); this.modalTarget.showModal(); } + close() { this.modalTarget.close() } @@ -38,6 +37,12 @@ export default class extends Controller { this.unassignEvaluatorSubmission() } + setValues(dataset) { + this.submissionIdValue = dataset.submissionId; + this.evaluatorIdValue = dataset.evaluatorId; + this.phaseIdValue = dataset.phaseId; + } + unassignEvaluatorSubmission() { const csrfToken = document.querySelector('meta[name="csrf-token"]').content; diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index 297a1171..ca54c0eb 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -16,15 +16,17 @@ class EvaluatorSubmissionAssignment < ApplicationRecord recused_unassigned: 6 } - STATUS_ORDER = [:recused, :unassigned, :recused_unassigned, :not_started, :in_progress, :completed] + STATUS_ORDER = %i[recused unassigned recused_unassigned not_started in_progress completed].freeze - scope :ordered_by_status, -> { - order(Arel.sql( - "CASE " + - STATUS_ORDER.map.with_index { |status, index| - "WHEN evaluator_submission_assignments.status = #{statuses[status]} THEN #{index + 1}" - }.join(" ") + - " ELSE #{STATUS_ORDER.length + 1} END" - )) + scope :ordered_by_status, lambda { + order( + Arel.sql( + [ + "CASE status", + *STATUS_ORDER.map.with_index { |status, index| "WHEN #{statuses[status]} THEN #{index}" }, + "ELSE #{STATUS_ORDER.length} END" + ].join(" ") + ) + ) } end From 01b132f9e7eab97aca9ab124f427c56653a407a1 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 12:41:17 -0600 Subject: [PATCH 24/38] Small fix --- app/controllers/evaluator_submission_assignments_controller.rb | 2 +- app/models/evaluator_submission_assignment.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/evaluator_submission_assignments_controller.rb b/app/controllers/evaluator_submission_assignments_controller.rb index ab89070b..c257ebd4 100644 --- a/app/controllers/evaluator_submission_assignments_controller.rb +++ b/app/controllers/evaluator_submission_assignments_controller.rb @@ -13,7 +13,7 @@ def index where(status: %i[completed in_progress not_started recused]). ordered_by_status @unassigned_submissions = @evaluator_assignments. - where(status: %i[:unassigned, :recused_unassigned]). + where(status: %i[unassigned recused_unassigned]). ordered_by_status @submissions_count = @assigned_submissions.group('evaluator_submission_assignments.status').count end diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index ca54c0eb..c0d37a7e 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -22,7 +22,7 @@ class EvaluatorSubmissionAssignment < ApplicationRecord order( Arel.sql( [ - "CASE status", + "CASE evaluator_submission_assignments.status", *STATUS_ORDER.map.with_index { |status, index| "WHEN #{statuses[status]} THEN #{index}" }, "ELSE #{STATUS_ORDER.length} END" ].join(" ") From 1a6531ef15f5bd272a2071a64325042e3af9ae07 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 13:06:53 -0600 Subject: [PATCH 25/38] Update order by status on evaluation submission assignments --- app/models/evaluator_submission_assignment.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index c0d37a7e..c4834787 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class EvaluatorSubmissionAssignment < ApplicationRecord + include ActiveRecord::Sanitization + belongs_to :submission belongs_to :evaluator, class_name: "User", foreign_key: :user_id, inverse_of: :assigned_submissions @@ -21,11 +23,15 @@ class EvaluatorSubmissionAssignment < ApplicationRecord scope :ordered_by_status, lambda { order( Arel.sql( - [ - "CASE evaluator_submission_assignments.status", - *STATUS_ORDER.map.with_index { |status, index| "WHEN #{statuses[status]} THEN #{index}" }, - "ELSE #{STATUS_ORDER.length} END" - ].join(" ") + sanitize_sql_array( + [ + "CASE evaluator_submission_assignments.status " + + STATUS_ORDER.map.with_index { |status, index| "WHEN ? THEN ?" }.join(" ") + + " ELSE ? END", + *STATUS_ORDER.flat_map { |status| [statuses[status], STATUS_ORDER.index(status)] }, + STATUS_ORDER.length + ] + ) ) ) } From 3da992d0e1486139d9ba07d628935c658cefbb98 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 13:20:23 -0600 Subject: [PATCH 26/38] Remove unused argument --- ...uator_submission_assignments_controller.rb | 4 ++-- app/models/evaluator_submission_assignment.rb | 23 ++++++++----------- 2 files changed, 11 insertions(+), 16 deletions(-) diff --git a/app/controllers/evaluator_submission_assignments_controller.rb b/app/controllers/evaluator_submission_assignments_controller.rb index c257ebd4..141b0f58 100644 --- a/app/controllers/evaluator_submission_assignments_controller.rb +++ b/app/controllers/evaluator_submission_assignments_controller.rb @@ -52,7 +52,7 @@ def update_assignment_status(new_status) end def handle_successful_update(new_status) - flash[:success] = t("evaluator_submission_assignments.#{new_status}.success") + flash.now[:success] = t("evaluator_submission_assignments.#{new_status}.success") respond_to do |format| format.html { redirect_to_assignment_path } format.json { render json: { success: true, message: flash[:success] } } @@ -60,7 +60,7 @@ def handle_successful_update(new_status) end def handle_failed_update(new_status) - flash[:error] = t("evaluator_submission_assignments.#{new_status}.failure") + flash.now[:error] = t("evaluator_submission_assignments.#{new_status}.failure") respond_to do |format| format.html { redirect_to_assignment_path } format.json { render json: { success: false, message: flash[:error] }, status: :unprocessable_entity } diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index c4834787..e3d6f050 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -20,19 +20,14 @@ class EvaluatorSubmissionAssignment < ApplicationRecord STATUS_ORDER = %i[recused unassigned recused_unassigned not_started in_progress completed].freeze - scope :ordered_by_status, lambda { - order( - Arel.sql( - sanitize_sql_array( - [ - "CASE evaluator_submission_assignments.status " + - STATUS_ORDER.map.with_index { |status, index| "WHEN ? THEN ?" }.join(" ") + - " ELSE ? END", - *STATUS_ORDER.flat_map { |status| [statuses[status], STATUS_ORDER.index(status)] }, - STATUS_ORDER.length - ] - ) - ) - ) + scope :ordered_by_status, -> { + order(Arel.sql( + sanitize_sql_array([ + "CASE evaluator_submission_assignments.status #{STATUS_ORDER.map { ' + WHEN ? THEN ?' }.join} ELSE ? END", + *STATUS_ORDER.flat_map { |status| [statuses[status], STATUS_ORDER.index(status)] }, + STATUS_ORDER.length + ]) + )) } end From e541c135013f619025e97f7464aa3e9b6877543f Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 13:25:07 -0600 Subject: [PATCH 27/38] Update ordered by status --- app/models/evaluator_submission_assignment.rb | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index e3d6f050..f29cfd37 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -20,14 +20,15 @@ class EvaluatorSubmissionAssignment < ApplicationRecord STATUS_ORDER = %i[recused unassigned recused_unassigned not_started in_progress completed].freeze - scope :ordered_by_status, -> { - order(Arel.sql( - sanitize_sql_array([ - "CASE evaluator_submission_assignments.status #{STATUS_ORDER.map { ' - WHEN ? THEN ?' }.join} ELSE ? END", - *STATUS_ORDER.flat_map { |status| [statuses[status], STATUS_ORDER.index(status)] }, - STATUS_ORDER.length - ]) - )) + scope :ordered_by_status, lambda { + order( + Arel.sql( + [ + "CASE evaluator_submission_assignments.status", + *STATUS_ORDER.map.with_index { |status, index| "WHEN #{statuses[status]} THEN #{index}" }, + "ELSE #{STATUS_ORDER.length} END" + ].join(" ") + ) + ) } end From 8e207e13b23aafb144a2b6aec1aa972629137fc1 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 15:22:36 -0600 Subject: [PATCH 28/38] 179 | Display score for evaluator submission assignments --- ...uator_submission_assignments_controller.rb | 2 +- app/helpers/evaluators_helper.rb | 14 ++++---- .../_submission_row.html.erb | 2 +- .../_unassigned_submission_row.html.erb | 2 +- spec/helpers/evaluators_helper_spec.rb | 34 ++++++++++++++++--- 5 files changed, 40 insertions(+), 14 deletions(-) diff --git a/app/controllers/evaluator_submission_assignments_controller.rb b/app/controllers/evaluator_submission_assignments_controller.rb index 141b0f58..2a51a864 100644 --- a/app/controllers/evaluator_submission_assignments_controller.rb +++ b/app/controllers/evaluator_submission_assignments_controller.rb @@ -7,7 +7,7 @@ class EvaluatorSubmissionAssignmentsController < ApplicationController def index @evaluator_assignments = @phase.evaluator_submission_assignments. - includes(:submission). + includes(:submission, :evaluation). where(user_id: @evaluator.id) @assigned_submissions = @evaluator_assignments. where(status: %i[completed in_progress not_started recused]). diff --git a/app/helpers/evaluators_helper.rb b/app/helpers/evaluators_helper.rb index bb16dd9f..62f21839 100644 --- a/app/helpers/evaluators_helper.rb +++ b/app/helpers/evaluators_helper.rb @@ -40,11 +40,13 @@ def evaluation_status(status) end end - # TODO: Display score for the evaluation submission assignment after EvaluationScore is added - # def display_score(assignment, evaluator_id) - # evaluation = Evaluation.find_by(evaluator_submission_assignment: assignment, user_id: evaluator_id) - # score = evaluation&.evaluation_scores&.effective_score + def display_score(assignment, evaluator_id) + evaluation = assignment.evaluation - # evaluation&.completed? && score ? score : 'N/A' - # end + if evaluation && evaluation.total_score.present? + evaluation.total_score + else + 'N/A' + end + end end diff --git a/app/views/evaluator_submission_assignments/_submission_row.html.erb b/app/views/evaluator_submission_assignments/_submission_row.html.erb index 48658c2c..1fba439c 100644 --- a/app/views/evaluator_submission_assignments/_submission_row.html.erb +++ b/app/views/evaluator_submission_assignments/_submission_row.html.erb @@ -3,7 +3,7 @@ <%= assignment.status.titleize %> - N/A<%# display_score(assignment) %> + <%= display_score(assignment, evaluator.id) %>
<% if assignment.status == "completed" %> diff --git a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb index 92dde7c3..221667dc 100644 --- a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb +++ b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb @@ -3,7 +3,7 @@ <%= assignment.status.titleize %> - N/A<%# display_score(assignment) %> + <%= display_score(assignment, evaluator.id) %>
<%= button_to "Reassign", phase_evaluator_submission_assignment_path(phase, assignment.submission, evaluator_id: evaluator.id, submission_id: assignment.submission.id, status: :not_started), method: :patch, class: 'usa-button usa-button--outline font-body-3xs' %> diff --git a/spec/helpers/evaluators_helper_spec.rb b/spec/helpers/evaluators_helper_spec.rb index bc08793b..47bf956b 100644 --- a/spec/helpers/evaluators_helper_spec.rb +++ b/spec/helpers/evaluators_helper_spec.rb @@ -3,12 +3,12 @@ require 'rails_helper' RSpec.describe EvaluatorsHelper, type: :helper do - describe '#assigned_submissions_count' do - let(:challenge) { create(:challenge) } - let(:phase) { create(:phase, challenge: challenge) } - let(:evaluator) { create(:user, role: :evaluator) } - let(:submission) { create(:submission, challenge: challenge, phase: phase) } + let(:challenge) { create(:challenge) } + let(:phase) { create(:phase, challenge: challenge) } + let(:evaluator) { create(:user, role: :evaluator) } + let(:submission) { create(:submission, challenge: challenge, phase: phase) } + describe '#assigned_submissions_count' do it 'returns the correct count of assigned submissions' do create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission, status: :not_started) create(:evaluator_submission_assignment, evaluator: evaluator, submission: create(:submission, challenge: challenge, phase: phase), status: :in_progress) @@ -34,4 +34,28 @@ expect(helper.assigned_submissions_count(evaluator, challenge, phase)).to eq(1) end end + + describe '#display_score' do + let(:assignment) { create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission) } + + context 'when evaluation exists and has a total score' do + it 'returns the total score' do + create(:evaluation, evaluator_submission_assignment: assignment, user: evaluator, total_score: 85) + expect(helper.display_score(assignment, evaluator.id)).to eq(85) + end + end + + context 'when evaluation exists but has no total score' do + it 'returns N/A' do + create(:evaluation, evaluator_submission_assignment: assignment, user: evaluator, total_score: nil) + expect(helper.display_score(assignment, evaluator.id)).to eq('N/A') + end + end + + context 'when evaluation does not exist' do + it 'returns N/A' do + expect(helper.display_score(assignment, evaluator.id)).to eq('N/A') + end + end +end end From f0013f6bf3e22bf2caa3cb1a28a47590e4968d42 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 15:31:51 -0600 Subject: [PATCH 29/38] 179 | Adjust SQL statement for status order --- app/models/evaluator_submission_assignment.rb | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index 5ff18cf7..b575a404 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true class EvaluatorSubmissionAssignment < ApplicationRecord - include ActiveRecord::Sanitization - belongs_to :submission belongs_to :evaluator, class_name: "User", foreign_key: :user_id, inverse_of: :assigned_submissions has_one :evaluation, dependent: :destroy @@ -19,16 +17,18 @@ class EvaluatorSubmissionAssignment < ApplicationRecord recused_unassigned: 6 } - STATUS_ORDER = %i[recused unassigned recused_unassigned not_started in_progress completed].freeze - scope :ordered_by_status, lambda { order( Arel.sql( - [ - "CASE evaluator_submission_assignments.status", - *STATUS_ORDER.map.with_index { |status, index| "WHEN #{statuses[status]} THEN #{index}" }, - "ELSE #{STATUS_ORDER.length} END" - ].join(" ") + "CASE evaluator_submission_assignments.status + WHEN #{statuses[:recused]} THEN 0 + WHEN #{statuses[:unassigned]} THEN 1 + WHEN #{statuses[:recused_unassigned]} THEN 2 + WHEN #{statuses[:not_started]} THEN 3 + WHEN #{statuses[:in_progress]} THEN 4 + WHEN #{statuses[:completed]} THEN 5 + ELSE 6 + END" ) ) } From 6602fb357ee19d652e633b22ce9df845d2152934 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 15:52:42 -0600 Subject: [PATCH 30/38] 179 | Scope score to evaluator's evaluation score --- app/helpers/evaluators_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/evaluators_helper.rb b/app/helpers/evaluators_helper.rb index 62f21839..b79e8b31 100644 --- a/app/helpers/evaluators_helper.rb +++ b/app/helpers/evaluators_helper.rb @@ -43,7 +43,7 @@ def evaluation_status(status) def display_score(assignment, evaluator_id) evaluation = assignment.evaluation - if evaluation && evaluation.total_score.present? + if evaluation && evaluation.user_id == evaluator_id && evaluation.total_score.present? evaluation.total_score else 'N/A' From 9f44ad8764d094b39aaa1b01bd8bff6d841b6f1f Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Fri, 22 Nov 2024 16:14:10 -0600 Subject: [PATCH 31/38] 179 | Recused unassigned cannot be reassign to the submission --- .../_unassigned_submission_row.html.erb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb index 221667dc..a8e574c5 100644 --- a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb +++ b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb @@ -6,7 +6,9 @@ <%= display_score(assignment, evaluator.id) %>
- <%= button_to "Reassign", phase_evaluator_submission_assignment_path(phase, assignment.submission, evaluator_id: evaluator.id, submission_id: assignment.submission.id, status: :not_started), method: :patch, class: 'usa-button usa-button--outline font-body-3xs' %> + <% if assignment.status == "unassigned" %> + <%= button_to "Reassign", phase_evaluator_submission_assignment_path(phase, assignment.submission, evaluator_id: evaluator.id, submission_id: assignment.submission.id, status: :not_started), method: :patch, class: 'usa-button usa-button--outline font-body-3xs' %> + <% end %>
From d029c20f8cc234aae2e947e3bb08d5360dce1b17 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 25 Nov 2024 16:49:33 -0600 Subject: [PATCH 32/38] 179 | Update route for evaluation submission assignments --- .../evaluator_submission_assignments_controller.rb | 13 +++++-------- .../_submission_row.html.erb | 8 +++----- .../_unassigned_submission_row.html.erb | 7 +++++-- config/routes.rb | 6 +----- 4 files changed, 14 insertions(+), 20 deletions(-) diff --git a/app/controllers/evaluator_submission_assignments_controller.rb b/app/controllers/evaluator_submission_assignments_controller.rb index 2a51a864..f9c4774c 100644 --- a/app/controllers/evaluator_submission_assignments_controller.rb +++ b/app/controllers/evaluator_submission_assignments_controller.rb @@ -1,14 +1,14 @@ # frozen_string_literal: true +# Controller for evaluator submissions assignments index and update status class EvaluatorSubmissionAssignmentsController < ApplicationController + before_action -> { authorize_user('challenge_manager') } before_action :set_challenge_and_phase before_action :set_evaluator, only: [:index] before_action :set_assignment, only: [:update] def index - @evaluator_assignments = @phase.evaluator_submission_assignments. - includes(:submission, :evaluation). - where(user_id: @evaluator.id) + @evaluator_assignments = @phase.evaluator_submission_assignments.where(user_id: @evaluator.id) @assigned_submissions = @evaluator_assignments. where(status: %i[completed in_progress not_started recused]). ordered_by_status @@ -37,14 +37,11 @@ def set_challenge_and_phase end def set_evaluator - @evaluator = params[:evaluator_id] ? User.find(params[:evaluator_id]) : current_user + @evaluator = @phase.evaluators.find(params[:evaluator_id]) end def set_assignment - @assignment = EvaluatorSubmissionAssignment.find_by!( - user_id: params[:evaluator_id], - submission_id: params[:submission_id] - ) + @assignment = @phase.evaluator_submission_assignments.find(params[:id]) end def update_assignment_status(new_status) diff --git a/app/views/evaluator_submission_assignments/_submission_row.html.erb b/app/views/evaluator_submission_assignments/_submission_row.html.erb index 1fba439c..5636f286 100644 --- a/app/views/evaluator_submission_assignments/_submission_row.html.erb +++ b/app/views/evaluator_submission_assignments/_submission_row.html.erb @@ -6,15 +6,13 @@ <%= display_score(assignment, evaluator.id) %>
- <% if assignment.status == "completed" %> + <% if assignment.completed? %> <%= link_to "View Evaluation", submissions_path(@challenge), class: 'usa-button font-body-3xs margin-right-1' %> <% end %> <%= button_tag "Unassign", type: 'button', class: 'usa-button usa-button--outline font-body-3xs', data: { action: "click->unassign-evaluator-submission-modal#open", - submission_id: assignment.submission.id, - evaluator_id: evaluator.id, - phase_id: @phase.id, - status: :unassigned + assignment_id: assignment.id, + phase_id: @phase.id } %>
diff --git a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb index a8e574c5..49fc1d60 100644 --- a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb +++ b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb @@ -6,8 +6,11 @@ <%= display_score(assignment, evaluator.id) %>
- <% if assignment.status == "unassigned" %> - <%= button_to "Reassign", phase_evaluator_submission_assignment_path(phase, assignment.submission, evaluator_id: evaluator.id, submission_id: assignment.submission.id, status: :not_started), method: :patch, class: 'usa-button usa-button--outline font-body-3xs' %> + <% if assignment.unassigned? %> + <%= button_to "Reassign", phase_evaluator_submission_assignment_path(@phase, assignment), + method: :patch, + params: { status: :not_started, evaluator_id: evaluator.id }, + class: 'usa-button usa-button--outline font-body-3xs' %> <% end %>
diff --git a/config/routes.rb b/config/routes.rb index b6d71289..2e7fff68 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -27,11 +27,7 @@ post 'resend_invite' end end - resources :evaluator_submission_assignments, only: [:index, :update] do - collection do - patch '', to: 'evaluator_submission_assignments#update' - end - end + resources :evaluator_submission_assignments, only: [:index, :update] end resources :submissions, only: [:index, :show, :update] From c890db4f99b004665764eb60c2c7dfc0ac760d72 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 25 Nov 2024 16:49:49 -0600 Subject: [PATCH 33/38] 179 | Update js to use assignment --- ...unassign_evaluator_submission_modal_controller.js | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js b/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js index 526db01d..dc58b0f7 100644 --- a/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js +++ b/app/javascript/controllers/unassign_evaluator_submission_modal_controller.js @@ -4,8 +4,7 @@ export default class extends Controller { static targets = ["modal", "confirmButton"] static values = { phaseId: String, - submissionId: String, - evaluatorId: String + assignmentId: String } connect() { @@ -38,15 +37,14 @@ export default class extends Controller { } setValues(dataset) { - this.submissionIdValue = dataset.submissionId; - this.evaluatorIdValue = dataset.evaluatorId; + this.assignmentIdValue = dataset.assignmentId; this.phaseIdValue = dataset.phaseId; } unassignEvaluatorSubmission() { const csrfToken = document.querySelector('meta[name="csrf-token"]').content; - fetch(`/phases/${this.phaseIdValue}/evaluator_submission_assignments?evaluator_id=${this.evaluatorIdValue}`, { + fetch(`/phases/${this.phaseIdValue}/evaluator_submission_assignments/${this.assignmentIdValue}`, { method: 'PATCH', headers: { 'Content-Type': 'application/json', @@ -54,14 +52,14 @@ export default class extends Controller { 'Accept': 'application/json' }, body: JSON.stringify({ - submission_id: this.submissionIdValue, status: 'unassigned' }) }) .then(response => response.json()) .then(data => { if (data.success) { - window.location.href = `/phases/${this.phaseIdValue}/evaluator_submission_assignments?evaluator_id=${this.evaluatorIdValue}`; + const evaluatorId = new URLSearchParams(window.location.search).get('evaluator_id'); + window.location.href = `/phases/${this.phaseIdValue}/evaluator_submission_assignments?evaluator_id=${evaluatorId}`; } else { throw new Error(data.message || 'Failed to unassign evaluator from submission'); } From 01209a4755b83830365b32068ea162edb4da9c14 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 25 Nov 2024 16:51:33 -0600 Subject: [PATCH 34/38] 179 | Update the display score to check for assignment completion --- app/helpers/evaluators_helper.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/app/helpers/evaluators_helper.rb b/app/helpers/evaluators_helper.rb index b79e8b31..df558921 100644 --- a/app/helpers/evaluators_helper.rb +++ b/app/helpers/evaluators_helper.rb @@ -41,10 +41,8 @@ def evaluation_status(status) end def display_score(assignment, evaluator_id) - evaluation = assignment.evaluation - - if evaluation && evaluation.user_id == evaluator_id && evaluation.total_score.present? - evaluation.total_score + if assignment.completed? && assignment.evaluation&.total_score + assignment.evaluation.total_score else 'N/A' end From ec365f98db5a335c44857b478a78a645c655bf5b Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 25 Nov 2024 16:52:11 -0600 Subject: [PATCH 35/38] 179 | Update ordered by status query --- app/models/evaluator_submission_assignment.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/models/evaluator_submission_assignment.rb b/app/models/evaluator_submission_assignment.rb index b575a404..926f5c49 100644 --- a/app/models/evaluator_submission_assignment.rb +++ b/app/models/evaluator_submission_assignment.rb @@ -21,12 +21,12 @@ class EvaluatorSubmissionAssignment < ApplicationRecord order( Arel.sql( "CASE evaluator_submission_assignments.status - WHEN #{statuses[:recused]} THEN 0 - WHEN #{statuses[:unassigned]} THEN 1 - WHEN #{statuses[:recused_unassigned]} THEN 2 - WHEN #{statuses[:not_started]} THEN 3 - WHEN #{statuses[:in_progress]} THEN 4 - WHEN #{statuses[:completed]} THEN 5 + WHEN #{ActiveRecord::Base.connection.quote(statuses[:recused])} THEN 0 + WHEN #{ActiveRecord::Base.connection.quote(statuses[:unassigned])} THEN 1 + WHEN #{ActiveRecord::Base.connection.quote(statuses[:recused_unassigned])} THEN 2 + WHEN #{ActiveRecord::Base.connection.quote(statuses[:not_started])} THEN 3 + WHEN #{ActiveRecord::Base.connection.quote(statuses[:in_progress])} THEN 4 + WHEN #{ActiveRecord::Base.connection.quote(statuses[:completed])} THEN 5 ELSE 6 END" ) From 575eda4896c58f4213a74a6a7a189d883c9253c3 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 25 Nov 2024 16:52:30 -0600 Subject: [PATCH 36/38] 179 | Update tests wip --- spec/helpers/evaluators_helper_spec.rb | 34 ++++++------ .../evaluator_submission_assignments_spec.rb | 53 +++++-------------- 2 files changed, 30 insertions(+), 57 deletions(-) diff --git a/spec/helpers/evaluators_helper_spec.rb b/spec/helpers/evaluators_helper_spec.rb index 47bf956b..9e807a46 100644 --- a/spec/helpers/evaluators_helper_spec.rb +++ b/spec/helpers/evaluators_helper_spec.rb @@ -36,26 +36,28 @@ end describe '#display_score' do - let(:assignment) { create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission) } - - context 'when evaluation exists and has a total score' do - it 'returns the total score' do - create(:evaluation, evaluator_submission_assignment: assignment, user: evaluator, total_score: 85) - expect(helper.display_score(assignment, evaluator.id)).to eq(85) + let(:assignment) { create(:evaluator_submission_assignment, evaluator: evaluator, submission: submission) } + + context 'when assignment is completed and has an evaluation with a total score' do + it 'returns the total score' do + assignment.update(status: :completed) + create(:evaluation, evaluator_submission_assignment: assignment, total_score: 85) + expect(helper.display_score(assignment, evaluator.id)).to eq(85) + end end - end - context 'when evaluation exists but has no total score' do - it 'returns N/A' do - create(:evaluation, evaluator_submission_assignment: assignment, user: evaluator, total_score: nil) - expect(helper.display_score(assignment, evaluator.id)).to eq('N/A') + context 'when assignment is not completed' do + it 'returns N/A' do + create(:evaluation, evaluator_submission_assignment: assignment, total_score: 85) + expect(helper.display_score(assignment, evaluator.id)).to eq('N/A') + end end - end - context 'when evaluation does not exist' do - it 'returns N/A' do - expect(helper.display_score(assignment, evaluator.id)).to eq('N/A') + context 'when evaluation does not exist' do + it 'returns N/A' do + assignment.update(status: :completed) + expect(helper.display_score(assignment, evaluator.id)).to eq('N/A') + end end end end -end diff --git a/spec/requests/evaluator_submission_assignments_spec.rb b/spec/requests/evaluator_submission_assignments_spec.rb index 794f6f10..9f7c07d5 100644 --- a/spec/requests/evaluator_submission_assignments_spec.rb +++ b/spec/requests/evaluator_submission_assignments_spec.rb @@ -6,15 +6,18 @@ let(:phase) { create(:phase, challenge: challenge) } let(:evaluator) { create(:user, role: 'evaluator') } let(:submission) { create(:submission, challenge: challenge, phase: phase) } + let(:unassigned_submission) { create(:submission, challenge: challenge, phase: phase) } + let!(:not_started_assignment) do create(:evaluator_submission_assignment, submission: submission, evaluator: evaluator, status: :not_started) end + let!(:unassigned_assignment) do create(:evaluator_submission_assignment, - submission: create(:submission, challenge: challenge, phase: phase), + submission: unassigned_submission, evaluator: evaluator, status: :unassigned) end @@ -23,38 +26,12 @@ ChallengeManager.create(user: challenge_manager, challenge: challenge) end - describe 'GET #index' do - before do - get phase_evaluator_submission_assignments_path(phase, evaluator_id: evaluator.id) - end - - it 'assigns @evaluator_assignments' do - expect(assigns(:evaluator_assignments)).to include(not_started_assignment, unassigned_assignment) - end - - it 'assigns @assigned_submissions' do - expect(assigns(:assigned_submissions)).to include(not_started_assignment) - expect(assigns(:assigned_submissions)).not_to include(unassigned_assignment) - end - - it 'assigns @unassigned_submissions' do - expect(assigns(:unassigned_submissions)).to include(unassigned_assignment) - expect(assigns(:unassigned_submissions)).not_to include(not_started_assignment) - end - - it 'assigns @submissions_count' do - expect(assigns(:submissions_count)).to eq({ "not_started" => 1 }) - end - end - describe 'PATCH #update' do context 'when reassigning' do it 'reassigns the evaluator successfully and updates counts' do - patch phase_evaluator_submission_assignments_path(phase, - evaluator_id: evaluator.id, - submission_id: unassigned_assignment.submission_id, - status: :not_started) + patch phase_evaluator_submission_assignment_path(phase, unassigned_assignment), + params: { status: :not_started, evaluator_id: evaluator.id } expect(unassigned_assignment.reload.status).to eq('not_started') expect(flash[:success]).to eq(I18n.t('evaluator_submission_assignments.not_started.success')) @@ -64,10 +41,8 @@ it 'fails to reassign when the assignment is invalid' do allow_any_instance_of(EvaluatorSubmissionAssignment).to receive(:update).and_return(false) - patch phase_evaluator_submission_assignments_path(phase, - evaluator_id: evaluator.id, - submission_id: unassigned_assignment.submission_id, - status: :not_started) + patch phase_evaluator_submission_assignment_path(phase, unassigned_assignment), + params: { status: :not_started, evaluator_id: evaluator.id } expect(unassigned_assignment.reload.status).to eq('unassigned') expect(flash[:error]).to eq(I18n.t('evaluator_submission_assignments.not_started.failure')) @@ -77,10 +52,8 @@ context 'when unassigning' do it 'unassigns the evaluator successfully' do - patch phase_evaluator_submission_assignments_path(phase, - evaluator_id: evaluator.id, - submission_id: not_started_assignment.submission_id, - status: :unassigned) + patch phase_evaluator_submission_assignment_path(phase, not_started_assignment), + params: { status: :unassigned, evaluator_id: evaluator.id } expect(not_started_assignment.reload.status).to eq('unassigned') expect(flash[:success]).to eq(I18n.t('evaluator_submission_assignments.unassigned.success')) @@ -90,10 +63,8 @@ it 'fails to unassign when the assignment is invalid' do allow_any_instance_of(EvaluatorSubmissionAssignment).to receive(:update).and_return(false) - patch phase_evaluator_submission_assignments_path(phase, - evaluator_id: evaluator.id, - submission_id: not_started_assignment.submission_id, - status: :unassigned) + patch phase_evaluator_submission_assignment_path(phase, not_started_assignment), + params: { status: :unassigned, evaluator_id: evaluator.id } expect(not_started_assignment.reload.status).to eq('not_started') expect(flash[:error]).to eq(I18n.t('evaluator_submission_assignments.unassigned.failure')) From f824247b64e54bff7c06f64903f6987805bb0b75 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 25 Nov 2024 17:16:49 -0600 Subject: [PATCH 37/38] 179 | Remove unused argument in display_score --- app/helpers/evaluators_helper.rb | 2 +- .../evaluator_submission_assignments/_submission_row.html.erb | 2 +- .../_unassigned_submission_row.html.erb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/helpers/evaluators_helper.rb b/app/helpers/evaluators_helper.rb index df558921..27e74970 100644 --- a/app/helpers/evaluators_helper.rb +++ b/app/helpers/evaluators_helper.rb @@ -40,7 +40,7 @@ def evaluation_status(status) end end - def display_score(assignment, evaluator_id) + def display_score(assignment) if assignment.completed? && assignment.evaluation&.total_score assignment.evaluation.total_score else diff --git a/app/views/evaluator_submission_assignments/_submission_row.html.erb b/app/views/evaluator_submission_assignments/_submission_row.html.erb index 5636f286..11f37ff6 100644 --- a/app/views/evaluator_submission_assignments/_submission_row.html.erb +++ b/app/views/evaluator_submission_assignments/_submission_row.html.erb @@ -3,7 +3,7 @@ <%= assignment.status.titleize %> - <%= display_score(assignment, evaluator.id) %> + <%= display_score(assignment) %>
<% if assignment.completed? %> diff --git a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb index 49fc1d60..71689f00 100644 --- a/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb +++ b/app/views/evaluator_submission_assignments/_unassigned_submission_row.html.erb @@ -3,7 +3,7 @@ <%= assignment.status.titleize %> - <%= display_score(assignment, evaluator.id) %> + <%= display_score(assignment) %>
<% if assignment.unassigned? %> From d99a8b4b59374e31acd3c590902c65c59fc03ff5 Mon Sep 17 00:00:00 2001 From: Emma Whamond Date: Mon, 25 Nov 2024 17:27:36 -0600 Subject: [PATCH 38/38] 179 | Update tests for display scores --- spec/helpers/evaluators_helper_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/helpers/evaluators_helper_spec.rb b/spec/helpers/evaluators_helper_spec.rb index 9e807a46..e65fea18 100644 --- a/spec/helpers/evaluators_helper_spec.rb +++ b/spec/helpers/evaluators_helper_spec.rb @@ -42,21 +42,21 @@ it 'returns the total score' do assignment.update(status: :completed) create(:evaluation, evaluator_submission_assignment: assignment, total_score: 85) - expect(helper.display_score(assignment, evaluator.id)).to eq(85) + expect(helper.display_score(assignment)).to eq(85) end end context 'when assignment is not completed' do it 'returns N/A' do create(:evaluation, evaluator_submission_assignment: assignment, total_score: 85) - expect(helper.display_score(assignment, evaluator.id)).to eq('N/A') + expect(helper.display_score(assignment)).to eq('N/A') end end context 'when evaluation does not exist' do it 'returns N/A' do assignment.update(status: :completed) - expect(helper.display_score(assignment, evaluator.id)).to eq('N/A') + expect(helper.display_score(assignment)).to eq('N/A') end end end