diff --git a/app/controllers/evaluations_controller.rb b/app/controllers/evaluations_controller.rb index 2142a0ad..e49ddd8f 100644 --- a/app/controllers/evaluations_controller.rb +++ b/app/controllers/evaluations_controller.rb @@ -10,9 +10,12 @@ class EvaluationsController < ApplicationController before_action :set_phase, only: [:submissions] def index - @challenges = Challenge.joins(phases: :challenge_phases_evaluators). - where(challenge_phases_evaluators: { user_id: current_user.id }). - includes(phases: [:evaluation_form]). + @phases = Phase.joins(:evaluator_submission_assignments). + where(evaluator_submission_assignments: { + user_id: current_user.id, + status: [:assigned, :recused] + }). + includes(:challenge, :evaluation_form). distinct end @@ -171,4 +174,4 @@ def evaluation_params end end # TODO: Remove this after above refactor -# rubocop:enable all \ No newline at end of file +# rubocop:enable all diff --git a/app/helpers/evaluators_helper.rb b/app/helpers/evaluators_helper.rb index fddb0f33..7247c337 100644 --- a/app/helpers/evaluators_helper.rb +++ b/app/helpers/evaluators_helper.rb @@ -5,6 +5,13 @@ module EvaluatorsHelper def user_status(evaluator) return "Invite Sent" unless evaluator.is_a?(User) - evaluator.status == 'active' ? "Available" : "Awaiting Approval" + case evaluator.status + when 'active' + "Available" + when 'evaluator_role_requested' + "Role Change Needed" + else + "Awaiting Approval" # pending + end end end diff --git a/app/models/user.rb b/app/models/user.rb index 5207461a..43825a80 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -95,6 +95,9 @@ class User < ApplicationRecord ROLES = %w[super_admin admin challenge_manager evaluator solver].freeze validates :role, inclusion: { in: ROLES } + USER_STATUSES = %w[pending active suspended revoked deactivated decertified evaluator_role_requested].freeze + validates :status, inclusion: { in: USER_STATUSES } + # Finds, creates, or updates user from userinfo # Find in case of user with existing token matching userinfo["sub"] # Create in case of no token or email matching in userinfo diff --git a/app/services/evaluator_management_service.rb b/app/services/evaluator_management_service.rb index 30bd8fe5..eeba636a 100644 --- a/app/services/evaluator_management_service.rb +++ b/app/services/evaluator_management_service.rb @@ -45,36 +45,35 @@ def resend_invitation(invitation) private def add_existing_user_as_evaluator(user) - if @phase.evaluators.include?(user) - return { - success: true, - message: I18n.t('evaluators.process_evaluator_invitation.already_added', - email: user.email) - } - end + return user_already_added(user) if @phase.evaluators.include?(user) + return invalid_role(user) unless User::VALID_EVALUATOR_ROLES.include?(user.role) - unless User::VALID_EVALUATOR_ROLES.include?(user.role) - return { - success: false, - message: I18n.t('evaluators.process_evaluator_invitation.invalid_role', - email: user.email) - } - end + user.role == 'evaluator' ? handle_evaluator_creation(user) : handle_evaluator_role_requested(user) + end - cpe = ChallengePhasesEvaluator.find_or_create_by(challenge: @challenge, phase: @phase, user:) + def user_already_added(user) + { success: true, message: I18n.t('evaluators.process_evaluator_invitation.already_added', email: user.email) } + end + def invalid_role(user) + { success: false, message: I18n.t('evaluators.process_evaluator_invitation.invalid_role', email: user.email) } + end + + def handle_evaluator_role_requested(user) + user.update!(status: 'evaluator_role_requested') + ChallengePhasesEvaluator.find_or_create_by(challenge: @challenge, phase: @phase, user:) + { + success: true, + message: I18n.t('evaluators.process_evaluator_invitation.evaluator_role_requested', email: user.email) + } + end + + def handle_evaluator_creation(user) + cpe = ChallengePhasesEvaluator.find_or_create_by(challenge: @challenge, phase: @phase, user:) if cpe.persisted? - { - success: true, - message: I18n.t('evaluators.process_evaluator_invitation.add_success', - email: user.email) - } + { success: true, message: I18n.t('evaluators.process_evaluator_invitation.add_success', email: user.email) } else - { - success: false, - message: I18n.t('evaluators.process_evaluator_invitation.add_failure', - email: user.email) - } + { success: false, message: I18n.t('evaluators.process_evaluator_invitation.add_failure', email: user.email) } end end @@ -91,18 +90,13 @@ def create_new_invitation(invitation_params) ) ) if invitation.save - { - success: true, + { success: true, message: I18n.t( 'evaluators.process_evaluator_invitation.invitation_sent', email: invitation_params[:email] - ) - } + ) } else - { - success: false, - message: invitation.errors.full_messages.join(", ") - } + { success: false, message: invitation.errors.full_messages.join(", ") } end end diff --git a/app/views/evaluations/_phases_table.html.erb b/app/views/evaluations/_phases_table.html.erb index 71163935..c3f899f1 100644 --- a/app/views/evaluations/_phases_table.html.erb +++ b/app/views/evaluations/_phases_table.html.erb @@ -9,36 +9,32 @@ - <% @challenges.each do |challenge| %> - <% challenge.phases.each do |phase| %> - - - <%= challenge_phase_title(challenge, phase) %> - - - <%= assigned_submissions_count(current_user, challenge, phase) %> - - - <%= remaining_evaluations_count(current_user, challenge, phase) %> - - - <% if phase.evaluation_form %> - <%= phase.evaluation_form.closing_date %> - <% else %> - N/A + <% @phases.each do |phase| %> + + + <%= challenge_phase_title(phase.challenge, phase) %> + + + <%= assigned_submissions_count(current_user, phase.challenge, phase) %> + + + <%= remaining_evaluations_count(current_user, phase.challenge, phase) %> + + + <% if phase.evaluation_form %> + <%= phase.evaluation_form.closing_date %> + <% else %> + N/A + <% end %> + + +
+ <%= link_to submissions_evaluation_path(phase), class: "usa-button font-body-2xs text-no-wrap width-full" do %> + View Submissions <% end %> - - -
- <%= link_to submissions_evaluation_path(phase), class: "usa-button font-body-2xs text-no-wrap width-full" do %> - View Submissions - <% end %> -
-
- - - - <% end %> - <% end %> + + + + <% end %> diff --git a/app/views/evaluations/index.html.erb b/app/views/evaluations/index.html.erb index db563021..a19d9710 100644 --- a/app/views/evaluations/index.html.erb +++ b/app/views/evaluations/index.html.erb @@ -1,7 +1,7 @@

Evaluations

View challenges with submissions assigned to me to evaluate.

-<% if @challenges.empty? %> +<% if @phases.empty? %>

You currently do not have any challenges.

diff --git a/config/locales/en.yml b/config/locales/en.yml index 1728dedc..d80d2f4a 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -59,6 +59,7 @@ en: process_evaluator_invitation: already_added: "%{email} has already been added as an evaluator for this phase." invalid_role: "%{email} does not have a valid evaluator role." + evaluator_role_requested: "%{email} requires a role change to evaluator. Please contact an admin." add_success: "%{email} has been added as an evaluator for this phase." add_failure: "Failed to add %{email} as an evaluator." invitation_sent: "Invitation sent to %{email} for this challenge phase." diff --git a/spec/requests/evaluations_spec.rb b/spec/requests/evaluations_spec.rb index 6ad2b742..e2a351b9 100644 --- a/spec/requests/evaluations_spec.rb +++ b/spec/requests/evaluations_spec.rb @@ -62,6 +62,102 @@ expect(response).to redirect_to(ENV.fetch("PHOENIX_URI", nil)) end end + + context "when logged in as an evaluator" do + let(:evaluator) { create_and_log_in_user(role: 'evaluator') } + + let(:challenge_with_submissions) { create(:challenge, title: "Challenge with Submissions", is_multi_phase: false) } + let(:phase_with_submissions) { challenge_with_submissions.phases.first } + + let(:multi_phase_challenge) { create(:challenge, title: "Multi-Phase Challenge", is_multi_phase: true) } + let(:phase1) { create(:phase, challenge: multi_phase_challenge) } + let(:phase2) { create(:phase, challenge: multi_phase_challenge) } + + let(:challenge_without_submissions) { create(:challenge, title: "Challenge without Submissions", is_multi_phase: false) } + let(:phase_without_submissions) { challenge_without_submissions.phases.first } + + before do + ChallengePhasesEvaluator.create!(challenge: challenge_with_submissions, phase: phase_with_submissions, user: evaluator) + ChallengePhasesEvaluator.create!(challenge: multi_phase_challenge, phase: phase1, user: evaluator) + ChallengePhasesEvaluator.create!(challenge: challenge_without_submissions, phase: phase_without_submissions, user: evaluator) + end + + it "only shows phases with assigned or recused submissions" do + # Submissions for single phase challenge + assigned_submission = create(:submission, phase: phase_with_submissions, challenge: challenge_with_submissions) + create(:evaluator_submission_assignment, + submission: assigned_submission, + evaluator: evaluator, + status: :assigned + ) + + recused_submission = create(:submission, phase: phase_with_submissions, challenge: challenge_with_submissions) + create(:evaluator_submission_assignment, + submission: recused_submission, + evaluator: evaluator, + status: :recused + ) + + unassigned_submission = create(:submission, phase: phase_with_submissions, challenge: challenge_with_submissions) + create(:evaluator_submission_assignment, + submission: unassigned_submission, + evaluator: evaluator, + status: :unassigned + ) + + recused_unassigned_submission = create(:submission, phase: phase_with_submissions, challenge: challenge_with_submissions) + create(:evaluator_submission_assignment, + submission: recused_unassigned_submission, + evaluator: evaluator, + status: :recused_unassigned + ) + + # Submissions for multi-phase challenge + multi_phase_submission = create(:submission, phase: phase1, challenge: multi_phase_challenge) + create(:evaluator_submission_assignment, + submission: multi_phase_submission, + evaluator: evaluator, + status: :assigned + ) + + multi_phase_recused_submission = create(:submission, phase: phase1, challenge: multi_phase_challenge) + create(:evaluator_submission_assignment, + submission: multi_phase_recused_submission, + evaluator: evaluator, + status: :recused + ) + + multi_phase_unassigned_submission = create(:submission, phase: phase1, challenge: multi_phase_challenge) + create(:evaluator_submission_assignment, + submission: multi_phase_unassigned_submission, + evaluator: evaluator, + status: :unassigned + ) + + multi_phase_recused_unassigned_submission = create(:submission, phase: phase1, challenge: multi_phase_challenge) + create(:evaluator_submission_assignment, + submission: multi_phase_recused_unassigned_submission, + evaluator: evaluator, + status: :recused_unassigned + ) + + get evaluations_path + + expect(response.body).to include(challenge_with_submissions.title) + expect(response.body).not_to include(challenge_without_submissions.title) + + expect(response.body.scan(/data-label="Challenge Title"/).count).to eq(2) + expect(response.body.scan(/#{multi_phase_challenge.title}/).count).to eq(1) + + within("#phase_#{phase_with_submissions.id}") do + expect(page).to have_css('td[data-label="Assigned to me"]', text: '2') + end + + within("#phase_#{phase1.id}") do + expect(page).to have_css('td[data-label="Assigned to me"]', text: '2') + end + end + end end describe "GET /evaluations/:id/submissions" do diff --git a/spec/services/evaluator_management_service_spec.rb b/spec/services/evaluator_management_service_spec.rb index 82faca8c..56a39317 100644 --- a/spec/services/evaluator_management_service_spec.rb +++ b/spec/services/evaluator_management_service_spec.rb @@ -58,6 +58,26 @@ expect(result[:message]).to include('Invitation has been resent') end end + + context 'with an existing user needing role change' do + let(:solver) { create(:user, role: 'solver', status: 'active') } + + it 'sets status to evaluator_role_requested when user is not an evaluator role' do + result = service.process_evaluator_invitation(solver.email, {}) + + expect(result[:success]).to be true + expect(result[:message]).to include('requires a role change to evaluator') + expect(solver.reload.status).to eq('evaluator_role_requested') + end + + it 'does not set evaluator_role_requested for users with evaluator role' do + evaluator = create(:user, role: 'evaluator', status: 'active') + result = service.process_evaluator_invitation(evaluator.email, {}) + + expect(result[:success]).to be true + expect(evaluator.reload.status).to eq('active') + end + end end describe '#remove_evaluator' do