Validate trip start and end dates

app/javascript/controllers/datetime_controller.js

@@ -11,9 +11,57 @@ export default class extends BaseController {
   connect() {
     console.log("Datetime controller connected")
     this.debounceTimer = null;
+
+    // Add validation listeners
+    if (this.hasStartedAtTarget && this.hasEndedAtTarget) {
+      // Validate on change to set validation state
+      this.startedAtTarget.addEventListener('change', () => this.validateDates())
+      this.endedAtTarget.addEventListener('change', () => this.validateDates())
+
+      // Validate on blur to set validation state
+      this.startedAtTarget.addEventListener('blur', () => this.validateDates())
+      this.endedAtTarget.addEventListener('blur', () => this.validateDates())
+
+      // Add form submit validation
+      const form = this.element.closest('form')
+      if (form) {
+        form.addEventListener('submit', (e) => {
+          if (!this.validateDates()) {
+            e.preventDefault()
+            this.endedAtTarget.reportValidity()
+          }
+        })
+      }
+    }
   }
 
-  async updateCoordinates(event) {
+  validateDates(showPopup = false) {
+    const startDate = new Date(this.startedAtTarget.value)
+    const endDate = new Date(this.endedAtTarget.value)
+
+    // Clear any existing custom validity
+    this.startedAtTarget.setCustomValidity('')
+    this.endedAtTarget.setCustomValidity('')
+
+    // Check if both dates are valid
+    if (isNaN(startDate.getTime()) || isNaN(endDate.getTime())) {
+      return true
+    }
+
+    // Validate that start date is before end date
+    if (startDate >= endDate) {
+      const errorMessage = 'Start date must be earlier than end date'
+      this.endedAtTarget.setCustomValidity(errorMessage)
+      if (showPopup) {
+        this.endedAtTarget.reportValidity()
+      }
+      return false
+    }
+
+    return true
+  }
+
+  async updateCoordinates() {
     // Clear any existing timeout
     if (this.debounceTimer) {
       clearTimeout(this.debounceTimer);
@@ -25,6 +73,11 @@ export default class extends BaseController {
       const endedAt = this.endedAtTarget.value
       const apiKey = this.apiKeyTarget.value
 
+      // Validate dates before making API call (don't show popup, already shown on change)
+      if (!this.validateDates(false)) {
+        return
+      }
+
       if (startedAt && endedAt) {
         try {
           const params = new URLSearchParams({

app/models/trip.rb

@@ -9,6 +9,7 @@ class Trip < ApplicationRecord
   belongs_to :user
 
   validates :name, :started_at, :ended_at, presence: true
+  validate :started_at_before_ended_at
 
   after_create :enqueue_calculation_jobs
   after_update :enqueue_calculation_jobs, if: -> { saved_change_to_started_at? || saved_change_to_ended_at? }
@@ -47,4 +48,11 @@ class Trip < ApplicationRecord
     # to show all photos in the same height
     vertical_photos.count > horizontal_photos.count ? vertical_photos : horizontal_photos
   end
+
+  def started_at_before_ended_at
+    return if started_at.blank? || ended_at.blank?
+    return unless started_at >= ended_at
+
+    errors.add(:ended_at, 'must be after start date')
+  end
 end

e2e/v2/trips.spec.js

@@ -0,0 +1,100 @@
+import { test, expect } from '@playwright/test'
+import { closeOnboardingModal } from '../helpers/navigation.js'
+
+test.describe('Trips Date Validation', () => {
+  test.beforeEach(async ({ page }) => {
+    await page.goto('/trips/new')
+    await closeOnboardingModal(page)
+  });
+
+  test('validates that start date is earlier than end date on new trip form', async ({ page }) => {
+    // Wait for the form to load
+    await page.waitForSelector('input[name="trip[started_at]"]')
+
+    // Fill in trip name
+    await page.fill('input[name="trip[name]"]', 'Test Trip')
+
+    // Set end date before start date
+    await page.fill('input[name="trip[started_at]"]', '2024-12-25T10:00')
+    await page.fill('input[name="trip[ended_at]"]', '2024-12-20T10:00')
+
+    // Get the current URL to verify we stay on the same page
+    const currentUrl = page.url()
+
+    // Try to submit the form
+    const submitButton = page.locator('input[type="submit"], button[type="submit"]')
+    await submitButton.click()
+
+    // Wait a bit for potential navigation
+    await page.waitForTimeout(500)
+
+    // Verify we're still on the same page (form wasn't submitted)
+    expect(page.url()).toBe(currentUrl)
+
+    // Verify the dates are still there (form wasn't cleared)
+    const startValue = await page.locator('input[name="trip[started_at]"]').inputValue()
+    const endValue = await page.locator('input[name="trip[ended_at]"]').inputValue()
+    expect(startValue).toBe('2024-12-25T10:00')
+    expect(endValue).toBe('2024-12-20T10:00')
+  });
+
+  test('allows valid date range on new trip form', async ({ page }) => {
+    // Wait for the form to load
+    await page.waitForSelector('input[name="trip[started_at]"]')
+
+    // Fill in trip name
+    await page.fill('input[name="trip[name]"]', 'Valid Test Trip')
+
+    // Set valid date range (start before end)
+    await page.fill('input[name="trip[started_at]"]', '2024-12-20T10:00')
+    await page.fill('input[name="trip[ended_at]"]', '2024-12-25T10:00')
+
+    // Trigger blur to validate
+    await page.locator('input[name="trip[ended_at]"]').blur()
+
+    // Give the validation time to run
+    await page.waitForTimeout(200)
+
+    // Check that the end date field has no validation error
+    const endDateInput = page.locator('input[name="trip[ended_at]"]')
+    const validationMessage = await endDateInput.evaluate(el => el.validationMessage)
+    const isValid = await endDateInput.evaluate(el => el.validity.valid)
+
+    expect(validationMessage).toBe('')
+    expect(isValid).toBe(true)
+  });
+
+  test('validates dates when updating end date to be earlier than start date', async ({ page }) => {
+    // Wait for the form to load
+    await page.waitForSelector('input[name="trip[started_at]"]')
+
+    // Fill in trip name
+    await page.fill('input[name="trip[name]"]', 'Test Trip')
+
+    // First set a valid range
+    await page.fill('input[name="trip[started_at]"]', '2024-12-20T10:00')
+    await page.fill('input[name="trip[ended_at]"]', '2024-12-25T10:00')
+
+    // Now change start date to be after end date
+    await page.fill('input[name="trip[started_at]"]', '2024-12-26T10:00')
+
+    // Get the current URL to verify we stay on the same page
+    const currentUrl = page.url()
+
+    // Try to submit the form
+    const submitButton = page.locator('input[type="submit"], button[type="submit"]')
+    await submitButton.click()
+
+    // Wait a bit for potential navigation
+    await page.waitForTimeout(500)
+
+    // Verify we're still on the same page (form wasn't submitted)
+    expect(page.url()).toBe(currentUrl)
+
+    // Verify the dates are still there (form wasn't cleared)
+    const startValue = await page.locator('input[name="trip[started_at]"]').inputValue()
+    const endValue = await page.locator('input[name="trip[ended_at]"]').inputValue()
+    expect(startValue).toBe('2024-12-26T10:00')
+    expect(endValue).toBe('2024-12-25T10:00')
+  });
+});

spec/models/trip_spec.rb

@@ -7,6 +7,33 @@ RSpec.describe Trip, type: :model do
     it { is_expected.to validate_presence_of(:name) }
     it { is_expected.to validate_presence_of(:started_at) }
     it { is_expected.to validate_presence_of(:ended_at) }
+
+    context 'date range validation' do
+      let(:user) { create(:user) }
+
+      it 'is valid when started_at is before ended_at' do
+        trip = build(:trip, user: user, started_at: 1.day.ago, ended_at: Time.current)
+        expect(trip).to be_valid
+      end
+
+      it 'is invalid when started_at is after ended_at' do
+        trip = build(:trip, user: user, started_at: Time.current, ended_at: 1.day.ago)
+        expect(trip).not_to be_valid
+        expect(trip.errors[:ended_at]).to include('must be after start date')
+      end
+
+      it 'is invalid when started_at equals ended_at' do
+        time = Time.current
+        trip = build(:trip, user: user, started_at: time, ended_at: time)
+        expect(trip).not_to be_valid
+        expect(trip.errors[:ended_at]).to include('must be after start date')
+      end
+
+      it 'is valid when both dates are blank during initialization' do
+        trip = Trip.new(user: user, name: 'Test Trip')
+        expect(trip.errors[:ended_at]).to be_empty
+      end
+    end
   end
 
   describe 'associations' do