Feature/859 team skill tracking over time (#890)

* Generate SkillSnapshot model

* Make skills discardable

* Rename SkillSnapshot to DepartmentSkillSnapshot, serialize skills as json and implement first logic that creates snapshots

* Add explanatory comment to the department skill snapshot builder

* Install and configure delayed_job_activerecord

* Install and configure delayed cron job

* Add delayed cron job that is executed once a month and reates department skills snapshots

* Run rubocop fix

* Rename field skills on department skill snapshot to department_skill_levels

* Start writing domain specs for departments

* Write test that tests creation of snapshots

* Add delayed_job model to be able to read djs from console, make snapshot logic ignore people without a department and revert unwanted changes to old migration

* Delete old migration test that is not needed anymore

Author: RandomTannenbaum

Gemfile

@@ -17,7 +17,10 @@ gem 'countries'
 gem 'cssbundling-rails'
 gem 'csv'
 gem 'database_cleaner'
+gem 'delayed_cron_job'
+gem 'delayed_job_active_record'
 gem 'devise'
+gem 'discard', '~> 1.4'
 gem 'drb'
 gem 'faker'
 gem 'haml-rails'

Gemfile.lock

@@ -141,18 +141,29 @@ GEM
     database_cleaner-core (2.0.1)
     date (3.4.1)
     deep_merge (1.2.2)
+    delayed_cron_job (0.9.0)
+      fugit (>= 1.5)
+    delayed_job (4.1.13)
+      activesupport (>= 3.0, < 9.0)
+    delayed_job_active_record (4.1.11)
+      activerecord (>= 3.0, < 9.0)
+      delayed_job (>= 3.0, < 5)
     devise (4.9.4)
       bcrypt (~> 3.0)
       orm_adapter (~> 0.1)
       railties (>= 4.1.0)
       responders
       warden (~> 1.2.3)
     diff-lcs (1.6.0)
+    discard (1.4.0)
+      activerecord (>= 4.2, < 9.0)
     docile (1.4.1)
     domain_name (0.6.20240107)
     dotenv (3.1.7)
     drb (2.2.1)
     erubi (1.13.1)
+    et-orbi (1.2.11)
+      tzinfo
     faker (3.5.1)
       i18n (>= 1.8.11, < 2)
     faraday (2.12.2)
@@ -164,6 +175,9 @@ GEM
     faraday-net_http (3.4.0)
       net-http (>= 0.5.0)
     ffi (1.17.1-x86_64-linux-gnu)
+    fugit (1.11.1)
+      et-orbi (~> 1, >= 1.2.11)
+      raabro (~> 1.4)
     globalid (1.2.1)
       activesupport (>= 6.1)
     haml (6.3.0)
@@ -318,6 +332,7 @@ GEM
     public_suffix (6.0.1)
     puma (6.6.0)
       nio4r (~> 2.0)
+    raabro (1.4.0)
     racc (1.8.1)
     rack (3.1.12)
     rack-protection (4.1.1)
@@ -534,7 +549,10 @@ DEPENDENCIES
   cssbundling-rails
   csv
   database_cleaner
+  delayed_cron_job
+  delayed_job_active_record
   devise
+  discard (~> 1.4)
   dotenv
   drb
   faker

app/controllers/admin/unified_skills_controller.rb

@@ -71,8 +71,8 @@ def merge_skills(old_skill1, old_skill2)
     UnifiedSkill.create!(skill1_attrs: old_skill1.attributes, skill2_attrs: old_skill2.attributes,
                          unified_skill_attrs: new_skill.attributes)
 
-    old_skill1.delete
-    old_skill2.delete
+    old_skill1.discard!
+    old_skill2.discard!
 
     new_skill
   end

app/domain/department_skills_snapshot_builder.rb

@@ -0,0 +1,19 @@
+class DepartmentSkillsSnapshotBuilder
+  # This will create snapshots of all departments with their respective skills and levels.
+  # Each snapshot is an instance of DepartmentSkillSnapshot which takes a hash of skills.
+  # This hash has the format:
+  # { <skill_id1> => [skill_level1, skill_level2], <skill_id2> => [skill_level1, skill_level2] }
+  # The count of skill_levels per skill_id is equal to the count of people that have rated that
+  # skill in a given department.
+
+  def snapshot_all_departments
+    Person.where.not(department_id: nil).group_by(&:department_id).each do |department_id, people|
+      department_skill_levels = PeopleSkill
+                                .where(person_id: people)
+                                .group_by(&:skill_id)
+                                .transform_values { |value| value.pluck(:level) }
+
+      DepartmentSkillSnapshot.create!(department_id:, department_skill_levels:)
+    end
+  end
+end

app/jobs/cron_job.rb

@@ -0,0 +1,23 @@
+class CronJob < ApplicationJob
+  class_attribute :cron_expression
+
+  class << self
+    def schedule
+      set(cron: cron_expression).perform_later unless scheduled?
+    end
+
+    def remove
+      delayed_job.destroy if scheduled?
+    end
+
+    def scheduled?
+      delayed_job.present?
+    end
+
+    def delayed_job
+      Delayed::Job
+        .where('handler LIKE ?', "%job_class: #{name}%")
+        .first
+    end
+  end
+end

app/jobs/monthly_department_skills_snapshot_job.rb

@@ -0,0 +1,8 @@
+class MonthlyDepartmentSkillsSnapshotJob < CronJob
+  # Perform job on first day of month at 3am
+  self.cron_expression = '0 3 1 * *'
+
+  def perform
+    DepartmentSkillsSnapshotBuilder.new.snapshot_all_departments
+  end
+end

app/models/delayed_job.rb

@@ -0,0 +1,2 @@
+class DelayedJob < ApplicationRecord
+end

app/models/department_skill_snapshot.rb

@@ -0,0 +1,5 @@
+class DepartmentSkillSnapshot < ApplicationRecord
+  belongs_to :department
+
+  serialize :department_skill_levels, type: Hash, coder: JSON
+end

app/models/skill.rb

@@ -15,6 +15,8 @@
 #
 
 class Skill < ApplicationRecord
+  include Discard::Model
+
   has_many :people_skills, dependent: :destroy
   has_many :people, through: :people_skills
   belongs_to :category
@@ -25,6 +27,8 @@ class Skill < ApplicationRecord
 
   validates :title, presence: true, length: { maximum: 100 }, uniqueness: { scope: :category_id }
 
+  default_scope { kept }
+
   scope :list, -> { order(:title) }
 
   scope :default_set, -> { where(default_set: true) }

bin/delayed_job

@@ -0,0 +1,5 @@
+#!/usr/bin/env ruby
+
+require File.expand_path(File.join(File.dirname(__FILE__), '..', 'config', 'environment'))
+require 'delayed/command'
+Delayed::Command.new(ARGV).daemonize

config/application.rb

@@ -22,6 +22,8 @@ class Application < Rails::Application
 
     config.active_record.verify_foreign_keys_for_fixtures = false
 
+    config.active_job.queue_adapter = :delayed_job
+
     # Bullet tries to add finish_at to insert statement, which does not exist anymore
     config.active_record.partial_inserts = true
 

db/migrate/20250509105746_create_department_skill_snapshots.rb

@@ -0,0 +1,10 @@
+class CreateDepartmentSkillSnapshots < ActiveRecord::Migration[8.0]
+  def change
+    create_table :department_skill_snapshots do |t|
+      t.references :department, null: false, foreign_key: true
+      t.text :department_skill_levels
+
+      t.timestamps
+    end
+  end
+end

db/migrate/20250509113422_add_discarded_at_to_skills.rb

@@ -0,0 +1,6 @@
+class AddDiscardedAtToSkills < ActiveRecord::Migration[8.0]
+  def change
+    add_column :skills, :discarded_at, :datetime
+    add_index :skills, :discarded_at
+  end
+end

db/migrate/20250512073835_create_delayed_jobs.rb

@@ -0,0 +1,22 @@
+class CreateDelayedJobs < ActiveRecord::Migration[8.0]
+  def self.up
+    create_table :delayed_jobs do |table|
+      table.integer :priority, default: 0, null: false # Allows some jobs to jump to the front of the queue
+      table.integer :attempts, default: 0, null: false # Provides for retries, but still fail eventually.
+      table.text :handler,                 null: false # YAML-encoded string of the object that will do work
+      table.text :last_error                           # reason for last failure (See Note below)
+      table.datetime :run_at                           # When to run. Could be Time.zone.now for immediately, or sometime in the future.
+      table.datetime :locked_at                        # Set when a client is working on this object
+      table.datetime :failed_at                        # Set when all retries have failed (actually, by default, the record is deleted instead)
+      table.string :locked_by                          # Who is working on this object (if locked)
+      table.string :queue                              # The name of the queue this job is in
+      table.timestamps null: true
+    end
+
+    add_index :delayed_jobs, [:priority, :run_at], name: "delayed_jobs_priority"
+  end
+
+  def self.down
+    drop_table :delayed_jobs
+  end
+end

db/migrate/20250512074440_add_cron_to_delayed_jobs.rb

@@ -0,0 +1,9 @@
+class AddCronToDelayedJobs < ActiveRecord::Migration[8.0]
+  def self.up
+    add_column :delayed_jobs, :cron, :string
+  end
+
+  def self.down
+    remove_column :delayed_jobs, :cron
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.0].define(version: 2025_03_25_145055) do
+ActiveRecord::Schema[8.0].define(version: 2025_05_12_074440) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_catalog.plpgsql"
 
@@ -93,6 +93,30 @@
     t.datetime "updated_at", precision: nil, null: false
   end
 
+  create_table "delayed_jobs", force: :cascade do |t|
+    t.integer "priority", default: 0, null: false
+    t.integer "attempts", default: 0, null: false
+    t.text "handler", null: false
+    t.text "last_error"
+    t.datetime "run_at"
+    t.datetime "locked_at"
+    t.datetime "failed_at"
+    t.string "locked_by"
+    t.string "queue"
+    t.datetime "created_at"
+    t.datetime "updated_at"
+    t.string "cron"
+    t.index ["priority", "run_at"], name: "delayed_jobs_priority"
+  end
+
+  create_table "department_skill_snapshots", force: :cascade do |t|
+    t.bigint "department_id", null: false
+    t.text "department_skill_levels"
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["department_id"], name: "index_department_skill_snapshots_on_department_id"
+  end
+
   create_table "departments", force: :cascade do |t|
     t.string "name", null: false
     t.datetime "created_at", null: false
@@ -235,7 +259,9 @@
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false
     t.bigint "category_id"
+    t.datetime "discarded_at"
     t.index ["category_id"], name: "index_skills_on_category_id"
+    t.index ["discarded_at"], name: "index_skills_on_discarded_at"
   end
 
   create_table "unified_skills", force: :cascade do |t|
@@ -247,6 +273,7 @@
   end
 
   add_foreign_key "categories", "categories", column: "parent_id"
+  add_foreign_key "department_skill_snapshots", "departments"
   add_foreign_key "language_skills", "people"
   add_foreign_key "people", "companies"
   add_foreign_key "project_technologies", "projects"

lib/tasks/jobs.rake

@@ -0,0 +1,16 @@
+namespace :db do
+  desc 'Schedule all cron jobs'
+  task :schedule_jobs => :environment do
+    # Need to load all jobs definitions in order to find subclasses
+    glob = Rails.root.join('app', 'jobs', '**', '*_job.rb')
+    Dir.glob(glob).each { |file| require file }
+    CronJob.subclasses.each(&:schedule)
+  end
+end
+
+# invoke schedule_jobs automatically after every migration and schema load.
+%w(db:migrate db:schema:load).each do |task|
+  Rake::Task[task].enhance do
+    Rake::Task['db:schedule_jobs'].invoke
+  end
+end

spec/controllers/unified_skills_controller_spec.rb

@@ -23,7 +23,7 @@ def check_merged_people_skill_values(merged_people_skill, original_people_skill)
     post :create, params: { unified_skill_form: { old_skill_id1: skill1.id, old_skill_id2: skill2.id, checked_conflicts: true, new_skill: new_skill } }
 
     unified_skill = UnifiedSkill.find_by(skill1_attrs: skill1.attributes, skill2_attrs: skill2.attributes)
-    expect(unified_skill.unified_skill_attrs.excluding('id', 'updated_at', 'created_at')).to eql(new_skill.stringify_keys)
+    expect(unified_skill.unified_skill_attrs.excluding('id', 'updated_at', 'created_at', 'discarded_at')).to eql(new_skill.stringify_keys)
 
     expect(Skill.find_by(id: skill1.id)).to be_nil
     expect(Skill.find_by(id: skill2.id)).to be_nil

spec/domain/department_skills_snapshot_spec.rb

@@ -0,0 +1,24 @@
+require 'rails_helper'
+
+describe PeopleSearch do
+  it 'should create snapshot of all departments that have members' do
+    DepartmentSkillsSnapshotBuilder.new.snapshot_all_departments
+
+    snapshots = DepartmentSkillSnapshot.all
+
+    expect(snapshots.count).to eql(Person.distinct.pluck(:department_id).count)
+
+    department = departments(:sys)
+    snapshot = snapshots.find_by(department_id: department)
+
+    expect(snapshot).not_to be_nil
+
+    department_skill_levels = snapshot.department_skill_levels
+    people_skills_of_department = PeopleSkill.joins(:person).where(people: {department_id: department.id})
+
+    expect(department_skill_levels.keys).to match_array(people_skills_of_department.distinct.pluck(:skill_id).map(&:to_s))
+    department_skill_levels.each do |k, v|
+      expect(people_skills_of_department.where(skill_id: k.to_i).pluck(:level)).to match_array(v)
+    end
+  end
+end

spec/migrations/create_department_spec.rb

@@ -0,0 +1,9 @@
+require 'rails_helper'
+
+RSpec.describe DepartmentSkillSnapshot, type: :model do
+  it 'should correctly serialize department_skill_levels' do
+    department_skill_snapshot = DepartmentSkillSnapshot.create!(department_id: Department.first.id, department_skill_levels: {"1" => [2, 2, 3], "2" => [3, 3, 1]})
+
+    expect(department_skill_snapshot.department_skill_levels.is_a? Hash).to eql(true)
+  end
+end