Refactoring: Not to use project setting tab but to use project menu.

Related to #127
Update test code and css.

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/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>
+

assets/stylesheets/banner.css

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

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,6 +20,10 @@ 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'
@@ -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

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'