Merge pull request #130 from akiko-pusu/dev

Update version to 0.3.1

README.md

@@ -51,6 +51,11 @@ Please use ver **0.1.x** or ``v0.1.x-support-Redmine3`` branch in case using Red
 
 ## Changelog
 
+### 0.3.1
+
+* Feature: Enabled to switch who can see the global banner. (#126)
+* Refactor: Change to use project menu to prevent the project setting tab's conflict. (#127)
+
 ### 0.3.0
 
 * Add feature: Give the ability to specific users to manage the site-wide banner. (GitHub: #86 / #113)

app/controllers/banner_controller.rb

@@ -31,15 +31,24 @@ def project_banner_off
     render action: '_project_banner_off', layout: false
   end
 
+  def show
+    @banner = Banner.find_or_create(@project.id)
+    render layout: !request.xhr?
+  end
+
   def edit
     return if params[:setting].nil?
 
     @banner = Banner.find_or_create(@project.id)
     @banner.safe_attributes = banner_params
-    @banner.save
-    flash[:notice] = l(:notice_successful_update)
-    redirect_to controller: 'projects',
-                action: 'settings', id: @project, tab: 'banner'
+
+    if @banner.save
+      flash[:notice] = l(:notice_successful_update)
+    else
+      flash[:error] = @banner.errors.messages
+    end
+    redirect_to action: 'show'
+    nil
   end
 
   private

app/models/global_banner.rb

@@ -7,7 +7,7 @@ class GlobalBanner < Setting
   GLOBAL_BANNER_DEFAULT_SETTING = {
     enable: 'false',
     banner_description: 'exp. Information about upcoming Service Interruption.',
-    only_authenticated: nil,
+    display_for: 'all',
     display_only_login_page: nil,
     type: 'info',
     display_part: 'both',

app/views/banner/show.html.erb

@@ -0,0 +1,47 @@
+<% banner = @banner %>
+
+<%= form_tag({ action: 'edit' }, id: 'banner') do %>
+
+<div id="banner_setting'>
+<%= error_messages_for 'banner' %>
+<div class='box tabular'>
+
+<p>
+  <%= content_tag(:label, l(:button_activate))%>
+  <%= hidden_field_tag('setting[enabled]', false).html_safe %>
+  <%= check_box_tag 'setting[enabled]', true, banner['enabled'] %>
+</p>
+
+<p>
+  <%= content_tag(:label, l(:label_message_type))%>
+  <%= radio_button_tag 'setting[style]', 'info', banner['style'] == 'info' %> <span class="banner_info'>Info</span><br/>
+  <%= radio_button_tag 'setting[style]', 'warn', banner['style'] == 'warn' %> <span class='banner_warn'>Warn</span><br/>
+  <%= radio_button_tag 'setting[style]', 'alert', banner['style'] == 'alert' %> <span class='banner_alert'>Alert </span><br/>
+  <%= radio_button_tag 'setting[style]', 'normal', banner['style'] == 'normal' %> <span class='banner_normal'>Normal </span><br/>
+  <%= radio_button_tag 'setting[style]', 'nodata', banner['style'] == 'nodata' %> <span class='banner_normal'>Nodata (Like Redmine.org style) </span>
+</p>
+
+<p>
+<%= content_tag(:label, l(:setting_banner_display_part))%>
+<%= select_tag('setting[display_part]',
+               options_for_select([[l(:page_overview_only), 'overview'],
+                [l(:page_new_issue_only), 'new_issue'],
+                [l(:page_overview_and_issues), 'overview_and_issues'], [l(:page_all), 'all']],
+              banner['display_part'])) %>
+<br/>Now project banner is displayed at the top only, but you can select which page to show banner.
+</p>
+
+<p>
+  <%=content_tag(:label, l(:label_banner_message)) %>
+  <%= text_area_tag 'setting[banner_description]',  banner['banner_description'], size: '40x3',
+        class: 'wiki-edit', required: true  %><br/>
+ <%= wikitoolbar_for "setting_banner_description" %>
+
+<div id='banner_description_preview' class='wiki'></div>
+
+</div>
+<%= submit_tag l(:button_save) %>
+
+<% end %>
+</div>
+

app/views/global_banner/show.html.erb

@@ -12,9 +12,12 @@
     </p>
 
     <p>
-      <%= content_tag(:label, l(:label_display_authenticated_user_only))%>
-      <%= hidden_field_tag('setting[only_authenticated]', false).html_safe %>
-      <%= check_box_tag 'setting[only_authenticated]', true, setting['only_authenticated'] == 'true' %>
+      <%= content_tag(:label, l(:setting_banner_display_for, default: 'Display for'))%>
+      <%= select_tag('setting[display_for]',
+                 options_for_select([[l(:label_anonymous_only, default: 'Anonymous'),'anonymous'],
+                                     [l(:label_authenticated_only, default: 'Authenticated'),'authenticated'],
+                                     [l(:label_display_all, default: 'All'), 'all']],
+                                     { selected: setting['display_for'] })) %>
     </p>
 
     <p>
@@ -29,7 +32,9 @@
       <%= content_tag(:label, l(:setting_banner_display_part))%>
       <%= select_tag('setting[display_part]',
                  options_for_select([[l(:label_header_only),'header'],
-                                     [l(:label_footer_only),'footer'],[l(:label_both),'both']],setting['display_part'])) %>
+                                     [l(:label_footer_only),'footer'],
+                                     [l(:label_both),'both']],
+                                     setting['display_part'])) %>
       <%= hidden_field_tag('setting[display_only_login_page]', false).html_safe %>
       <%= check_box_tag 'setting[display_only_login_page]', true, setting['display_only_login_page'] == 'true' %>
       <%= l(:label_check_if_banner_show_only_login_page) %>

assets/stylesheets/banner.css

@@ -1,4 +1,4 @@
-.banner {
+div.banner {
   border: 1px solid;
   margin: 2px 0;
   padding: 2px 20px;

config/locales/en.yml

@@ -28,3 +28,7 @@ en:
   label_more_info: "Link to more information."
   banner_admin_group: "Banner Admin Group"
   description_for_banner_admin_group: "Specify a group that can manage the global banner without admin rights."
+  setting_banner_display_for: "Display for"
+  label_anonymous_only: "Anonymous users"
+  label_authenticated_only: "Authenticated users"
+  label_display_all: "All"

config/locales/ja.yml

@@ -28,4 +28,7 @@ ja:
   label_more_info: "詳細はこちら。"
   banner_admin_group: "バナー管理者グループ"
   description_for_banner_admin_group: "バナー管理者グループのユーザは、Redmine全体の管理者権限なしでもグローバルバナーを編集できるようになります。"
-
+  setting_banner_display_for: "表示対象"
+  label_anonymous_only: "ログインしていないユーザのみ"
+  label_authenticated_only: "ログインユーザのみ"
+  label_display_all: "全てのユーザ"

config/routes.rb

@@ -7,9 +7,9 @@
   end
 
   resources :projects do
-    resources :banner do
-      patch 'edit', on: :member
-      put 'edit', on: :collection
+    resources :banner, only: %i[show] do
+      get '/', to: 'banner#show', on: :collection
+      post '/', to: 'banner#edit', on: :collection
       post 'project_banner_off', on: :collection
       get 'project_banner_off', on: :collection
     end

init.rb

@@ -3,7 +3,6 @@
 require 'redmine'
 require_relative 'app/models/global_banner'
 require 'banners/application_hooks'
-require 'banners/projects_helper_patch'
 
 # NOTE: Keep error message for a while to support Redmine3.x users.
 def banner_version_message(original_message = nil)
@@ -21,13 +20,17 @@ def banner_admin?
   GlobalBanner.banner_admin?(User.current)
 end
 
+def project_menu_allowed?
+  proc { |p| User.current.allowed_to?({ controller: 'banner', action: 'show' }, p) }
+end
+
 Redmine::Plugin.register :redmine_banner do
   begin
     name 'Redmine Banner plugin'
     author 'Akiko Takano'
     author_url 'http://twitter.com/akiko_pusu'
     description 'Plugin to show site-wide message, such as maintenacne informations or notifications.'
-    version '0.3.0'
+    version '0.3.1'
     requires_redmine version_or_higher: '4.0'
     url 'https://github.com/akiko-pusu/redmine_banner'
 
@@ -37,6 +40,8 @@ def banner_admin?
     menu :top_menu, :redmine_banner, { controller: 'global_banner', action: 'show', "id": nil }, caption: :banner,
                                                                                                  last: true,
                                                                                                  if: proc { banner_admin? }
+    menu :project_menu, :banner, { controller: 'banner', action: 'show', "id": nil },
+         caption: :banner, param: :project_id, after: :settings, if: project_menu_allowed?
 
     project_module :banner do
       permission :manage_banner, { banner: %I[show edit project_banner_off] }, require: :member

lib/banners/application_hooks.rb

@@ -91,9 +91,17 @@ def should_display_on_current_page?(context, setting)
           (context[:controller].action_name != 'login')) &&
                       (setting['display_only_login_page'] == 'true')
 
-      return false if !User.current.logged? && setting['only_authenticated'] == 'true'
+      should_display_for?(setting)
+    end
+
+    def should_display_for?(setting)
+      target = setting['display_for'] || 'all'
+      return true if target == 'all'
+
+      return true if target == 'authenticated' && User.current.logged?
+      return true if target == 'anonymous' && User.current.anonymous?
 
-      true
+      false
     end
   end
 end

script/swagger.yml

@@ -119,6 +119,15 @@ definitions:
       banner_description:
         type: string
         example: "Message for Global Banner"
+      display_for:
+        type: "string"
+        example: "all"
+        enum: [all, anonymous, authenticated]
+        description: >
+            Display for:
+             * `all` - For all users
+             * `anonymous` - For anonymous users
+             * `authenticated` - For authenticated users
       display_part:
         type: "string"
         example: "both"

test/controller/api_global_banner_controller_test.rb

@@ -20,7 +20,7 @@ def test_routing
   def test_get_global_banner_when_api_disable
     Setting.rest_api_enabled = '0'
     get '/banners/api/global_banner.json', headers: { 'X-Redmine-API-Key' => @user.api_key }
-    assert_response 403
+    assert_response :forbidden
   end
 
   def test_get_global_banner_when_api_enable

test/functional/banner_controller_test.rb

@@ -90,8 +90,7 @@ def test_redirect_post
                           setting: { enabled: '1', description: 'Edit test',
                                      display_part: 'all', style: 'alert' } }
     assert_response :redirect
-    assert_redirected_to controller: 'projects',
-                         action: 'settings', id: @project, tab: 'banner'
+    assert_redirected_to controller: 'banner', action: 'show'
   end
 
   def test_return_success_when_banner_off_with_manage_permission

test/functional/projects_controller_test.rb

@@ -28,26 +28,20 @@ def setup
     @banner.save!
   end
 
-  def test_settings
-    get :settings, params: { id: 1 }
-    assert_response :success
-    assert_select 'a#tab-banner'
-    assert_select 'div#project_banner_area div.banner_info', false,
-                  'Banner should be displayed Overview only.'
-  end
-
   # project 1 is enabled banner and type is info, display_part is overview only.
   def test_show_overview
     get :show, params: { id: 1 }
     assert_response :success
-    assert_select 'div#project_banner_area div.banner_info'
+    assert_select '#main-menu ul li a.banner'
+    assert_select 'div#project_banner_area div.banner_info', true,
+                  'Banner should be displayed Overview only.'
   end
 
   def test_show_all
     @banner.display_part = 'all'
     @banner.style = 'warn'
     @banner.save!
-    get :settings, params: { id: 1 }
+    get :show, params: { id: 1 }
     assert_response :success
     assert_select 'div#project_banner_area div.banner_warn'
   end

test/integration/layout_test.rb

@@ -36,8 +36,8 @@ def test_project_banner_visible_when_module_on
     get '/projects/ecookbook/issues'
     assert_select 'div#project_banner_area div.banner_info', 0
 
-    put '/projects/ecookbook/banner/edit',
-        params: { setting: { enabled: '1', style: 'warn', display_part: 'all', banner_description: 'Test banner message.' }, project_id: 'ecookbook' }
+    post '/projects/ecookbook/banner',
+         params: { setting: { enabled: '1', style: 'warn', display_part: 'all', banner_description: 'Test banner message.' }, project_id: 'ecookbook' }
     assert_response :redirect
 
     get '/projects/ecookbook/issues'
@@ -75,8 +75,7 @@ def test_display_only_for_login_page
            enable: 'true', type: 'warn', display_part: 'both',
            use_timer: 'false',
            banner_description: 'h1. Test data.',
-           display_only_login_page: 'true',
-           only_authenticated: 'true'
+           display_for: 'authenticated'
          } }
 
     # Session is cleared
@@ -100,11 +99,12 @@ def test_display_more_link
            use_timer: 'false',
            banner_description: 'h1. Test data.',
            display_only_login_page: 'false',
-           only_authenticated: 'false',
+           display_for: 'authenticated',
            related_link: ''
          } }
 
     get '/'
+    assert_select 'div.banner_area', 1
     assert_select 'div.banner_more_info', 0
 
     # Update setting.
@@ -114,7 +114,7 @@ def test_display_more_link
            use_timer: 'false',
            banner_description: 'h1. Test data.',
            display_only_login_page: 'false',
-           only_authenticated: 'false',
+           display_for: 'all',
            related_link: 'http://www.redmine.org/'
          } }