Merge branch 'dev' into 58_export_submissions

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
+# rubocop:enable all

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

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

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
 

app/views/evaluations/_phases_table.html.erb

@@ -9,36 +9,32 @@
     </tr>
   </thead>
   <tbody>
-    <% @challenges.each do |challenge| %>
-      <% challenge.phases.each do |phase| %>
-        <tr>
-          <th data-label="Challenge Title" scope="row" class="text-top">
-            <%= challenge_phase_title(challenge, phase) %>
-          </th>
-          <td data-label="Assigned to me" class="text-top">
-            <%= assigned_submissions_count(current_user, challenge, phase) %>
-          </td>
-          <td data-label="Remaining to evaluate" class="text-top">
-            <%= remaining_evaluations_count(current_user, challenge, phase) %>
-          </td>
-          <td data-label="Due by" class="text-top">
-            <% if phase.evaluation_form %>
-              <%= phase.evaluation_form.closing_date %>
-            <% else %>
-              N/A
+    <% @phases.each do |phase| %>
+      <tr>
+        <th data-label="Challenge Title" scope="row" class="text-top">
+          <%= challenge_phase_title(phase.challenge, phase) %>
+        </th>
+        <td data-label="Assigned to me" class="text-top">
+          <%= assigned_submissions_count(current_user, phase.challenge, phase) %>
+        </td>
+        <td data-label="Remaining to evaluate" class="text-top">
+          <%= remaining_evaluations_count(current_user, phase.challenge, phase) %>
+        </td>
+        <td data-label="Due by" class="text-top">
+          <% if phase.evaluation_form %>
+            <%= phase.evaluation_form.closing_date %>
+          <% else %>
+            N/A
+          <% end %>
+        </td>
+        <td>
+          <div class="display-flex flex-column grid-row grid-gap-1 padding-bottom-2">
+            <%= link_to submissions_evaluation_path(phase), class: "usa-button font-body-2xs text-no-wrap width-full" do %>
+              View Submissions
             <% end %>
-          </td>
-          <td>
-            <div class="display-flex flex-column grid-row grid-gap-1 padding-bottom-2">
-              <%= link_to submissions_evaluation_path(phase), class: "usa-button font-body-2xs text-no-wrap width-full" do %>
-                View Submissions
-              <% end %>
-            </div>
-            </div>  
-          </td>
-          </td>  
-        </tr>
-      <% end %>  
-    <% end %>  
+          </div>
+        </td>  
+      </tr>
+    <% end %>
   </tbody>
 </table>

app/views/evaluations/index.html.erb

@@ -1,7 +1,7 @@
 <h1>Evaluations</h1>
 <p class="text-normal padding-bottom-2">View challenges with submissions assigned to me to evaluate.</p>
 
-<% if @challenges.empty? %>
+<% if @phases.empty? %>
   <div class="text-normal">
     <p>You currently do not have any challenges.</p>
   </div>

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."

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

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