Merge pull request #370 from we-promise/copilot/fix-custom-range-error

Fix 500 error when custom date range end date precedes start date

app/controllers/reports_controller.rb

@@ -11,6 +11,9 @@ class ReportsController < ApplicationController
     @start_date = parse_date_param(:start_date) || default_start_date
     @end_date = parse_date_param(:end_date) || default_end_date
 
+    # Validate and fix date range if end_date is before start_date
+    validate_and_fix_date_range(show_flash: true)
+
     # Build the period
     @period = Period.custom(start_date: @start_date, end_date: @end_date)
     @previous_period = build_previous_period
@@ -41,6 +44,11 @@ class ReportsController < ApplicationController
     @period_type = params[:period_type]&.to_sym || :monthly
     @start_date = parse_date_param(:start_date) || default_start_date
     @end_date = parse_date_param(:end_date) || default_end_date
+
+    # Validate and fix date range if end_date is before start_date
+    # Don't show flash message since we're returning CSV data
+    validate_and_fix_date_range(show_flash: false)
+
     @period = Period.custom(start_date: @start_date, end_date: @end_date)
 
     # Build monthly breakdown data for export
@@ -93,6 +101,14 @@ class ReportsController < ApplicationController
 
   private
 
+    def validate_and_fix_date_range(show_flash: false)
+      return unless @start_date > @end_date
+
+      # Swap the dates to maintain user's intended date range
+      @start_date, @end_date = @end_date, @start_date
+      flash.now[:alert] = t("reports.invalid_date_range") if show_flash
+    end
+
     def ensure_money(value)
       return value if value.is_a?(Money)
       # Value is numeric (BigDecimal or Integer) in dollars - pass directly to Money.new

app/views/reports/index.html.erb

@@ -9,6 +9,13 @@
       </p>
     </div>
 
+    <%# Flash messages %>
+    <% if flash[:alert].present? %>
+      <div class="p-3 rounded-lg bg-destructive-surface text-sm text-destructive">
+        <%= flash[:alert] %>
+      </div>
+    <% end %>
+
     <%# Period Navigation Tabs %>
     <div class="flex items-center gap-2 overflow-x-auto pb-2">
       <%= render DS::Link.new(

config/locales/views/reports/de.yml

@@ -15,6 +15,7 @@ de:
         from: Von
         to: Bis
       showing_period: "Zeitraum: %{start} bis %{end}"
+    invalid_date_range: "Das Enddatum kann nicht vor dem Startdatum liegen. Die Daten wurden vertauscht."
     summary:
       total_income: Gesamteinnahmen
       total_expenses: Gesamtausgaben

config/locales/views/reports/en.yml

@@ -15,6 +15,7 @@ en:
         from: From
         to: To
       showing_period: "Showing data from %{start} to %{end}"
+    invalid_date_range: "End date cannot be before start date. The dates have been swapped."
     summary:
       total_income: Total Income
       total_expenses: Total Expenses

test/controllers/reports_controller_test.rb

@@ -92,6 +92,25 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest
     assert_response :ok # Should not crash, uses defaults
   end
 
+  test "index swaps dates when end_date is before start_date" do
+    start_date = Date.current
+    end_date = 1.month.ago.to_date
+
+    get reports_path(
+      period_type: :custom,
+      start_date: start_date.to_s,
+      end_date: end_date.to_s
+    )
+
+    assert_response :ok
+    # Should show flash message about invalid date range
+    assert flash[:alert].present?, "Flash alert should be present"
+    assert_match /End date cannot be before start date/, flash[:alert]
+    # Verify the response body contains the swapped date range in the correct order
+    assert_includes @response.body, end_date.strftime("%b %-d, %Y")
+    assert_includes @response.body, start_date.strftime("%b %-d, %Y")
+  end
+
   test "spending patterns returns data when expense transactions exist" do
     # Create expense category
     expense_category = @family.categories.create!(
@@ -185,4 +204,21 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest
     assert_response :ok, "Export should work with session auth. Response: #{@response.body}"
     assert_equal "text/csv", @response.media_type
   end
+
+  test "export transactions swaps dates when end_date is before start_date" do
+    start_date = Date.current
+    end_date = 1.month.ago.to_date
+
+    get export_transactions_reports_path(
+      format: :csv,
+      period_type: :custom,
+      start_date: start_date.to_s,
+      end_date: end_date.to_s
+    )
+
+    assert_response :ok
+    assert_equal "text/csv", @response.media_type
+    # Verify the CSV content is generated (should not crash)
+    assert_not_nil @response.body
+  end
 end