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/mailers/recruiter_mailer.rb b/app/mailers/recruiter_mailer.rb index 8d9e66c..d9dff41 100644 --- a/app/mailers/recruiter_mailer.rb +++ b/app/mailers/recruiter_mailer.rb @@ -1,30 +1,23 @@ # frozen_string_literal: true class RecruiterMailer < ApplicationMailer def candidate_created candidate - @candidate = candidate + @candidate = Candidate.find_by(id: candidate.to_i) mail to: @candidate.recruiter.email, subject: "Skills Assessment Test - #{candidate.name}" end def candidate_submitted candidate - @candidate = candidate + @candidate = Candidate.find_by(id: candidate.to_i) mail to: @candidate.recruiter.email, subject: "Skills Assessment Test - #{candidate.name}" end - def interview_requested candidate - @candidate = candidate + def candidate_reviewed candidate + @candidate = Candidate.find_by(id: candidate.to_i) mail to: @candidate.recruiter.email, - subject: "Skills Assesment - Interview Request for #{candidate.name}" - end - - def interview_declined candidate - @candidate = candidate - - mail to: @candidate.recruiter.email, - subject: "Skills Assesment - Interview Declined for #{candidate.name}" + subject: "Skills Assesment - Review Complete for #{candidate.name}" 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/policies/reviewer_vote_policy.rb b/app/policies/reviewer_vote_policy.rb index 00f28a7..a5d2193 100644 --- a/app/policies/reviewer_vote_policy.rb +++ b/app/policies/reviewer_vote_policy.rb @@ -21,13 +21,7 @@ class ReviewerVotePolicy < ApplicationPolicy user.acts_as_reviewer? end - def approve? - return true if user.acts_as_admin? - return false unless record.candidate.reviewers.include? user - user.acts_as_manager? - end - - def decline? + def interview_request? return true if user.acts_as_admin? return false unless record.candidate.reviewers.include? user user.acts_as_manager? 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/app/views/recruiter_mailer/candidate_reviewed.html.inky b/app/views/recruiter_mailer/candidate_reviewed.html.inky new file mode 100644 index 0000000..1eecc06 --- /dev/null +++ b/app/views/recruiter_mailer/candidate_reviewed.html.inky @@ -0,0 +1,9 @@ + + + diff --git a/app/views/recruiter_mailer/candidate_reviewed.text.erb b/app/views/recruiter_mailer/candidate_reviewed.text.erb new file mode 100644 index 0000000..0beabee --- /dev/null +++ b/app/views/recruiter_mailer/candidate_reviewed.text.erb @@ -0,0 +1,7 @@ +PERFICIENT/digital - Skills Assessment Test + +The team has <%= @candidate.review_status %> an interview with <%= @candidate.name %> with the following comments: + +<%= @candidate.review_comments %> + +Thank you. diff --git a/app/views/recruiter_mailer/interview_declined.html.inky b/app/views/recruiter_mailer/interview_declined.html.inky deleted file mode 100644 index 1dd0d42..0000000 --- a/app/views/recruiter_mailer/interview_declined.html.inky +++ /dev/null @@ -1,6 +0,0 @@ - - - diff --git a/app/views/recruiter_mailer/interview_declined.text.erb b/app/views/recruiter_mailer/interview_declined.text.erb deleted file mode 100644 index 1ef40ae..0000000 --- a/app/views/recruiter_mailer/interview_declined.text.erb +++ /dev/null @@ -1,5 +0,0 @@ -PERFICIENT/digital - Skills Assessment Test - -The team has declined an interview with <%= @candidate.name %>. - -Thank you. diff --git a/app/views/recruiter_mailer/interview_requested.html.inky b/app/views/recruiter_mailer/interview_requested.html.inky deleted file mode 100644 index bbaa0df..0000000 --- a/app/views/recruiter_mailer/interview_requested.html.inky +++ /dev/null @@ -1,6 +0,0 @@ - - - diff --git a/app/views/recruiter_mailer/interview_requested.text.erb b/app/views/recruiter_mailer/interview_requested.text.erb deleted file mode 100644 index ca5d7f3..0000000 --- a/app/views/recruiter_mailer/interview_requested.text.erb +++ /dev/null @@ -1,5 +0,0 @@ -PERFICIENT/digital - Skills Assessment Test - -The team has requested an interview with <%= @candidate.name %>. - -Thank you. 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/db/migrate/20170228161543_add_comments_to_candidate.rb b/db/migrate/20170228161543_add_comments_to_candidate.rb new file mode 100644 index 0000000..960f178 --- /dev/null +++ b/db/migrate/20170228161543_add_comments_to_candidate.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true +class AddCommentsToCandidate < ActiveRecord::Migration[5.0] + def change + add_column :candidates, :review_comments, :text, after: :review_status + end +end diff --git a/db/migrate/20170228161729_remove_last_reminded_from_reviewer_votes.rb b/db/migrate/20170228161729_remove_last_reminded_from_reviewer_votes.rb new file mode 100644 index 0000000..1631176 --- /dev/null +++ b/db/migrate/20170228161729_remove_last_reminded_from_reviewer_votes.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true +class RemoveLastRemindedFromReviewerVotes < ActiveRecord::Migration[5.0] + def change + remove_column :reviewer_votes, :last_reminded, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index ddc3263..243d9ea 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170227154554) do +ActiveRecord::Schema.define(version: 20170228161729) do create_table "answers", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t| t.integer "candidate_id" @@ -37,10 +37,11 @@ ActiveRecord::Schema.define(version: 20170227154554) do t.boolean "completed" t.datetime "completed_at" t.boolean "reminded" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.integer "quiz_id" - t.integer "review_status", default: 0, null: false + t.integer "review_status", default: 0, null: false + t.text "review_comments", limit: 65535 t.index ["quiz_id"], name: "index_candidates_on_quiz_id", using: :btree t.index ["recruiter_id"], name: "index_candidates_on_recruiter_id", using: :btree t.index ["test_hash"], name: "index_candidates_on_test_hash", unique: true, using: :btree @@ -89,11 +90,10 @@ ActiveRecord::Schema.define(version: 20170227154554) do create_table "reviewer_votes", force: :cascade, options: "ENGINE=InnoDB DEFAULT CHARSET=utf8" do |t| t.integer "candidate_id" t.integer "user_id" - t.integer "vote", default: 0, null: false - t.integer "veto", default: 0, null: false - t.datetime "last_reminded" - t.datetime "created_at", null: false - t.datetime "updated_at", null: false + t.integer "vote", default: 0, null: false + t.integer "veto", default: 0, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false t.index ["candidate_id", "user_id"], name: "index_reviewer_votes_on_candidate_id_and_user_id", unique: true, using: :btree end 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 diff --git a/test/fixtures/candidates.yml b/test/fixtures/candidates.yml index 46608f7..d298cc5 100644 --- a/test/fixtures/candidates.yml +++ b/test/fixtures/candidates.yml @@ -79,6 +79,7 @@ richard: # Richard has completed AND submitted the test reminded: false test_hash: 6NjnourLE6Y review_status: 1 + review_comments: "Some reasons why or why not, but here they are." juan: # Juan has chosen "finish later" for live coders name: Juan Campbell @@ -107,6 +108,7 @@ stacy: # Stacy has completed AND submitted the test reminded: false test_hash: s6oFExZliYYFx review_status: 2 + review_comments: "Some reasons why or why not, but here they are." henry: # Henry has completed AND submitted the test name: Henry Butler diff --git a/test/mailers/previews/recruiter_mailer_preview.rb b/test/mailers/previews/recruiter_mailer_preview.rb index 257f254..f287a52 100644 --- a/test/mailers/previews/recruiter_mailer_preview.rb +++ b/test/mailers/previews/recruiter_mailer_preview.rb @@ -9,11 +9,7 @@ class RecruiterMailerPreview < ActionMailer::Preview RecruiterMailer.candidate_submitted Candidate.find_by(test_hash: 'OvP0ZqGKwJ0') # Dawn end - def interview_requested - RecruiterMailer.interview_requested Candidate.find_by(test_hash: '6NjnourLE6Y') # Richard - end - - def interview_declined - RecruiterMailer.interview_declined Candidate.find_by(test_hash: 's6oFExZliYYFx') # Stacy + def candidate_reviewed + RecruiterMailer.candidate_reviewed Candidate.find_by(test_hash: 's6oFExZliYYFx') # Stacy end end diff --git a/test/mailers/recruiter_mailer_test.rb b/test/mailers/recruiter_mailer_test.rb index d563fd5..bf1341e 100644 --- a/test/mailers/recruiter_mailer_test.rb +++ b/test/mailers/recruiter_mailer_test.rb @@ -22,21 +22,14 @@ class RecruiterMailerTest < ActionMailer::TestCase assert_match manager.name, mail.body.encoded end - test "interview_requested" do - candidate = candidates :richard - mail = RecruiterMailer.interview_requested candidate - assert_match candidate.name, mail.subject - assert_equal [candidate.recruiter.email], mail.to - assert_equal [ENV["default_mail_from"]], mail.from - assert_match candidate.name, mail.body.encoded - end - - test "interview_declined" do + test "candidate_reviewed" do candidate = candidates :stacy - mail = RecruiterMailer.interview_declined candidate + mail = RecruiterMailer.candidate_reviewed candidate assert_match candidate.name, mail.subject assert_equal [candidate.recruiter.email], mail.to assert_equal [ENV["default_mail_from"]], mail.from + assert_match candidate.review_status, mail.body.encoded assert_match candidate.name, mail.body.encoded + assert_match candidate.review_comments, mail.body.encoded end end diff --git a/test/policies/reviewer_vote_policy_test.rb b/test/policies/reviewer_vote_policy_test.rb index 95c8e82..e529e10 100644 --- a/test/policies/reviewer_vote_policy_test.rb +++ b/test/policies/reviewer_vote_policy_test.rb @@ -48,15 +48,7 @@ class ReviewerVotePolicyTest < PolicyAssertions::Test refute_permit users(:manager), reviewer_votes(:gustov) end - def approve - assert_permit users(:manager), reviewer_votes(:manager_richard) - assert_permit users(:admin), reviewer_votes(:manager_henry) - - refute_permit users(:recruiter), reviewer_votes(:manager_henry) - refute_permit users(:reviewer), reviewer_votes(:reviewer_richard) - end - - def decline + def interview_request assert_permit users(:manager), reviewer_votes(:manager_richard) assert_permit users(:admin), reviewer_votes(:manager_henry)