Added dismissible and required features to announcements (#3667)

* Added dismissible and required features to announcements

* Implementation improvements for dismissible and required features for Annoncements

* Added 404 JSON reponse to projects.show method

* Fixed SettingsController read parameters method

* Simplfied SettingsController logic based on feedback from review

apps/dashboard/app/controllers/application_controller.rb

@@ -7,6 +7,13 @@ class ApplicationController < ActionController::Base
   before_action :set_user, :set_user_configuration, :set_pinned_apps, :set_nav_groups, :set_announcements
   before_action :set_my_balances, only: [:index, :new, :featured]
   before_action :set_featured_group, :set_custom_navigation
+  before_action :check_required_announcements
+
+  def check_required_announcements
+    return if instance_of?(SettingsController)
+
+    render inline: '', layout: :default if @announcements.select(&:required?).reject(&:completed?).any?
+  end
 
   def set_user
     @user = CurrentUser

apps/dashboard/app/controllers/settings_controller.rb

@@ -1,9 +1,10 @@
 # frozen_string_literal: true
 
 # The Controller for user level settings /dashboard/settings.
+# Current supported settings: profile, announcement
 class SettingsController < ApplicationController
   include UserSettingStore
-  ALLOWED_SETTINGS = [:profile].freeze
+  ALLOWED_SETTINGS = [:profile, { announcements: {} }].freeze
 
   def update
     new_settings = read_settings(settings_param)
@@ -23,8 +24,6 @@ def settings_param
   end
 
   def read_settings(params)
-    {}.tap do |settings|
-      params&.each { |key, value| settings[key] = value }
-    end
+    params.to_h
   end
 end

apps/dashboard/app/models/announcement.rb

@@ -2,6 +2,7 @@
 
 # Announcements show up on the dashboard to convey a message to users.
 class Announcement
+  include UserSettingStore
   # List of valid announcement types
   TYPES = [:warning, :info, :success, :danger].freeze
 
@@ -31,10 +32,48 @@ def msg
     msg.blank? ? nil : msg
   end
 
+  # The announcement's id. Used when storing that it has been dismissed.
+  #  @return [String] the id
+  def id
+    @id ||= begin
+              default_id = Digest::SHA1.hexdigest(msg) if msg
+              opts.fetch(:id, default_id)
+            end
+  end
+
+  # The announcement's button text displayed for required or dismissible announcements
+  #  @return [String] the button text
+  def button_text
+    default_text = required? ? I18n.t('dashboard.announcements_required_button') : I18n.t('dashboard.announcements_dismissible_button')
+    opts.fetch(:button_text, default_text).to_s
+  end
+
   # Whether this is a valid announcement
   # @return [Boolean] whether it is valid
   def valid?
-    !!msg
+    return false unless msg
+
+    return false if dismissible? && !id
+
+    true
+  end
+
+  # Whether this announcement has been dismissed.
+  # @return [Boolean] whether it has been dismissed
+  def completed?
+    dismissible? && user_settings.dig(:announcements, id.to_s.to_sym).present?
+  end
+
+  # Whether this is a dismissible announcement.
+  # @return [Boolean] whether it is dismissible
+  def dismissible?
+    required? || opts.fetch(:dismissible, true)
+  end
+
+  # Whether this is a required announcement.
+  # @return [Boolean] whether it is required
+  def required?
+    opts.fetch(:required, false)
   end
 
   private

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

@@ -0,0 +1,20 @@
+<% @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') } } }
+          ) do
+            announcement.button_text
+          end
+        %>
+      </div>
+    <% end %>
+  </div>
+<% end %>

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

@@ -63,11 +63,7 @@
 
   <div class="<%= local_assigns[:layout_container_class] || 'container-md' %> content mt-4" role="main">
 
-    <% @announcements.select(&:valid?).each do |announcement| %>
-      <div class="alert alert-<%= announcement.type %> announcement" role="alert">
-        <%= raw OodAppkit.markdown.render(announcement.msg) %>
-      </div>
-    <% end %>
+    <%= render "layouts/announcements" %>
 
     <%= render "layouts/browser_warning" %>
 

apps/dashboard/config/locales/en.yml

@@ -260,6 +260,10 @@ en:
     jobs_scripts_fixed_field: "Fixed Value"
 
     settings_updated: "Settings updated"
+    settings_invalid_request: "Invalid settings submitted"
+
+    announcements_required_button: "Accept"
+    announcements_dismissible_button: "OK"
 
     bc_saved_settings:
       title: "Saved Settings"

apps/dashboard/test/fixtures/config/announcements/announcement_id.yml

@@ -0,0 +1,2 @@
+type: info
+msg: This is yaml

apps/dashboard/test/fixtures/config/announcements/announcement_md.md

@@ -0,0 +1 @@
+This is md

apps/dashboard/test/fixtures/config/announcements/announcement_view.yml

@@ -0,0 +1,4 @@
+type: danger
+msg: This is the announcement.
+id: view_id
+dismissible: true

apps/dashboard/test/fixtures/config/announcements/announcement_yml.yml

@@ -0,0 +1,5 @@
+type: danger
+msg: This is yaml
+id: yml_id
+dismissible: true
+required: true

apps/dashboard/test/fixtures/file_output/user_settings/announcements.yml

@@ -0,0 +1,3 @@
+---
+announcements:
+  completed_id: '2024-07-11 13:37:03'

apps/dashboard/test/integration/announcement_views_test.rb

@@ -1,17 +1,34 @@
 require 'test_helper'
 
 class AnnouncementViewsTest < ActionDispatch::IntegrationTest
-  test "announcement is displayed if exists" do
-    f = Tempfile.open(["announcement", ".md"])
-    f.write %{Test announcement.}
+  test 'announcement is displayed if exists' do
+    f = Tempfile.open(%w[announcement .md])
+    f.write %(Test announcement.)
     f.close
 
-    stub_user_configuration({announcement_path: [f.path]})
+    stub_user_configuration({ announcement_path: [f.path] })
 
     begin
-      get "/"
+      get '/'
       assert_response :success
-      assert_select "div.announcement", "Test announcement."
+      assert_select 'div.announcement div.announcement-body', 'Test announcement.'
+    ensure
+      stub_user_configuration({})
+    end
+  end
+
+  test 'dismissible announcement have a button to close the announcement' do
+    file = "#{Rails.root}/test/fixtures/config/announcements/announcement_view.yml"
+    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']
     ensure
       stub_user_configuration({})
     end

apps/dashboard/test/integration/settings_controller_test.rb

@@ -12,45 +12,102 @@ def setup
     @headers = { 'X-CSRF-Token' => @token }
   end
 
-  test "should save user_settings when posting settings data" do
-    data = {
-      settings: {
-        profile: "test_profile"
-      }
-    }
-    Dir.mktmpdir {|temp_data_dir|
+  test 'should save and override profile settings when posting profile' do
+    Dir.mktmpdir do |temp_data_dir|
       Configuration.stubs(:user_settings_file).returns("#{temp_data_dir}/settings.yml")
+      data = { settings: {} }
 
+      data[:settings][:profile] = 'first_profile'
       post settings_path, params: data, headers: @headers
       assert_response :redirect
-      assert_equal "test_profile", TestUserSettings.new.user_settings[:profile]
-    }
-  end
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
+      assert_equal 'first_profile', TestUserSettings.new.user_settings[:profile]
 
-  test "should not save user settings when no data" do
-    data = { settings: {} }
+      data[:settings][:profile] = 'override_profile'
+      post settings_path, params: data, headers: @headers
+      assert_response :redirect
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
+      assert_equal 'override_profile', TestUserSettings.new.user_settings[:profile]
+    end
+  end
 
-    Dir.mktmpdir {|temp_data_dir|
+  test 'should allow empty or nil profile settings when posting profile' do
+    Dir.mktmpdir do |temp_data_dir|
       Configuration.stubs(:user_settings_file).returns("#{temp_data_dir}/settings.yml")
+      data = { settings: {} }
+
+      data[:settings][:profile] = ''
+      post settings_path, params: data, headers: @headers
+      assert_response :redirect
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
+      assert_equal '', TestUserSettings.new.user_settings[:profile]
+
+      data[:settings][:profile] = nil
       post settings_path, params: data, headers: @headers
       assert_response :redirect
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
       assert_nil TestUserSettings.new.user_settings[:profile]
-    }
+    end
+  end
+
+  test 'should save announcement settings and allow multiple announcements when posting announcement' do
+    Dir.mktmpdir do |temp_data_dir|
+      Configuration.stubs(:user_settings_file).returns("#{temp_data_dir}/settings.yml")
+      data = { settings: {} }
+
+      value =  Time.now.localtime.strftime('%Y-%m-%d %H:%M:%S')
+      data[:settings] = { announcements: { 'announcement_id' => value } }
+      post settings_path, params: data, headers: @headers
+      assert_response :redirect
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
+      assert_equal value, TestUserSettings.new.user_settings[:announcements][:announcement_id]
+
+      data[:settings] = { announcements: { 'other_announcement_id' => value } }
+      post settings_path, params: data, headers: @headers
+      assert_response :redirect
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
+      assert_equal value, TestUserSettings.new.user_settings[:announcements][:announcement_id]
+      assert_equal value, TestUserSettings.new.user_settings[:announcements][:other_announcement_id]
+    end
+  end
+
+  test 'should not save user_settings when no data' do
+    Dir.mktmpdir do |temp_data_dir|
+      Configuration.stubs(:user_settings_file).returns("#{temp_data_dir}/settings.yml")
+      data = { settings: {} }
+
+      post settings_path, params: data, headers: @headers
+      assert_response :redirect
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
+      assert_empty TestUserSettings.new.user_settings
+    end
   end
 
   test "should not save user_settings when parameters are outside the settings namespace" do
-    data = { profile: "root_value" }
+    Dir.mktmpdir do |temp_data_dir|
+      Configuration.stubs(:user_settings_file).returns("#{temp_data_dir}/settings.yml")
+      data = { profile: "root_value" }
 
-    Dir.mktmpdir {|temp_data_dir|
+      post settings_path, params: data, headers: @headers
+      assert_response :redirect
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
+      assert_empty TestUserSettings.new.user_settings
+    end
+  end
+
+  test 'should not save user_settings when parameters are not in the allowed list' do
+    Dir.mktmpdir do |temp_data_dir|
       Configuration.stubs(:user_settings_file).returns("#{temp_data_dir}/settings.yml")
+      data = { settings: { not_allowed: 'root_value' } }
+
       post settings_path, params: data, headers: @headers
       assert_response :redirect
-      assert_nil TestUserSettings.new.user_settings[:profile]
-    }
+      assert_equal I18n.t('dashboard.settings_updated'), flash[:notice]
+      assert_empty TestUserSettings.new.user_settings
+    end
   end
 
   class TestUserSettings
     include UserSettingStore
   end
-
-end
+end