From c38e0c9bf87cbd6a4fe929f3b67312a3d6f40ed2 Mon Sep 17 00:00:00 2001 From: Mark Moser Date: Thu, 2 Mar 2017 10:32:02 -0600 Subject: [PATCH] refactored approval system - now with comments --- app/assets/javascripts/ajax-links.js | 9 --- .../stylesheets/molecules/_admin_review.scss | 28 +++++++- app/controllers/admin/vote_controller.rb | 18 +++-- app/models/reviewer_vote.rb | 2 +- app/models/user.rb | 18 ++--- app/views/admin/result/_voting.html.erb | 26 +++---- app/views/admin/vote/approve.json.erb | 5 -- app/views/admin/vote/decline.json.erb | 5 -- config/routes.rb | 7 +- .../controllers/admin/vote_controller_test.rb | 72 +++++++------------ 10 files changed, 84 insertions(+), 106 deletions(-) delete mode 100644 app/views/admin/vote/approve.json.erb delete mode 100644 app/views/admin/vote/decline.json.erb diff --git a/app/assets/javascripts/ajax-links.js b/app/assets/javascripts/ajax-links.js index 55ee365..4a700c2 100644 --- a/app/assets/javascripts/ajax-links.js +++ b/app/assets/javascripts/ajax-links.js @@ -18,11 +18,6 @@ function updateVotes(data){ $("[data-id=my-vote]").html(data.myVote); } -function updateVeto(data){ - $("[data-id=interview-request]").html(data.requestCopy); - $("[data-id=interview-decline]").html(data.declineCopy); -} - $(document).ready(function() { $('[data-id=ajax-action]').each(function(){ handleAjaxResponse($(this)); }); }); @@ -30,7 +25,3 @@ $(document).ready(function() { $(document).ready(function() { $('[data-id=vote-count]').each(function(){ handleAjaxResponse($(this), updateVotes); }); }); - -$(document).ready(function() { - $('[data-id=veto-status]').each(function(){ handleAjaxResponse($(this), updateVeto); }); -}); diff --git a/app/assets/stylesheets/molecules/_admin_review.scss b/app/assets/stylesheets/molecules/_admin_review.scss index ab119d1..dc1f8fb 100644 --- a/app/assets/stylesheets/molecules/_admin_review.scss +++ b/app/assets/stylesheets/molecules/_admin_review.scss @@ -83,8 +83,32 @@ & > div { flex: 1 1 auto; } } - .review_meta__votes, - .review_meta__vetos { + .review_meta__votes { + margin-bottom: 15px; a { padding: 5px; } } + + .review_meta__vetos { + label { + display: inline-block; + margin-left: 15px; + } + + .review-status-comments { + opacity: 0; + padding: 15px 0; + height: 0; + max-height: 0; + overflow: hidden; + transition: all 0.500s; + } + + input:checked ~ .review-status-comments { + opacity: 1; + height: auto; + max-height: 9999px; + overflow: auto; + transition: all 0.7500s; + } + } } diff --git a/app/controllers/admin/vote_controller.rb b/app/controllers/admin/vote_controller.rb index acf2008..538bebf 100644 --- a/app/controllers/admin/vote_controller.rb +++ b/app/controllers/admin/vote_controller.rb @@ -13,22 +13,20 @@ module Admin current_user.cast_nay_on(@candidate) end - def approve + def interview_request @candidate = Candidate.find_by(test_hash: params[:test_hash]) authorize ReviewerVote.find_by(user_id: current_user.id, candidate_id: @candidate.id) - if current_user.approve_candidate(@candidate) - RecruiterMailer.interview_requested(@candidate).deliver_later - end + current_user.review_candidate(@candidate, interview_params) + RecruiterMailer.candidate_reviewed(@candidate).deliver_later + redirect_to admin_result_path(@candidate.test_hash), + flash: { notice: "Quiz #{interview_params[:review_status]}" } end - def decline - @candidate = Candidate.find_by(test_hash: params[:test_hash]) - authorize ReviewerVote.find_by(user_id: current_user.id, candidate_id: @candidate.id) + private - if current_user.decline_candidate(@candidate) - RecruiterMailer.interview_declined(@candidate).deliver_later + def interview_params + params.permit(:review_status, :review_comments) end - end end end diff --git a/app/models/reviewer_vote.rb b/app/models/reviewer_vote.rb index 2dce2f4..26770f3 100644 --- a/app/models/reviewer_vote.rb +++ b/app/models/reviewer_vote.rb @@ -15,7 +15,7 @@ class ReviewerVote < ApplicationRecord enum veto: { approved: 1, - rejected: 2 + declined: 2 } private diff --git a/app/models/user.rb b/app/models/user.rb index 02b88bf..26f5957 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -38,20 +38,16 @@ class User < ApplicationRecord vote.save end - def approve_candidate candidate + def review_candidate candidate, parms_hash candidate = Candidate.find(candidate.to_i) vote = votes.find_by(candidate_id: candidate.to_i) - vote.veto = :approved - candidate.update_attribute(:review_status, :approved) if vote.save - end - - def decline_candidate candidate - candidate = Candidate.find(candidate.to_i) - - vote = votes.find_by(candidate_id: candidate.to_i) - vote.veto = :rejected - candidate.update_attribute(:review_status, :declined) if vote.save + vote.veto = parms_hash[:review_status] + if vote.save + # skipping validations on candidate because that's not the managers responsibility + candidate.review_comments = parms_hash[:review_comments] + candidate.update_attribute(:review_status, parms_hash[:review_status]) + end end def my_vote candidate diff --git a/app/views/admin/result/_voting.html.erb b/app/views/admin/result/_voting.html.erb index 42316c5..689a7ae 100644 --- a/app/views/admin/result/_voting.html.erb +++ b/app/views/admin/result/_voting.html.erb @@ -23,22 +23,22 @@ <% end %> <% if current_user.acts_as_manager? %> -
- Interview: +
+ <%= form_tag admin_interview_path(test_hash: @candidate.test_hash) do %> + Interview: - <% if !@candidate.approved? %> - <%= link_to admin_approve_vote_path(test_hash: @candidate.test_hash), remote: true do %> - Yes - <% end %> - <% else %>Yes<% end %> + <%= radio_button_tag :review_status, :approved, checked = @candidate.approved? %> + <%= label_tag :review_status_approved, "Yes" %> + <%= radio_button_tag :review_status, :declined, checked = @candidate.declined? %> + <%= label_tag :review_status_declined, "No" %> - <% if !@candidate.declined? %> - <%= link_to admin_decline_vote_path(test_hash: @candidate.test_hash), remote: true do %> - No - <% end %> - <% else %>No<% end %> - +
+ Review comments for recruiter + <%= text_area_tag :review_comments, @candidate.review_comments %> + <%= submit_tag 'Send Request' %> +
+ <% end %>
<% else %> diff --git a/app/views/admin/vote/approve.json.erb b/app/views/admin/vote/approve.json.erb deleted file mode 100644 index 1aa0072..0000000 --- a/app/views/admin/vote/approve.json.erb +++ /dev/null @@ -1,5 +0,0 @@ -{ - "message" : "Interview requested!", - "requestCopy" : "Requested", - "declineCopy" : "Decline Interview" -} diff --git a/app/views/admin/vote/decline.json.erb b/app/views/admin/vote/decline.json.erb deleted file mode 100644 index 6d98faf..0000000 --- a/app/views/admin/vote/decline.json.erb +++ /dev/null @@ -1,5 +0,0 @@ -{ - "message" : "Interview declined.", - "requestCopy" : "Request Interview", - "declineCopy" : "Declined" -} diff --git a/config/routes.rb b/config/routes.rb index c806362..6996760 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -55,10 +55,9 @@ Rails.application.routes.draw do post "/admin/comment/:test_hash/:id", to: "admin/comment#update", as: :admin_update_comment post "/admin/comment/:test_hash", to: "admin/comment#create", as: :admin_create_comment - get "admin/vote/:test_hash/up", to: "admin/vote#up", as: :admin_up_vote, defaults: { format: 'json' } - get "admin/vote/:test_hash/down", to: "admin/vote#down", as: :admin_down_vote, defaults: { format: 'json' } - get "admin/vote/:test_hash/approve", to: "admin/vote#approve", as: :admin_approve_vote, defaults: { format: 'json' } - get "admin/vote/:test_hash/decline", to: "admin/vote#decline", as: :admin_decline_vote, defaults: { format: 'json' } + get "admin/vote/:test_hash/up", to: "admin/vote#up", as: :admin_up_vote, defaults: { format: 'json' } + get "admin/vote/:test_hash/down", to: "admin/vote#down", as: :admin_down_vote, defaults: { format: 'json' } + post "admin/vote/:test_hash", to: "admin/vote#interview_request", as: :admin_interview get "/admin", to: "admin/dashboard#show", as: :admin diff --git a/test/controllers/admin/vote_controller_test.rb b/test/controllers/admin/vote_controller_test.rb index 23e202e..79686c6 100644 --- a/test/controllers/admin/vote_controller_test.rb +++ b/test/controllers/admin/vote_controller_test.rb @@ -5,6 +5,24 @@ module Admin class VoteControllerTest < ActionDispatch::IntegrationTest include ActiveJob::TestHelper + test "reviewer can only vote after commenting" do + auth_user users(:reviewer) + henry = candidates(:henry) + + assert_difference("Candidate.find(#{henry.id}).votes.yea.count", 0) do + get admin_up_vote_url(henry.test_hash) + end + + post admin_create_comment_url(test_hash: henry.test_hash), + params: { quiz_comment: { message: 'this is a comment to vote' } } + + assert_difference("Candidate.find(#{henry.id}).votes.yea.count", 1) do + get admin_up_vote_url(henry.test_hash) + end + + assert_response :success + end + test "reviewer can up vote henry" do auth_user users(:reviewer2) henry = candidates(:henry) @@ -38,57 +56,19 @@ module Admin assert_response :success end - test "manager can approve henry" do + test "manager can approve henry and notify recruiter" do auth_user users(:manager) henry = candidates(:henry) - get admin_approve_vote_url(henry.test_hash) - assert_equal 1, henry.votes.approved.count - assert_equal 'approved', Candidate.find(henry.to_i).review_status, xhr: true - assert_response :success - data = JSON.parse(response.body) - assert_match 'requested', data["message"] - end - - test "manager can decline henry" do - auth_user users(:manager) - henry = candidates(:henry) - get admin_decline_vote_url(henry.test_hash), xhr: true - - assert_equal 1, henry.votes.rejected.count - assert_equal 'declined', Candidate.find(henry.to_i).review_status - assert_response :success - data = JSON.parse(response.body) - assert_match 'declined', data["message"] - end - - test "should queue up a notification when manager approves henry" do - auth_user users(:manager) - henry = candidates(:henry) assert_enqueued_jobs 1 do - get admin_approve_vote_url(henry.test_hash), xhr: true + post admin_interview_url(henry.test_hash), params: { + review_status: 'approved', + review_comments: 'ipsum' + } end - assert_response :success - data = JSON.parse(response.body) - assert_match 'requested', data["message"] - end - - test "reviewer can only vote after commenting" do - auth_user users(:reviewer) - henry = candidates(:henry) - - assert_difference("Candidate.find(#{henry.id}).votes.yea.count", 0) do - get admin_up_vote_url(henry.test_hash) - end - - post admin_create_comment_url(test_hash: henry.test_hash), - params: { quiz_comment: { message: 'this is a comment to vote' } } - - assert_difference("Candidate.find(#{henry.id}).votes.yea.count", 1) do - get admin_up_vote_url(henry.test_hash) - end - - assert_response :success + assert_equal 'approved', Candidate.find(henry.to_i).review_status + assert_equal 'ipsum', Candidate.find(henry.to_i).review_comments + assert_redirected_to admin_result_url(henry.test_hash) end end end