[75] Assign Evaluators to a Submission (WIP) #329

stonefilipczak · 2024-12-20T14:07:17Z

a mostly working WIP of assigning evaluators to a submission from the submission details page.

TODO:

implement unassign modal. The existing code for this always sends the user to a different page so I need to modify what's there to accept a return path or something along those lines.
styling mystery. The center bar of the hotdog gets pushed to the right when the "assigned evaluators" accordion is open. IDK why
tests

Co-authored-by: Stephen Chudleigh <[email protected]>

…signment

stepchud

initial review feedback, there are some codeclimate issues also

stepchud · 2024-12-20T16:44:53Z

.zshrc

should probably remove this file, not sure why we need it?

app/controllers/evaluator_submission_assignments_controller.rb

stepchud · 2024-12-20T17:01:11Z

db/structure.sql

@@ -1109,7 +1109,7 @@ CREATE TABLE public.submissions (
    description_delta text,
    brief_description_delta text,
    pdf_reference character varying(255),
-    comments text
+    comments character varying


not needed here

stepchud · 2024-12-20T17:01:33Z

app/controllers/evaluator_submission_assignments_controller.rb

+  def unassign
+
+  end  


this is unused, we can rely on the update route for unassigning

stepchud · 2024-12-20T17:03:50Z

app/controllers/evaluator_submission_assignments_controller.rb

+    if @evaluator_submission_assignment.save
+      redirect_to submission_path(@submission), notice: I18n.t("evaluator_submission_assignment_saved")
+    else
+      redirect_to confirmation_evaluation_form_path(@evaluator_submission_assignment), notice: I18n.t("evaluation_form_saved")


this is the wrong path to redirect to when it failed to save. it should probably redirect to the current evaluator_submission_assignment with a warning error message instead of evaluation_form_saved flash notice.

stepchud · 2024-12-20T18:02:52Z

app/views/submissions/_submission_evaluation.html.erb

+<% else %>
+    <%= render partial: 'assigned_evaluators' %>
+    <%= render partial: 'available_evaluators' %>
+<% end %>


Don't forget the link to "Manage my evaluation panel"

stepchud · 2024-12-20T18:13:18Z

app/views/submissions/_assigned_evaluators.html.erb

+                    <%= form_with url: phase_evaluator_submission_assignments_path(@submission.phase) do |f| %>
+                      <%= f.hidden_field :evaluator_id, value: evaluator.id %>
+                      <%= f.hidden_field :submission_id, value: @submission.id %>
+                      <%= f.submit "Unassign" %>


this form routes to the create action when it should issue an update to the existing evaluator_submission_assignment to mark it unassigned if it's assigned and recused_unassigned if it is recused status.
assigned => unassigned
recused => recused_unassigned
because it creates a new evaluator_submission_assignment, that also creates duplicate assignment records. we should also validate uniqueness on the combination of evaluator_id/phase_id/submission_id in the EvaluatorSubmissionAssignment model, so we don't end up with duplicate records (just in case someone tries to create one).

stepchud · 2024-12-20T18:18:46Z

app/views/submissions/_assigned_evaluators.html.erb

+            </tr>
+          </thead>
+          <tbody>
+            <% @submission.evaluators.each do |evaluator| %>


we should filter this by evaluator_submission_assignments with assigned or recused status. (this should work, or you can assign the filtered list in the controller like you do with @available_evaluators)

Suggested change

<% @submission.evaluators.each do |evaluator| %>

<% @submission.evaluators.where("evaluator_submission_assignments.status" => ["assigned", "recused"]).each do |evaluator| %>

stepchud · 2024-12-20T18:26:06Z

app/controllers/submissions_controller.rb

@@ -5,7 +5,9 @@ class SubmissionsController < ApplicationController
  before_action -> { authorize_user('challenge_manager') }
  before_action :set_submission, only: [:show, :update]

-  def show; end
+  def show
+    @available_evaluators = @submission.phase.evaluators - @submission.evaluators


this list needs a bit of tweaking to account for different statuses. available evaluators should include evaluators that were assigned and then unassigned. when you add specs we should test all these scenarios where the evaluators are in different states of assigned/unassigned/recused/recused_unassigned and make sure it's working for each case.

See this Figma comment thread.. if the evaluator was unassigned the button CTA should read "Reassign to Submission".

stepchud · 2025-01-03T23:08:20Z

app/views/submissions/_details_form.html.erb

+            type="checkbox"
+            id="<%= "#{rand}-judging-status-selected" %>"
+            value="selected"
+            <%= "disabled" if @submission.judging_status == "winner" %>


it looks like #321 will merge first so I wanted to leave a reminder to update this disabled state from that logic once it's merged:

Challenge_platform/app/models/submission.rb

Lines 78 to 84 in bde972d

def eligibility_deselection_disabled?

evaluator_submission_assignments.exists?(status: [:assigned, :recused])

end

def advancement_checkbox_disabled?

!eligible_for_evaluation? || !all_evaluations_completed? || evaluators.empty?

end

(also the winner status disabled below)

stepchud · 2025-01-03T23:15:19Z

app/helpers/submissions_helper.rb

@@ -9,4 +9,8 @@ def eligible_for_evaluation?(submission)
  def selected_to_advance?(submission)
    submission.judging_status.in?(%w[winner])
  end
+
+  def available_evaluators(submission)
+    submission.phase.evaluators - submission.evaluators || []


you don't need the || [] fallback right? [] - [] == []

Suggested change

submission.phase.evaluators - submission.evaluators || []

submission.phase.evaluators - submission.evaluators

stepchud · 2025-01-03T23:20:07Z

.zshrc

@@ -0,0 +1 @@
+export PHOENIX_URI="localhost:4000"


are you using nix? this value is defined in .env_dev so you can source that directly instead if you don't use nix

stonefilipczak and others added 26 commits December 9, 2024 17:24

working

40ce276

disabling and verbage

3f1e16d

specs

acf9d04

reorder

ff410cc

Merge branch 'dev' into 305_submission_details

7ac20ae

render everything

2145636

fix spec

a10efbb

Merge branch 'dev' into 305_submission_details

7e8e97b

Merge branch 'dev' into 305_submission_details

2a5de81

initialize

55401a6

missing evaluation form

e321bf7

Update spec/system/submission_details_spec.rb

c405259

Co-authored-by: Stephen Chudleigh <[email protected]>

Update spec/system/submission_details_spec.rb

9d313a1

Co-authored-by: Stephen Chudleigh <[email protected]>

Update spec/system/submission_details_spec.rb

81a8020

Co-authored-by: Stephen Chudleigh <[email protected]>

Update app/views/submissions/_judging_status.html.erb

f4231db

Co-authored-by: Stephen Chudleigh <[email protected]>

not working single form attempt

1d11c91

weird WIP

ff79c48

working single form approach

ef9808f

fix test

9d6de87

Merge branch 'dev' into 305_submission_details

22d736c

Merge branch '305_submission_details' into 75_evaluator_submission_as…

faf6aba

…signment

WIP

efb3ed5

both accordions working

7779784

succesful assignment

ad1d718

working everything

63aa36b

unassign

f0548ee

stonefilipczak added the WIP label Dec 20, 2024

r-bartlett-gsa added this to the Sprint 12/31/24 milestone Dec 20, 2024

r-bartlett-gsa linked an issue Dec 20, 2024 that may be closed by this pull request

Assign Evaluators to a Submission - Manual #75

Open

48 tasks

Merge branch 'dev' into 75_evaluator_submission_assignment

3f4b79b

move form to partial

c942ba2

stepchud requested changes Dec 20, 2024

View reviewed changes

stonefilipczak and others added 2 commits December 30, 2024 09:26

Merge branch 'dev' into 75_evaluator_submission_assignment

85a8193

extract available evaluators to helper method

2e9236e

r-bartlett-gsa modified the milestones: Sprint 12/31/24, Sprint 01/14/25 Jan 2, 2025

stepchud reviewed Jan 3, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[75] Assign Evaluators to a Submission (WIP) #329

[75] Assign Evaluators to a Submission (WIP) #329

stonefilipczak commented Dec 20, 2024

stepchud left a comment •

edited

Loading

stepchud Dec 20, 2024

stepchud Dec 20, 2024

stepchud Dec 20, 2024

stepchud Dec 20, 2024

stepchud Dec 20, 2024

stepchud Dec 20, 2024

stepchud Dec 20, 2024

stepchud Dec 20, 2024

stepchud Dec 20, 2024

stepchud Jan 3, 2025

stepchud Jan 3, 2025

stepchud Jan 3, 2025

	<% @submission.evaluators.each do \|evaluator\| %>
	<% @submission.evaluators.where("evaluator_submission_assignments.status" => ["assigned", "recused"]).each do \|evaluator\| %>

	def eligibility_deselection_disabled?
	evaluator_submission_assignments.exists?(status: [:assigned, :recused])
	end

	def advancement_checkbox_disabled?
	!eligible_for_evaluation? \|\| !all_evaluations_completed? \|\| evaluators.empty?
	end

	submission.phase.evaluators - submission.evaluators \|\| []
	submission.phase.evaluators - submission.evaluators

[75] Assign Evaluators to a Submission (WIP) #329

Are you sure you want to change the base?

[75] Assign Evaluators to a Submission (WIP) #329

Conversation

stonefilipczak commented Dec 20, 2024

stepchud left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stepchud left a comment •

edited

Loading