From 3cc88f3e98be10d32d4a6583a9c8de66ef05c179 Mon Sep 17 00:00:00 2001 From: Josh Pigford Date: Mon, 26 May 2025 19:53:25 -0500 Subject: [PATCH] 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 | 11 +++++ app/views/pages/changelog.html.erb | 16 +++++--- test/controllers/mfa_controller_test.rb | 4 +- .../onboardings_controller_test.rb | 6 ++- test/controllers/pages_controller_test.rb | 31 ++++++++++++++ test/system/onboardings_test.rb | 41 ++++++++++--------- 6 files changed, 82 insertions(+), 27 deletions(-) diff --git a/app/controllers/pages_controller.rb b/app/controllers/pages_controller.rb index 32bca350..b921f89c 100644 --- a/app/controllers/pages_controller.rb +++ b/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: "

Unable to fetch the latest release notes at this time. Please check back later or visit our GitHub releases page directly.

" + } + end + render layout: "settings" end diff --git a/app/views/pages/changelog.html.erb b/app/views/pages/changelog.html.erb index e7192321..0756ca5c 100644 --- a/app/views/pages/changelog.html.erb +++ b/app/views/pages/changelog.html.erb @@ -4,18 +4,24 @@
-
- <%= image_tag @release_notes[:avatar], class: "rounded-full w-full h-full object-cover" %> -
+ <% if @release_notes[:avatar].present? %> +
+ <%= image_tag @release_notes[:avatar], class: "rounded-full w-full h-full object-cover" %> +
+ <% else %> +
+ <%= @release_notes[:username]&.first&.upcase || "?" %> +
+ <% end %>
<%= "@#{@release_notes[:username]}" %> -
<%= @release_notes[:published_at].strftime("%B %d, %Y") %>
+
<%= @release_notes[:published_at]&.strftime("%B %d, %Y") || "Date unavailable" %>

<%= @release_notes[:name] %>

- <%= @release_notes[:body].html_safe %> + <%= (@release_notes[:body] || "No release notes available").html_safe %>
diff --git a/test/controllers/mfa_controller_test.rb b/test/controllers/mfa_controller_test.rb index e2e24417..49e4b622 100644 --- a/test/controllers/mfa_controller_test.rb +++ b/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 diff --git a/test/controllers/onboardings_controller_test.rb b/test/controllers/onboardings_controller_test.rb index 129aaecf..c35c1409 100644 --- a/test/controllers/onboardings_controller_test.rb +++ b/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 diff --git a/test/controllers/pages_controller_test.rb b/test/controllers/pages_controller_test.rb index 47fdc936..49ee225b 100644 --- a/test/controllers/pages_controller_test.rb +++ b/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 diff --git a/test/system/onboardings_test.rb b/test/system/onboardings_test.rb index dd1c93c9..32dc1783 100644 --- a/test/system/onboardings_test.rb +++ b/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