-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[75] Assign Evaluators to a Submission (WIP) #329
base: dev
Are you sure you want to change the base?
Changes from 26 commits
40ce276
3f1e16d
acf9d04
ff410cc
7ac20ae
2145636
a10efbb
7e8e97b
2a5de81
55401a6
e321bf7
c405259
9d313a1
81a8020
f4231db
1d11c91
ff79c48
ef9808f
9d6de87
22d736c
faf6aba
efb3ed5
7779784
ad1d718
63aa36b
f0548ee
3f4b79b
c942ba2
85a8193
2e9236e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export PHOENIX_URI="localhost:4000" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are you using nix? this value is defined in |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,8 +4,26 @@ | |
class EvaluatorSubmissionAssignmentsController < ApplicationController | ||
before_action -> { authorize_user('challenge_manager') } | ||
before_action :set_challenge_and_phase | ||
before_action :set_evaluator, only: [:index] | ||
before_action :set_evaluator, only: [:index, :create] | ||
before_action :set_assignment, only: [:update] | ||
before_action :set_submission, only: [:create] | ||
|
||
def create | ||
@evaluator_submission_assignment = EvaluatorSubmissionAssignment.new( | ||
user_id: params["evaluator_id"], | ||
submission_id: @submission.id, | ||
status: :assigned | ||
) | ||
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") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
end | ||
end | ||
|
||
def unassign | ||
|
||
end | ||
Comment on lines
+24
to
+26
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is unused, we can rely on the |
||
|
||
def index | ||
@evaluator_assignments = @phase.evaluator_submission_assignments.includes(:submission).where(user_id: @evaluator.id) | ||
|
@@ -49,6 +67,10 @@ def set_assignment | |
@assignment = @phase.evaluator_submission_assignments.find(params[:id]) | ||
end | ||
|
||
def set_submission | ||
@submission = @phase.submissions.find(params[:submission_id]) | ||
stonefilipczak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
end | ||
|
||
def status_from_params | ||
status = params[:status] || params.dig(:evaluator_submission_assignment, :status) | ||
status&.to_sym | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,11 +5,13 @@ 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See this Figma comment thread.. if the evaluator was unassigned the button CTA should read "Reassign to Submission". |
||
end | ||
|
||
def update | ||
if @submission.update!(submission_params) | ||
flash.now[:success] = I18n.t("comments_saved") | ||
flash.now[:success] = I18n.t("submission_updated") | ||
render :show, submission: @submission | ||
else | ||
render :show, status: :unprocessable_entity, submission: @submission | ||
|
@@ -19,7 +21,7 @@ def update | |
private | ||
|
||
def submission_params | ||
params.require(:submission).permit(:comments) | ||
params.require(:submission).permit(:comments, :judging_status) | ||
end | ||
|
||
# User access enforced by role | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
import { Controller } from "@hotwired/stimulus" | ||
|
||
// Connects to data-controller="submission-details" | ||
export default class extends Controller { | ||
|
||
eligibleCheck(e) { | ||
const hiddenInput = e.target.form.elements["judging-status-hidden"] | ||
if (e.target.checked) { | ||
hiddenInput.value = "selected" | ||
} else { | ||
hiddenInput.value = "qualified" | ||
} | ||
} | ||
|
||
selectedCheck(e) { | ||
const hiddenInput = e.target.form.elements["judging-status-hidden"] | ||
if (e.target.checked) { | ||
hiddenInput.value = "winner" | ||
} else { | ||
hiddenInput.value = "selected" | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,46 @@ | ||||||
<div class="usa-accordion"> | ||||||
<% rand = SecureRandom.hex(8) %> | ||||||
<h2 class="usa-accordion__heading" id="<%= "#{rand}-accordion-heading" %>"> | ||||||
<button | ||||||
type="button" | ||||||
class="usa-accordion__button display-flex flex-justify icon-white" | ||||||
aria-expanded="true" | ||||||
aria-controls="<%= rand %>" | ||||||
> | ||||||
<div>Assigned Evaluators</div> | ||||||
</button> | ||||||
</h2> | ||||||
|
||||||
<div class="usa-accordion__content" id="<%= rand %>"> | ||||||
<table class="usa-table usa-table--stacked-header usa-table--borderless gray-header"> | ||||||
<thead> | ||||||
<tr> | ||||||
<th scope="col">Evaluator name</th> | ||||||
<th scope="col">Evaluation Status</th> | ||||||
<th scope="col">Score</th> | ||||||
<th scope="col" aria-label="Actions"></th> | ||||||
</tr> | ||||||
</thead> | ||||||
<tbody> | ||||||
<% @submission.evaluators.each do |evaluator| %> | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
|
||||||
<tr data-evaluator-id="<%= evaluator.id %>" data-evaluator-type="user"> | ||||||
<th scope="row" data-label="Evaluator name" class="text-top"> | ||||||
<span class="text-bold text-ls-1 line-height-sans-4"><%= "#{evaluator.first_name} #{evaluator.last_name}" %></span> | ||||||
</th> | ||||||
<td data-label="Evaluation Status" class="text-top"></td> | ||||||
<td data-label="Score" class="text-top"></td> | ||||||
<td data-label=""> | ||||||
<div class="display-flex flex-no-wrap grid-row"> | ||||||
<%= 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" %> | ||||||
Comment on lines
+34
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||||||
<% end %> | ||||||
</div> | ||||||
</td> | ||||||
</tr> | ||||||
<% end %> | ||||||
</tbody> | ||||||
</table> | ||||||
</div> | ||||||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
<div class="usa-accordion"> | ||
<% rand = SecureRandom.hex(8) %> | ||
<h2 class="usa-accordion__heading" id="<%= "#{rand}-accordion-heading" %>"> | ||
<button | ||
type="button" | ||
class="usa-accordion__button display-flex flex-justify icon-white" | ||
aria-expanded="true" | ||
aria-controls="<%= rand %>" | ||
> | ||
<div>Available Evaluators</div> | ||
</button> | ||
</h2> | ||
|
||
<div class="usa-accordion__content background-primary-lighter" id="<%= rand %>"> | ||
<% @available_evaluators.each do |evaluator| %> | ||
<div class="float-left"> | ||
<%= "#{evaluator.first_name} #{evaluator.last_name}" %> <br/> | ||
<%= evaluator.email %> | ||
</div> | ||
<div class="float-right"> | ||
<%= 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 "Assign" %> | ||
<% end %> | ||
</div> | ||
|
||
<% end %> | ||
</div> | ||
</div> |
This file was deleted.
Original file line number | Diff line number | Diff line change | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,51 @@ | ||||||||||||||||
<div data-controller="submission-details"> | ||||||||||||||||
<% rand = SecureRandom.hex(8) %> | ||||||||||||||||
<%= form_with(model: @submission, url: submission_path(@submission), class: "maxw-mobile-lg") do |form| %> | ||||||||||||||||
<div class="usa-checkbox" id="eligible-for-evaluation"> | ||||||||||||||||
<input | ||||||||||||||||
class="usa-checkbox__input usa-checkbox__input--tile" | ||||||||||||||||
type="checkbox" | ||||||||||||||||
id="<%= "#{rand}-judging-status-selected" %>" | ||||||||||||||||
value="selected" | ||||||||||||||||
<%= "disabled" if @submission.judging_status == "winner" %> | ||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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
(also the winner status disabled below) |
||||||||||||||||
<%= "checked" if @submission.judging_status.in?(["selected", "winner"]) %> | ||||||||||||||||
data-action="change->submission-details#eligibleCheck" | ||||||||||||||||
/> | ||||||||||||||||
<label class="usa-checkbox__label" for="<%= "#{rand}-judging-status-selected" %>" | ||||||||||||||||
>This submission is eligible for evaluation. | ||||||||||||||||
<span class="usa-checkbox__label-description" | ||||||||||||||||
>Check the box if the submission is pre-screened and is eligible for the evaluation process.</span | ||||||||||||||||
></label | ||||||||||||||||
> | ||||||||||||||||
</div> | ||||||||||||||||
<div class="usa-checkbox" id="selected-to-advance"> | ||||||||||||||||
<input | ||||||||||||||||
class="usa-checkbox__input usa-checkbox__input--tile" | ||||||||||||||||
id="<%= "#{rand}-judging-status-winner" %>" | ||||||||||||||||
type="checkbox" | ||||||||||||||||
<%= "disabled" unless @submission.judging_status.in?(["selected", "winner"]) %> | ||||||||||||||||
<%= "checked" if @submission.judging_status == "winner" %> | ||||||||||||||||
data-action="change->submission-details#selectedCheck" | ||||||||||||||||
/> | ||||||||||||||||
<label class="usa-checkbox__label" for="<%= "#{rand}-judging-status-winner" %>" | ||||||||||||||||
>This submission is selected to advance. | ||||||||||||||||
<span class="usa-checkbox__label-description" | ||||||||||||||||
>Check the box to select this submission to advance or for award after all evaluations are completed.</span | ||||||||||||||||
></label | ||||||||||||||||
> | ||||||||||||||||
</div> | ||||||||||||||||
<input | ||||||||||||||||
id="judging-status-hidden" | ||||||||||||||||
type="hidden" | ||||||||||||||||
name="submission[judging_status]" | ||||||||||||||||
data-submission-details-target="hiddenInput" | ||||||||||||||||
/> | ||||||||||||||||
<div class="usa-form-group"> | ||||||||||||||||
<%= form.label :comments, "Comments and notes:", class: "usa-label" %> | ||||||||||||||||
<%= form.text_area :comments, class: "usa-textarea", default: @submission.comments %> | ||||||||||||||||
</div> | ||||||||||||||||
<button type="submit" name="commit" class="usa-button font-body-2xs text-no-wrap margin-y-2"> | ||||||||||||||||
Save | ||||||||||||||||
</button> | ||||||||||||||||
<% end %> | ||||||||||||||||
</div> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
<%= render partial: 'details_form' %> | ||
<% if @submission.judging_status.in?(["selected", "winner"]) %> | ||
<%= render partial: 'submission_evaluation' %> | ||
<% end %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<h2>Submission Evaluation</h2> | ||
<% if @submission.phase.evaluation_form.nil? %> | ||
<%= render 'shared/alert_error', alert_heading: t('alerts.no_evaluation_form.heading'), alert_text: t('alerts.no_evaluation_form.text') %> | ||
<% else %> | ||
<%= render partial: 'assigned_evaluators' %> | ||
<%= render partial: 'available_evaluators' %> | ||
<% end %> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
<h1>Submission ID <%= @submission.id %></h1> | ||
<p class="text-normal">View submission information and assign evaluators to evaluate the submission.</p> | ||
<h2 class="font-body-sm text-normal">View submission information and assign evaluators to evaluate the submission.</h2> | ||
|
||
<%= render partial: "layouts/hotdog", locals: {left: 'submissions/comment_form', right: 'submissions/submission_materials', name: 'Submission Materials'} %> | ||
<%= render partial: "layouts/hotdog", locals: {left: 'submissions/submission_details', right: 'submissions/submission_materials', name: 'Submission Materials'} %> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not needed here |
||
); | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,5 +8,6 @@ | |
|
||
title { Faker::Lorem.sentence } | ||
status { "draft" } | ||
external_url { "www.example.com" } | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
# frozen_string_literal: true | ||
|
||
require 'rails_helper' | ||
|
||
describe "A11y", :js do | ||
describe "Logged-in as a Challenge Manager" do | ||
let(:user) { create_user(role: "challenge_manager") } | ||
let(:challenge) { create_challenge(user: user, title: "Boston Tea Party Cleanup") } | ||
let(:submission) { create(:submission, manager: user, challenge: challenge) } | ||
|
||
|
||
before { system_login_user(user) } | ||
|
||
it "submission details page is accessible" do | ||
visit submission_path(submission) | ||
expect(user.role).to eq("challenge_manager") | ||
expect(page).to have_css('h1', text: "Submission ID #{submission.id}") | ||
expect(page).to(be_axe_clean) | ||
end | ||
|
||
it "allows manipulation of judging status" do | ||
visit submission_path(submission) | ||
|
||
find_by_id('eligible-for-evaluation').click | ||
click_on('Save') | ||
updated_submission = Submission.find(submission.id) | ||
expect(updated_submission.judging_status).to eq('selected') | ||
|
||
find_by_id('selected-to-advance').click | ||
click_on('Save') | ||
updated_submission = Submission.find(submission.id) | ||
expect(updated_submission.judging_status).to eq('winner') | ||
end | ||
|
||
it "saves comments" do | ||
visit submission_path(submission) | ||
comments = Faker::Lorem.sentence | ||
|
||
fill_in "Comments and notes:", with: comments | ||
click_on('Save') | ||
updated_submission = Submission.find(submission.id) | ||
expect(updated_submission.comments).to eq(comments) | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should probably remove this file, not sure why we need it?