Fix changelog page crash when GitHub release notes are unavailable (#2314)

* Fix changelog page crash when GitHub release notes are unavailable

* Refactor changelog view to handle missing avatars gracefully and improve session sign-out logic in tests

* Enhance changelog view to display fallback messages for unavailable release notes and publication dates

* Update onboarding system tests to reflect UI changes and improve assertions

- Changed button labels from "Get started" to "Continue" and "Complete" to align with updated UI.
- Updated text assertions for clarity, changing "Set your preferences" to "Configure your preferences".
- Adjusted locale selection options to include language codes.
- Enhanced validation error handling in preferences form.
- Improved navigation assertions to ensure accurate path checks.

app/controllers/pages_controller.rb

@@ -29,6 +29,17 @@ class PagesController < ApplicationController
   def changelog
     @release_notes = github_provider.fetch_latest_release_notes
 
+    # Fallback if no release notes are available
+    if @release_notes.nil?
+      @release_notes = {
+        avatar: "https://github.com/maybe-finance.png",
+        username: "maybe-finance",
+        name: "Release notes unavailable",
+        published_at: Date.current,
+        body: "<p>Unable to fetch the latest release notes at this time. Please check back later or visit our <a href='https://github.com/maybe-finance/maybe/releases' target='_blank'>GitHub releases page</a> directly.</p>"
+      }
+    end
+
     render layout: "settings"
   end
 

app/views/pages/changelog.html.erb

@@ -4,18 +4,24 @@
   <div class="flex flex-col md:flex-row justify-between gap-4 mb-12 last:mb-0">
     <div class="w-full md:w-1/3">
       <div class="md:px-3 flex items-center gap-3">
-        <div class="text-white shrink-0 w-9 h-9">
-          <%= image_tag @release_notes[:avatar], class: "rounded-full w-full h-full object-cover" %>
-        </div>
+        <% if @release_notes[:avatar].present? %>
+          <div class="text-white shrink-0 w-9 h-9">
+            <%= image_tag @release_notes[:avatar], class: "rounded-full w-full h-full object-cover" %>
+          </div>
+        <% else %>
+          <div class="bg-gray-300 text-gray-600 shrink-0 w-9 h-9 rounded-full flex items-center justify-center text-sm font-medium">
+            <%= @release_notes[:username]&.first&.upcase || "?" %>
+          </div>
+        <% end %>
         <div>
           <a class="text-primary font-medium text-sm" href="https://github.com/<%= @release_notes[:username] %>"><%= "@#{@release_notes[:username]}" %></a>
-          <div class="text-secondary text-sm"><%= @release_notes[:published_at].strftime("%B %d, %Y") %></div>
+          <div class="text-secondary text-sm"><%= @release_notes[:published_at]&.strftime("%B %d, %Y") || "Date unavailable" %></div>
         </div>
       </div>
     </div>
     <div class="w-full md:w-2/3 text-secondary text-sm prose prose--github-release-notes">
       <h2 class="mb-5 text-xl text-primary"><%= @release_notes[:name] %></h2>
-      <%= @release_notes[:body].html_safe %>
+      <%= (@release_notes[:body] || "No release notes available").html_safe %>
     </div>
   </div>
 </div>

test/controllers/mfa_controller_test.rb

@@ -7,7 +7,9 @@ class MfaControllerTest < ActionDispatch::IntegrationTest
   end
 
   def sign_out
-    delete session_path(@user.sessions.last) if @user.sessions.any?
+    @user.sessions.each do |session|
+      delete session_path(session)
+    end
   end
 
   test "redirects to root if MFA already enabled" do

test/controllers/onboardings_controller_test.rb

@@ -162,6 +162,8 @@ end
     private
 
       def sign_out
-        delete session_path(@user.sessions.last) if @user.sessions.any?
-end
+        @user.sessions.each do |session|
+          delete session_path(session)
+        end
+      end
 end

test/controllers/pages_controller_test.rb

@@ -16,4 +16,35 @@ class PagesControllerTest < ActionDispatch::IntegrationTest
       assert_response :ok
     end
   end
+
+  test "changelog with nil release notes" do
+    # Mock the GitHub provider to return nil (simulating API failure or no releases)
+    github_provider = mock
+    github_provider.expects(:fetch_latest_release_notes).returns(nil)
+    Provider::Registry.stubs(:get_provider).with(:github).returns(github_provider)
+
+    get changelog_path
+    assert_response :ok
+    assert_select "h2", text: "Release notes unavailable"
+    assert_select "a[href='https://github.com/maybe-finance/maybe/releases']"
+  end
+
+  test "changelog with incomplete release notes" do
+    # Mock the GitHub provider to return incomplete data (missing some fields)
+    github_provider = mock
+    incomplete_data = {
+      avatar: nil,
+      username: "maybe-finance",
+      name: "Test Release",
+      published_at: nil,
+      body: nil
+    }
+    github_provider.expects(:fetch_latest_release_notes).returns(incomplete_data)
+    Provider::Registry.stubs(:get_provider).with(:github).returns(github_provider)
+
+    get changelog_path
+    assert_response :ok
+    assert_select "h2", text: "Test Release"
+    # Should not crash even with nil values
+  end
 end

test/system/onboardings_test.rb

@@ -16,25 +16,25 @@ class OnboardingsTest < ApplicationSystemTestCase
     visit onboarding_path
 
     assert_text "Let's set up your account"
-    assert_button "Get started"
+    assert_button "Continue"
 
     # Navigate to preferences
-    click_button "Get started"
+    click_button "Continue"
 
     assert_current_path preferences_onboarding_path
-    assert_text "Set your preferences"
+    assert_text "Configure your preferences"
 
     # Test that the chart renders without errors (this would catch the Series bug)
     assert_selector "[data-controller='time-series-chart']"
 
     # Fill out preferences form
-    select "English", from: "user_family_attributes_locale"
+    select "English (en)", from: "user_family_attributes_locale"
     select "United States Dollar (USD)", from: "user_family_attributes_currency"
     select "MM/DD/YYYY", from: "user_family_attributes_date_format"
     select "Light", from: "user_theme"
 
     # Submit preferences
-    click_button "Continue"
+    click_button "Complete"
 
     # Should redirect to goals page
     assert_current_path goals_onboarding_path
@@ -100,22 +100,25 @@ class OnboardingsTest < ApplicationSystemTestCase
 
     # Clear required fields and try to submit
     select "", from: "user_family_attributes_locale"
-    click_button "Continue"
+    click_button "Complete"
 
-    # Should stay on preferences page with validation errors
-    assert_current_path preferences_onboarding_path
+    # Should stay on preferences page with validation errors (may have query params)
+    assert_match %r{/onboarding/preferences}, current_path
   end
 
   test "preferences form saves data correctly" do
     visit preferences_onboarding_path
 
     # Fill out form with specific values
-    select "Spanish", from: "user_family_attributes_locale"
+    select "Spanish (es)", from: "user_family_attributes_locale"
     select "Euro (EUR)", from: "user_family_attributes_currency"
     select "DD/MM/YYYY", from: "user_family_attributes_date_format"
     select "Dark", from: "user_theme"
 
-    click_button "Continue"
+    click_button "Complete"
+
+    # Wait for redirect to goals page to ensure form was submitted
+    assert_current_path goals_onboarding_path
 
     # Verify data was saved
     @family.reload
@@ -123,7 +126,7 @@ class OnboardingsTest < ApplicationSystemTestCase
 
     assert_equal "es", @family.locale
     assert_equal "EUR", @family.currency
-    assert_equal "DD/MM/YYYY", @family.date_format
+    assert_equal "%d/%m/%Y", @family.date_format
     assert_equal "dark", @user.theme
     assert_not_nil @user.set_onboarding_preferences_at
   end
@@ -135,7 +138,7 @@ class OnboardingsTest < ApplicationSystemTestCase
     visit goals_onboarding_path
 
     assert_text "What brings you to Maybe?"
-    assert_button "Continue"
+    assert_button "Next"
   end
 
   test "trial page renders correctly" do
@@ -147,16 +150,16 @@ class OnboardingsTest < ApplicationSystemTestCase
   test "navigation between onboarding steps" do
     # Start at main onboarding
     visit onboarding_path
-    click_button "Get started"
+    click_button "Continue"
 
     # Should be at preferences
     assert_current_path preferences_onboarding_path
 
     # Complete preferences
-    select "English", from: "user_family_attributes_locale"
+    select "English (en)", from: "user_family_attributes_locale"
     select "United States Dollar (USD)", from: "user_family_attributes_currency"
     select "MM/DD/YYYY", from: "user_family_attributes_date_format"
-    click_button "Continue"
+    click_button "Complete"
 
     # Should be at goals
     assert_current_path goals_onboarding_path
@@ -166,14 +169,14 @@ class OnboardingsTest < ApplicationSystemTestCase
     visit preferences_onboarding_path
 
     # Check that navigation shows current step
-    assert_selector ".onboarding-nav" # Adjust selector based on actual implementation
+    assert_selector "ul.hidden.md\\:flex.items-center.gap-2"
   end
 
   test "logout option is available during onboarding" do
     visit preferences_onboarding_path
 
-    # Should have logout option
-    assert_link "Sign out" # Adjust based on actual implementation
+    # Should have logout option (rendered as a button component)
+    assert_text "Sign out"
   end
 
   private
@@ -188,5 +191,5 @@ class OnboardingsTest < ApplicationSystemTestCase
 
       # Wait for successful login
       assert_current_path root_path
-end
+    end
 end