From a5ae5d887f9575cd21c7daad1768418a06a4f2a2 Mon Sep 17 00:00:00 2001 From: Eugene Burmakin Date: Fri, 26 Dec 2025 16:15:06 +0100 Subject: [PATCH] Validate trip start and end dates --- .../controllers/datetime_controller.js | 55 +++++++++- app/models/trip.rb | 8 ++ e2e/v2/trips.spec.js | 100 ++++++++++++++++++ spec/models/trip_spec.rb | 27 +++++ 4 files changed, 189 insertions(+), 1 deletion(-) create mode 100644 e2e/v2/trips.spec.js diff --git a/app/javascript/controllers/datetime_controller.js b/app/javascript/controllers/datetime_controller.js index b03df4ca..cc78ea44 100644 --- a/app/javascript/controllers/datetime_controller.js +++ b/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({ diff --git a/app/models/trip.rb b/app/models/trip.rb index fca5e1e2..3b882f4b 100644 --- a/app/models/trip.rb +++ b/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 diff --git a/e2e/v2/trips.spec.js b/e2e/v2/trips.spec.js new file mode 100644 index 00000000..d03f39f3 --- /dev/null +++ b/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') + }); +}); diff --git a/spec/models/trip_spec.rb b/spec/models/trip_spec.rb index 8c46a65a..7b79da46 100644 --- a/spec/models/trip_spec.rb +++ b/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