Add invalid announcements error widget and add related test (#2568)#5250
Add invalid announcements error widget and add related test (#2568)#5250bstepanovski wants to merge 3 commits intomasterfrom
Conversation
Bubballoo3
left a comment
There was a problem hiding this comment.
The model updates here look good, but we are handling a bit too much logic in the view side. By checking validity in the controller and raising any errors from there, we will save a lot of code and headache in the views.
It looks like some of the other announcement tests are failing, so you will probably want to add an announcement to your dev dashboard to test it out manually and see what's going on. If you follow the trace of where the announcement path comes from, you'll see in UserConfiguration#announcement_path that it will read from an OOD_ANNOUNCEMENT_PATH environment variable before looking in the default locations. So you should be able to set this in a .env.local and point it to a file in your home directory.
| @@ -0,0 +1,24 @@ | |||
| require 'test_helper' | |||
|
|
|||
| class AnnouncementParseErrorTest < ActionDispatch::IntegrationTest | |||
There was a problem hiding this comment.
We can just add this to announcement_views_test to keep things in one place
| <% end %> | ||
| </div> | ||
| <% end %> No newline at end of file | ||
| <% if announcement.parse_error? %> |
There was a problem hiding this comment.
We typically want to do error checking in the controller (ApplicationController#set_announcements in this case)
| @@ -0,0 +1,8 @@ | |||
| <div class="alert alert-danger card"> | |||
There was a problem hiding this comment.
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.

Fixes #2658
Also manually tested in the dashboard with both valid and invalid announcements.