Leave types validations (#1702)

* validation for frquency and allocations

* rubocop fix

* spec fix and review comment

* validation for carry forward days

* fix failing specs

* rubocop fixes

* fixed allocation_value erro

* refactored to use concerns

---------

Co-authored-by: “Apoorv <“tiwari.apoorv1316@gmail.com”>

app/models/concerns/leave_type_validatable.rb

@@ -0,0 +1,84 @@
+# frozen_string_literal: true
+
+module LeaveTypeValidatable
+  extend ActiveSupport::Concern
+
+  included do
+    validate :valid_allocation_combination
+    validate :valid_allocation_value
+    validate :valid_carry_forward
+  end
+
+  private
+
+    def valid_allocation_combination
+      if allocation_period == "weeks" && allocation_frequency == "per_week"
+        errors.add(:base, "Invalid combination: Allocation period in weeks cannot have frequency per week")
+      end
+
+      if allocation_period == "months" && !(allocation_frequency == "per_quarter" || allocation_frequency == "per_year")
+        errors.add(
+          :base,
+          "Invalid combination: Allocation period in months can only have frequency per quarter or per year")
+      end
+    end
+
+    def valid_allocation_value
+      max_values = {
+        ["days", "per_week"] => 7,
+        ["days", "per_month"] => 31,
+        ["days", "per_quarter"] => 92,
+        ["days", "per_year"] => 366,
+        ["weeks", "per_month"] => 5,
+        ["weeks", "per_quarter"] => 13,
+        ["weeks", "per_year"] => 52,
+        ["months", "per_quarter"] => 3,
+        ["months", "per_year"] => 12
+      }
+
+      if allocation_value.present?
+        key = [allocation_period, allocation_frequency]
+        if max_values[key] && allocation_value > max_values[key]
+          errors.add(
+            :allocation_value,
+            "cannot exceed #{max_values[key]} #{allocation_period} for #{allocation_frequency} frequency")
+        end
+      end
+    end
+
+    def valid_carry_forward
+      total_days = convert_allocation_to_days
+
+      if carry_forward_days.present? && total_days.present? && carry_forward_days > total_days
+        errors.add(:carry_forward_days, "cannot exceed the total allocated days")
+      end
+    end
+
+    def convert_allocation_to_days
+      return nil unless allocation_value
+
+      base_days = case allocation_period
+                  when "days"
+                    allocation_value
+                  when "weeks"
+                    allocation_value * 7
+                  when "months"
+                    allocation_value * 31
+                  else
+                    return nil
+      end
+
+      case allocation_frequency
+      when "per_week"
+        base_days * 52
+      when "per_month"
+        base_days * 12
+      when "per_quarter"
+        base_days * 4
+      when "per_year"
+        base_days
+      else
+        nil
+      end
+    end
+end

app/models/leave_type.rb

@@ -30,6 +30,7 @@
 #
 class LeaveType < ApplicationRecord
   include Discard::Model
+  include LeaveTypeValidatable
 
   enum :color, {
     chart_blue: 0,

spec/models/concerns/leave_type_validatable_spec.rb

@@ -0,0 +1,98 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe LeaveTypeValidatable, type: :model do
+  let(:leave) { create(:leave) }
+
+  describe "Custom Validations" do
+    context "when validating allocation combinations" do
+      it "is not valid with weekly allocation period and weekly frequency" do
+        leave_type = build(:leave_type, allocation_period: "weeks", allocation_frequency: "per_week", leave:)
+        expect(leave_type.valid?).to be false
+        expect(leave_type.errors[:base]).to include(
+          "Invalid combination: Allocation period in weeks cannot have frequency per week")
+      end
+
+      it "is not valid with monthly allocation period and weekly or monthly frequencies" do
+        ["per_week", "per_month"].each do |freq|
+          leave_type = build(:leave_type, allocation_period: "months", allocation_frequency: freq, leave:)
+          expect(leave_type.valid?).to be false
+          expect(leave_type.errors[:base]).to include(
+            "Invalid combination: Allocation period in months can only have frequency per quarter or per year")
+        end
+      end
+
+      it "is valid with monthly allocation period and quarterly or yearly frequencies" do
+        ["per_quarter", "per_year"].each do |freq|
+          leave_type = build(
+            :leave_type, allocation_period: "months", allocation_frequency: freq, allocation_value: 2,
+            leave:)
+          expect(leave_type.valid?).to be true
+        end
+      end
+    end
+
+    context "when validating allocation values" do
+      it "is not valid with an allocation value exceeding the limit for the period and frequency" do
+        combinations = {
+          ["days", "per_week"] => 8,
+          ["weeks", "per_month"] => 6
+        }
+        combinations.each do |(period, freq), value|
+          leave_type = build(
+            :leave_type, allocation_period: period, allocation_frequency: freq,
+            allocation_value: value, leave:)
+          expect(leave_type).not_to be_valid
+          expect(leave_type.errors[:allocation_value]).to include(
+            "cannot exceed #{value - 1} #{period} for #{freq} frequency")
+        end
+      end
+
+      it "is valid with an allocation value within the limit for the period and frequency" do
+        combinations = {
+          ["days", "per_week"] => 7,
+          ["weeks", "per_month"] => 5
+        }
+        combinations.each do |(period, freq), value|
+          leave_type = build(
+            :leave_type, allocation_period: period, allocation_frequency: freq,
+            allocation_value: value, leave:)
+          expect(leave_type).to be_valid
+        end
+      end
+    end
+
+    context "when validating carry forward limits with frequency considerations" do
+      it "is valid when carry_forward is less than total days in a year for days per week" do
+        leave_type = build(
+          :leave_type, allocation_period: "days", allocation_frequency: "per_week",
+          allocation_value: 4, carry_forward_days: 5, leave:)
+        expect(leave_type).to be_valid
+      end
+
+      it "is not valid when carry_forward exceeds total days for weeks per year" do
+        leave_type = build(
+          :leave_type, allocation_period: "weeks", allocation_frequency: "per_year",
+          allocation_value: 2, carry_forward_days: 15, leave:)
+        expect(leave_type).not_to be_valid
+        expect(leave_type.errors[:carry_forward_days]).to include("cannot exceed the total allocated days")
+      end
+
+      it "is valid when carry_forward does not exceed total days for months per quarter" do
+        leave_type = build(
+          :leave_type, allocation_period: "months", allocation_frequency: "per_quarter",
+          allocation_value: 1, carry_forward_days: 30, leave:)
+        expect(leave_type).to be_valid
+      end
+
+      it "is not valid when carry_forward exceeds total days for months per year" do
+        leave_type = build(
+          :leave_type, allocation_period: "months", allocation_frequency: "per_year",
+          allocation_value: 2, carry_forward_days: 63, leave:)
+        expect(leave_type).not_to be_valid
+        expect(leave_type.errors[:carry_forward_days]).to include("cannot exceed the total allocated days")
+      end
+    end
+  end
+end

spec/models/leave_type_spec.rb

@@ -6,53 +6,73 @@
   let(:leave) { create(:leave) }
 
   describe "validations" do
-    it "is valid with valid attributes" do
-      leave_type = build(:leave_type, leave:)
-      expect(leave_type).to be_valid
-    end
-
-    it "is not valid without a name" do
-      leave_type = build(:leave_type, name: nil, leave:)
-      expect(leave_type).not_to be_valid
-      expect(leave_type.errors[:name]).to include("can't be blank")
-    end
-
-    it "is not valid with a duplicate color within the same leave" do
-      existing_leave_type = create(:leave_type, leave:)
-      leave_type = build(:leave_type, color: existing_leave_type.color, leave:)
-      expect(leave_type).not_to be_valid
-      expect(leave_type.errors[:color]).to include("has already been taken for this leave")
-    end
-
-    it "is not valid with a duplicate icon within the same leave" do
-      existing_leave_type = create(:leave_type, leave:)
-      leave_type = build(:leave_type, icon: existing_leave_type.icon, leave:)
-      expect(leave_type).not_to be_valid
-      expect(leave_type.errors[:icon]).to include("has already been taken for this leave")
-    end
-
-    it "is not valid without a allocation value" do
-      leave_type = build(:leave_type, allocation_value: nil, leave:)
-      expect(leave_type).not_to be_valid
-      expect(leave_type.errors[:allocation_value]).to include("can't be blank")
-    end
-
-    it "is not valid if allocation value is less than 1" do
-      leave_type = build(:leave_type, allocation_value: 0, leave:)
-      expect(leave_type).not_to be_valid
-      expect(leave_type.errors[:allocation_value]).to include("must be greater than or equal to 1")
-    end
-
-    it "is not valid without a allocation frequency" do
-      leave_type = build(:leave_type, allocation_frequency: nil, leave:)
-      expect(leave_type).not_to be_valid
-      expect(leave_type.errors[:allocation_frequency]).to include("can't be blank")
-    end
-
-    it "is not valid without a carry forward days" do
-      leave_type = build(:leave_type, carry_forward_days: nil, leave:)
-      expect(leave_type).not_to be_valid
-      expect(leave_type.errors[:carry_forward_days]).to include("can't be blank")
-    end
+  it "is valid with valid attributes" do
+    leave_type = build(
+      :leave_type, leave:, allocation_period: "weeks", allocation_frequency: "per_year",
+      allocation_value: 2)
+    expect(leave_type).to be_valid
   end
+
+  it "is not valid without a name" do
+    leave_type = build(
+      :leave_type, name: nil, leave:, allocation_period: "days", allocation_frequency: "per_week",
+      allocation_value: 5)
+    expect(leave_type).not_to be_valid
+    expect(leave_type.errors[:name]).to include("can't be blank")
+  end
+
+  it "is not valid with a duplicate color within the same leave" do
+    existing_leave_type = create(
+      :leave_type, leave:, allocation_period: "months",
+      allocation_frequency: "per_quarter", allocation_value: 1)
+    leave_type = build(
+      :leave_type, color: existing_leave_type.color, leave:, allocation_period: "months",
+      allocation_frequency: "per_quarter", allocation_value: 1)
+    expect(leave_type).not_to be_valid
+    expect(leave_type.errors[:color]).to include("has already been taken for this leave")
+  end
+
+  it "is not valid with a duplicate icon within the same leave" do
+    existing_leave_type = create(
+      :leave_type, leave:, allocation_period: "weeks", allocation_frequency: "per_month",
+      allocation_value: 2)
+    leave_type = build(
+      :leave_type, icon: existing_leave_type.icon, leave:, allocation_period: "weeks",
+      allocation_frequency: "per_month", allocation_value: 2)
+    expect(leave_type).not_to be_valid
+    expect(leave_type.errors[:icon]).to include("has already been taken for this leave")
+  end
+
+  it "is not valid without an allocation value" do
+    leave_type = build(
+      :leave_type, allocation_value: nil, leave:, allocation_period: "days",
+      allocation_frequency: "per_week")
+    expect(leave_type).not_to be_valid
+    expect(leave_type.errors[:allocation_value]).to include("can't be blank")
+  end
+
+  it "is not valid if allocation value is less than 1" do
+    leave_type = build(
+      :leave_type, allocation_value: 0, leave:, allocation_period: "days",
+      allocation_frequency: "per_month")
+    expect(leave_type).not_to be_valid
+    expect(leave_type.errors[:allocation_value]).to include("must be greater than or equal to 1")
+  end
+
+  it "is not valid without an allocation frequency" do
+    leave_type = build(
+      :leave_type, allocation_frequency: nil, leave:, allocation_period: "weeks",
+      allocation_value: 3)
+    expect(leave_type).not_to be_valid
+    expect(leave_type.errors[:allocation_frequency]).to include("can't be blank")
+  end
+
+  it "is not valid without carry forward days" do
+    leave_type = build(
+      :leave_type, carry_forward_days: nil, leave:, allocation_period: "months",
+      allocation_frequency: "per_year", allocation_value: 2)
+    expect(leave_type).not_to be_valid
+    expect(leave_type.errors[:carry_forward_days]).to include("can't be blank")
+  end
+end
 end

spec/requests/internal_api/v1/timeoff_entries/create_spec.rb

@@ -6,7 +6,10 @@
   let(:company) { create(:company) }
   let(:user) { create(:user, current_workspace_id: company.id) }
   let(:leave) { create(:leave, company:) }
-  let!(:leave_type) { create(:leave_type, name: "Annual", leave:) }
+  let!(:leave_type) { create(
+    :leave_type, name: "Annual", leave:, allocation_period: "months", allocation_frequency: "per_quarter",
+    allocation_value: 1, carry_forward_days: 30,)
+}
 
   context "when user is an admin" do
     before do

spec/requests/internal_api/v1/timeoff_entries/destroy_spec.rb

@@ -6,7 +6,10 @@
   let(:company) { create(:company) }
   let(:user) { create(:user, current_workspace_id: company.id) }
   let(:leave) { create(:leave, company:) }
-  let!(:leave_type) { create(:leave_type, leave:) }
+  let!(:leave_type) { create(
+    :leave_type, leave:, allocation_period: "months", allocation_frequency: "per_quarter",
+    allocation_value: 1, carry_forward_days: 30,)
+}
   let!(:timeoff_entry) { create(:timeoff_entry, user:, leave_type:) }
 
   context "when user is an admin" do

spec/requests/internal_api/v1/timeoff_entries/update_spec.rb

@@ -6,7 +6,10 @@
   let(:company) { create(:company) }
   let(:user) { create(:user, current_workspace_id: company.id) }
   let(:leave) { create(:leave, company:) }
-  let!(:leave_type) { create(:leave_type, name: "Annaul", leave:) }
+  let!(:leave_type) { create(
+    :leave_type, name: "Annaul", leave:, allocation_period: "months", allocation_frequency: "per_quarter",
+    allocation_value: 1, carry_forward_days: 30,)
+}
   let!(:timeoff_entry) { create(:timeoff_entry, user:, leave_type:) }
 
   context "when user is an admin" do