Add invalid announcements error widget and add related test (#2568)

apps/dashboard/app/controllers/application_controller.rb

@@ -77,7 +77,38 @@ def set_pinned_apps
   end
 
   def set_announcements
-    @announcements ||= Announcements.all(@user_configuration.announcement_path)
+    all_announcements = Announcements.all(@user_configuration.announcement_path)
+
+    parse_errors, announcements = all_announcements.partition(&:parse_error?)
+    if parse_errors.any?
+      flash.now[:alert] = helpers.safe_join(
+        parse_errors.map do |a|
+          source = a.source.presence || a.id.presence || 'unknown'
+          short_name =
+            begin
+              Pathname.new(source.to_s).basename.sub_ext('').to_s
+            rescue StandardError
+              source.to_s
+            end
+
+          headline = "Could not render announcement '#{short_name}' because of error"
+          details = [a.error_class.presence, a.error_message.presence].compact.join(': ')
+          details = a.message.presence || a.msg.presence || 'Error parsing announcement.' if details.blank?
+
+          helpers.content_tag(:div) do
+            helpers.safe_join(
+              [
+                helpers.content_tag(:div, headline, class: 'card-header bg-transparent'),
+                helpers.content_tag(:div, details.to_s, class: 'card-body py-2'),
+              ]
+            )
+          end
+        end,
+        helpers.safe_join([helpers.tag.br, helpers.tag.br])
+      )
+    end
+
+    @announcements ||= announcements
   rescue => e
     logger.warn "Error parsing announcements: #{e.message}"
     @announcements ||= []

apps/dashboard/app/models/announcement.rb

@@ -52,6 +52,7 @@ def button_text
   # Whether this is a valid announcement
   # @return [Boolean] whether it is valid
   def valid?
+    return false if parse_error?
     return false unless message
     return false if dismissible? && !id
     true
@@ -75,6 +76,30 @@ def required?
     opts.fetch(:required, false)
   end
 
+  # Whether this announcement is a parse/render error
+  # @return [Boolean] whether it is a parse/render error
+  def parse_error?
+    opts.fetch(:parse_error, false)
+  end
+
+  # Raw error message when parsing failed
+  # @return [String, nil] the error message if parsing failed
+  def error_message
+    opts[:error_message]
+  end
+
+  # Exception class name when parsing failed
+  # @return [String, nil] the exception class name if parsing failed
+  def error_class
+    opts[:error_class]
+  end
+
+  # Source file path for the announcement (if any)
+  # @return [String, nil] the source file path if it exists
+  def source
+    opts[:file]
+  end
+
   private
 
   def opts
@@ -86,15 +111,19 @@ def opts
               else
                 {}
               end
-    @opts.symbolize_keys.compact
-  rescue Errno::ENOENT # File does not exist
+    (@opts || {}).symbolize_keys.compact
+  rescue Errno::ENOENT => e # File does not exist
     Rails.logger.warn "Announcement file not found: #{@path}"
-    @opts = {}
+    @opts = build_error_opts("Announcement file not found: #{@path}", e)
   rescue SyntaxError => e # Syntax errors
     Rails.logger.warn "Syntax error in announcement file '#{@path}': #{e.message}. Please check the file for proper syntax."
-    @opts = {}
+    @opts = build_error_opts("Syntax error parsing announcement file '#{@path}': #{e.message}", e)
   rescue StandardError => e # Other exceptions
     Rails.logger.warn "Error parsing announcement file '#{@path}': #{e.message}"
-    @opts = {}
+    @opts = build_error_opts("Error parsing announcement file '#{@path}': #{e.message}", e)
+  end
+
+  def build_error_opts(message, e)
+    { type: :danger, message: message, parse_error: true, error_message: e.message, error_class: e.class.to_s, file: @path.to_s }
   end
 end

apps/dashboard/app/views/layouts/_announcements.html.erb

@@ -17,4 +17,4 @@
       </div>
     <% end %>
   </div>
-<% end %>
+<% end %>

apps/dashboard/test/integration/announcement_views_test.rb

@@ -9,9 +9,31 @@ class AnnouncementViewsTest < ActionDispatch::IntegrationTest
     stub_user_configuration({ announcement_path: [f.path] })
 
     begin
-      get '/'
-      assert_response :success
-      assert_select 'div.announcement div.announcement-body', 'Test announcement.'
+      with_modified_env('OOD_ANNOUNCEMENT_PATH' => nil) do
+        get '/'
+        assert_response :success
+        assert_select 'div.announcement div.announcement-body', 'Test announcement.'
+      end
+    ensure
+      stub_user_configuration({})
+    end
+  end
+
+  test 'parsing error shows a danger alert' do
+    f = Tempfile.open(%w[announcement .yml])
+    f.write("::: this is not yaml: [\n")
+    f.close
+
+    stub_user_configuration({ announcement_path: [f.path] })
+
+    begin
+      with_modified_env('OOD_ANNOUNCEMENT_PATH' => nil) do
+        get '/'
+        assert_response :success
+
+        assert_select 'div.alert.alert-danger', /Could not render announcement/
+        assert_select 'div.alert.alert-danger', /mapping values/
+      end
     ensure
       stub_user_configuration({})
     end
@@ -22,13 +44,15 @@ class AnnouncementViewsTest < ActionDispatch::IntegrationTest
     stub_user_configuration({ announcement_path: [file] })
 
     begin
-      get '/'
-      assert_response :success
-      assert_select 'div.announcement div.announcement-body', 'This is the announcement.'
-      assert_select 'div.announcement .announcement-button', I18n.t('dashboard.announcements_dismissible_button')
-      form = css_select('div.announcement .announcement-form')
-      assert_equal 1, form.size
-      assert_equal settings_path(action: 'announcement'), form[0]['action']
+      with_modified_env('OOD_ANNOUNCEMENT_PATH' => nil) do
+        get '/'
+        assert_response :success
+        assert_select 'div.announcement div.announcement-body', 'This is the announcement.'
+        assert_select 'div.announcement .announcement-button', I18n.t('dashboard.announcements_dismissible_button')
+        form = css_select('div.announcement .announcement-form')
+        assert_equal 1, form.size
+        assert_equal settings_path(action: 'announcement'), form[0]['action']
+      end
     ensure
       stub_user_configuration({})
     end

apps/dashboard/test/models/announcements_test.rb

@@ -86,7 +86,7 @@ class AnnouncementsTest < ActiveSupport::TestCase
 
     Rails.logger.expects(:warn).with(regexp_matches(/Syntax error in announcement file/))
     announcement = Announcements.all(f.path).first
-    assert_equal(:warning, announcement.type)
+    assert_equal(:danger, announcement.type)
   end
 
   test 'should create invalid announcement for invalid yaml file' do