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

[200] Build Manage Evaluators List #243

Merged
merged 56 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
589d27f
200 | Add frontend to manage evaluators list page
emmabjj Oct 29, 2024
8f3fa45
200 | Add delete modal to manage evaluators and functionality
emmabjj Oct 29, 2024
84f3b13
200 | Update header and add challenge status to manage submissions index
emmabjj Oct 29, 2024
12c95ff
200 | Add functionality to add/invite, delete, and resend evaluator +…
emmabjj Oct 29, 2024
89fedc4
200 | Center back link on delete evaluator modal
emmabjj Oct 29, 2024
4f48437
200 | Uniqueness message handled elsewhere
emmabjj Oct 29, 2024
3335e3f
200 | Add evaluator messages to locale
emmabjj Oct 29, 2024
b18a5af
200 | Indentation and lazy lookup for locale
emmabjj Oct 29, 2024
f13edf1
set node-version for circleci
emmabjj Oct 29, 2024
74ebbfc
200 | Break up logic into smaller methods
emmabjj Oct 29, 2024
25fc5b6
200 | Update call
emmabjj Oct 29, 2024
32dbc51
Merge branch 'dev' into 200_manage_evaluators_list
stepchud Oct 30, 2024
0453855
Update .circleci/config.yml
stepchud Oct 30, 2024
6bd9cf3
Merge branch 'dev' into 200_manage_evaluators_list
stepchud Oct 30, 2024
888d2ae
200 | Update queries and naming
emmabjj Nov 1, 2024
e4b10f7
200 | Use stimulus for delete evaluator modal
emmabjj Nov 1, 2024
2eaf379
Merge branch 'dev' into 200_manage_evaluators_list
emmabjj Nov 1, 2024
333a590
200 | Add missing bracket after merge conflict
emmabjj Nov 1, 2024
296a0fe
200 | Update query calls
emmabjj Nov 4, 2024
f25eb6d
200 | Update manage evaluators destroy route
emmabjj Nov 5, 2024
f587d15
[245] Invited Evaluators User Setup (#254)
emmabjj Nov 6, 2024
2a17abd
200 | Adjust challenge route
emmabjj Nov 6, 2024
219095e
200 | remove unused param
emmabjj Nov 6, 2024
db6e2a7
200 | Add guard clause
emmabjj Nov 6, 2024
ffb623d
200 | Remove redundant self check
emmabjj Nov 6, 2024
aaa1c69
Merge branch 'dev' into 200_manage_evaluators_list
emmabjj Nov 6, 2024
1764e7e
200 | Fix failing tests
emmabjj Nov 7, 2024
f77730f
consolidate routes
stepchud Nov 7, 2024
adc6422
revert structure.sql changes
stepchud Nov 7, 2024
98bb679
remove rbenv from .envrc
stepchud Nov 7, 2024
d23da73
200 | Remove unnecessary css property
emmabjj Nov 12, 2024
b9364aa
200 | Adjust UI
emmabjj Nov 12, 2024
b725416
200 | Move controller specs to spec/requests
emmabjj Nov 12, 2024
667cb62
200 | Require authorized user in evaluator invitations controller
emmabjj Nov 12, 2024
9f3d3af
200 | Move business logic to evaluator management service object
emmabjj Nov 12, 2024
c1cb69c
200 | Add evaluator management service object + tests
emmabjj Nov 12, 2024
a630bb9
200 | Use service object on after_create in user model for processign…
emmabjj Nov 12, 2024
5e7eb94
200 | Add translations
emmabjj Nov 12, 2024
de375d6
200 | Fix some formatting
emmabjj Nov 13, 2024
32e3205
scope the Challenge query to the current_user
stepchud Nov 13, 2024
0f60b7c
Merge remote-tracking branch 'origin/dev' into 200_manage_evaluators_…
stepchud Nov 15, 2024
a56ba86
review feedback
stepchud Nov 15, 2024
f15ffad
update javascript url
stepchud Nov 15, 2024
86c9b6f
fix n+1 queries
stepchud Nov 15, 2024
4fe4a1b
codeclimate shrink JS function
stepchud Nov 15, 2024
13a08bc
update the invitation routes
stepchud Nov 19, 2024
73af9de
missing status check
stepchud Nov 19, 2024
13dbc4c
rubocop specs
stepchud Nov 19, 2024
a30de58
Merge pull request #279 from GSA/200_manage_evaluators_restful
emmabjj Nov 19, 2024
15405e0
Merge branch 'dev' into 200_manage_evaluators_list
emmabjj Nov 19, 2024
7cecef3
Bullet failure: Remove unnecessary eager loaded :submissions not used…
emmabjj Nov 19, 2024
4064d4f
adjust error message
stepchud Nov 19, 2024
0df811a
inline another render
stepchud Nov 19, 2024
380c7ce
rename helper, remove duplicate helper method
stepchud Nov 19, 2024
b13b54f
Merge branch 'dev' into 200_manage_evaluators_list
stepchud Nov 19, 2024
e41dd2b
remove unnecessary code
stepchud Nov 19, 2024
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
3 changes: 0 additions & 3 deletions .envrc
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,5 @@ use nix
mkdir -p .nix-bundler
export BUNDLE_PATH=./.nix-bundler

# hook for rbenv locally
which rbenv &> /dev/null && eval "$(rbenv init - -zsh)"
stepchud marked this conversation as resolved.
Show resolved Hide resolved

# Login Env Vars
source .env_login
8 changes: 8 additions & 0 deletions app/assets/stylesheets/application.sass.scss
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ dialog::backdrop {
border-color: black;
}

.usa-table.usa-table--borderless.gray-header {
stepchud marked this conversation as resolved.
Show resolved Hide resolved
thead {
th {
background-color: #dfe1e2;
}
}
}

#add-criteria-button.usa-button {
background-color: #4d8055;
}
28 changes: 28 additions & 0 deletions app/controllers/evaluator_invitations_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# frozen_string_literal: true

class EvaluatorInvitationsController < ApplicationController
before_action -> { authorize_user('challenge_manager') }
before_action :set_challenge
before_action :set_evaluator_invitation

def resend
if @evaluator_invitation.update(last_invite_sent: Time.current)
# TODO: Implement sending the actual invitation email here
redirect_to challenge_manage_evaluators_path(@challenge),
notice: t('.success')
else
redirect_to challenge_manage_evaluators_path(@challenge),
alert: t('.failure')
end
end

private

def set_challenge
@challenge = current_user.challenge_manager_challenges.find(params[:challenge_id])
end

def set_evaluator_invitation
@evaluator_invitation = @challenge.evaluator_invitations.find(params[:id])
end
end
82 changes: 82 additions & 0 deletions app/controllers/manage_evaluators_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# frozen_string_literal: true

class ManageEvaluatorsController < ApplicationController
include ManageEvaluatorsHelper

before_action -> { authorize_user('challenge_manager') }
before_action :set_challenge
stepchud marked this conversation as resolved.
Show resolved Hide resolved
before_action :set_phase, only: [:index, :create, :destroy]

def index
@phase ? handle_existing_phases : handle_empty_phases
end

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

handle_invitation_result(result)
end

def destroy
result = evaluator_service.remove_evaluator(params[:evaluator_type], params[:id])
render_json_response(result)
end

private

def set_challenge
@challenge = Challenge.find(params[:challenge_id])
stepchud marked this conversation as resolved.
Show resolved Hide resolved
end

def set_phase
phase_id = params.dig(:evaluator_invitation, :phase_id) || params[:phase_id]
@phase = phase_id ? @challenge.phases.find(phase_id) : @challenge.phases.order(:start_date).first
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this before_action is overloaded with several routes and conditionals. I'm not sure how each route will work. before_actions should be simple logic and either check for error conditions in the request or load data in a predictable way, if they're used at all. I'm not sure what kinds of bugs might be caused by an incorrect param. why does it default to the first phase by start_date? which route(s) does that apply to? it gets pretty confusing.


def evaluator_service
@evaluator_service ||= EvaluatorManagementService.new(@challenge, @phase)
end

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

def handle_existing_phases
fetch_evaluators_and_invitations
end

def handle_empty_phases
flash.now[:alert] = t('.no_phases_alert')
@evaluator_invitations = []
@existing_evaluators = []
end

def fetch_evaluators_and_invitations
@evaluator_invitations = @phase.evaluator_invitations
@existing_evaluators = @phase.evaluators
end

def handle_invitation_result(result)
if result[:success]
redirect_to challenge_manage_evaluators_path(@challenge, phase_id: @phase.id), notice: result[:message]
else
flash.now[:alert] = result[:message]
handle_existing_phases
render :index
end
end

def render_json_response(result)
if result[:success]
flash[:notice] = result[:message]
render json: { success: true, message: result[:message] }
else
render json: { success: false, message: result[:message] }, status: :unprocessable_entity
end
end
end
22 changes: 22 additions & 0 deletions app/helpers/manage_evaluators_helper.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# frozen_string_literal: true

module ManageEvaluatorsHelper
def user_status(evaluator)
if evaluator.is_a?(User)
evaluator.status == 'active' ? "Available" : "Awaiting Approval"
else
"Invite Sent"
end
end

def assigned_submissions_count(evaluator, challenge, phase)
if evaluator.is_a?(User)
evaluator.evaluator_submission_assignments.
joins(:submission).
where(submissions: { challenge:, phase: }).
count
else
0
end
end
end
82 changes: 82 additions & 0 deletions app/javascript/controllers/delete_evaluator_modal_controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { Controller } from "@hotwired/stimulus"

export default class extends Controller {
static targets = ["modal", "confirmButton", "modalDescription"]
static values = {
challengeId: String,
evaluatorId: String,
evaluatorType: String,
phaseId: String
}

connect() {
this.modalTarget.addEventListener('click', this.handleOutsideClick.bind(this))
}

disconnect() {
this.modalTarget.removeEventListener('click', this.handleOutsideClick.bind(this))
}

open(event) {
event.preventDefault()
this.evaluatorIdValue = event.currentTarget.dataset.evaluatorId
this.evaluatorTypeValue = event.currentTarget.dataset.evaluatorType
this.challengeIdValue = event.currentTarget.dataset.challengeId
this.phaseIdValue = event.currentTarget.dataset.phaseId
this.modalTarget.showModal()
}

close() {
this.modalTarget.close()
this.resetModal()
}

handleOutsideClick(event) {
if (event.target === this.modalTarget) {
this.close()
}
}

confirm() {
this.deleteEvaluator()
}

deleteEvaluator(forceDelete = false) {
const csrfToken = document.querySelector('meta[name="csrf-token"]').content

fetch(`/challenges/${this.challengeIdValue}/manage_evaluators/${this.evaluatorIdValue}`, {
method: 'DELETE',
headers: {
'Content-Type': 'application/json',
'X-CSRF-Token': csrfToken
},
body: JSON.stringify({
evaluator_type: this.evaluatorTypeValue,
phase_id: this.phaseIdValue,
force_delete: forceDelete
})
})
.then(response => {
if (!response.ok) {
throw new Error('Network response was not ok')
}
return response.json()
})
.then(data => {
if (data.success) {
this.close()
window.location.reload()
} else {
throw new Error(data.message || 'Failed to remove evaluator')
}
})
.catch(error => {
alert(error.message || 'An error occurred while removing the evaluator')
})
}

resetModal() {
this.modalDescriptionTarget.textContent = 'Deleting an evaluator from the challenge will remove the evaluator from any submissions of this challenge that the evaluator is assigned to. It will also delete any of their completed or in progress evaluations for those submissions.'
this.confirmButtonTarget.textContent = 'Yes'
}
}
3 changes: 3 additions & 0 deletions app/javascript/controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ import EvaluationFormController from "./evaluation_form_controller";
import EvaluationCriteriaController from "./evaluation_criteria_controller";
application.register("evaluation-form", EvaluationFormController);
application.register("evaluation-criteria", EvaluationCriteriaController);

import DeleteEvaluatorModalController from "./delete_evaluator_modal_controller"
application.register("delete-evaluator-modal", DeleteEvaluatorModalController)
10 changes: 10 additions & 0 deletions app/models/challenge_phases_evaluator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,14 @@ class ChallengePhasesEvaluator < ApplicationRecord
belongs_to :challenge
belongs_to :phase
belongs_to :user

validate :user_has_valid_role

private

def user_has_valid_role
return if User::VALID_EVALUATOR_ROLES.include?(user.role)

errors.add(:user, "must have a valid evaluator role")
end
end
16 changes: 15 additions & 1 deletion app/models/user.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# frozen_string_literal: true

require_relative '../services/evaluator_management_service'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this line needed? I was under the impression that Rails handles requiring all the code under app/?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still works without it 👍


# == Schema Information
#
# Table name: users
Expand Down Expand Up @@ -33,6 +35,10 @@
# recertification_expired_at :datetime
#
class User < ApplicationRecord
after_create :process_evaluator_invitations

VALID_EVALUATOR_ROLES = %w[evaluator solver challenge_manager].freeze

belongs_to :agency, optional: true

has_many :challenges, dependent: :destroy
Expand Down Expand Up @@ -130,7 +136,9 @@ def self.create_user_from_userinfo(userinfo)
end

def self.default_role_and_status_for_email(email)
if default_challenge_manager?(email)
if EvaluatorInvitation.exists?(email:)
%w[evaluator pending]
elsif default_challenge_manager?(email)
%w[challenge_manager pending]
else
%w[solver active]
Expand All @@ -140,4 +148,10 @@ def self.default_role_and_status_for_email(email)
def self.default_challenge_manager?(email)
/\.(gov|mil)$/.match?(email)
end

private

def process_evaluator_invitations
EvaluatorManagementService.accept_evaluator_invitation(self)
end
end
Loading
Loading