From 6e4e44d08c55a231996cd12491be3f29e7781030 Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger@fusionary.com>
Date: Sat, 13 Mar 2021 07:59:04 -0500
Subject: [PATCH 01/14] #206 WIP start removing starting salary from employee
 into salaries

---
 app/controllers/employees_controller.rb |  3 ++-
 app/models/employee.rb                  | 12 ++++++++++--
 app/models/salary.rb                    | 14 ++++++++++++--
 app/views/employees/_form.html.haml     | 13 ++++++++-----
 app/views/employees/show.html.haml      |  4 ----
 spec/models/employee_spec.rb            |  1 -
 spec/models/salary_spec.rb              | 23 ++++++++++++++++++++++-
 7 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/app/controllers/employees_controller.rb b/app/controllers/employees_controller.rb
index 333378b0..f8a6e81a 100644
--- a/app/controllers/employees_controller.rb
+++ b/app/controllers/employees_controller.rb
@@ -52,6 +52,7 @@ def employee_params
                                      :planning_raise_date,
                                      :planning_raise_salary,
                                      :planning_notes,
-                                     tenures_attributes: [:id, :start_date, :end_date, :_destroy])
+                                     tenures_attributes: [:id, :start_date, :end_date, :_destroy],
+                                     salaries_attributes: [:id, :start_date, :annual_amount])
   end
 end
diff --git a/app/models/employee.rb b/app/models/employee.rb
index c6ef780e..23032742 100644
--- a/app/models/employee.rb
+++ b/app/models/employee.rb
@@ -6,9 +6,11 @@ class Employee < ActiveRecord::Base
   accepts_nested_attributes_for :tenures,
     reject_if: proc { |attributes| attributes['start_date'].blank? }, allow_destroy: true
 
+  accepts_nested_attributes_for :salaries,
+    reject_if: proc { |attributes| attributes['annual_amount'].blank? }
+
   validates :first_name, presence: true
   validates :last_name, presence: true
-  validates :starting_salary, presence: true
 
   default_scope { order :first_name }
   scope :past, -> { joins(:tenures).where 'tenures.end_date < :today', today: Time.zone.today }
@@ -77,6 +79,7 @@ def end_date
   end
 
   def employed_on?(date)
+    return nil if date.nil?
     tenures.any? \
       { |tenure| date >= tenure.start_date && (tenure.end_date.nil? || date <= tenure.end_date) }
   end
@@ -108,6 +111,11 @@ def salary_data
     data.sort_by { |salary| salary[:c][0]}
   end
 
+  def starting_salary
+    s = salaries.first&.annual_amount
+    s = s || 0
+  end
+
   def ending_salary
     end_date ? salary_on(end_date) : nil
   end
@@ -226,4 +234,4 @@ def format_salary(salary)
       "$#{format('%g', salary_in_ks)}K"
     end
   end
-end
+end
\ No newline at end of file
diff --git a/app/models/salary.rb b/app/models/salary.rb
index 55ce0dcb..17bfb328 100644
--- a/app/models/salary.rb
+++ b/app/models/salary.rb
@@ -3,12 +3,14 @@
 class Salary < ActiveRecord::Base
   belongs_to :employee
   validates :start_date, presence: true, uniqueness: { scope: :employee_id }
-  validates :employee_id, presence: true
+  validates_presence_of :employee
   validates :annual_amount, presence: true
   validate :no_salaries_outside_employment_dates, if: :employee
 
   delegate :first_name, :last_name, to: :employee, prefix: true
 
+  before_validation :ensure_start_date
+
   default_scope { order('start_date') }
 
   def self.ordered_dates
@@ -35,4 +37,12 @@ def no_salaries_outside_employment_dates
       errors.add(:start_date, 'must be between employee start and end dates')
     end
   end
-end
+
+  def ensure_start_date
+    # first salary is entered as part of employee form so get start date from first tenure
+    #  this depends on employee nested_attributes_for :tenures getting called first
+    if start_date.blank? && employee&.salaries&.count == 0
+      self.start_date = employee.start_date
+    end
+  end
+end
\ No newline at end of file
diff --git a/app/views/employees/_form.html.haml b/app/views/employees/_form.html.haml
index 7fa4d24b..79a0feb9 100644
--- a/app/views/employees/_form.html.haml
+++ b/app/views/employees/_form.html.haml
@@ -49,11 +49,14 @@
         .form-group.col-lg-12
           = f.label :indirect_experience, 'Indirect experience (months)', class: 'control-label'
           = f.number_field :indirect_experience, class: 'form-control'
-        .form-group.col-lg-12
-          = f.label :starting_salary, 'Starting salary annual amount', class: 'control-label'
-          .input-group
-            %span.input-group-addon $
-            = f.number_field :starting_salary, class: 'form-control', required: true
+        = f.fields_for :salaries, (employee.salaries.first || employee.salaries.build) do |salary_f|
+          = salary_f.hidden_field :id unless employee.salaries.count == 0
+          = salary_f.hidden_field :start_date unless employee.salaries.count == 0
+          .form-group.col-lg-12
+            = salary_f.label :annual_amount, 'Starting salary annual amount', class: 'control-label'
+            .input-group
+              %span.input-group-addon $
+              = salary_f.number_field :annual_amount, class: 'form-control', required: true
         .form-group.col-lg-12
           = f.label :notes, 'Notes', class: 'control-label'
           = f.text_area :notes, class: 'form-control'
diff --git a/app/views/employees/show.html.haml b/app/views/employees/show.html.haml
index dd3e7ca5..80aa6e37 100644
--- a/app/views/employees/show.html.haml
+++ b/app/views/employees/show.html.haml
@@ -38,10 +38,6 @@
           %td Salary
           %td
       %tbody
-        %tr
-          %td= @employee.start_date.strftime('%b %e, %Y')
-          %td= number_to_currency(@employee.starting_salary, precision: 0)
-          %td  (To change starting salary, use edit button above.)
         - @employee.salaries.each do |salary|
           %tr
             %td= salary.start_date.strftime('%b %e, %Y')
diff --git a/spec/models/employee_spec.rb b/spec/models/employee_spec.rb
index 464518a0..1dbad160 100644
--- a/spec/models/employee_spec.rb
+++ b/spec/models/employee_spec.rb
@@ -8,7 +8,6 @@
 
   it { should validate_presence_of :first_name }
   it { should validate_presence_of :last_name }
-  it { should validate_presence_of :starting_salary }
 
   it { should have_db_column(:billable).of_type(:boolean).with_options(default: true) }
   it { should have_db_column(:notes).of_type(:text) }
diff --git a/spec/models/salary_spec.rb b/spec/models/salary_spec.rb
index a7b77e06..e0e62831 100644
--- a/spec/models/salary_spec.rb
+++ b/spec/models/salary_spec.rb
@@ -6,7 +6,7 @@
   it { should belong_to :employee }
   it { should validate_presence_of :start_date }
   it { should validate_presence_of :annual_amount }
-  it { should validate_presence_of :employee_id }
+  it { should validate_presence_of :employee }
 
   it 'validates employee salary start dates unique' do
     create :salary
@@ -72,4 +72,25 @@
       expect(salary).to be_invalid
     end
   end
+
+  describe 'before_validation' do
+    let(:employee) do
+      build(:employee).tap do |employee|
+        employee.tenures = [build(:tenure, start_date: Time.zone.today - 5)]
+        employee.save
+      end
+    end
+
+    it 'should retrieve start date from related tenure if blank and first' do
+      salary = employee.salaries.create(annual_amount: 5)
+      expect(salary).to be_valid
+    end
+
+    it 'should be invalid if blank and second' do
+      salary = employee.salaries.create(annual_amount: 5)
+      salary2 = employee.salaries.create(annual_amount: 5)
+      expect(salary2).to be_invalid
+    end
+  end
+
 end

From 04d0594890506010c7ba9e5107695a7b5890722f Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger@fusionary.com>
Date: Tue, 23 Mar 2021 09:19:20 -0400
Subject: [PATCH 02/14] #206 move salaries to relate to tenures

---
 app/controllers/employees_controller.rb       |  4 +-
 app/controllers/salaries_controller.rb        |  7 ++-
 app/models/employee.rb                        | 11 ++--
 app/models/salary.rb                          | 19 +++---
 app/models/tenure.rb                          |  9 +++
 app/views/employees/show.html.haml            |  4 +-
 ...533_migrate_starting_salary_to_salaries.rb | 41 ++++++++++++
 db/schema.rb                                  |  9 ++-
 db/seeds.rb                                   | 27 ++++----
 spec/factories/employee_factory.rb            |  1 -
 spec/factories/salary_factory.rb              |  2 +-
 spec/features/manage_employee_spec.rb         |  4 +-
 spec/models/employee_spec.rb                  | 62 +++++++++++--------
 spec/models/salary_graph_spec.rb              |  6 +-
 spec/models/salary_spec.rb                    | 35 ++++++-----
 15 files changed, 157 insertions(+), 84 deletions(-)
 create mode 100644 db/migrate/20210315122533_migrate_starting_salary_to_salaries.rb

diff --git a/app/controllers/employees_controller.rb b/app/controllers/employees_controller.rb
index f8a6e81a..a5f146b2 100644
--- a/app/controllers/employees_controller.rb
+++ b/app/controllers/employees_controller.rb
@@ -12,8 +12,10 @@ def new
   def edit; end
 
   def create
-    @employee = Employee.new(employee_params)
+    salary_params = employee_params.dig(:salaries_attributes, "0")
+    @employee = Employee.new(employee_params.except(:salaries_attributes))
     if @employee.save
+      @employee.tenures.first.salaries.create(salary_params) unless salary_params.blank?
       redirect_to @employee, notice: 'Employee successfully created.'
     else
       @errors = @employee.errors.full_messages
diff --git a/app/controllers/salaries_controller.rb b/app/controllers/salaries_controller.rb
index a9d96870..c48cdaeb 100644
--- a/app/controllers/salaries_controller.rb
+++ b/app/controllers/salaries_controller.rb
@@ -3,14 +3,15 @@
 class SalariesController < ApplicationController
   def new
     employee = Employee.find(params[:employee_id])
-    @salary = employee.salaries.new
+    @salary = employee.tenures.last.salaries.new
   end
 
   def create
     employee = Employee.find(params[:employee_id])
-    @salary = employee.salaries.new(salary_params)
+    tenure = employee.tenures.last
+    @salary = tenure.salaries.new(salary_params)
     if @salary.save
-      redirect_to employee_path(@salary.employee), notice: 'Successfully recorded raise'
+      redirect_to employee_path(employee), notice: 'Successfully recorded raise'
     else
       render :new
     end
diff --git a/app/models/employee.rb b/app/models/employee.rb
index 23032742..babdeac4 100644
--- a/app/models/employee.rb
+++ b/app/models/employee.rb
@@ -1,8 +1,9 @@
 # frozen_string_literal: true
 
 class Employee < ActiveRecord::Base
-  has_many :salaries, dependent: :destroy
   has_many :tenures, dependent: :destroy
+  has_many :salaries, through: :tenures, dependent: :destroy
+  
   accepts_nested_attributes_for :tenures,
     reject_if: proc { |attributes| attributes['start_date'].blank? }, allow_destroy: true
 
@@ -91,16 +92,12 @@ def display_name
   def salary_on(date)
     return nil unless employed_on?(date)
 
-    salary_match = salaries.where('start_date <= ?', date).last
+    salary_match = salaries.where('salaries.start_date <= ?', date).last
     salary_match ? salary_match.annual_amount : starting_salary
   end
 
   def salary_data
     data = []
-    tenures.each do |tenure|
-      data << { c: [date_for_js(tenure.start_date), salary_on(tenure.start_date)] }
-    end
-
     salaries.ordered_dates_with_previous_dates.each do |date|
       data << { c: [date_for_js(date), salary_on(date)] }
     end
@@ -163,7 +160,7 @@ def display_previous_pay
   end
 
   def previous_pay
-    if salaries.empty?
+    if salaries.empty? || salaries.count == 1
       nil
     else
       salaries[-2].try(:annual_amount) || starting_salary
diff --git a/app/models/salary.rb b/app/models/salary.rb
index 17bfb328..7c310ac9 100644
--- a/app/models/salary.rb
+++ b/app/models/salary.rb
@@ -1,24 +1,25 @@
 # frozen_string_literal: true
 
 class Salary < ActiveRecord::Base
-  belongs_to :employee
-  validates :start_date, presence: true, uniqueness: { scope: :employee_id }
-  validates_presence_of :employee
+  belongs_to :tenure
+  has_one :employee, through: :tenure
+  validates :start_date, presence: true, uniqueness: { scope: :tenure_id }
+  validates_presence_of :tenure
   validates :annual_amount, presence: true
-  validate :no_salaries_outside_employment_dates, if: :employee
+  validate :no_salaries_outside_tenure_dates, if: :tenure
 
   delegate :first_name, :last_name, to: :employee, prefix: true
 
   before_validation :ensure_start_date
 
-  default_scope { order('start_date') }
+  default_scope { order('salaries.start_date') }
 
   def self.ordered_dates
-    select('distinct start_date').order('start_date').map(&:start_date)
+    select('salaries.start_date').order('salaries.start_date').map(&:start_date).uniq
   end
 
   def self.ordered_dates_with_previous_dates
-    ordered_dates.map { |date| [date - 1, date] }.flatten
+    ordered_dates.map { |date| [date - 1, date] }.flatten.tap(&:shift)
   end
 
   def self.all_dates
@@ -32,8 +33,8 @@ def self.history_dates
 
   private
 
-  def no_salaries_outside_employment_dates
-    unless employee.employed_on?(start_date)
+  def no_salaries_outside_tenure_dates
+    unless tenure.employed_on?(start_date)
       errors.add(:start_date, 'must be between employee start and end dates')
     end
   end
diff --git a/app/models/tenure.rb b/app/models/tenure.rb
index 2f08898e..be721daa 100644
--- a/app/models/tenure.rb
+++ b/app/models/tenure.rb
@@ -2,11 +2,15 @@
 
 class Tenure < ActiveRecord::Base
   belongs_to :employee
+  has_many :salaries
+
   validates :start_date, presence: true, uniqueness: { scope: :employee_id }
   validates :employee, presence: true
   validate :tenures_are_sequential, if: :employee
   validate :start_date_is_before_end_date
 
+  default_scope { order :start_date }
+
   def self.ordered_start_dates
     select('distinct start_date').unscoped.order('start_date').map(&:start_date).uniq
   end
@@ -24,6 +28,11 @@ def next_tenure
     employee.tenures.where("id > ?", id).first
   end
 
+  def employed_on?(date)
+    return nil if date.nil?
+    date >= start_date && (end_date.nil? || date <= end_date)
+  end
+
   private
 
   def start_date_is_before_end_date
diff --git a/app/views/employees/show.html.haml b/app/views/employees/show.html.haml
index 80aa6e37..2bead11e 100644
--- a/app/views/employees/show.html.haml
+++ b/app/views/employees/show.html.haml
@@ -38,11 +38,11 @@
           %td Salary
           %td
       %tbody
-        - @employee.salaries.each do |salary|
+        - @employee.salaries.each_with_index do |salary, i|
           %tr
             %td= salary.start_date.strftime('%b %e, %Y')
             %td= number_to_currency(salary.annual_amount, precision: 0)
-            %td= link_to 'Delete', employee_salary_path(employee_id: @employee, id: salary), method: :delete
+            %td= link_to 'Delete', employee_salary_path(employee_id: @employee, id: salary), method: :delete unless i == 0
   - if @employee.notes?
     %li.active
       = link_to 'Notes', "#"
diff --git a/db/migrate/20210315122533_migrate_starting_salary_to_salaries.rb b/db/migrate/20210315122533_migrate_starting_salary_to_salaries.rb
new file mode 100644
index 00000000..0b8afbd8
--- /dev/null
+++ b/db/migrate/20210315122533_migrate_starting_salary_to_salaries.rb
@@ -0,0 +1,41 @@
+class MigrateStartingSalaryToSalaries < ActiveRecord::Migration[6.1]
+  def up
+    add_reference :salaries, :tenure, foreign_key: true
+
+    Employee.all.each do |employee|
+      Salary.where(employee_id: employee.id).all.each do |salary| 
+        # look for tenure date range containing salary start_date.  Fall back to first if none found
+        tenure = employee.tenures
+          .where('start_date <= :date and (end_date >= :date or end_date is NULL)', date: salary.start_date).first
+        tenure = employee.tenures.first unless tenure
+        salary.update(tenure_id: tenure.id)
+      end
+      tenure = employee.tenures.first
+      tenure.salaries << Salary.new(
+        start_date: tenure.start_date, 
+        annual_amount: employee.attributes['starting_salary'], 
+        employee_id: employee.id)
+      tenure.save
+    end
+
+    remove_reference :salaries, :employee, foreign_key: true
+    remove_column :employees, :starting_salary
+  end
+
+  def down
+    add_column :employees, :starting_salary, :decimal, default: 0, null: false
+    add_reference :salaries, :employee, foreign_key: true
+
+    Employee.all.each do |employee|
+      employee.tenures.each do |tenure|
+        tenure.salaries.each { |salary| salary.update(employee_id: employee.id) }
+      end
+      tenure = employee.tenures.first
+      salary = tenure.salaries.first if tenure
+      employee.update(starting_salary: salary.annual_amount) if salary
+      salary.destroy
+    end
+
+    remove_reference :salaries, :tenure
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 02d466db..bfc7746e 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: 2020_12_14_224051) do
+ActiveRecord::Schema.define(version: 2021_03_15_122533) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -46,7 +46,6 @@
     t.datetime "updated_at"
     t.integer "direct_experience", default: 0, null: false
     t.integer "indirect_experience", default: 0, null: false
-    t.decimal "starting_salary", default: "0.0", null: false
     t.text "notes"
     t.date "planning_raise_date"
     t.string "planning_raise_salary"
@@ -55,11 +54,11 @@
 
   create_table "salaries", force: :cascade do |t|
     t.date "start_date", null: false
-    t.integer "employee_id", null: false
     t.decimal "annual_amount", null: false
     t.datetime "created_at"
     t.datetime "updated_at"
-    t.index ["employee_id"], name: "index_salaries_on_employee_id"
+    t.bigint "tenure_id"
+    t.index ["tenure_id"], name: "index_salaries_on_tenure_id"
   end
 
   create_table "tenures", force: :cascade do |t|
@@ -102,5 +101,5 @@
   end
 
   add_foreign_key "balances", "accounts"
-  add_foreign_key "salaries", "employees", name: "salaries_employee_id_fk"
+  add_foreign_key "salaries", "tenures"
 end
diff --git a/db/seeds.rb b/db/seeds.rb
index 02c713bb..b752c8da 100644
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -5,7 +5,7 @@
 
 Salary.delete_all
 Employee.delete_all
-
+Tenure.delete_all
 User.delete_all
 
 User.create!(email: 'admin@bendyworks.com',
@@ -18,56 +18,58 @@
 darkwing_start_date = Date.parse('2012-11-11')
 darkwing_end_date = Date.parse('2014-1-1')
 darkwing = Employee.create!(first_name: 'Darkwing (gone)', last_name: 'Duck',
-                            starting_salary: '100000',
                             tenures: [Tenure.new(start_date: darkwing_start_date,
                                                  end_date: darkwing_end_date)])
-Salary.create!(employee: darkwing, start_date: darkwing_start_date + 90, annual_amount: '200000')
+Salary.create!(tenure: darkwing.tenures.first, start_date: darkwing_start_date, annual_amount: '100000')
+Salary.create!(tenure: darkwing.tenures.first, start_date: darkwing_start_date + 90, annual_amount: '200000')
 
 ###### FUTURE EMPLOYEES ######
 
 daffy_start_date = Time.zone.today + 10
 _daffy = Employee.create!(first_name: 'Daffy (future)',
                           last_name: 'Duck',
-                          starting_salary: '51000.00',
                           direct_experience: 2,
                           indirect_experience: 8,
                           tenures: [Tenure.new(start_date: daffy_start_date)])
 
+Salary.create!(tenure: _daffy.tenures.first, start_date: daffy_start_date, annual_amount: '51000.00')
+
 ###### CURRENT EMPLOYEES #######
 ## Daisie
 daisie_start_date = Date.parse('2013-1-1')
 daisie = Employee.create!(first_name: 'Daisie',
                           last_name: 'Duck',
-                          starting_salary: '57000.00',
                           direct_experience: 6,
                           indirect_experience: 4,
                           tenures: [Tenure.new(start_date: daisie_start_date)])
 
-Salary.create!(employee: daisie,
+Salary.create!(tenure: daisie.tenures.first, start_date: daisie_start_date, annual_amount: '57000.00')
+Salary.create!(tenure: daisie.tenures.first,
                start_date: daisie_start_date + 90, # 4-1-2013
                annual_amount: '59000.00')
 
 ## Minnie
 minnie_start_date = daisie_start_date + 90
 minnie = Employee.create!(first_name: 'Minnie', last_name: 'Mouse',
-                          starting_salary: '46000.00',
                           tenures: [Tenure.new(start_date: minnie_start_date)])
-Salary.create!(employee: minnie,
+
+Salary.create!(tenure: minnie.tenures.first, start_date: minnie_start_date, annual_amount: '46000.00')
+Salary.create!(tenure: minnie.tenures.first,
                start_date: minnie_start_date + 90, # 6-30-2013
                annual_amount: '49000.00')
 
 ## Mickey
 mickey_start_date = daisie_start_date + 180
 mickey = Employee.create!(first_name: 'Mickey', last_name: 'Mouse',
-                          starting_salary: '45000.00',
                           indirect_experience: 18,
                           tenures: [Tenure.new(start_date: mickey_start_date)])
 
-Salary.create!(employee: mickey,
+Salary.create!(tenure: mickey.tenures.first, start_date: mickey_start_date, annual_amount: '45000.00')
+Salary.create!(tenure: mickey.tenures.first,
                start_date: mickey_start_date + 40, # 8-9-2013
                annual_amount: '50000.00')
 
-Salary.create!(employee: mickey,
+Salary.create!(tenure: mickey.tenures.first,
                start_date: mickey_start_date + 80, # 9-18-2013
                annual_amount: '55000.00')
 
@@ -75,10 +77,11 @@
 donald_start_date = daisie_start_date + 270
 _donald = Employee.create!(first_name: 'Donald (support)',
                            last_name: 'Duck',
-                           starting_salary: '40000.00',
                            direct_experience: 9,
                            billable: false,
                            tenures: [Tenure.new(start_date: donald_start_date)])
 
+Salary.create!(tenure: _donald.tenures.first, start_date: donald_start_date, annual_amount: '40000.00')
+
 #########################
 puts 'Seed data created.'
diff --git a/spec/factories/employee_factory.rb b/spec/factories/employee_factory.rb
index f5739e85..25a657cf 100644
--- a/spec/factories/employee_factory.rb
+++ b/spec/factories/employee_factory.rb
@@ -4,7 +4,6 @@
   factory :employee do
     sequence(:first_name) { |n| "##{n}" }
     last_name { 'Bendyworker' }
-    starting_salary { 500 }
     after :create do |employee|
       create_list :tenure, 1, employee: employee
     end
diff --git a/spec/factories/salary_factory.rb b/spec/factories/salary_factory.rb
index 22ddd6fa..5e11d6b0 100644
--- a/spec/factories/salary_factory.rb
+++ b/spec/factories/salary_factory.rb
@@ -4,6 +4,6 @@
   factory :salary do
     sequence(:start_date) { |n| 6.months.ago + n.day }
     annual_amount { 700 }
-    employee
+    tenure
   end
 end
diff --git a/spec/features/manage_employee_spec.rb b/spec/features/manage_employee_spec.rb
index 4cdebfb9..8821620c 100644
--- a/spec/features/manage_employee_spec.rb
+++ b/spec/features/manage_employee_spec.rb
@@ -15,7 +15,7 @@
     fill_in 'Last name', with: 'Bendyworker'
     fill_in 'Start date', with: Time.zone.today
     fill_in 'Direct experience', with: 5
-    fill_in 'Starting salary', with: 40_000
+    fill_in 'Starting salary annual amount', with: 40_000
     click_button 'Save'
 
     expect(page).to have_content 'Newest Bendyworker'
@@ -24,7 +24,7 @@
 
   context 'employee exists' do
     let(:employee) { create :employee }
-    let!(:salary) { create :salary, employee: employee }
+    let!(:salary) { create :salary, tenure: employee.tenures.first }
 
     scenario 'edit employee basic info' do
       visit "/employees/#{employee.id}"
diff --git a/spec/models/employee_spec.rb b/spec/models/employee_spec.rb
index 1dbad160..f7c33222 100644
--- a/spec/models/employee_spec.rb
+++ b/spec/models/employee_spec.rb
@@ -26,8 +26,9 @@
     let(:third_raise_date) { 1.months.ago.to_date }
 
     let!(:employee) do
-      build(:employee, starting_salary: 1_000).tap do |employee|
+      build(:employee).tap do |employee|
         employee.tenures = [build(:tenure, start_date: start_date)]
+        employee.tenures.first.salaries = [build(:salary, start_date: start_date, annual_amount: 1_000)]
         employee.save
       end
     end
@@ -40,18 +41,18 @@
         end
       end
       context 'with one raise' do
-        let!(:only_raise) { create :salary, employee: employee, start_date: first_raise_date }
+        let!(:only_raise) { create :salary, tenure: employee.tenures.first, start_date: first_raise_date }
         it 'returns starting salary' do
           expect(last_raise_date).to eq(first_raise_date)
         end
       end
       context 'with two raises' do
         let!(:first_raise) do
-          create :salary, employee: employee, start_date: first_raise_date
+          create :salary, tenure: employee.tenures.first, start_date: first_raise_date
         end
 
         let!(:second_raise) do
-          create :salary, employee: employee, start_date: second_raise_date
+          create :salary, tenure: employee.tenures.first, start_date: second_raise_date
         end
 
         it 'returns first raise' do
@@ -60,15 +61,15 @@
       end
       context 'with three raises' do
         let!(:first_raise) do
-          create :salary, employee: employee, start_date: first_raise_date
+          create :salary, tenure: employee.tenures.first, start_date: first_raise_date
         end
 
         let!(:second_raise) do
-          create :salary, employee: employee, start_date: second_raise_date
+          create :salary, tenure: employee.tenures.first, start_date: second_raise_date
         end
 
         let!(:third_raise) do
-          create :salary, employee: employee, start_date: third_raise_date
+          create :salary, tenure: employee.tenures.first, start_date: third_raise_date
         end
 
         it 'returns second raise' do
@@ -79,7 +80,14 @@
   end
 
   describe '#previous_pay' do
-    let(:employee) { create :employee, starting_salary: 1_000 }
+    let(:start_date) { 4.months.ago.to_date }
+    let!(:employee) do
+      build(:employee).tap do |employee|
+        employee.tenures = [build(:tenure, start_date: start_date)]
+        employee.tenures.first.salaries = [build(:salary, start_date: start_date, annual_amount: 1_000)]
+        employee.save
+      end
+    end
     let(:previous_pay) { employee.reload.previous_pay }
 
     context 'current employee' do
@@ -89,18 +97,18 @@
         end
       end
       context 'with one raise' do
-        let!(:only_raise) { create :salary, employee: employee, annual_amount: 2_000 }
+        let!(:only_raise) { create :salary, tenure: employee.tenures.first, annual_amount: 2_000, start_date: 3.months.ago }
         it 'returns starting salary' do
           expect(previous_pay).to eq(1_000)
         end
       end
       context 'with two raises' do
         let!(:first_raise) do
-          create :salary, employee: employee, annual_amount: 2_000, start_date: 3.months.ago
+          create :salary, tenure: employee.tenures.first, annual_amount: 2_000, start_date: 3.months.ago
         end
 
         let!(:second_raise) do
-          create :salary, employee: employee, annual_amount: 3_000, start_date: 2.months.ago
+          create :salary, tenure: employee.tenures.first, annual_amount: 3_000, start_date: 2.months.ago
         end
 
         it 'returns first raise' do
@@ -109,15 +117,15 @@
       end
       context 'with three raises' do
         let!(:first_raise) do
-          create :salary, employee: employee, annual_amount: 2_000, start_date: 3.months.ago
+          create :salary, tenure: employee.tenures.first, annual_amount: 2_000, start_date: 3.months.ago
         end
 
         let!(:second_raise) do
-          create :salary, employee: employee, annual_amount: 3_000, start_date: 2.months.ago
+          create :salary, tenure: employee.tenures.first, annual_amount: 3_000, start_date: 2.months.ago
         end
 
         let!(:third_raise) do
-          create :salary, employee: employee, annual_amount: 4_000, start_date: 1.months.ago
+          create :salary, tenure: employee.tenures.first, annual_amount: 4_000, start_date: 1.months.ago
         end
 
         it 'returns second raise' do
@@ -132,7 +140,7 @@
     let(:employee) { create :employee, tenures_attributes: [{start_date: start_date}] }
 
     let!(:starting_salary) do
-      create(:salary, employee: employee, start_date: start_date, annual_amount: pay)
+      create(:salary, tenure: employee.tenures.first, start_date: start_date, annual_amount: pay)
     end
 
     context 'when pay is a whole number of thousands' do
@@ -157,9 +165,9 @@
 
     let(:daisie) { create(:employee, tenures_attributes: [{start_date: start_date, end_date: end_date}]) }
 
-    let!(:starting_salary) { create(:salary, employee: daisie, start_date: start_date) }
+    let!(:starting_salary) { create(:salary, tenure: daisie.tenures.first, start_date: start_date, annual_amount: 800) }
     let!(:raise_salary) do
-      create(:salary, employee: daisie, start_date: raise_date, annual_amount: '900')
+      create(:salary, tenure: daisie.tenures.first, start_date: raise_date, annual_amount: '900')
     end
 
     it 'returns nil, given date before employee has started' do
@@ -190,8 +198,8 @@
           employee.save
         end
       end
-      let!(:salary) { create :salary, employee: employee }
-      let!(:raise_salary) { create :salary, employee: employee, start_date: salary.start_date + 5 }
+      let!(:salary) { create :salary, tenure: employee.tenures.first }
+      let!(:raise_salary) { create :salary, tenure: employee.tenures.first, start_date: salary.start_date + 5 }
 
       context 'no end date' do
         let(:end_date) { nil }
@@ -219,8 +227,8 @@
           employee.save
         end
       end
-      let!(:returned_salary) { create :salary, employee: returned, start_date: Time.zone.today - 28 }
-      let!(:returned_raise_salary) { create :salary, employee: returned, start_date: start_date + 5 }
+      let!(:returned_salary) { create :salary, tenure: returned.tenures.first, start_date: Time.zone.today - 28 }
+      let!(:returned_raise_salary) { create :salary, tenure: returned.tenures.last, start_date: start_date + 5 }
 
       context 'has multiple tenures and no end date on the latest one' do
         let(:end_date) { nil }
@@ -364,13 +372,14 @@
       let(:raise_date) { Date.parse '2002-10-10' }
 
       let(:employee) do
-        build(:employee, first_name: 'Joan', starting_salary: 100).tap do |employee|
+        build(:employee, first_name: 'Joan').tap do |employee|
           employee.tenures = [build(:tenure, start_date: start_date, end_date: end_date)]
+          employee.tenures.first.salaries = [build(:salary, start_date: start_date, annual_amount: 100)]
           employee.save
         end
       end
 
-      let!(:raise) { create :salary, employee: employee, start_date: raise_date, annual_amount: 200 }
+      let!(:raise) { create :salary, tenure: employee.tenures.first, start_date: raise_date, annual_amount: 200 }
       let(:last_pay_date) { end_date || Time.zone.today }
 
       let(:expected_salary_data) do
@@ -425,14 +434,16 @@
       let(:second_start_date) { Date.parse '2004-10-10' }
 
       let(:employee) do
-        build(:employee, first_name: 'Joan', starting_salary: 100).tap do |employee|
+        build(:employee, first_name: 'Joan').tap do |employee|
           employee.tenures = [build(:tenure, start_date: start_date, end_date: end_date),
                               build(:tenure, start_date: second_start_date, end_date: second_end_date)]
+          employee.tenures.first.salaries = [build(:salary, start_date: start_date, annual_amount: 100)]
+          employee.tenures.last.salaries = [build(:salary, start_date: second_start_date, annual_amount: 200)]
           employee.save
         end
       end
 
-      let!(:raise) { create :salary, employee: employee, start_date: raise_date, annual_amount: 200 }
+      let!(:raise) { create :salary, tenure: employee.tenures.first, start_date: raise_date, annual_amount: 200 }
       let(:last_pay_date) { second_end_date || Time.zone.today }
 
       let(:expected_salary_data) do
@@ -444,6 +455,7 @@
           { c: [date_for_js.call(raise_date), 200] },
           { c: [date_for_js.call(end_date), 200] },
           { c: [date_for_js.call(end_date + 1), nil] },
+          { c: [date_for_js.call(second_start_date - 1), nil] },
           { c: [date_for_js.call(second_start_date), 200] },
           { c: [date_for_js.call(last_pay_date), 200] }
         ]
diff --git a/spec/models/salary_graph_spec.rb b/spec/models/salary_graph_spec.rb
index 4201f791..af975df6 100644
--- a/spec/models/salary_graph_spec.rb
+++ b/spec/models/salary_graph_spec.rb
@@ -26,8 +26,10 @@
         second_expected_date_format = second_existing_date.to_time.to_f * 1000
         expected_second_salary = 150_000
 
-        employee = create(:employee, starting_salary: expected_first_salary)
-        employee.salaries.create(start_date: second_existing_date - 1.day, annual_amount: expected_second_salary)
+        employee = create(:employee)
+        tenure = employee.tenures.first
+        tenure.salaries.create(annual_amount: expected_first_salary)
+        tenure.salaries.create(start_date: second_existing_date - 1.day, annual_amount: expected_second_salary)
 
         first_expected_tooltip = "#{first_existing_date}\n#{employee.display_name}: $130K"
         second_expected_tooltip = "#{second_existing_date}\n#{employee.display_name}: $150K"
diff --git a/spec/models/salary_spec.rb b/spec/models/salary_spec.rb
index e0e62831..6875f934 100644
--- a/spec/models/salary_spec.rb
+++ b/spec/models/salary_spec.rb
@@ -3,14 +3,21 @@
 require 'rails_helper'
 
 describe Salary do
-  it { should belong_to :employee }
+  it { should belong_to :tenure }
   it { should validate_presence_of :start_date }
   it { should validate_presence_of :annual_amount }
-  it { should validate_presence_of :employee }
+  it { should validate_presence_of :tenure }
 
+  let(:employee) do
+    build(:employee).tap do |employee|
+      employee.tenures = [build(:tenure)]
+      employee.save
+    end
+  end
   it 'validates employee salary start dates unique' do
-    create :salary
-    should validate_uniqueness_of(:start_date).scoped_to(:employee_id)
+    # actually instantiate the salary record so matcher has something to go by
+    salary = employee.tenures.first.salaries.create(start_date: employee.start_date, annual_amount: 5)
+    should validate_uniqueness_of(:start_date).scoped_to(:tenure_id)
   end
 
   describe 'ordered_dates' do
@@ -18,11 +25,12 @@
     let(:first_date) { 7.months.ago.to_date }
     let(:third_date) { 5.months.ago.to_date }
     let(:second_date) { 6.months.ago.to_date }
+    let(:tenure) { create :tenure, start_date: first_date }
 
-    let!(:first_added_salary) { create :salary, start_date: fourth_date }
-    let!(:second_added_salary) { create :salary, start_date: first_date }
-    let!(:third_added_salary) { create :salary, start_date: third_date }
-    let!(:fourth_added_salary) { create :salary, start_date: second_date }
+    let!(:first_added_salary) { create :salary, tenure: tenure, start_date: fourth_date }
+    let!(:second_added_salary) { create :salary, tenure: tenure, start_date: first_date }
+    let!(:third_added_salary) { create :salary, tenure: tenure, start_date: third_date }
+    let!(:fourth_added_salary) { create :salary, tenure: tenure, start_date: second_date }
 
     it 'returns all salary dates in order' do
       expect(Salary.ordered_dates).to eq([first_date, second_date, third_date, fourth_date])
@@ -30,13 +38,12 @@
 
     it 'removes duplicate dates' do
       create :salary, start_date: third_date
-
       expect(Salary.ordered_dates).to eq([first_date, second_date, third_date, fourth_date])
     end
 
     describe 'with_previous_dates' do
       it 'returns an array of dates, including each interesting date and the day before it' do
-        expect(Salary.ordered_dates_with_previous_dates).to eq([first_date - 1, first_date,
+        expect(Salary.ordered_dates_with_previous_dates).to eq([first_date,
                                                                 second_date - 1, second_date,
                                                                 third_date - 1, third_date,
                                                                 fourth_date - 1, fourth_date])
@@ -53,17 +60,17 @@
     end
 
     it 'allows salary starting on employee start date' do
-      salary = employee.salaries.create(start_date: employee.start_date, annual_amount: 5)
+      salary = employee.tenures.first.salaries.create(start_date: employee.start_date, annual_amount: 5)
       expect(salary).to be_valid
     end
 
     it 'allows salary starting on employee end date' do
-      salary = employee.salaries.create(start_date: employee.end_date, annual_amount: 5)
+      salary = employee.tenures.first.salaries.create(start_date: employee.end_date, annual_amount: 5)
       expect(salary).to be_valid
     end
 
     it 'prevents salary before employee start date' do
-      salary = employee.salaries.create(start_date: employee.start_date - 1, annual_amount: 5)
+      salary = employee.tenures.first.salaries.create(start_date: employee.start_date - 1, annual_amount: 5)
       expect(salary).to be_invalid
     end
 
@@ -82,7 +89,7 @@
     end
 
     it 'should retrieve start date from related tenure if blank and first' do
-      salary = employee.salaries.create(annual_amount: 5)
+      salary = employee.tenures.first.salaries.create(annual_amount: 5)
       expect(salary).to be_valid
     end
 

From 62e971edb5bac9d20a8f18ad26d1ab12d52ebd4c Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger@fusionary.com>
Date: Tue, 23 Mar 2021 10:07:57 -0400
Subject: [PATCH 03/14] #206 restore edit message for first salary

---
 app/views/employees/show.html.haml | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/app/views/employees/show.html.haml b/app/views/employees/show.html.haml
index 2bead11e..ee8cdbab 100644
--- a/app/views/employees/show.html.haml
+++ b/app/views/employees/show.html.haml
@@ -42,7 +42,11 @@
           %tr
             %td= salary.start_date.strftime('%b %e, %Y')
             %td= number_to_currency(salary.annual_amount, precision: 0)
-            %td= link_to 'Delete', employee_salary_path(employee_id: @employee, id: salary), method: :delete unless i == 0
+            %td
+              -if i == 0
+                (To change starting salary, use edit button above.)
+              -else
+                = link_to 'Delete', employee_salary_path(employee_id: @employee, id: salary), method: :delete
   - if @employee.notes?
     %li.active
       = link_to 'Notes', "#"

From 21b0a89f866be579c9a091cca941aee14617124f Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger@fusionary.com>
Date: Sat, 13 Mar 2021 07:59:04 -0500
Subject: [PATCH 04/14] #206 WIP start removing starting salary from employee
 into salaries

---
 app/controllers/employees_controller.rb |  3 ++-
 app/models/employee.rb                  | 12 ++++++++++--
 app/models/salary.rb                    | 14 ++++++++++++--
 app/views/employees/_form.html.haml     | 13 ++++++++-----
 app/views/employees/show.html.haml      |  4 ----
 spec/models/employee_spec.rb            |  1 -
 spec/models/salary_spec.rb              | 23 ++++++++++++++++++++++-
 7 files changed, 54 insertions(+), 16 deletions(-)

diff --git a/app/controllers/employees_controller.rb b/app/controllers/employees_controller.rb
index 333378b0..f8a6e81a 100644
--- a/app/controllers/employees_controller.rb
+++ b/app/controllers/employees_controller.rb
@@ -52,6 +52,7 @@ def employee_params
                                      :planning_raise_date,
                                      :planning_raise_salary,
                                      :planning_notes,
-                                     tenures_attributes: [:id, :start_date, :end_date, :_destroy])
+                                     tenures_attributes: [:id, :start_date, :end_date, :_destroy],
+                                     salaries_attributes: [:id, :start_date, :annual_amount])
   end
 end
diff --git a/app/models/employee.rb b/app/models/employee.rb
index 1d684dcf..c1ecf674 100644
--- a/app/models/employee.rb
+++ b/app/models/employee.rb
@@ -6,9 +6,11 @@ class Employee < ActiveRecord::Base
   accepts_nested_attributes_for :tenures,
     reject_if: proc { |attributes| attributes['start_date'].blank? }, allow_destroy: true
 
+  accepts_nested_attributes_for :salaries,
+    reject_if: proc { |attributes| attributes['annual_amount'].blank? }
+
   validates :first_name, presence: true
   validates :last_name, presence: true
-  validates :starting_salary, presence: true
 
   default_scope { order :first_name }
   scope :past, -> { joins(:tenures).where 'tenures.end_date < :today', today: Time.zone.today }
@@ -77,6 +79,7 @@ def end_date
   end
 
   def employed_on?(date)
+    return nil if date.nil?
     tenures.any? \
       { |tenure| date >= tenure.start_date && (tenure.end_date.nil? || date <= tenure.end_date) }
   end
@@ -108,6 +111,11 @@ def salary_data
     data.sort_by { |salary| salary[:c][0]}
   end
 
+  def starting_salary
+    s = salaries.first&.annual_amount
+    s = s || 0
+  end
+
   def ending_salary
     end_date ? salary_on(end_date) : nil
   end
@@ -203,4 +211,4 @@ def format_salary(salary)
       "$#{format('%g', salary_in_ks)}K"
     end
   end
-end
+end
\ No newline at end of file
diff --git a/app/models/salary.rb b/app/models/salary.rb
index 55ce0dcb..17bfb328 100644
--- a/app/models/salary.rb
+++ b/app/models/salary.rb
@@ -3,12 +3,14 @@
 class Salary < ActiveRecord::Base
   belongs_to :employee
   validates :start_date, presence: true, uniqueness: { scope: :employee_id }
-  validates :employee_id, presence: true
+  validates_presence_of :employee
   validates :annual_amount, presence: true
   validate :no_salaries_outside_employment_dates, if: :employee
 
   delegate :first_name, :last_name, to: :employee, prefix: true
 
+  before_validation :ensure_start_date
+
   default_scope { order('start_date') }
 
   def self.ordered_dates
@@ -35,4 +37,12 @@ def no_salaries_outside_employment_dates
       errors.add(:start_date, 'must be between employee start and end dates')
     end
   end
-end
+
+  def ensure_start_date
+    # first salary is entered as part of employee form so get start date from first tenure
+    #  this depends on employee nested_attributes_for :tenures getting called first
+    if start_date.blank? && employee&.salaries&.count == 0
+      self.start_date = employee.start_date
+    end
+  end
+end
\ No newline at end of file
diff --git a/app/views/employees/_form.html.haml b/app/views/employees/_form.html.haml
index 7fa4d24b..79a0feb9 100644
--- a/app/views/employees/_form.html.haml
+++ b/app/views/employees/_form.html.haml
@@ -49,11 +49,14 @@
         .form-group.col-lg-12
           = f.label :indirect_experience, 'Indirect experience (months)', class: 'control-label'
           = f.number_field :indirect_experience, class: 'form-control'
-        .form-group.col-lg-12
-          = f.label :starting_salary, 'Starting salary annual amount', class: 'control-label'
-          .input-group
-            %span.input-group-addon $
-            = f.number_field :starting_salary, class: 'form-control', required: true
+        = f.fields_for :salaries, (employee.salaries.first || employee.salaries.build) do |salary_f|
+          = salary_f.hidden_field :id unless employee.salaries.count == 0
+          = salary_f.hidden_field :start_date unless employee.salaries.count == 0
+          .form-group.col-lg-12
+            = salary_f.label :annual_amount, 'Starting salary annual amount', class: 'control-label'
+            .input-group
+              %span.input-group-addon $
+              = salary_f.number_field :annual_amount, class: 'form-control', required: true
         .form-group.col-lg-12
           = f.label :notes, 'Notes', class: 'control-label'
           = f.text_area :notes, class: 'form-control'
diff --git a/app/views/employees/show.html.haml b/app/views/employees/show.html.haml
index dd3e7ca5..80aa6e37 100644
--- a/app/views/employees/show.html.haml
+++ b/app/views/employees/show.html.haml
@@ -38,10 +38,6 @@
           %td Salary
           %td
       %tbody
-        %tr
-          %td= @employee.start_date.strftime('%b %e, %Y')
-          %td= number_to_currency(@employee.starting_salary, precision: 0)
-          %td  (To change starting salary, use edit button above.)
         - @employee.salaries.each do |salary|
           %tr
             %td= salary.start_date.strftime('%b %e, %Y')
diff --git a/spec/models/employee_spec.rb b/spec/models/employee_spec.rb
index 464518a0..1dbad160 100644
--- a/spec/models/employee_spec.rb
+++ b/spec/models/employee_spec.rb
@@ -8,7 +8,6 @@
 
   it { should validate_presence_of :first_name }
   it { should validate_presence_of :last_name }
-  it { should validate_presence_of :starting_salary }
 
   it { should have_db_column(:billable).of_type(:boolean).with_options(default: true) }
   it { should have_db_column(:notes).of_type(:text) }
diff --git a/spec/models/salary_spec.rb b/spec/models/salary_spec.rb
index b374d04b..44ef2017 100644
--- a/spec/models/salary_spec.rb
+++ b/spec/models/salary_spec.rb
@@ -6,7 +6,7 @@
   it { should belong_to :employee }
   it { should validate_presence_of :start_date }
   it { should validate_presence_of :annual_amount }
-  it { should validate_presence_of :employee_id }
+  it { should validate_presence_of :employee }
 
   it 'validates employee salary start dates unique' do
     create :salary
@@ -113,4 +113,25 @@
       expect(salary).to be_invalid
     end
   end
+
+  describe 'before_validation' do
+    let(:employee) do
+      build(:employee).tap do |employee|
+        employee.tenures = [build(:tenure, start_date: Time.zone.today - 5)]
+        employee.save
+      end
+    end
+
+    it 'should retrieve start date from related tenure if blank and first' do
+      salary = employee.salaries.create(annual_amount: 5)
+      expect(salary).to be_valid
+    end
+
+    it 'should be invalid if blank and second' do
+      salary = employee.salaries.create(annual_amount: 5)
+      salary2 = employee.salaries.create(annual_amount: 5)
+      expect(salary2).to be_invalid
+    end
+  end
+
 end

From 69a0538c024a5092a74362ec76b294b81a1bf4ff Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger@fusionary.com>
Date: Tue, 23 Mar 2021 16:38:41 -0400
Subject: [PATCH 05/14] #206 update specs to match new salary data structure

---
 spec/features/manage_salaries_spec.rb |  3 ++-
 spec/models/salary_spec.rb            | 10 +++++++---
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/spec/features/manage_salaries_spec.rb b/spec/features/manage_salaries_spec.rb
index cf44b367..8442e809 100644
--- a/spec/features/manage_salaries_spec.rb
+++ b/spec/features/manage_salaries_spec.rb
@@ -23,7 +23,8 @@
   end
 
   context 'Raise salary exists' do
-    let!(:salary) { create :salary }
+    let!(:salary) { create :salary, tenure: employee.tenures.first }
+    let!(:raise) { create :salary, tenure: employee.tenures.first, start_date: employee.tenures.first.start_date + 30 }
 
     scenario 'delete salary' do
       visit "/employees/#{salary.employee.id}"
diff --git a/spec/models/salary_spec.rb b/spec/models/salary_spec.rb
index 45ebe8ab..f5fe56db 100644
--- a/spec/models/salary_spec.rb
+++ b/spec/models/salary_spec.rb
@@ -63,27 +63,31 @@
     let!(:first_employee) do
       build(:employee).tap do |employee|
         employee.tenures = [build(:tenure, start_date: first_start_date, end_date: first_end_date)]
+        employee.tenures.first.salaries = [build(:salary, start_date: first_start_date)]
         employee.save
       end
     end
     let!(:second_employee) do
       build(:employee).tap do |employee|
         employee.tenures = [build(:tenure, start_date: first_start_date, end_date: second_end_date)]
+        employee.tenures.first.salaries = [build(:salary, start_date: first_start_date)]
         employee.save
       end
     end
     let!(:third_employee) do
       build(:employee).tap do |employee|
         employee.tenures = [build(:tenure, start_date: second_start_date)]
+        employee.tenures.first.salaries = [build(:salary, start_date: second_start_date)]
         employee.save
       end
     end
 
-    let!(:first_added_salary) { create :salary, employee: second_employee, start_date: first_salary_date }
-    let!(:second_added_salary) { create :salary, employee: third_employee, start_date: second_salary_date }
+    let!(:first_added_salary) { create :salary, tenure: second_employee.tenures.first, start_date: first_salary_date }
+    let!(:second_added_salary) { create :salary, tenure: third_employee.tenures.first, start_date: second_salary_date }
 
     it 'returns an ordered list of history dates' do
-      expect(Salary.history_dates).to eq([first_start_date, second_start_date,
+      expect(Salary.history_dates).to eq([first_start_date, second_start_date - 1, 
+                                          second_start_date,
                                           first_end_date, first_end_date + 1,
                                           first_salary_date - 1, first_salary_date,
                                           second_salary_date - 1, second_salary_date,

From e73347195c8bcc4d92b1832aacaebc8d794f1e1c Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger@fusionary.com>
Date: Wed, 24 Mar 2021 09:19:56 -0400
Subject: [PATCH 06/14] #206 improve employee factory and make specs match

---
 app/models/tenure.rb                         |  2 +-
 spec/controllers/charts_controller_spec.rb   | 16 ++-----------
 spec/controllers/salaries_controller_spec.rb |  1 +
 spec/factories/employee_factory.rb           | 12 ++++++++--
 spec/features/manage_salaries_spec.rb        |  3 +--
 spec/models/employee_spec.rb                 | 17 ++++----------
 spec/models/salary_spec.rb                   | 24 +++++---------------
 7 files changed, 25 insertions(+), 50 deletions(-)

diff --git a/app/models/tenure.rb b/app/models/tenure.rb
index be721daa..6224fdd9 100644
--- a/app/models/tenure.rb
+++ b/app/models/tenure.rb
@@ -2,7 +2,7 @@
 
 class Tenure < ActiveRecord::Base
   belongs_to :employee
-  has_many :salaries
+  has_many :salaries, dependent: :destroy
 
   validates :start_date, presence: true, uniqueness: { scope: :employee_id }
   validates :employee, presence: true
diff --git a/spec/controllers/charts_controller_spec.rb b/spec/controllers/charts_controller_spec.rb
index f74a7574..d6c8faed 100644
--- a/spec/controllers/charts_controller_spec.rb
+++ b/spec/controllers/charts_controller_spec.rb
@@ -16,20 +16,8 @@
   end
 
   describe 'GET salaries' do
-    let!(:current_employee)  do
-      build(:employee).tap do |employee|
-        employee.tenures = [build(:tenure, end_date: Time.zone.today + 1)]
-        employee.save
-      end
-    end
-    let!(:salary_1) { create :salary, employee: current_employee }
-    let!(:former_employee) do
-      build(:employee).tap do |employee|
-        employee.tenures = [build(:tenure, end_date: Time.zone.today - 1)]
-        employee.save
-      end
-    end
-    let!(:salary_2) { create :salary, employee: former_employee }
+    let!(:current_employee) { create(:employee, end_date: Time.zone.today + 1) }
+    let!(:former_employee) { create(:employee, end_date: Time.zone.today - 1) }
 
     it 'returns http success' do
       get :salaries
diff --git a/spec/controllers/salaries_controller_spec.rb b/spec/controllers/salaries_controller_spec.rb
index 6fff85dc..cefed714 100644
--- a/spec/controllers/salaries_controller_spec.rb
+++ b/spec/controllers/salaries_controller_spec.rb
@@ -32,6 +32,7 @@
       end
 
       it 'creates a new Salary' do
+        employee.reload
         expect do
           post :create, params: post_params
         end.to change(Salary, :count).by(1)
diff --git a/spec/factories/employee_factory.rb b/spec/factories/employee_factory.rb
index 25a657cf..ac150bfc 100644
--- a/spec/factories/employee_factory.rb
+++ b/spec/factories/employee_factory.rb
@@ -4,17 +4,25 @@
   factory :employee do
     sequence(:first_name) { |n| "##{n}" }
     last_name { 'Bendyworker' }
-    after :create do |employee|
-      create_list :tenure, 1, employee: employee
+    transient do
+      starting_salary { 40_000 }
+      sequence(:start_date) { |n| 1.year.ago + n.day }
+      end_date { nil }
+    end
+    after :create do |employee, evaluator|
+      create :tenure, employee: employee, start_date: evaluator.start_date, end_date: evaluator.end_date
+      create :salary, tenure: employee.tenures.first, start_date: employee.tenures.first.start_date, annual_amount: evaluator.starting_salary
     end
 
     trait :current do
       transient do
         end_date { Time.zone.today + 1 }
+        starting_salary { 40_000 }
       end
       first_name { 'Current' }
       after(:create) do |employee, evaluator|
         employee.tenures = [FactoryBot.create(:tenure, end_date: evaluator.end_date)]
+        create :salary, tenure: employee.tenures.first, start_date: employee.tenures.first.start_date, annual_amount: evaluator.starting_salary
       end
     end
 
diff --git a/spec/features/manage_salaries_spec.rb b/spec/features/manage_salaries_spec.rb
index 8442e809..bd14225a 100644
--- a/spec/features/manage_salaries_spec.rb
+++ b/spec/features/manage_salaries_spec.rb
@@ -23,8 +23,7 @@
   end
 
   context 'Raise salary exists' do
-    let!(:salary) { create :salary, tenure: employee.tenures.first }
-    let!(:raise) { create :salary, tenure: employee.tenures.first, start_date: employee.tenures.first.start_date + 30 }
+    let!(:salary) { create :salary, tenure: employee.tenures.first, start_date: employee.tenures.first.start_date + 30 }
 
     scenario 'delete salary' do
       visit "/employees/#{salary.employee.id}"
diff --git a/spec/models/employee_spec.rb b/spec/models/employee_spec.rb
index f7c33222..f5a87a95 100644
--- a/spec/models/employee_spec.rb
+++ b/spec/models/employee_spec.rb
@@ -26,11 +26,7 @@
     let(:third_raise_date) { 1.months.ago.to_date }
 
     let!(:employee) do
-      build(:employee).tap do |employee|
-        employee.tenures = [build(:tenure, start_date: start_date)]
-        employee.tenures.first.salaries = [build(:salary, start_date: start_date, annual_amount: 1_000)]
-        employee.save
-      end
+      create(:employee, starting_salary: 1_000, start_date: start_date)
     end
     let(:last_raise_date) { employee.reload.last_raise_date }
 
@@ -137,11 +133,7 @@
 
   describe '#display_pay' do
     let(:start_date) { Date.parse('2015-08-07') }
-    let(:employee) { create :employee, tenures_attributes: [{start_date: start_date}] }
-
-    let!(:starting_salary) do
-      create(:salary, tenure: employee.tenures.first, start_date: start_date, annual_amount: pay)
-    end
+    let(:employee) { create :employee, start_date: start_date, starting_salary: pay }
 
     context 'when pay is a whole number of thousands' do
       let(:pay) { 73_000 }
@@ -163,9 +155,8 @@
     let(:raise_date) { Date.parse('2013-12-10') }
     let(:end_date) { Date.parse('2014-12-31') }
 
-    let(:daisie) { create(:employee, tenures_attributes: [{start_date: start_date, end_date: end_date}]) }
+    let(:daisie) { create(:employee, starting_salary: 800, tenures_attributes: [{start_date: start_date, end_date: end_date}]) }
 
-    let!(:starting_salary) { create(:salary, tenure: daisie.tenures.first, start_date: start_date, annual_amount: 800) }
     let!(:raise_salary) do
       create(:salary, tenure: daisie.tenures.first, start_date: raise_date, annual_amount: '900')
     end
@@ -186,7 +177,7 @@
       expect(daisie.salary_on(raise_date + 5)).to eq raise_salary.annual_amount
     end
     it 'returns correct salary, given a date between two salary `start_date`s' do
-      expect(daisie.salary_on(raise_date - 5)).to eq starting_salary.annual_amount
+      expect(daisie.salary_on(raise_date - 5)).to eq daisie.salaries.first.annual_amount
     end
   end
 
diff --git a/spec/models/salary_spec.rb b/spec/models/salary_spec.rb
index f5fe56db..27ea4c06 100644
--- a/spec/models/salary_spec.rb
+++ b/spec/models/salary_spec.rb
@@ -25,10 +25,10 @@
     let(:first_date) { 7.months.ago.to_date }
     let(:third_date) { 5.months.ago.to_date }
     let(:second_date) { 6.months.ago.to_date }
-    let(:tenure) { create :tenure, start_date: first_date }
+    let (:employee) { create :employee, start_date: first_date }
+    let(:tenure) { employee.tenures.first }
 
     let!(:first_added_salary) { create :salary, tenure: tenure, start_date: fourth_date }
-    let!(:second_added_salary) { create :salary, tenure: tenure, start_date: first_date }
     let!(:third_added_salary) { create :salary, tenure: tenure, start_date: third_date }
     let!(:fourth_added_salary) { create :salary, tenure: tenure, start_date: second_date }
 
@@ -37,7 +37,7 @@
     end
 
     it 'removes duplicate dates' do
-      create :salary, start_date: third_date
+      create :employee, start_date: third_date
       expect(Salary.ordered_dates).to eq([first_date, second_date, third_date, fourth_date])
     end
 
@@ -61,25 +61,13 @@
 
 
     let!(:first_employee) do
-      build(:employee).tap do |employee|
-        employee.tenures = [build(:tenure, start_date: first_start_date, end_date: first_end_date)]
-        employee.tenures.first.salaries = [build(:salary, start_date: first_start_date)]
-        employee.save
-      end
+      create(:employee, start_date: first_start_date, end_date: first_end_date)
     end
     let!(:second_employee) do
-      build(:employee).tap do |employee|
-        employee.tenures = [build(:tenure, start_date: first_start_date, end_date: second_end_date)]
-        employee.tenures.first.salaries = [build(:salary, start_date: first_start_date)]
-        employee.save
-      end
+      create(:employee, start_date: first_start_date, end_date: second_end_date)
     end
     let!(:third_employee) do
-      build(:employee).tap do |employee|
-        employee.tenures = [build(:tenure, start_date: second_start_date)]
-        employee.tenures.first.salaries = [build(:salary, start_date: second_start_date)]
-        employee.save
-      end
+      create(:employee, start_date: second_start_date)
     end
 
     let!(:first_added_salary) { create :salary, tenure: second_employee.tenures.first, start_date: first_salary_date }

From 032333a40b0e9b4003f58353beddbd050615bc8b Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger@fusionary.com>
Date: Wed, 24 Mar 2021 10:21:50 -0400
Subject: [PATCH 07/14] #206 make rails_best_practice happy

---
 app/models/employee.rb                        |  2 +-
 ...533_migrate_starting_salary_to_salaries.rb | 15 ++++++-------
 db/seeds.rb                                   | 21 ++++++++++++-------
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/app/models/employee.rb b/app/models/employee.rb
index 71188167..70d0a571 100644
--- a/app/models/employee.rb
+++ b/app/models/employee.rb
@@ -3,7 +3,7 @@
 class Employee < ActiveRecord::Base
   has_many :tenures, dependent: :destroy
   has_many :salaries, through: :tenures, dependent: :destroy
-  
+
   accepts_nested_attributes_for :tenures,
     reject_if: proc { |attributes| attributes['start_date'].blank? }, allow_destroy: true
 
diff --git a/db/migrate/20210315122533_migrate_starting_salary_to_salaries.rb b/db/migrate/20210315122533_migrate_starting_salary_to_salaries.rb
index 0b8afbd8..a9e114a8 100644
--- a/db/migrate/20210315122533_migrate_starting_salary_to_salaries.rb
+++ b/db/migrate/20210315122533_migrate_starting_salary_to_salaries.rb
@@ -3,19 +3,20 @@ def up
     add_reference :salaries, :tenure, foreign_key: true
 
     Employee.all.each do |employee|
-      Salary.where(employee_id: employee.id).all.each do |salary| 
+      Salary.where(employee_id: employee.id).all.each do |salary|
         # look for tenure date range containing salary start_date.  Fall back to first if none found
-        tenure = employee.tenures
-          .where('start_date <= :date and (end_date >= :date or end_date is NULL)', date: salary.start_date).first
+        tenure = employee.tenures.where(
+          'start_date <= :date and (end_date >= :date or end_date is NULL)',
+          date: salary.start_date).first
         tenure = employee.tenures.first unless tenure
         salary.update(tenure_id: tenure.id)
       end
       tenure = employee.tenures.first
       tenure.salaries << Salary.new(
-        start_date: tenure.start_date, 
-        annual_amount: employee.attributes['starting_salary'], 
+        start_date: tenure.start_date,
+        annual_amount: employee.attributes['starting_salary'],
         employee_id: employee.id)
-      tenure.save
+      tenure.save!
     end
 
     remove_reference :salaries, :employee, foreign_key: true
@@ -33,7 +34,7 @@ def down
       tenure = employee.tenures.first
       salary = tenure.salaries.first if tenure
       employee.update(starting_salary: salary.annual_amount) if salary
-      salary.destroy
+      salary.destroy!
     end
 
     remove_reference :salaries, :tenure
diff --git a/db/seeds.rb b/db/seeds.rb
index 4917cb00..728f31b0 100644
--- a/db/seeds.rb
+++ b/db/seeds.rb
@@ -19,8 +19,10 @@
 darkwing = Employee.create!(first_name: 'Darkwing (gone)', last_name: 'Duck',
                             tenures: [Tenure.new(start_date: darkwing_start_date,
                                                  end_date: darkwing_end_date)])
-Salary.create!(tenure: darkwing.tenures.first, start_date: darkwing_start_date, annual_amount: '100000')
-Salary.create!(tenure: darkwing.tenures.first, start_date: darkwing_start_date + 90, annual_amount: '200000')
+Salary.create!(tenure: darkwing.tenures.first, start_date: darkwing_start_date,
+               annual_amount: '100000')
+Salary.create!(tenure: darkwing.tenures.first, start_date: darkwing_start_date + 90,
+               annual_amount: '200000')
 
 ###### FUTURE EMPLOYEES ######
 
@@ -31,7 +33,8 @@
                           indirect_experience: 8,
                           tenures: [Tenure.new(start_date: daffy_start_date)])
 
-Salary.create!(tenure: _daffy.tenures.first, start_date: daffy_start_date, annual_amount: '51000.00')
+Salary.create!(tenure: _daffy.tenures.first, start_date: daffy_start_date,
+               annual_amount: '51000.00')
 
 ###### CURRENT EMPLOYEES #######
 ## Daisie
@@ -42,7 +45,8 @@
                           indirect_experience: 4,
                           tenures: [Tenure.new(start_date: daisie_start_date)])
 
-Salary.create!(tenure: daisie.tenures.first, start_date: daisie_start_date, annual_amount: '57000.00')
+Salary.create!(tenure: daisie.tenures.first, start_date: daisie_start_date,
+               annual_amount: '57000.00')
 Salary.create!(tenure: daisie.tenures.first,
                start_date: daisie_start_date + 90, # 4-1-2013
                annual_amount: '59000.00')
@@ -52,7 +56,8 @@
 minnie = Employee.create!(first_name: 'Minnie', last_name: 'Mouse',
                           tenures: [Tenure.new(start_date: minnie_start_date)])
 
-Salary.create!(tenure: minnie.tenures.first, start_date: minnie_start_date, annual_amount: '46000.00')
+Salary.create!(tenure: minnie.tenures.first, start_date: minnie_start_date,
+               annual_amount: '46000.00')
 Salary.create!(tenure: minnie.tenures.first,
                start_date: minnie_start_date + 90, # 6-30-2013
                annual_amount: '49000.00')
@@ -63,7 +68,8 @@
                           indirect_experience: 18,
                           tenures: [Tenure.new(start_date: mickey_start_date)])
 
-Salary.create!(tenure: mickey.tenures.first, start_date: mickey_start_date, annual_amount: '45000.00')
+Salary.create!(tenure: mickey.tenures.first, start_date: mickey_start_date,
+               annual_amount: '45000.00')
 Salary.create!(tenure: mickey.tenures.first,
                start_date: mickey_start_date + 40, # 8-9-2013
                annual_amount: '50000.00')
@@ -80,7 +86,8 @@
                            billable: false,
                            tenures: [Tenure.new(start_date: donald_start_date)])
 
-Salary.create!(tenure: _donald.tenures.first, start_date: donald_start_date, annual_amount: '40000.00')
+Salary.create!(tenure: _donald.tenures.first, start_date: donald_start_date,
+               annual_amount: '40000.00')
 
 #########################
 puts 'Seed data created.'

From 8ee9daa31ebb184912391f507d1456979cbeb423 Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger@fusionary.com>
Date: Thu, 25 Mar 2021 08:41:33 -0400
Subject: [PATCH 08/14] #206 get salary start date from tenure rather than from
 a hidden field

---
 app/controllers/employees_controller.rb       | 31 ++++++++++++-------
 app/views/employees/_form.html.haml           |  3 +-
 spec/controllers/employees_controller_spec.rb |  6 ++--
 3 files changed, 23 insertions(+), 17 deletions(-)

diff --git a/app/controllers/employees_controller.rb b/app/controllers/employees_controller.rb
index a5f146b2..151f02e9 100644
--- a/app/controllers/employees_controller.rb
+++ b/app/controllers/employees_controller.rb
@@ -44,17 +44,24 @@ def set_employee
   end
 
   def employee_params
-    params.require(:employee).permit(:first_name,
-                                     :last_name,
-                                     :starting_salary,
-                                     :direct_experience,
-                                     :indirect_experience,
-                                     :billable,
-                                     :notes,
-                                     :planning_raise_date,
-                                     :planning_raise_salary,
-                                     :planning_notes,
-                                     tenures_attributes: [:id, :start_date, :end_date, :_destroy],
-                                     salaries_attributes: [:id, :start_date, :annual_amount])
+    emp_params = params.require(:employee)
+      .permit(:first_name,
+              :last_name,
+              :starting_salary,
+              :direct_experience,
+              :indirect_experience,
+              :billable,
+              :notes,
+              :planning_raise_date,
+              :planning_raise_salary,
+              :planning_notes,
+              tenures_attributes: [:id, :start_date, :end_date, :_destroy],
+              salaries_attributes: [:id, :annual_amount])
+    salary = emp_params.dig(:salaries_attributes, "0")
+    tenure = emp_params.dig(:tenures_attributes, "0")
+    if salary && tenure
+      emp_params[:salaries_attributes]["0"][:start_date] = tenure[:start_date]
+    end
+    emp_params
   end
 end
diff --git a/app/views/employees/_form.html.haml b/app/views/employees/_form.html.haml
index 79a0feb9..cb320ed2 100644
--- a/app/views/employees/_form.html.haml
+++ b/app/views/employees/_form.html.haml
@@ -50,8 +50,7 @@
           = f.label :indirect_experience, 'Indirect experience (months)', class: 'control-label'
           = f.number_field :indirect_experience, class: 'form-control'
         = f.fields_for :salaries, (employee.salaries.first || employee.salaries.build) do |salary_f|
-          = salary_f.hidden_field :id unless employee.salaries.count == 0
-          = salary_f.hidden_field :start_date unless employee.salaries.count == 0
+          = salary_f.hidden_field :id
           .form-group.col-lg-12
             = salary_f.label :annual_amount, 'Starting salary annual amount', class: 'control-label'
             .input-group
diff --git a/spec/controllers/employees_controller_spec.rb b/spec/controllers/employees_controller_spec.rb
index 82bea6a2..53eb0937 100644
--- a/spec/controllers/employees_controller_spec.rb
+++ b/spec/controllers/employees_controller_spec.rb
@@ -50,7 +50,7 @@
 
   describe 'POST create' do
     describe 'with valid params' do
-      let(:valid_attributes) {{ employee: attributes_for(:employee).merge(tenures_attributes: [{ start_date: Time.zone.today }]) }}
+      let(:valid_attributes) {{ employee: attributes_for(:employee).merge(tenures_attributes: {"0" => { start_date: Time.zone.today }}) }}
       it 'creates a new Employee' do
         expect do
           post :create, params: valid_attributes
@@ -59,7 +59,7 @@
 
       it 'creates a new Employee with the expected start date' do
         post :create, params: valid_attributes
-        expect(assigns(:employee).start_date).to eq(valid_attributes[:employee][:tenures_attributes][0][:start_date])
+        expect(assigns(:employee).start_date).to eq(valid_attributes[:employee][:tenures_attributes]["0"][:start_date])
       end
 
       it 'assigns a newly created employee as @employee' do
@@ -70,7 +70,7 @@
     end
 
     describe 'with invalid params' do
-      let(:invalid_attributes) {{ employee: attributes_for(:employee).merge(first_name: nil).merge(tenures_attributes: [{ start_date: Time.zone.today }]) }}
+      let(:invalid_attributes) {{ employee: attributes_for(:employee).merge(first_name: nil).merge(tenures_attributes: {"0" => { start_date: Time.zone.today }}) }}
 
       it 'assigns a newly created but unsaved employee as @employee' do
         post :create, params: invalid_attributes

From 1a3ff98129d09a6ec5376df30ead342582433a4f Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger.roelofs@gmail.com>
Date: Fri, 26 Mar 2021 11:04:04 -0400
Subject: [PATCH 09/14] #206 WIP change how starting salary UI works

---
 app/controllers/employees_controller.rb | 13 ++------
 app/controllers/salaries_controller.rb  | 42 ++++++++++++++++++++-----
 app/models/employee.rb                  |  8 -----
 app/views/employees/_form.html.haml     |  7 -----
 app/views/employees/show.html.haml      | 11 ++++---
 app/views/salaries/_form.html.haml      | 20 ++++++++++++
 app/views/salaries/edit.html.haml       |  3 ++
 app/views/salaries/new.html.haml        | 28 ++++-------------
 config/routes.rb                        |  2 +-
 9 files changed, 73 insertions(+), 61 deletions(-)
 create mode 100644 app/views/salaries/_form.html.haml
 create mode 100644 app/views/salaries/edit.html.haml

diff --git a/app/controllers/employees_controller.rb b/app/controllers/employees_controller.rb
index 151f02e9..5b697629 100644
--- a/app/controllers/employees_controller.rb
+++ b/app/controllers/employees_controller.rb
@@ -12,10 +12,8 @@ def new
   def edit; end
 
   def create
-    salary_params = employee_params.dig(:salaries_attributes, "0")
-    @employee = Employee.new(employee_params.except(:salaries_attributes))
+    @employee = Employee.new(employee_params)
     if @employee.save
-      @employee.tenures.first.salaries.create(salary_params) unless salary_params.blank?
       redirect_to @employee, notice: 'Employee successfully created.'
     else
       @errors = @employee.errors.full_messages
@@ -55,13 +53,6 @@ def employee_params
               :planning_raise_date,
               :planning_raise_salary,
               :planning_notes,
-              tenures_attributes: [:id, :start_date, :end_date, :_destroy],
-              salaries_attributes: [:id, :annual_amount])
-    salary = emp_params.dig(:salaries_attributes, "0")
-    tenure = emp_params.dig(:tenures_attributes, "0")
-    if salary && tenure
-      emp_params[:salaries_attributes]["0"][:start_date] = tenure[:start_date]
-    end
-    emp_params
+              tenures_attributes: [:id, :start_date, :end_date, :_destroy])
   end
 end
diff --git a/app/controllers/salaries_controller.rb b/app/controllers/salaries_controller.rb
index c48cdaeb..5652b371 100644
--- a/app/controllers/salaries_controller.rb
+++ b/app/controllers/salaries_controller.rb
@@ -1,30 +1,58 @@
 # frozen_string_literal: true
 
 class SalariesController < ApplicationController
+  before_action :set_employee, except: [:edit, :update, :destroy]
+  before_action :set_salary, only: [:edit, :update, :destroy]
+
   def new
-    employee = Employee.find(params[:employee_id])
-    @salary = employee.tenures.last.salaries.new
+    salaries_count = last_tenure.salaries.count
+    @salary = last_tenure.salaries.new
+    # pre-fill start date from tenure for first salary to help user
+    @salary.start_date = last_tenure.start_date if salaries_count == 0
   end
 
   def create
-    employee = Employee.find(params[:employee_id])
-    tenure = employee.tenures.last
-    @salary = tenure.salaries.new(salary_params)
+    update_params = salary_params
+    update_params["start_date"] = @employee.tenures.last.start_date unless update_params.dig(:start_date)
+    @salary = @employee.tenures.last.salaries.new(update_params)
     if @salary.save
-      redirect_to employee_path(employee), notice: 'Successfully recorded raise'
+      redirect_to employee_path(@employee), notice: 'Successfully recorded raise'
     else
       render :new
     end
   end
 
+  def edit; end
+
+  def update
+    update_params = salary_params
+    update_params["start_date"] = @salary.tenure.start_date unless update_params.dig(:start_date)
+    if @salary.update(update_params)
+      redirect_to employee_path(@salary.employee), notice: 'Successfully upddated salary'
+    else
+      render :edit
+    end
+  end
+
   def destroy
-    @salary = Salary.find(params[:id])
     msg = @salary.destroy ? 'Salary deleted.' : 'Failed to delete salary. Please try again.'
     redirect_to employee_path(params[:employee_id]), notice: msg
   end
 
   private
 
+  def set_employee
+    @employee = Employee.find(params[:employee_id])
+  end
+
+  def set_salary
+    @salary = Salary.find(params[:id])
+  end
+
+  def last_tenure
+    @employee.tenures.last
+  end
+
   def salary_params
     params.require(:salary).permit(:start_date, :annual_amount)
   end
diff --git a/app/models/employee.rb b/app/models/employee.rb
index 70d0a571..2bace083 100644
--- a/app/models/employee.rb
+++ b/app/models/employee.rb
@@ -7,18 +7,10 @@ class Employee < ActiveRecord::Base
   accepts_nested_attributes_for :tenures,
     reject_if: proc { |attributes| attributes['start_date'].blank? }, allow_destroy: true
 
-  accepts_nested_attributes_for :salaries,
-    reject_if: proc { |attributes| attributes['annual_amount'].blank? }
-
   validates :first_name, presence: true
   validates :last_name, presence: true
 
   default_scope { order :first_name }
-  scope :past, -> { joins(:tenures).where 'tenures.end_date < :today', today: Time.zone.today }
-  scope :current, lambda {
-    joins(:tenures).where 'tenures.start_date <= :today' \
-          ' AND (tenures.end_date IS NULL OR tenures.end_date >= :today)', today: Time.zone.today
-  }
   scope :future, -> { joins(:tenures).where 'tenures.start_date > :today', today: Time.zone.today }
   scope :non_current, lambda {
     joins(:tenures)
diff --git a/app/views/employees/_form.html.haml b/app/views/employees/_form.html.haml
index cb320ed2..201c8ede 100644
--- a/app/views/employees/_form.html.haml
+++ b/app/views/employees/_form.html.haml
@@ -49,13 +49,6 @@
         .form-group.col-lg-12
           = f.label :indirect_experience, 'Indirect experience (months)', class: 'control-label'
           = f.number_field :indirect_experience, class: 'form-control'
-        = f.fields_for :salaries, (employee.salaries.first || employee.salaries.build) do |salary_f|
-          = salary_f.hidden_field :id
-          .form-group.col-lg-12
-            = salary_f.label :annual_amount, 'Starting salary annual amount', class: 'control-label'
-            .input-group
-              %span.input-group-addon $
-              = salary_f.number_field :annual_amount, class: 'form-control', required: true
         .form-group.col-lg-12
           = f.label :notes, 'Notes', class: 'control-label'
           = f.text_area :notes, class: 'form-control'
diff --git a/app/views/employees/show.html.haml b/app/views/employees/show.html.haml
index ee8cdbab..14c9ca48 100644
--- a/app/views/employees/show.html.haml
+++ b/app/views/employees/show.html.haml
@@ -43,12 +43,13 @@
             %td= salary.start_date.strftime('%b %e, %Y')
             %td= number_to_currency(salary.annual_amount, precision: 0)
             %td
-              -if i == 0
-                (To change starting salary, use edit button above.)
-              -else
-                = link_to 'Delete', employee_salary_path(employee_id: @employee, id: salary), method: :delete
+              = link_to 'Edit', edit_employee_salary_path(employee_id: @employee, id: salary)
+              = link_to 'Delete', employee_salary_path(employee_id: @employee, id: salary), method: :delete
   - if @employee.notes?
     %li.active
       = link_to 'Notes', "#"
       %pre= @employee.notes
-  = link_to 'Record a Raise', new_employee_salary_path(@employee), class: 'btn btn-success'
+  - if @employee.tenures.last.salaries.count == 0
+    = link_to 'Record Starting Salary', new_employee_salary_path(@employee), class: 'btn btn-success'
+  - else
+    = link_to 'Record a Raise', new_employee_salary_path(@employee), class: 'btn btn-success'
diff --git a/app/views/salaries/_form.html.haml b/app/views/salaries/_form.html.haml
new file mode 100644
index 00000000..07f0315b
--- /dev/null
+++ b/app/views/salaries/_form.html.haml
@@ -0,0 +1,20 @@
+- if @salary.errors.any?
+  .alert.alert-danger
+    %p #{pluralize(@salary.errors.count, "error")} prohibited this raise from being saved:
+    %ul
+      - @salary.errors.full_messages.each do |msg|
+        %li= msg
+.col-lg-9
+  .well.bs-component
+    %fieldset
+      .form-group.col-lg-12
+        = f.label :start_date, class: 'control-label'
+        = f.date_field :start_date, class: 'form-control', required: true, autofocus: @salary.start_date != @salary.tenure.start_date, disabled: (@salary.start_date == @salary.tenure.start_date)
+      .form-group.col-lg-12
+        = f.label :annual_amount, class: 'control-label'
+        .input-group
+          %span.input-group-addon $
+          = f.number_field :annual_amount, class: 'form-control', required: true
+      .form-group.col-lg-12
+        = f.submit 'Save', class: 'btn btn-primary'
+        = link_to 'Cancel', employee_path(@salary.employee), class: 'btn btn-default'
diff --git a/app/views/salaries/edit.html.haml b/app/views/salaries/edit.html.haml
new file mode 100644
index 00000000..7be0265d
--- /dev/null
+++ b/app/views/salaries/edit.html.haml
@@ -0,0 +1,3 @@
+%h2= "Edit Salary for #{@salary.employee_first_name} #{@salary.employee_last_name}"
+= form_for(@salary, url: employee_salary_path(@salary), html: { class: 'form-horizontal' }) do |f|
+    = render 'form', salary: @salary, f: f
\ No newline at end of file
diff --git a/app/views/salaries/new.html.haml b/app/views/salaries/new.html.haml
index ac111851..5cce402d 100644
--- a/app/views/salaries/new.html.haml
+++ b/app/views/salaries/new.html.haml
@@ -1,22 +1,6 @@
-%h2= "Record a Raise for #{@salary.employee_first_name} #{@salary.employee_last_name}"
-- if @salary.errors.any?
-  .alert.alert-danger
-    %p= "#{pluralize(@salary.errors.count, "error")} prohibited this raise from being saved:"
-    %ul
-      - @salary.errors.full_messages.each do |msg|
-        %li= msg
-.col-lg-9
-  .well.bs-component
-    = form_for(@salary, url: employee_salaries_path(@salary.employee), html: { class: 'form-horizontal' }) do |f|
-      %fieldset
-        .form-group.col-lg-12
-          = f.label :start_date, class: 'control-label'
-          = f.date_field :start_date, class: 'form-control', required: true, autofocus: true
-        .form-group.col-lg-12
-          = f.label :annual_amount, class: 'control-label'
-          .input-group
-            %span.input-group-addon $
-            = f.number_field :annual_amount, class: 'form-control', required: true, value: @salary.employee.salary_on(Time.zone.today)
-        .form-group.col-lg-12
-          = f.submit 'Save', class: 'btn btn-primary'
-          = link_to 'Cancel', employee_path(@salary.employee), class: 'btn btn-default'
+- if @salary.tenure.salaries.count >= 1
+  %h2= "Record a Raise for #{@salary.employee_first_name} #{@salary.employee_last_name}"
+- else
+  %h2= "Record Starting Salary for #{@salary.employee_first_name} #{@salary.employee_last_name}"
+= form_for(@salary, url: employee_salaries_path(@salary.employee), html: { class: 'form-horizontal' }) do |f|
+  = render 'form', salary: @salary, f: f
\ No newline at end of file
diff --git a/config/routes.rb b/config/routes.rb
index a38e7690..a87fdd21 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -17,7 +17,7 @@
   get 'planning', to: 'planning#index'
 
   resources :employees, except: [:index, :destroy] do
-    resources :salaries, only: [:new, :create, :destroy]
+    resources :salaries, only: [:new, :create, :edit, :update, :destroy]
   end
 
   resources :users, only: [:index, :destroy] do

From 13f90d89f92eda7a47e470c28a29d5d6e8ebebab Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger.roelofs@gmail.com>
Date: Fri, 26 Mar 2021 13:37:21 -0400
Subject: [PATCH 10/14] #206 WIP add after save to keep tenure and salary dates
 in sync and fix specs

---
 app/models/tenure.rb                  |  8 ++++++++
 spec/features/manage_employee_spec.rb | 12 ++----------
 spec/features/manage_salaries_spec.rb |  4 ++--
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/app/models/tenure.rb b/app/models/tenure.rb
index 6224fdd9..05ed41ec 100644
--- a/app/models/tenure.rb
+++ b/app/models/tenure.rb
@@ -9,6 +9,8 @@ class Tenure < ActiveRecord::Base
   validate :tenures_are_sequential, if: :employee
   validate :start_date_is_before_end_date
 
+  after_save :sync_first_salary
+
   default_scope { order :start_date }
 
   def self.ordered_start_dates
@@ -49,4 +51,10 @@ def tenures_are_sequential
       errors.add(:end_date, 'must be before the next start date')
     end
   end
+
+  def sync_first_salary
+    if salaries.count > 0 && salaries.first&.start_date != start_date
+      salaries.first.update(start_date: start_date)
+    end
+  end
 end
diff --git a/spec/features/manage_employee_spec.rb b/spec/features/manage_employee_spec.rb
index 8821620c..24a85197 100644
--- a/spec/features/manage_employee_spec.rb
+++ b/spec/features/manage_employee_spec.rb
@@ -15,7 +15,6 @@
     fill_in 'Last name', with: 'Bendyworker'
     fill_in 'Start date', with: Time.zone.today
     fill_in 'Direct experience', with: 5
-    fill_in 'Starting salary annual amount', with: 40_000
     click_button 'Save'
 
     expect(page).to have_content 'Newest Bendyworker'
@@ -25,10 +24,11 @@
   context 'employee exists' do
     let(:employee) { create :employee }
     let!(:salary) { create :salary, tenure: employee.tenures.first }
+    let(:link) { "a[href$=\"#{employee.id}/edit\"]" }
 
     scenario 'edit employee basic info' do
       visit "/employees/#{employee.id}"
-      click_on 'Edit'
+      find(:css, link).click
       fill_in 'First name', with: 'Different'
       fill_in 'Indirect experience', with: 11
       click_button 'Save'
@@ -37,13 +37,5 @@
       expect(page).to have_content '11 months indirect'
     end
 
-    scenario 'edit employee starting salary amount' do
-      visit "/employees/#{employee.id}"
-      click_on 'Edit'
-      fill_in 'Starting salary', with: 41_000
-      click_button 'Save'
-
-      expect(page).to have_content '$41,000'
-    end
   end
 end
diff --git a/spec/features/manage_salaries_spec.rb b/spec/features/manage_salaries_spec.rb
index bd14225a..f61f5cda 100644
--- a/spec/features/manage_salaries_spec.rb
+++ b/spec/features/manage_salaries_spec.rb
@@ -24,12 +24,12 @@
 
   context 'Raise salary exists' do
     let!(:salary) { create :salary, tenure: employee.tenures.first, start_date: employee.tenures.first.start_date + 30 }
+    let(:link) { "a[href$=\"/salaries/#{salary.id}\"]" }
 
     scenario 'delete salary' do
       visit "/employees/#{salary.employee.id}"
-      click_on 'Delete'
+      find(:css, link).click
 
-      expect(page).not_to have_link 'Delete'
       expect(page).to have_content 'Salary deleted'
     end
   end

From 2993f4b217b20a74b27569f0502b5f013fdc4868 Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger.roelofs@gmail.com>
Date: Fri, 26 Mar 2021 17:36:28 -0400
Subject: [PATCH 11/14] #206 update cucumber tests

---
 features/employees/employees.feature   | 3 +--
 features/step_definitions/web_steps.rb | 4 ++++
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/features/employees/employees.feature b/features/employees/employees.feature
index 9bc0fbbb..2a4034e7 100644
--- a/features/employees/employees.feature
+++ b/features/employees/employees.feature
@@ -12,7 +12,6 @@ Feature: Administer employees
     When I fill in "First name" with "Scrooge"
       And I fill in "Last name" with "McDuck"
       And I fill in "Start date" with "03/01/2016"
-      And I fill in "Starting salary annual amount" with "10000.00"
       And I fill in "Notes" with "New employee"
       And I press "Save"
     Then I see "Employee successfully created."
@@ -22,7 +21,7 @@ Feature: Administer employees
       And I'm logged in
       And I follow "Employees"
       And I follow "Scrooge"
-      And I press "Edit"
+      And I press success button with text "Edit"
     When I fill in "Notes" with "Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua."
       And I press "Save"
     Then I should see a pre tag
diff --git a/features/step_definitions/web_steps.rb b/features/step_definitions/web_steps.rb
index fbcc797f..f794300f 100644
--- a/features/step_definitions/web_steps.rb
+++ b/features/step_definitions/web_steps.rb
@@ -16,6 +16,10 @@
   click_on button_text
 end
 
+When(/^I press success button with text "(.*?)"$/) do |button_text|
+  find(:css, "p > a.btn-success", text: button_text).click
+end
+
 Then(/^I see "(.*?)"$/) do |text|
   expect(page.body).to have_text(text)
 end

From beea651b19303cb1c2156ed2655ab596ab4b3b51 Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger.roelofs@gmail.com>
Date: Fri, 26 Mar 2021 18:22:36 -0400
Subject: [PATCH 12/14] #206 commit last change I missed

---
 features/step_definitions/web_steps.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/features/step_definitions/web_steps.rb b/features/step_definitions/web_steps.rb
index f794300f..b87bc0a2 100644
--- a/features/step_definitions/web_steps.rb
+++ b/features/step_definitions/web_steps.rb
@@ -17,7 +17,7 @@
 end
 
 When(/^I press success button with text "(.*?)"$/) do |button_text|
-  find(:css, "p > a.btn-success", text: button_text).click
+  find(:css, "a.btn-success", text: button_text).click
 end
 
 Then(/^I see "(.*?)"$/) do |text|

From 6e0ec6cb654f68d500cd9fcea0eb875fe0db8d8f Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger.roelofs@gmail.com>
Date: Sat, 27 Mar 2021 10:47:16 -0400
Subject: [PATCH 13/14] #206 rails_best_practice updates

---
 app/controllers/salaries_controller.rb | 12 ++++++++----
 app/models/salary.rb                   |  1 +
 app/views/salaries/_form.html.haml     | 10 +++++-----
 3 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/app/controllers/salaries_controller.rb b/app/controllers/salaries_controller.rb
index 5652b371..c13b8633 100644
--- a/app/controllers/salaries_controller.rb
+++ b/app/controllers/salaries_controller.rb
@@ -12,8 +12,7 @@ def new
   end
 
   def create
-    update_params = salary_params
-    update_params["start_date"] = @employee.tenures.last.start_date unless update_params.dig(:start_date)
+    update_params = fill_start_date(@employee.tenures.last.start_date)
     @salary = @employee.tenures.last.salaries.new(update_params)
     if @salary.save
       redirect_to employee_path(@employee), notice: 'Successfully recorded raise'
@@ -25,8 +24,7 @@ def create
   def edit; end
 
   def update
-    update_params = salary_params
-    update_params["start_date"] = @salary.tenure.start_date unless update_params.dig(:start_date)
+    update_params = fill_start_date(@salary.tenure_start_date)
     if @salary.update(update_params)
       redirect_to employee_path(@salary.employee), notice: 'Successfully upddated salary'
     else
@@ -53,6 +51,12 @@ def last_tenure
     @employee.tenures.last
   end
 
+  def fill_start_date(date)
+    update_params = salary_params
+    update_params[:start_date] = date unless update_params.dig(:start_date)
+    update_params
+  end
+
   def salary_params
     params.require(:salary).permit(:start_date, :annual_amount)
   end
diff --git a/app/models/salary.rb b/app/models/salary.rb
index 7c310ac9..85edd220 100644
--- a/app/models/salary.rb
+++ b/app/models/salary.rb
@@ -9,6 +9,7 @@ class Salary < ActiveRecord::Base
   validate :no_salaries_outside_tenure_dates, if: :tenure
 
   delegate :first_name, :last_name, to: :employee, prefix: true
+  delegate :start_date, to: :tenure, prefix: true
 
   before_validation :ensure_start_date
 
diff --git a/app/views/salaries/_form.html.haml b/app/views/salaries/_form.html.haml
index 07f0315b..e41d1bae 100644
--- a/app/views/salaries/_form.html.haml
+++ b/app/views/salaries/_form.html.haml
@@ -1,15 +1,15 @@
-- if @salary.errors.any?
+- if salary.errors.any?
   .alert.alert-danger
-    %p #{pluralize(@salary.errors.count, "error")} prohibited this raise from being saved:
+    %p #{pluralize(salary.errors.count, "error")} prohibited this raise from being saved:
     %ul
-      - @salary.errors.full_messages.each do |msg|
+      - salary.errors.full_messages.each do |msg|
         %li= msg
 .col-lg-9
   .well.bs-component
     %fieldset
       .form-group.col-lg-12
         = f.label :start_date, class: 'control-label'
-        = f.date_field :start_date, class: 'form-control', required: true, autofocus: @salary.start_date != @salary.tenure.start_date, disabled: (@salary.start_date == @salary.tenure.start_date)
+        = f.date_field :start_date, class: 'form-control', required: true, autofocus: salary.start_date != salary.tenure_start_date, disabled: (salary.start_date == salary.tenure_start_date)
       .form-group.col-lg-12
         = f.label :annual_amount, class: 'control-label'
         .input-group
@@ -17,4 +17,4 @@
           = f.number_field :annual_amount, class: 'form-control', required: true
       .form-group.col-lg-12
         = f.submit 'Save', class: 'btn btn-primary'
-        = link_to 'Cancel', employee_path(@salary.employee), class: 'btn btn-default'
+        = link_to 'Cancel', employee_path(salary.employee), class: 'btn btn-default'

From f5994ef681ac30dd2c9c07ffcc39cb0e0c59dc24 Mon Sep 17 00:00:00 2001
From: Roger Roelofs <roger.roelofs@gmail.com>
Date: Wed, 31 Mar 2021 14:27:40 -0400
Subject: [PATCH 14/14] #206 simplify haml interpolation

---
 app/views/salaries/edit.html.haml | 2 +-
 app/views/salaries/new.html.haml  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/views/salaries/edit.html.haml b/app/views/salaries/edit.html.haml
index 7be0265d..7222e1c8 100644
--- a/app/views/salaries/edit.html.haml
+++ b/app/views/salaries/edit.html.haml
@@ -1,3 +1,3 @@
-%h2= "Edit Salary for #{@salary.employee_first_name} #{@salary.employee_last_name}"
+%h2 Edit Salary for #{@salary.employee_first_name} #{@salary.employee_last_name}
 = form_for(@salary, url: employee_salary_path(@salary), html: { class: 'form-horizontal' }) do |f|
     = render 'form', salary: @salary, f: f
\ No newline at end of file
diff --git a/app/views/salaries/new.html.haml b/app/views/salaries/new.html.haml
index 5cce402d..70c7bc61 100644
--- a/app/views/salaries/new.html.haml
+++ b/app/views/salaries/new.html.haml
@@ -1,6 +1,6 @@
 - if @salary.tenure.salaries.count >= 1
-  %h2= "Record a Raise for #{@salary.employee_first_name} #{@salary.employee_last_name}"
+  %h2 Record a Raise for #{@salary.employee_first_name} #{@salary.employee_last_name}
 - else
-  %h2= "Record Starting Salary for #{@salary.employee_first_name} #{@salary.employee_last_name}"
+  %h2 Record Starting Salary for #{@salary.employee_first_name} #{@salary.employee_last_name}
 = form_for(@salary, url: employee_salaries_path(@salary.employee), html: { class: 'form-horizontal' }) do |f|
   = render 'form', salary: @salary, f: f
\ No newline at end of file