Add fields for logging interviews

api/app/controllers/v1/new_club_applications_controller.rb

@@ -36,6 +36,10 @@ def update
       c = NewClubApplication.find(params[:id])
       authorize c
 
+      if c.submitted? && !current_user.admin?
+        return render_field_error(:base, 'cannot edit application after submit')
+      end
+
       if c.update_attributes(club_application_params)
         render_success(c)
       else
@@ -47,7 +51,7 @@ def add_user
       app = NewClubApplication.find(params[:new_club_application_id])
       authorize app
 
-      if app.submitted_at.present?
+      if app.submitted?
         return render_field_error(:base, 'cannot edit application after submit')
       end
 
@@ -76,7 +80,7 @@ def remove_user
 
       authorize app
 
-      if app.submitted_at.present?
+      if app.submitted?
         return render_field_error(:base, 'cannot edit application after submit')
       end
 

api/app/models/new_club_application.rb

@@ -22,8 +22,7 @@ class NewClubApplication < ApplicationRecord
 
   enum high_school_type: %i[
     public_school
-    private_school
-    charter_school
+    private_school charter_school
   ]
 
   with_options if: -> { submitted_at.present? } do |application|
@@ -52,17 +51,17 @@ class NewClubApplication < ApplicationRecord
 
       errors.add(:base, 'leader profiles not complete') unless all_complete
     end
-
-    # make model immutable
-    application.validate do |app|
-      # if model has changed and it wasn't us changing submitted_at away from
-      # nil
-      if app.changed? && app.changes['submitted_at'] != [nil, submitted_at]
-        errors.add(:base, 'cannot edit application after submit')
-      end
-    end
   end
 
+  # submitted_at must be set for interviewed_at to be set
+  validates :submitted_at, presence: true, if: -> { interviewed_at.present? }
+  validates :interviewed_at, :interview_duration, :interview_notes,
+            presence: true, if: lambda {
+              interviewed_at.present? ||
+                interview_duration.present? ||
+                interview_notes.present?
+            }
+
   def submit!
     self.submitted_at = Time.current
 
@@ -84,6 +83,10 @@ def submit!
     end
   end
 
+  def submitted?
+    submitted_at.present?
+  end
+
   # ensure that the point of contact is an associated applicant
   def point_of_contact_is_associated
     return unless point_of_contact

api/db/migrate/20180127234239_add_interview_fields_to_new_club_applications.rb

@@ -0,0 +1,9 @@
+# frozen_string_literal: true
+
+class AddInterviewFieldsToNewClubApplications < ActiveRecord::Migration[5.1]
+  def change
+    add_column :new_club_applications, :interviewed_at, :datetime
+    add_column :new_club_applications, :interview_duration, :interval
+    add_column :new_club_applications, :interview_notes, :text
+  end
+end

api/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: 20180127102450) do
+ActiveRecord::Schema.define(version: 20180127234239) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -275,6 +275,9 @@
     t.integer "point_of_contact_id"
     t.datetime "submitted_at"
     t.json "legacy_fields"
+    t.datetime "interviewed_at"
+    t.interval "interview_duration"
+    t.text "interview_notes"
     t.index ["point_of_contact_id"], name: "index_new_club_applications_on_point_of_contact_id"
   end
 

api/spec/models/new_club_application_spec.rb

@@ -51,6 +51,60 @@
 
   it { should have_db_column :submitted_at }
 
+  it { should have_db_column :interviewed_at }
+  it { should have_db_column :interview_duration }
+  it { should have_db_column :interview_notes }
+
+  describe 'interview fields' do
+    subject { create(:completed_new_club_application) }
+    before { subject.submit! }
+
+    it 'does not allow interviewed_at to be set unless submitted_at is' do
+      subject.submitted_at = nil
+      subject.interviewed_at = Time.current
+
+      subject.validate
+      expect(subject.errors).to include('submitted_at')
+    end
+
+    it 'should require other fields to be set if interviewed_at is' do
+      expect(subject.valid?).to be(true)
+
+      subject.interviewed_at = Time.current
+
+      subject.validate
+      expect(subject.errors).to include('interview_duration')
+      expect(subject.errors).to include('interview_notes')
+    end
+
+    it 'should require other fields to be set if interview_duration is' do
+      expect(subject.valid?).to be(true)
+
+      subject.interview_duration = 1.hour
+
+      subject.validate
+      expect(subject.errors).to include('interviewed_at')
+      expect(subject.errors).to include('interview_notes')
+    end
+
+    it 'should require other fields to be set if interview_notes is' do
+      expect(subject.valid?).to be(true)
+
+      subject.interview_notes = "They're ready to start."
+
+      subject.validate
+      expect(subject.errors).to include('interviewed_at')
+      expect(subject.errors).to include('interview_duration')
+    end
+
+    it 'is valid if all interview fields are set' do
+      subject.interviewed_at = Time.current
+      subject.interview_duration = 1.hour
+      subject.interview_notes = 'Ready to go.'
+      expect(subject.valid?).to eq(true)
+    end
+  end
+
   ## enums ##
 
   it { should define_enum_for :high_school_type }
@@ -126,14 +180,18 @@
       # three emails to applicants and one email to staff
       expect(ApplicantMailer.deliveries.length).to eq(3 + 1)
     end
+  end
 
-    it 'makes the model immutable' do
-      subject.submit!
+  describe ':submitted?' do
+    subject { create(:completed_new_club_application) }
 
-      subject.update_attributes(high_school_name: 'Superhero High School')
-      expect(subject.errors[:base]).to include(
-        'cannot edit application after submit'
-      )
+    it 'return false when not submitted' do
+      expect(subject.submitted?).to eq(false)
+    end
+
+    it 'returns true when submitted' do
+      subject.submit!
+      expect(subject.submitted?).to eq(true)
     end
   end
 end