diff --git a/frontend/src/app/shared/helpers/chronic_duration.js b/frontend/src/app/shared/helpers/chronic_duration.js index de4fac624ff..d96a3f56c59 100644 --- a/frontend/src/app/shared/helpers/chronic_duration.js +++ b/frontend/src/app/shared/helpers/chronic_duration.js @@ -125,18 +125,38 @@ function durationUnitsSecondsMultiplier(unit, opts) { function calculateFromWords(string, opts) { let val = 0; + let lastExplicitUnit = null; const words = string.split(' '); + words.forEach((v, k) => { if (v === '') { return; } if (v.search(FLOAT_MATCHER) >= 0) { - val += - convertToNumber(v) * - durationUnitsSecondsMultiplier( - words[parseInt(k, 10) + 1] || opts.defaultUnit || 'seconds', - opts, - ); + const nextWord = words[parseInt(k, 10) + 1]; + let unit; + + if (nextWord && DURATION_UNITS_LIST.includes(nextWord)) { + // Next word is a valid unit, use it and remember as last explicit unit + unit = nextWord; + lastExplicitUnit = nextWord; + } else if (lastExplicitUnit) { + // No explicit unit, but we have a previous unit - use next smaller unit + const currentIndex = DURATION_UNITS_LIST.indexOf(lastExplicitUnit); + if (currentIndex > 0) { + // Use next smaller unit (lower index in array) + unit = DURATION_UNITS_LIST[currentIndex - 1]; + } else { + // Already at smallest unit (seconds), keep using it + unit = lastExplicitUnit; + } + lastExplicitUnit = unit; + } else { + // No explicit unit and no previous unit, fall back to default + unit = opts.defaultUnit || 'seconds'; + } + + val += convertToNumber(v) * durationUnitsSecondsMultiplier(unit, opts); } }); return val; diff --git a/frontend/src/app/shared/helpers/chronic_duration.spec.ts b/frontend/src/app/shared/helpers/chronic_duration.spec.ts index 8946fbb9848..22d678a7469 100644 --- a/frontend/src/app/shared/helpers/chronic_duration.spec.ts +++ b/frontend/src/app/shared/helpers/chronic_duration.spec.ts @@ -73,8 +73,9 @@ describe('parseChronicDuration', () => { '2 months': 3600 * 24 * 30 * 2, '18 months': 3600 * 24 * 30 * 18, '1 year 6 months': 3600 * 24 * (365.25 + 6 * 30), - day: 3600 * 24, + 'day': 3600 * 24, 'minute 30s': 90, + '5d 2 3 9': (5 * 24 * 3600) + (2 * 3600) + (3 * 60) + 9, // 5 days, 2 hours, 3 minutes, 9 seconds }; describe("when string can't be parsed", () => { @@ -115,6 +116,11 @@ describe('parseChronicDuration', () => { expect(parseChronicDuration('5', { defaultUnit: 'minutes' })).toBe(300); }); + /* The cecile case */ + it('it parses 2h15 correctly to 2h 15 minutes even when the default unit is hours', () => { + expect(parseChronicDuration('2h15', { defaultUnit: 'hours' })).toBe(2 * 3600 + 15 * 60); + }); + Object.entries(EXEMPLARS).forEach(([k, v]) => { it(`parses a duration like ${k}`, () => { expect(parseChronicDuration(k)).toBe(v); @@ -139,6 +145,7 @@ describe('parseChronicDuration', () => { expect(parseChronicDuration('1mo', { daysPerMonth: 30, hoursPerDay: 24 })).toBe( 30 * 24 * 60 * 60, ); + expect(parseChronicDuration('1mo', { daysPerMonth: 20, hoursPerDay: 8 })).toBe( 20 * 8 * 60 * 60, ); @@ -146,6 +153,7 @@ describe('parseChronicDuration', () => { expect(parseChronicDuration('1w', { daysPerMonth: 30, hoursPerDay: 24 })).toBe( 7 * 24 * 60 * 60, ); + expect(parseChronicDuration('1w', { daysPerMonth: 20, hoursPerDay: 8 })).toBe( 5 * 8 * 60 * 60, ); @@ -322,6 +330,7 @@ describe('outputChronicDuration', () => { expect(outputChronicDuration(7 * 24 * 60 * 60, { weeks: true, daysPerMonth: 30 })).toBe( '1 wk', ); + expect(outputChronicDuration(7 * 24 * 60 * 60, { weeks: true, daysPerMonth: 20 })).toBe( '1 wk 2 days', ); @@ -331,6 +340,7 @@ describe('outputChronicDuration', () => { expect( outputChronicDuration(7 * 24 * 60 * 60, { weeks: true, daysPerMonth: 30, hoursPerDay: 24 }), ).toBe('1 wk'); + expect( outputChronicDuration(5 * 8 * 60 * 60, { weeks: true, daysPerMonth: 20, hoursPerDay: 8 }), ).toBe('1 wk'); @@ -340,6 +350,7 @@ describe('outputChronicDuration', () => { expect(outputChronicDuration(30 * 24 * 60 * 60, { daysPerMonth: 30, hoursPerDay: 24 })).toBe( '1 mo', ); + expect( outputChronicDuration(30 * 24 * 60 * 60, { daysPerMonth: 30, @@ -351,6 +362,7 @@ describe('outputChronicDuration', () => { expect(outputChronicDuration(20 * 8 * 60 * 60, { daysPerMonth: 20, hoursPerDay: 8 })).toBe( '1 mo', ); + expect( outputChronicDuration(20 * 8 * 60 * 60, { daysPerMonth: 20, hoursPerDay: 8, weeks: true }), ).toBe('1 mo'); @@ -392,6 +404,7 @@ describe('outputChronicDuration', () => { describe('work week', () => { it('should parse knowing the work week', () => { const week = parseChronicDuration('5d', { hoursPerDay: 8, daysPerMonth: 20 }); + expect(parseChronicDuration('40h', { hoursPerDay: 8, daysPerMonth: 20 })).toBe(week); expect(parseChronicDuration('1w', { hoursPerDay: 8, daysPerMonth: 20 })).toBe(week); }); diff --git a/lib/chronic_duration.rb b/lib/chronic_duration.rb index de6db62414d..b0f1039e95a 100644 --- a/lib/chronic_duration.rb +++ b/lib/chronic_duration.rb @@ -255,15 +255,37 @@ module ChronicDuration res end - def calculate_from_words(string, opts) + def calculate_from_words(string, opts) # rubocop:disable Metrics/AbcSize, Metrics/PerceivedComplexity val = 0 + last_explicit_unit = nil words = string.split + words.each_with_index do |v, k| next unless v&.match?(float_matcher) - val += (convert_to_number(v) * duration_units_seconds_multiplier( - words[k + 1] || (opts[:default_unit] || "seconds"), opts - )) + next_word = words[k + 1] + + if next_word && duration_units_list.include?(next_word) + # Next word is a valid unit, use it and remember as last explicit unit + unit = next_word + last_explicit_unit = next_word + elsif last_explicit_unit + # No explicit unit, but we have a previous unit - use next smaller unit + current_index = duration_units_list.index(last_explicit_unit) + unit = if current_index > 0 + # Use next smaller unit (lower index in array) + duration_units_list[current_index - 1] + else + # Already at smallest unit (seconds), keep using it + last_explicit_unit + end + last_explicit_unit = unit + else + # No explicit unit and no previous unit, fall back to default + unit = opts[:default_unit] || "seconds" + end + + val += convert_to_number(v) * duration_units_seconds_multiplier(unit, opts) end val end diff --git a/spec/lib/chronic_duration_spec.rb b/spec/lib/chronic_duration_spec.rb index 258b05e6084..bf37aff8fd7 100644 --- a/spec/lib/chronic_duration_spec.rb +++ b/spec/lib/chronic_duration_spec.rb @@ -113,6 +113,57 @@ RSpec.describe ChronicDuration do expect(described_class.parse("5", default_unit: "minutes")).to eq(300) end + # Tests for intelligent unit inference + context "when using intelligent unit inference" do + it "interprets subsequent numbers without units as next smaller unit" do + expect(described_class.parse("2 hours 15", default_unit: "hours")).to eq(8100) # 2h 15m = 8100s + end + + it "handles multiple numbers without units in descending order" do + expect(described_class.parse("2 hours 15 30", default_unit: "hours")).to eq(8130) # 2h 15m 30s = 8130s + end + + it "works with different starting units" do + expect(described_class.parse("1 day 5", default_unit: "days")).to eq(104400) # 1d 5h = 104400s + expect(described_class.parse("3 minutes 45", default_unit: "minutes")).to eq(225) # 3m 45s = 225s + expect(described_class.parse("1 week 2", default_unit: "weeks")).to eq(777600) # 1w 2d = 777600s + end + + it "falls back to default unit when no previous unit exists" do + expect(described_class.parse("5", default_unit: "minutes")).to eq(300) # 5m = 300s + expect(described_class.parse("10", default_unit: "hours")).to eq(36000) # 10h = 36000s + end + + it "handles mixed explicit and implicit units" do + expect(described_class.parse("1 hour 30 20 seconds", default_unit: "hours")).to eq(5420) # 1h 30m 20s = 5420s + end + + it "keeps using smallest unit when already at seconds" do + expect(described_class.parse("3 minutes 45 10", default_unit: "minutes")).to eq(235) # 3m 45s 10s = 235s + end + + it "works with fractional numbers" do + expect(described_class.parse("2.5 hours 30", default_unit: "hours")).to eq(10800) # 2.5h 30m = 10800s + expect(described_class.parse("1 hour 15.5", default_unit: "hours")).to eq(4530) # 1h 15.5m = 4530s + end + + it "maintains backward compatibility with existing formats" do + expect(described_class.parse("2 hours 20 minutes")).to eq(8400) # Explicit units should still work + expect(described_class.parse("1 day 5 hours 30 minutes")).to eq(106200) # Multiple explicit units + end + + it "handles edge cases correctly" do + # Multiple numbers without units at the beginning (should use default) + expect(described_class.parse("5 10", default_unit: "minutes")).to eq(900) # 5m + 10m = 900s + + # Unit at the end with numbers before - first number uses default, then intelligent inference kicks in + expect(described_class.parse("2 15 minutes")).to eq(902) # 2s (default) + 15m = 902s + + # Unit at beginning gets implicit "1", then intelligent inference + expect(described_class.parse("minutes 2 15")).to eq(77) # 1m + 2s + 15s = 77s + end + end + exemplars.each do |k, v| it "parses a duration like #{k}" do expect(described_class.parse(k)).to eq(v)