Handle additional GraduationHelper edge cases

Analysing production data showed variants for the requested embargo
duration and graduation date that were not covered by the test suite
and had unclear behaviors in the code.

This PR adds tests to cover the newly identified edge cases
and updates the GraduationHelper to match.

app/helpers/graduation_helper.rb

@@ -1,13 +1,23 @@
 module GraduationHelper
   # Given a graduation date and an embargo length, calculate the embargo_release_date.
-  # This assumes embargo_length values like "6 months", "2 months", "6 years"
-  def self.embargo_length_to_embargo_release_date(graduation_date, embargo_length)
-    if embargo_length == InProgressEtd::NO_EMBARGO
+  def self.embargo_length_to_embargo_release_date(graduation_date, requested_embargo)
+    if requested_embargo == InProgressEtd::NO_EMBARGO || requested_embargo.blank?
+      # No post-graduation embargo to apply
+      Rails.logger.warn "Treating empty requested_embargo as 'None'" if requested_embargo.blank?
       graduation_date
     else
-      number, units = embargo_length.split(" ")
+      # Calculate embargo expiration date
+      number, units = requested_embargo.split(" ")
+      raise ArgumentError, "Unexpected embargo length '#{requested_embargo}'" unless valid_length(number, units)
       graduation_date = Date.parse(graduation_date) if graduation_date.class == String
       graduation_date + Integer(number).send(units.to_sym)
     end
   end
+
+  # Ensure valid values like "6 months", "2 months", "6 years"
+  def self.valid_length(number, units)
+    valid_quantity = number.to_i > 0
+    valid_units = ['months', 'year', 'years'].include?(units)
+    valid_quantity && valid_units
+  end
 end

spec/helpers/graduation_helper_spec.rb

@@ -3,18 +3,45 @@
 require 'rails_helper'
 
 RSpec.describe GraduationHelper, type: :helper do
-  context "calculating embargo_release_date" do
-    it "can interpret a length of '6 months'" do
-      e = described_class.embargo_length_to_embargo_release_date(Time.zone.today, "6 months")
-      expect(e).to eq Time.zone.today + 6.months
+  let(:graduation_date) { "2019-08-17" }
+
+  context "with valid embargo_length and graduation_date" do
+    it "inteprets month units" do
+      release_date = described_class.embargo_length_to_embargo_release_date(graduation_date, "6 months")
+      expect(release_date).to eq graduation_date.to_datetime + 6.months
     end
-    it "can interpret a length of '3 years'" do
-      e = described_class.embargo_length_to_embargo_release_date(Time.zone.today, "3 years")
-      expect(e).to eq Time.zone.today + 3.years
+
+    it "interprets year units" do
+      release_date = described_class.embargo_length_to_embargo_release_date(graduation_date, "3 years")
+      expect(release_date).to eq graduation_date.to_datetime + 3.years
     end
+
     it "can interpret a length of 'None - open access immediately'" do
-      e = described_class.embargo_length_to_embargo_release_date(Time.zone.today, "None - open access immediately")
-      expect(e).to be <= Time.zone.today
+      release_date = described_class.embargo_length_to_embargo_release_date(graduation_date, "None - open access immediately")
+      expect(release_date).to eq graduation_date
+    end
+  end
+
+  context "with unexpected values" do
+    before { allow(Rails.logger).to receive(:warn) }
+
+    it "handles exmpty embargo_length as 'None'" do
+      embargo_length = ''
+      release_date = described_class.embargo_length_to_embargo_release_date(graduation_date, embargo_length)
+      expect(release_date).to eq graduation_date
+      expect(Rails.logger).to have_received(:warn).with("Treating empty requested_embargo as 'None'")
+    end
+
+    it "handles nil embargo_length as 'None'" do
+      embargo_length = nil
+      release_date = described_class.embargo_length_to_embargo_release_date(graduation_date, embargo_length)
+      expect(release_date).to eq graduation_date
+      expect(Rails.logger).to have_received(:warn).with("Treating empty requested_embargo as 'None'")
+    end
+
+    it "raises an error for random embargo_lengths" do
+      embargo_length = 'Four score and seven years'
+      expect { described_class.embargo_length_to_embargo_release_date(graduation_date, embargo_length) }.to raise_error ArgumentError
     end
   end
 end