chore(validation): Refactor Expiration Date Validation to Use Centralized Validator (#3312)

Author: brunomiguelpinto

app/services/coupons/validate_service.rb

@@ -16,18 +16,9 @@ def valid?
     private
 
     def valid_expiration_at?
-      return true if args[:expiration_at].blank?
-
-      expiration_at = if args[:expiration_at].is_a?(Time)
-        args[:expiration_at]
-      elsif args[:expiration_at].is_a?(String) && Date._strptime(args[:expiration_at]).present?
-        Date.strptime(args[:expiration_at])
-      end
-
-      return true if expiration_at && expiration_at.to_date >= Time.current.to_date
+      return true if Validators::ExpirationDateValidator.valid?(args[:expiration_at])
 
       add_error(field: :expiration_at, error_code: "invalid_date")
-
       false
     end
   end

app/services/utils/datetime.rb

@@ -8,6 +8,14 @@ def self.valid_format?(datetime)
       false
     end
 
+    def self.future_date?(datetime)
+      return true if datetime.is_a?(ActiveSupport::TimeWithZone) && datetime.future?
+      return false unless valid_format?(datetime)
+
+      parsed_date = Time.zone.parse(datetime.to_s)
+      parsed_date&.future? || false
+    end
+
     def self.date_diff_with_timezone(from_datetime, to_datetime, timezone)
       from = from_datetime
       from = Time.zone.parse(from.to_s) unless from.is_a?(ActiveSupport::TimeWithZone)

app/services/validators/expiration_date_validator.rb

@@ -0,0 +1,12 @@
+# frozen_string_literal: true
+
+module Validators
+  module ExpirationDateValidator
+    def self.valid?(expiration_at)
+      return true if expiration_at.blank?
+
+      Utils::Datetime.valid_format?(expiration_at) &&
+        Utils::Datetime.future_date?(expiration_at)
+    end
+  end
+end

app/services/wallets/update_service.rb

@@ -46,20 +46,9 @@ def valid_recurring_transaction_rules?
     end
 
     def valid_expiration_at?(expiration_at:)
-      return true if expiration_at.blank?
-
-      if Utils::Datetime.valid_format?(expiration_at)
-        parsed_expiration_at = if expiration_at.is_a?(String)
-          DateTime.strptime(expiration_at)
-        else
-          expiration_at
-        end
-
-        return true if parsed_expiration_at.to_date > Time.current.to_date
-      end
+      return true if Validators::ExpirationDateValidator.valid?(expiration_at)
 
       result.single_validation_failure!(field: :expiration_at, error_code: "invalid_date")
-
       false
     end
   end

app/services/wallets/validate_service.rb

@@ -48,24 +48,12 @@ def valid_granted_credits_amount?
     end
 
     def valid_expiration_at?
-      return true if args[:expiration_at].blank?
-
-      future = Utils::Datetime.valid_format?(args[:expiration_at]) && expiration_at.to_date > Time.current.to_date
-      return true if future
+      return true if Validators::ExpirationDateValidator.valid?(args[:expiration_at])
 
       add_error(field: :expiration_at, error_code: "invalid_date")
-
       false
     end
 
-    def expiration_at
-      @expiration_at ||= if args[:expiration_at].is_a?(String)
-        DateTime.strptime(args[:expiration_at])
-      else
-        args[:expiration_at]
-      end
-    end
-
     def valid_recurring_transaction_rules?
       if args[:recurring_transaction_rules].count > 1
         return add_error(field: :recurring_transaction_rules, error_code: "invalid_number_of_recurring_rules")

spec/services/validators/expiration_date_validator_spec.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require "rails_helper"
+
+RSpec.describe Validators::ExpirationDateValidator do
+  describe ".valid?" do
+    subject { described_class.valid?(expiration_at) }
+
+    context "when expiration_at is blank" do
+      let(:expiration_at) { nil }
+
+      it { is_expected.to be true }
+    end
+
+    context "when expiration_at is an empty string" do
+      let(:expiration_at) { "" }
+
+      it { is_expected.to be true }
+    end
+
+    context "when expiration_at is an invalid format" do
+      let(:expiration_at) { "invalid-date" }
+
+      it { is_expected.to be false }
+    end
+
+    context "when expiration_at is an integer" do
+      let(:expiration_at) { 123 }
+
+      it { is_expected.to be false }
+    end
+
+    context "when expiration_at is a past date" do
+      let(:expiration_at) { (Time.current - 1.day).iso8601 }
+
+      it { is_expected.to be false }
+    end
+
+    context "when expiration_at is a past datetime" do
+      let(:expiration_at) { (Time.current - 1.hour).iso8601 }
+
+      it { is_expected.to be false }
+    end
+
+    context "when expiration_at is today but not in the future" do
+      let(:expiration_at) { Time.current.beginning_of_day.iso8601 }
+
+      it { is_expected.to be false }
+    end
+
+    context "when expiration_at is a valid future date" do
+      let(:expiration_at) { (Time.current + 1.day).iso8601 }
+
+      it { is_expected.to be true }
+    end
+
+    context "when expiration_at is a valid future datetime" do
+      let(:expiration_at) { (Time.current + 1.hour).iso8601 }
+
+      it { is_expected.to be true }
+    end
+
+    context "when expiration_at is an ActiveSupport::TimeWithZone object in the future" do
+      let(:expiration_at) { Time.zone.now + 1.day }
+
+      it { is_expected.to be true }
+    end
+
+    context "when expiration_at is an ActiveSupport::TimeWithZone object in the past" do
+      let(:expiration_at) { Time.zone.now - 1.day }
+
+      it { is_expected.to be false }
+    end
+  end
+end

spec/services/wallets/validate_service_spec.rb

@@ -84,7 +84,7 @@
         end
       end
 
-      context "when expiration_at is integer" do
+      context "when expiration_at is an integer" do
         let(:expiration_at) { 123 }
 
         it "returns false and result has errors" do
@@ -93,34 +93,42 @@
         end
       end
 
-      context "when expiration_at is less than current time" do
-        let(:expiration_at) { (Time.current - 1.year).iso8601 }
+      context "when expiration_at is in the past" do
+        let(:expiration_at) { (Time.current - 1.hour).iso8601 }
 
         it "returns false and result has errors" do
           expect(validate_service).not_to be_valid
           expect(result.error.messages[:expiration_at]).to eq(["invalid_date"])
         end
       end
 
-      context "with invalid transaction metadata" do
-        let(:args) do
-          {
-            customer:,
-            organization_id: organization.id,
-            paid_credits:,
-            granted_credits:,
-            expiration_at:,
-            transaction_metadata: [{key: "valid key", value1: "invalid value"}]
-          }
-        end
+      context "when expiration_at is a valid datetime string but in the future" do
+        let(:expiration_at) { (Time.current + 1.hour).iso8601 }
 
-        it "returns false and result has errors" do
-          expect(validate_service).not_to be_valid
-          expect(result.error.messages[:metadata]).to eq(["invalid_key_value_pair"])
+        it "returns true and has no errors" do
+          expect(validate_service).to be_valid
         end
       end
     end
 
+    context "with invalid transaction metadata" do
+      let(:args) do
+        {
+          customer:,
+          organization_id: organization.id,
+          paid_credits:,
+          granted_credits:,
+          expiration_at:,
+          transaction_metadata: [{key: "valid key", value1: "invalid value"}]
+        }
+      end
+
+      it "returns false and result has errors" do
+        expect(validate_service).not_to be_valid
+        expect(result.error.messages[:metadata]).to eq(["invalid_key_value_pair"])
+      end
+    end
+
     context "with recurring transaction rules" do
       let(:rules) do
         [