Skip to content
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

[74] Enter Evaluators A11y Fix #343

Merged
merged 30 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ddf39d7
74 | Use inline error messages for form validation
emmabjj Jan 3, 2025
bf53f50
74 | Update first/last name for existing user without one
emmabjj Jan 3, 2025
77aa9d7
74 | Challenge phase title format
emmabjj Jan 3, 2025
0dcc23b
74 | Remove duplicate validation error on full_name field
emmabjj Jan 3, 2025
e4ecf73
74 | Omit hash
emmabjj Jan 3, 2025
ecdf880
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 7, 2025
d0b4922
Fix back_link path
stepchud Jan 7, 2025
c942595
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 8, 2025
95e863c
74 | Extract form validation into stimulus controller for reusability
emmabjj Jan 8, 2025
0589077
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 8, 2025
cf1cdd4
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 13, 2025
a40ee3f
74 | Handle existing user inline validation errors
emmabjj Jan 15, 2025
066b678
74 | First & Last Name form label red
emmabjj Jan 15, 2025
e631fa8
74 | Use evaluator invitation split_full_name
emmabjj Jan 15, 2025
3f10e8e
74 | Use temp EvaluatorInvitation model to check validation
emmabjj Jan 15, 2025
6e02a4f
74 | Update tests
emmabjj Jan 15, 2025
342f363
CodeClimate reduce lines
emmabjj Jan 15, 2025
d421b42
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 15, 2025
cce0a3b
codeclimate reduce to single lines
emmabjj Jan 15, 2025
c6995bb
small tweak
emmabjj Jan 15, 2025
fadb770
74 | Move invitation logic to evaluator_invitation_service
emmabjj Jan 15, 2025
dc33cba
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 15, 2025
03636e6
alignment tweak
emmabjj Jan 15, 2025
cf18396
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 15, 2025
22319cd
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 15, 2025
a78aac7
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 15, 2025
11c045a
remove white space
emmabjj Jan 15, 2025
1d20d98
74 | Check full name is provided when updating
emmabjj Jan 15, 2025
ab4a417
74 | Update tests to include full_name in creation of evaluator invit…
emmabjj Jan 16, 2025
415d134
Merge branch 'dev' into 74_enter_evaluators_a11y_fix
emmabjj Jan 16, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 22 additions & 6 deletions app/controllers/evaluators_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,20 @@ def index
end

def create
@evaluator_invitation = EvaluatorInvitation.new(evaluator_invitation_params)

result = evaluator_service.process_evaluator_invitation(
evaluator_invitation_params[:email],
evaluator_invitation_params
)

if result[:success]
redirect_to phase_evaluators_path(@phase), notice: result[:message]
else
flash.now[:alert] = result[:message]
@evaluator_invitations = @phase.evaluator_invitations
@existing_evaluators = @phase.evaluators
render :index
return
end

handle_failed_invitation(result)
stepchud marked this conversation as resolved.
Show resolved Hide resolved
render :index, status: :unprocessable_entity
end

def destroy
Expand Down Expand Up @@ -64,7 +65,22 @@ def evaluator_service

def evaluator_invitation_params
params.require(:evaluator_invitation).permit(
:first_name, :last_name, :email, :challenge_id, :phase_id, :last_invite_sent
:full_name, :email, :challenge_id, :phase_id, :last_invite_sent
)
end

def handle_failed_invitation(result)
@evaluator_invitations = @phase.evaluator_invitations
@existing_evaluators = @phase.evaluators

unless @evaluator_invitation.valid?
return
end

if result[:evaluator_invitation].present?
@evaluator_invitation = result[:evaluator_invitation]
else
@evaluator_invitation.errors.add(:base, result[:message])
end
end
end
8 changes: 4 additions & 4 deletions app/helpers/form_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

# Helpers for rendering various form elements and errors
module FormHelper
def label_error_class(form, field)
def label_error_class(form, fields)
object = form.object
object.errors[field].present? ? "text-secondary" : ""
Array(fields).any? { |field| object.errors[field].present? } ? "text-secondary" : ""
end

def input_error_class(form, field)
def input_error_class(form, fields)
object = form.object
object.errors[field].present? ? "border-secondary" : ""
Array(fields).any? { |field| object.errors[field].present? } ? "border-secondary" : ""
end

def inline_error(form, field)
Expand Down
42 changes: 0 additions & 42 deletions app/javascript/controllers/evaluation_form_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,46 +69,4 @@ export default class extends Controller {
);
accordions.forEach((content) => content.removeAttribute("hidden"));
}

validatePresence(e) {
const target = e.target;
const formGroup = target.closest(".usa-form-group");
const fieldName = target.dataset.fieldName || target.id;

const label = this.findLabel(target, formGroup);

if (!target.value) {
this.addErrorClasses(target, label);
this.updateErrorMessage(fieldName, "can't be blank");
} else {
this.removeErrorClasses(target, label);
this.updateErrorMessage(fieldName, "");
}
}

findLabel(target, formGroup) {
const isSelect =
target.tagName === "SELECT" ||
target.classList.contains("usa-combo-box__input");
const isRadio = target.type === "radio";

const labelId = isSelect ? target.name : target.id;
const labelQuery = isRadio ? "legend" : `label[for="${labelId}"]`;

return formGroup.querySelector(labelQuery);
}

addErrorClasses(target, label) {
target.classList.add("border-secondary");
if (label) label.classList.add("text-secondary");
}

removeErrorClasses(target, label) {
target.classList.remove("border-secondary");
if (label) label.classList.remove("text-secondary");
}

updateErrorMessage(field, message) {
document.getElementById(field + "_error").innerHTML = message;
}
}
46 changes: 46 additions & 0 deletions app/javascript/controllers/form_validation_controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { Controller } from "@hotwired/stimulus"

// Connects to data-controller="form-validation"
export default class extends Controller {
validatePresence(e) {
const target = e.target;
const formGroup = target.closest(".usa-form-group");
const fieldName = target.dataset.fieldName || target.id;
const label = this.findLabel(target, formGroup);

if (!target.value) {
this.addErrorClasses(target, label);
this.updateErrorMessage(fieldName, "can't be blank");
} else {
this.removeErrorClasses(target, label);
this.updateErrorMessage(fieldName, "");
}
}

findLabel(target, formGroup) {
const isSelect = target.tagName === "SELECT" || target.classList.contains("usa-combo-box__input");
const isRadio = target.type === "radio";

const labelId = isSelect ? target.name : target.id;
const labelQuery = isRadio ? "legend" : `label[for="${labelId}"]`;

return formGroup.querySelector(labelQuery);
}

addErrorClasses(target, label) {
target.classList.add("border-secondary");
if (label) label.classList.add("text-secondary");
}

removeErrorClasses(target, label) {
target.classList.remove("border-secondary");
if (label) label.classList.remove("text-secondary");
}

updateErrorMessage(field, message) {
const errorElement = document.getElementById(field + "_error");
if (errorElement) {
errorElement.innerHTML = message;
}
}
}
3 changes: 3 additions & 0 deletions app/javascript/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ application.register("evaluation-criteria", EvaluationCriteriaController);
import EvaluationFormController from "./evaluation_form_controller";
application.register("evaluation-form", EvaluationFormController);

import FormValidationController from "./form_validation_controller";
application.register("form-validation", FormValidationController);

import HotdogController from "./hotdog_controller";
application.register("hotdog", HotdogController);

Expand Down
13 changes: 13 additions & 0 deletions app/models/evaluator_invitation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,17 @@ class EvaluatorInvitation < ApplicationRecord
validates :last_invite_sent, presence: true

validates :email, uniqueness: { scope: [:challenge_id, :phase_id] }

def self.split_full_name(name)
names = name.to_s.strip.split(/\s+/, 2)
[names[0], names[1]]
end

def full_name=(name)
self.first_name, self.last_name = self.class.split_full_name(name)
end

def full_name
"#{first_name} #{last_name}".strip
end
end
53 changes: 53 additions & 0 deletions app/services/evaluator_invitation_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
# frozen_string_literal: true

# This service handles evaluator invitations to a challenge phase.
class EvaluatorInvitationService
def initialize(challenge, phase)
@challenge = challenge
@phase = phase
end

def handle_invitation(email, invitation_params)
existing_invitation = @challenge.evaluator_invitations.find_by(email:, phase: @phase)
existing_invitation ? resend_invitation(existing_invitation) : create_new_invitation(invitation_params)
end

# TODO: Implement sending the invitation email here
def resend_invitation(invitation)
if invitation.update(last_invite_sent: Time.current)
{
success: true,
message: I18n.t(
'evaluators.process_evaluator_invitation.invitation_resent',
email: invitation.email
)
}
else
{
success: false, message: I18n.t('evaluators.resend_invite.failure')
}
end
end

private

def create_new_invitation(invitation_params)
invitation = @challenge.evaluator_invitations.new(
invitation_params.merge(
phase: @phase,
last_invite_sent: Time.current
)
)
if invitation.save
{
success: true,
message: I18n.t(
'evaluators.process_evaluator_invitation.invitation_sent',
email: invitation_params[:email]
)
}
else
{ success: false, message: invitation.errors.full_messages.join(", "), evaluator_invitation: invitation }
end
end
end
64 changes: 32 additions & 32 deletions app/services/evaluator_management_service.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
# frozen_string_literal: true

# This service handles evaluator invitation as well as adding and removing evalutors to challenge phases.
# This service handles adding and removing evalutors to and from challenge phases.
class EvaluatorManagementService
def initialize(challenge, phase)
@challenge = challenge
@phase = phase
@invitation_service = EvaluatorInvitationService.new(challenge, phase)
end

def process_evaluator_invitation(email, invitation_params)
@invitation_params = invitation_params
user = User.find_by(email:)
user ? add_existing_user_as_evaluator(user) : handle_invitation(email, invitation_params)
user ? add_existing_user_as_evaluator(user) : @invitation_service.handle_invitation(email, invitation_params)
end

def remove_evaluator(evaluator_type, evaluator_id)
Expand All @@ -32,22 +34,43 @@ def self.accept_evaluator_invitation(user)
{ success: true, message: I18n.t('evaluators.accept_evaluator_invitation.success') }
end

# TODO: Implement sending the actual invitation email here
def resend_invitation(invitation)
if invitation.update(last_invite_sent: Time.current)
{ success: true,
message: I18n.t('evaluators.process_evaluator_invitation.invitation_resent', email: invitation.email) }
else
{ success: false, message: I18n.t('evaluators.resend_invite.failure') }
end
@invitation_service.resend_invitation(invitation)
end

private

def update_name_for_existing_user(user)
temp_invitation = EvaluatorInvitation.new(
full_name: @invitation_params[:full_name],
email: user.email,
challenge: @challenge,
phase: @phase,
last_invite_sent: Time.current
)

if temp_invitation.valid?
user.update(
first_name: temp_invitation.first_name,
last_name: temp_invitation.last_name
)
{ success: true }
else
name_errors = temp_invitation.errors.messages.slice(:first_name, :last_name)
first_error_field, first_error_message = name_errors.first
{ success: false,
message: "#{first_error_field.to_s.humanize} #{first_error_message.first}",
errors: name_errors }
end
end

def add_existing_user_as_evaluator(user)
return user_already_added(user) if @phase.evaluators.include?(user)
return invalid_role(user) unless User::VALID_EVALUATOR_ROLES.include?(user.role)

updated_name = update_name_for_existing_user(user)
return updated_name unless updated_name[:success]

user.role == 'evaluator' ? handle_evaluator_creation(user) : handle_evaluator_role_requested(user)
end

Expand Down Expand Up @@ -77,29 +100,6 @@ def handle_evaluator_creation(user)
end
end

def handle_invitation(email, invitation_params)
existing_invitation = @challenge.evaluator_invitations.find_by(email:, phase: @phase)
existing_invitation ? resend_invitation(existing_invitation) : create_new_invitation(invitation_params)
end

def create_new_invitation(invitation_params)
invitation = @challenge.evaluator_invitations.new(
invitation_params.merge(
phase: @phase,
last_invite_sent: Time.current
)
)
if invitation.save
{ success: true,
message: I18n.t(
'evaluators.process_evaluator_invitation.invitation_sent',
email: invitation_params[:email]
) }
else
{ success: false, message: invitation.errors.full_messages.join(", ") }
end
end

def remove_user_evaluator(evaluator_id)
evaluator = User.find(evaluator_id)
cpe = ChallengePhasesEvaluator.find_by(challenge: @challenge, phase: @phase, user: evaluator)
Expand Down
10 changes: 5 additions & 5 deletions app/views/evaluation_forms/_evaluation_criterion_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
disabled: is_template || form_disabled,
data: {
"evaluation-criteria-target": "titleField",
"action": "evaluation-form#validatePresence focusout->evaluation-form#validatePresence"
"action": "form-validation#validatePresence focusout->form-validation#validatePresence"
}
%>

Expand All @@ -55,7 +55,7 @@
disabled: is_template || form_disabled,
data: {
"evaluation-criteria-target": "descriptionField",
"action": "evaluation-form#validatePresence focusout->evaluation-form#validatePresence"
"action": "form-validation#validatePresence focusout->form-validation#validatePresence"
}
%>
<span class="usa-character-count__message">You can enter up to 1000 characters</span>
Expand Down Expand Up @@ -83,7 +83,7 @@
"evaluation-criteria-target": "pointsOrWeightField",
action: "
input->evaluation-criteria#checkPointsOrWeightMax blur->evaluation-criteria#checkPointsOrWeightMax
evaluation-form#validatePresence focusout->evaluation-form#validatePresence
form-validation#validatePresence focusout->form-validation#validatePresence
"
}
%>
Expand Down Expand Up @@ -111,7 +111,7 @@
disabled: is_template || form_disabled,
data: {
"evaluation-criteria-target": "scoringTypeRadio",
action: "click->evaluation-criteria#toggleScoringType evaluation-form#validatePresence",
action: "click->evaluation-criteria#toggleScoringType form-validation#validatePresence",
field_name: criteria_field_id(f, "scoring_type", is_template)
}
%>
Expand Down Expand Up @@ -187,7 +187,7 @@
value: f.object.option_labels[index.to_s],
disabled: disabledOptionLabel || form_disabled,
data: {
"action": "evaluation-form#validatePresence focusout->evaluation-form#validatePresence",
"action": "form-validation#validatePresence focusout->form-validation#validatePresence",
"evaluation-criteria-target": "optionLabelInput"
}
%>
Expand Down
Loading
Loading