new approval process with comments

completes #101

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); });
-});

app/assets/stylesheets/molecules/_admin_review.scss

@@ -83,8 +83,32 @@
     & > div { flex: 1 1 auto; }
   }
 
-  .review_meta__votes,
+  .review_meta__votes {
-  .review_meta__vetos {
+    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;
+    }
+  }
 }

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)
+      current_user.review_candidate(@candidate, interview_params)
-        RecruiterMailer.interview_requested(@candidate).deliver_later
+      RecruiterMailer.candidate_reviewed(@candidate).deliver_later
-      end
+      redirect_to admin_result_path(@candidate.test_hash),
+                  flash: { notice: "Quiz #{interview_params[:review_status]}" }
     end
 
-    def decline
+    private
-      @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.decline_candidate(@candidate)
+      def interview_params
-        RecruiterMailer.interview_declined(@candidate).deliver_later
+        params.permit(:review_status, :review_comments)
-      end
       end
   end
 end

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
+  def candidate_reviewed candidate
-    @candidate = candidate
+    @candidate = Candidate.find_by(id: candidate.to_i)
 
     mail to: @candidate.recruiter.email,
-         subject: "Skills Assesment - Interview Request for #{candidate.name}"
+         subject: "Skills Assesment - Review Complete for #{candidate.name}"
-  end
-
-  def interview_declined candidate
-    @candidate = candidate
-
-    mail to: @candidate.recruiter.email,
-         subject: "Skills Assesment - Interview Declined for #{candidate.name}"
   end
 end

app/models/reviewer_vote.rb

@@ -15,7 +15,7 @@ class ReviewerVote < ApplicationRecord
 
   enum veto: {
     approved: 1,
-    rejected: 2
+    declined: 2
   }
 
   private

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
+    vote.veto = parms_hash[:review_status]
-    candidate.update_attribute(:review_status, :approved) if vote.save
+    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
-
-  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
   end
 
   def my_vote candidate

app/policies/reviewer_vote_policy.rb

@@ -21,13 +21,7 @@ class ReviewerVotePolicy < ApplicationPolicy
     user.acts_as_reviewer?
   end
 
-  def approve?
+  def interview_request?
-    return true if user.acts_as_admin?
-    return false unless record.candidate.reviewers.include? user
-    user.acts_as_manager?
-  end
-
-  def decline?
     return true if user.acts_as_admin?
     return false unless record.candidate.reviewers.include? user
     user.acts_as_manager?

app/views/admin/result/_voting.html.erb

@@ -23,22 +23,22 @@
 <% end %>
 
 <% if current_user.acts_as_manager? %>
-  <div class="review_meta__vetos" data-id="veto-status">
+  <div class="review_meta__vetos">
+    <%= form_tag admin_interview_path(test_hash: @candidate.test_hash) do %>
       <strong>Interview: </strong>
 
-    <% if !@candidate.approved? %>
+      <%= radio_button_tag :review_status, :approved, checked = @candidate.approved? %>
-      <%= link_to admin_approve_vote_path(test_hash: @candidate.test_hash), remote: true do %>
+      <%= label_tag :review_status_approved, "Yes" %>
-        <span data-id="interview-request">Yes</span>
+
+      <%= radio_button_tag :review_status, :declined, checked = @candidate.declined? %>
+      <%= label_tag :review_status_declined, "No" %>
+
+      <div class="review-status-comments">
+        <span>Review comments for recruiter</span>
+        <%= text_area_tag :review_comments, @candidate.review_comments %>
+        <%= submit_tag 'Send Request' %>
+      </div>
     <% end %>
-    <% else %>Yes<% end %>
-
-
-    <% if !@candidate.declined? %>
-      <%= link_to admin_decline_vote_path(test_hash: @candidate.test_hash), remote: true do %>
-        <span data-id="interview-decline">No</span>
-      <% end %>
-    <% else %>No<% end %>
-
   </div>
 <% else %>
 

app/views/admin/vote/approve.json.erb

@@ -1,5 +0,0 @@
-{
- "message" : "Interview requested!",
- "requestCopy" : "Requested",
- "declineCopy" : "Decline Interview"
-}

app/views/admin/vote/decline.json.erb

@@ -1,5 +0,0 @@
-{
-  "message" : "Interview declined.",
-  "requestCopy" : "Request Interview",
-  "declineCopy" : "Declined"
-}

app/views/recruiter_mailer/candidate_reviewed.html.inky

@@ -0,0 +1,9 @@
+<row>
+  <columns class="email-body">
+    <p>The team has <%= @candidate.review_status %> an interview with <strong><%= @candidate.name %></strong> with the following comments:</p>
+
+    <p><%= @candidate.review_comments %></p>
+
+    <p>Thank you</p>
+  </columns>
+</row>

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.

app/views/recruiter_mailer/interview_declined.html.inky

@@ -1,6 +0,0 @@
-<row>
-  <columns class="email-body">
-    <p>The team has declined an interview with <strong><%= @candidate.name %></strong>.</p>
-    <p>Thank you</p>
-  </columns>
-</row>

app/views/recruiter_mailer/interview_declined.text.erb

@@ -1,5 +0,0 @@
-PERFICIENT/digital - Skills Assessment Test
-
-The team has declined an interview with <%= @candidate.name %>.
-
-Thank you.

app/views/recruiter_mailer/interview_requested.html.inky

@@ -1,6 +0,0 @@
-<row>
-  <columns class="email-body">
-    <p>The team has requested an interview with <strong><%= @candidate.name %></strong>.</p>
-    <p>Thank you</p>
-  </columns>
-</row>

app/views/recruiter_mailer/interview_requested.text.erb

@@ -1,5 +0,0 @@
-PERFICIENT/digital - Skills Assessment Test
-
-The team has requested an interview with <%= @candidate.name %>.
-
-Thank you.

config/routes.rb

@@ -57,8 +57,7 @@ Rails.application.routes.draw do
 
   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' }
+  post "admin/vote/:test_hash",         to: "admin/vote#interview_request", as: :admin_interview
-  get "admin/vote/:test_hash/decline", to: "admin/vote#decline", as: :admin_decline_vote, defaults: { format: 'json' }
 
   get "/admin", to: "admin/dashboard#show", as: :admin
 

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

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

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"
@@ -41,6 +41,7 @@ ActiveRecord::Schema.define(version: 20170227154554) do
     t.datetime "updated_at",                                null: false
     t.integer  "quiz_id"
     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
@@ -91,7 +92,6 @@ ActiveRecord::Schema.define(version: 20170227154554) do
     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.index ["candidate_id", "user_id"], name: "index_reviewer_votes_on_candidate_id_and_user_id", unique: true, using: :btree

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
+      assert_equal 'approved', Candidate.find(henry.to_i).review_status
-      data = JSON.parse(response.body)
+      assert_equal 'ipsum', Candidate.find(henry.to_i).review_comments
-      assert_match 'requested', data["message"]
+      assert_redirected_to admin_result_url(henry.test_hash)
-    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
     end
   end
 end

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

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
+  def candidate_reviewed
-    RecruiterMailer.interview_requested Candidate.find_by(test_hash: '6NjnourLE6Y') # Richard
+    RecruiterMailer.candidate_reviewed Candidate.find_by(test_hash: 's6oFExZliYYFx') # Stacy
-  end
-
-  def interview_declined
-    RecruiterMailer.interview_declined Candidate.find_by(test_hash: 's6oFExZliYYFx') # Stacy
   end
 end

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
+  test "candidate_reviewed" 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
     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

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
+  def interview_request
-    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
     assert_permit users(:manager), reviewer_votes(:manager_richard)
     assert_permit users(:admin), reviewer_votes(:manager_henry)