Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 34 additions & 5 deletions apps/dashboard/app/models/announcement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ def button_text
# Whether this is a valid announcement
# @return [Boolean] whether it is valid
def valid?
return true if parse_error?
return false unless message
return false if dismissible? && !id
true
Expand All @@ -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
Expand All @@ -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
42 changes: 23 additions & 19 deletions apps/dashboard/app/views/layouts/_announcements.html.erb
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
<% @announcements.select(&:valid?).reject(&:completed?).each do |announcement| %>
<div id="announcement-<%=announcement.id%>" class="alert alert-<%= announcement.type %> announcement" role="alert">
<div class="announcement-body"><%= raw OodAppkit.markdown.render(announcement.msg) %></div>
<% if announcement.dismissible? %>
<div class="d-grid gap-2 d-md-flex justify-content-md-end">
<%=
button_to(
settings_path,
method: :post,
form_class: 'announcement-form',
class: "btn btn-#{announcement.type} me-md-2 announcement-button",
params: { settings: { announcements: { announcement.id => Time.now.localtime.strftime('%Y-%m-%d %H:%M:%S') } }, back: true }
) do
announcement.button_text
end
%>
</div>
<% end %>
</div>
<% end %>
<% if announcement.parse_error? %>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We typically want to do error checking in the controller (ApplicationController#set_announcements in this case)

<%= render partial: 'shared/announcement_error', locals: { announcement: announcement } %>
<% else %>
<div id="announcement-<%=announcement.id%>" class="alert alert-<%= announcement.type %> announcement" role="alert">
<div class="announcement-body"><%= raw OodAppkit.markdown.render(announcement.msg) %></div>
<% if announcement.dismissible? %>
<div class="d-grid gap-2 d-md-flex justify-content-md-end">
<%=
button_to(
settings_path,
method: :post,
form_class: 'announcement-form',
class: "btn btn-#{announcement.type} me-md-2 announcement-button",
params: { settings: { announcements: { announcement.id => Time.now.localtime.strftime('%Y-%m-%d %H:%M:%S') } }, back: true }
) do
announcement.button_text
end
%>
</div>
<% end %>
</div>
<% end %>
<% end %>
8 changes: 8 additions & 0 deletions apps/dashboard/app/views/shared/_announcement_error.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<div class="alert alert-danger card">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't have to make a new alert partial for this error, we can use a shared alert structure by calling flash.now from the controller.

<div class="card-header">
Could not render announcement '<%= announcement.source || announcement.id || "unknown" %>' because of error
</div>
<div class="card-body">
<p><%= announcement.error_message || announcement.msg %></p>
</div>
</div>
24 changes: 24 additions & 0 deletions apps/dashboard/test/integration/announcement_parse_error_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
require 'test_helper'

class AnnouncementParseErrorTest < ActionDispatch::IntegrationTest
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can just add this to announcement_views_test to keep things in one place

test 'parsing error shows danger announcement' do
f = Tempfile.open(%w[announcement .yml])
# write malformed YAML
f.write("::: this is not yaml: [\n")
f.close

stub_user_configuration({ announcement_path: [f.path] })

begin
get '/'
assert_response :success

assert_select 'div.alert-danger.card' do
assert_select 'div.card-header', /Could not render announcement/
assert_select 'div.card-body', /mapping values/
end
ensure
stub_user_configuration({})
end
end
end
Loading