Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added dismissible and required features to announcements #3667

Merged
merged 5 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
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
7 changes: 7 additions & 0 deletions apps/dashboard/app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion apps/dashboard/app/controllers/projects_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ def show
project_id = show_project_params[:id]
@project = Project.find(project_id)
if @project.nil?
redirect_to(projects_path, alert: I18n.t('dashboard.jobs_project_not_found', project_id: project_id))
respond_to do |format|
message = I18n.t('dashboard.jobs_project_not_found', project_id: project_id)
format.html { redirect_to(projects_path, alert: message) }
format.json { render json: { message: message }, status: :not_found }
end
else
@scripts = Launcher.all(@project.directory)
@valid_project = Launcher.clusters?
Expand Down
7 changes: 3 additions & 4 deletions apps/dashboard/app/controllers/settings_controller.rb
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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
41 changes: 40 additions & 1 deletion apps/dashboard/app/models/announcement.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
20 changes: 20 additions & 0 deletions apps/dashboard/app/views/layouts/_announcements.html.erb
Original file line number Diff line number Diff line change
@@ -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 %>
6 changes: 1 addition & 5 deletions apps/dashboard/app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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" %>

Expand Down
4 changes: 4 additions & 0 deletions apps/dashboard/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
type: info
msg: This is yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This is md
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type: danger
msg: This is the announcement.
id: view_id
dismissible: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: danger
msg: This is yaml
id: yml_id
dismissible: true
required: true
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
announcements:
completed_id: '2024-07-11 13:37:03'
29 changes: 23 additions & 6 deletions apps/dashboard/test/integration/announcement_views_test.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down
97 changes: 77 additions & 20 deletions apps/dashboard/test/integration/settings_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading
Loading