From 91a459ee20ab9b42448a3c800f28bf70ae097471 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Thu, 19 Dec 2019 11:40:35 +0100 Subject: [PATCH 1/4] Add forum management in "Forums" module --- app/controllers/forums_controller.rb | 12 +++---- app/views/forums/edit.html.erb | 2 +- app/views/forums/index.html.erb | 52 +++++++++++++++++++++++++++- 3 files changed, 56 insertions(+), 10 deletions(-) diff --git a/app/controllers/forums_controller.rb b/app/controllers/forums_controller.rb index c6d03c192d..5ee64e2044 100644 --- a/app/controllers/forums_controller.rb +++ b/app/controllers/forums_controller.rb @@ -96,7 +96,7 @@ class ForumsController < ApplicationController def create if @forum.save flash[:notice] = l(:notice_successful_create) - redirect_to_settings_in_projects + redirect_to action: 'index' else render :new end @@ -107,7 +107,7 @@ class ForumsController < ApplicationController def update if @forum.update(permitted_params.forum) flash[:notice] = l(:notice_successful_update) - redirect_to_settings_in_projects + redirect_to action: 'index' else render :edit end @@ -120,21 +120,17 @@ class ForumsController < ApplicationController flash.now[:error] = t('forum_could_not_be_saved') render action: 'edit' end - redirect_to_settings_in_projects(@forum.project_id) + redirect_to action: 'index' end def destroy @forum.destroy flash[:notice] = l(:notice_successful_delete) - redirect_to_settings_in_projects + redirect_to action: 'index' end private - def redirect_to_settings_in_projects(id = @project) - redirect_to controller: '/project_settings', action: 'show', id: id, tab: 'forums' - end - def find_forum @forum = @project.forums.find(params[:id]) rescue ActiveRecord::RecordNotFound diff --git a/app/views/forums/edit.html.erb b/app/views/forums/edit.html.erb index d7ddd88d99..bcff16b45b 100644 --- a/app/views/forums/edit.html.erb +++ b/app/views/forums/edit.html.erb @@ -31,5 +31,5 @@ See docs/COPYRIGHT.rdoc for more details. <%= labelled_tabular_form_for [@project, @forum] do |f| %> <%= render partial: 'form', locals: { f: f } %> <%= f.button t(:button_save), class: 'button -highlight -with-icon icon-checkmark' %> - <%= link_to t(:button_cancel), project_forum_path(@project, @forum), class: 'button -with-icon icon-cancel' %> + <%= link_to t(:button_cancel), project_forums_path(@project), class: 'button -with-icon icon-cancel' %> <% end %> diff --git a/app/views/forums/index.html.erb b/app/views/forums/index.html.erb index c8af86e9dd..017d321974 100644 --- a/app/views/forums/index.html.erb +++ b/app/views/forums/index.html.erb @@ -27,7 +27,19 @@ See docs/COPYRIGHT.rdoc for more details. ++#%> <% html_title t(:label_forum_plural) %> -<%= toolbar title: t(:label_forum_plural) %> +<%= toolbar title: t(:label_forum_plural) do %> + <% if User.current.allowed_to?(:manage_forums, @project) %> +
  • + <%= link_to({ controller: '/forums', action: 'new', project_id: @project }, + { aria: { label: t(:label_forum_new) }, + title: t(:label_forum_new), + class: 'button -highlight' }) do %> + <%= op_icon('button--icon icon-add') %> + <%= t('activerecord.models.forum') %> + <% end %> +
  • + <% end %> +<% end %>
    @@ -36,6 +48,10 @@ See docs/COPYRIGHT.rdoc for more details. + <% if User.current.allowed_to?(:manage_forums, @project) %> + + + <% end %> @@ -75,6 +91,18 @@ See docs/COPYRIGHT.rdoc for more details. + <% if User.current.allowed_to?(:manage_forums, @project) %> + + + <% end %> @@ -93,6 +121,28 @@ See docs/COPYRIGHT.rdoc for more details. <%= link_to_message forum.last_message, no_root: true %> <% end %> + <% if User.current.allowed_to?(:manage_forums, @project) %> + + + <% end %> <% end %> From 73a03c07f821134779712398beffd778a0f86768 Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Thu, 19 Dec 2019 11:41:59 +0100 Subject: [PATCH 2/4] Delete forum from project settings as it is now within the "Forums" module --- app/helpers/projects_helper.rb | 6 - app/views/project_settings/_forums.html.erb | 136 -------------------- 2 files changed, 142 deletions(-) delete mode 100644 app/views/project_settings/_forums.html.erb diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 95bff11f2e..b26df63453 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -79,12 +79,6 @@ module ProjectsHelper partial: 'repositories/settings', label: :label_repository }, - { - name: 'forums', - action: :manage_forums, - partial: 'project_settings/forums', - label: :label_forum_plural - }, { name: 'activities', action: :manage_project_activities, diff --git a/app/views/project_settings/_forums.html.erb b/app/views/project_settings/_forums.html.erb deleted file mode 100644 index d9ee51e2c4..0000000000 --- a/app/views/project_settings/_forums.html.erb +++ /dev/null @@ -1,136 +0,0 @@ -<%#-- copyright -OpenProject is a project management system. -Copyright (C) 2012-2018 the OpenProject Foundation (OPF) - -This program is free software; you can redistribute it and/or -modify it under the terms of the GNU General Public License version 3. - -OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: -Copyright (C) 2006-2017 Jean-Philippe Lang -Copyright (C) 2010-2013 the ChiliProject Team - -This program is free software; you can redistribute it and/or -modify it under the terms of the GNU General Public License -as published by the Free Software Foundation; either version 2 -of the License, or (at your option) any later version. - -This program is distributed in the hope that it will be useful, -but WITHOUT ANY WARRANTY; without even the implied warranty of -MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -GNU General Public License for more details. - -You should have received a copy of the GNU General Public License -along with this program; if not, write to the Free Software -Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. - -See docs/COPYRIGHT.rdoc for more details. - -++#%> - -<% if @project.forums.any? %> -
    - - <%= l(:label_available_project_forums) %> - -
    - - <%= link_to_if_authorized({ controller: '/forums', action: 'new', project_id: @project }, - { aria: { label: t(:label_forum_new) }, - title: t(:label_forum_new) }) do %> - <%= op_icon('icon-add icon-small') %> - <%= t('activerecord.models.forum') %> - <% end %> - -
    -
    -
    -
    +
    +
    + + <%=l(:button_sort)%> + +
    +
    +
    + <% if authorize_for 'forums', 'edit' %> + <%= reorder_links 'forum', controller: '/forums', + action: 'move', + project_id: @project, + id: forum %> + <% end %> + + <%= link_to_if_authorized '', + { controller: '/forums', action: 'edit', project_id: @project, id: forum }, + class: 'icon icon-edit', + title: t(:button_edit) %> + <%= link_to_if_authorized '', + { controller: '/forums', action: 'destroy', project_id: @project, id: forum }, + data: { confirm: l(:text_are_you_sure) }, + method: :delete, + class: 'icon icon-delete', + title: t(:button_delete) %> +
    - - - - - - - - - - - - - - - - <% @project.forums.each do |forum| - next if forum.new_record? %> - - - - - - - <% end %> - -
    -
    -
    - - <%= Forum.model_name.human %> - -
    -
    -
    -
    -
    - - <%= Forum.human_attribute_name(:description) %> - -
    -
    -
    -
    -
    - - <%=l(:button_sort)%> - -
    -
    -
    <%=h forum.name %><%= format_text(forum.description) %> - <% if authorize_for 'forums', 'edit' %> - <%= reorder_links 'forum', controller: '/forums', - action: 'move', - project_id: @project, - id: forum %> - <% end %> - - <%= link_to_if_authorized '', - { controller: '/forums', action: 'edit', project_id: @project, id: forum }, - class: 'icon icon-edit', - title: t(:button_edit) %> - <%= link_to_if_authorized '', - { controller: '/forums', action: 'destroy', project_id: @project, id: forum }, - data: { confirm: l(:text_are_you_sure) }, - method: :delete, - class: 'icon icon-delete', - title: t(:button_delete) %> -
    - -
    -
    - <% if @project.forums.any? %> -
    - <%= link_to_if_authorized({ controller: '/forums', action: 'new', project_id: @project }, - { class: 'button -alt-highlight', - aria: {label: t(:label_forum_new)}, - title: t(:label_forum_new)}) do %> - <%= op_icon('button--icon icon-add') %> - <%= t('activerecord.models.forum') %> - <% end %> -
    - <% end %> - <% else %> - <%= no_results_box action_url: new_project_forum_path(@project), - display_action: authorize_for('forums', 'new'), - custom_title: t('projects.settings.forums.no_results_title_text'), - custom_action_text: t('projects.settings.forums.no_results_content_text') %> - <% end %> - From 35cb01b9feb6363773d1d0eae27f76cc74648def Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Thu, 19 Dec 2019 13:40:53 +0100 Subject: [PATCH 3/4] Show forum module although there are no forums --- app/controllers/forums_controller.rb | 6 - app/views/forums/index.html.erb | 197 ++++++++++++++------------- config/initializers/menus.rb | 1 - config/locales/en.yml | 1 + 4 files changed, 103 insertions(+), 102 deletions(-) diff --git a/app/controllers/forums_controller.rb b/app/controllers/forums_controller.rb index 5ee64e2044..e4b320b4ea 100644 --- a/app/controllers/forums_controller.rb +++ b/app/controllers/forums_controller.rb @@ -41,12 +41,6 @@ class ForumsController < ApplicationController def index @forums = @project.forums - render_404 if @forums.empty? - # show the forum if there is only one - if @forums.size == 1 - @forum = @forums.first - show - end end current_menu_item [:index, :show] do diff --git a/app/views/forums/index.html.erb b/app/views/forums/index.html.erb index 017d321974..345370933b 100644 --- a/app/views/forums/index.html.erb +++ b/app/views/forums/index.html.erb @@ -40,116 +40,123 @@ See docs/COPYRIGHT.rdoc for more details. <% end %> <% end %> -
    -
    - - - - - - - <% if User.current.allowed_to?(:manage_forums, @project) %> + +<% if @forums.empty? %> + <%= no_results_box(action_url: new_project_forum_path(@project), + display_action: true, + custom_action_text: t('forums.no_results_content_text')) %> +<% else %> +
    +
    +
    + - - <% end %> - - - - - + + + <% if User.current.allowed_to?(:manage_forums, @project) %> + + + <% end %> + + + + - + - + - <% if User.current.allowed_to?(:manage_forums, @project) %> + - - <% end %> - - - - <% for forum in @forums %> - - - - - <% if User.current.allowed_to?(:manage_forums, @project) %> - + + <% end %> + + + + <% for forum in @forums %> + + - + + - <% end %> - - <% end %> - -
    -
    -
    - - <%= Forum.name.humanize %> - -
    -
    -
    -
    -
    - - <%= t(:label_topic_plural) %> - +
    +
    +
    + + <%= Forum.name.humanize %> + +
    - -
    -
    -
    - - <%= t(:label_message_plural) %> - +
    +
    +
    + + <%= t(:label_topic_plural) %> + +
    - -
    -
    -
    - - <%= t(:label_message_last) %> - +
    +
    +
    + + <%= t(:label_message_plural) %> + +
    - -
    - - <%=l(:button_sort)%> - + + <%= t(:label_message_last) %> +
    - <%= link_to h(forum.name), { action: 'show', id: forum }, class: "forum" %>
    - <%= format_text(forum.description) %> -
    <%= forum.topics_count %><%= forum.messages_count %> - <% if forum.last_message %> - <% forum.last_message %> - <%= authoring forum.last_message.created_on, forum.last_message.author %>
    - <%= link_to_message forum.last_message, no_root: true %> - <% end %> -
    - <% if authorize_for 'forums', 'edit' %> - <%= reorder_links 'forum', controller: '/forums', - action: 'move', - project_id: @project, - id: forum %> - <% end %> + +
    +
    + + <%=l(:button_sort)%> + +
    +
    +
    + <%= link_to h(forum.name), { action: 'show', id: forum }, class: "forum" %>
    + <%= format_text(forum.description) %>
    - <%= link_to_if_authorized '', - { controller: '/forums', action: 'edit', project_id: @project, id: forum }, - class: 'icon icon-edit', - title: t(:button_edit) %> - <%= link_to_if_authorized '', - { controller: '/forums', action: 'destroy', project_id: @project, id: forum }, - data: { confirm: l(:text_are_you_sure) }, - method: :delete, - class: 'icon icon-delete', - title: t(:button_delete) %> + <%= forum.topics_count %><%= forum.messages_count %> + <% if forum.last_message %> + <% forum.last_message %> + <%= authoring forum.last_message.created_on, forum.last_message.author %>
    + <%= link_to_message forum.last_message, no_root: true %> + <% end %>
    + <% if User.current.allowed_to?(:manage_forums, @project) %> + + <% if authorize_for 'forums', 'edit' %> + <%= reorder_links 'forum', controller: '/forums', + action: 'move', + project_id: @project, + id: forum %> + <% end %> + + + <%= link_to_if_authorized '', + { controller: '/forums', action: 'edit', project_id: @project, id: forum }, + class: 'icon icon-edit', + title: t(:button_edit) %> + <%= link_to_if_authorized '', + { controller: '/forums', action: 'destroy', project_id: @project, id: forum }, + data: { confirm: l(:text_are_you_sure) }, + method: :delete, + class: 'icon icon-delete', + title: t(:button_delete) %> + + <% end %> + + <% end %> + + +
    - +<% end %> <%= other_formats_links do |f| %> <%= f.link_to 'Atom', url: { controller: '/activities', action: 'index', id: @project, show_messages: 1, key: User.current.rss_key } %> <% end %> diff --git a/config/initializers/menus.rb b/config/initializers/menus.rb index 7681b9cbc1..a6c8fdd540 100644 --- a/config/initializers/menus.rb +++ b/config/initializers/menus.rb @@ -336,7 +336,6 @@ Redmine::MenuManager.map :project_menu do |menu| menu.push :forums, { controller: '/forums', action: 'index', id: nil }, param: :project_id, - if: Proc.new { |p| p.forums.any? }, caption: :label_forum_plural, icon: 'icon2 icon-ticket-note' diff --git a/config/locales/en.yml b/config/locales/en.yml index 615901b2b1..ed1efbaece 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -125,6 +125,7 @@ en: For more information, visit the Net::LDAP documentation. forums: + no_results_content_text: Create a new forum show: no_results_title_text: There are currently no posts for the forum. From 97a2f8477b1b899ff961fac55184bc8d52134e6b Mon Sep 17 00:00:00 2001 From: Henriette Dinger Date: Thu, 19 Dec 2019 15:19:56 +0100 Subject: [PATCH 4/4] Adapt tests to new forum redirects && remove doubled possibility to add a new forum --- app/views/forums/index.html.erb | 4 +-- config/locales/en.yml | 1 - spec/controllers/forums_controller_spec.rb | 28 ++++++++++--------- .../features/forums/attachment_upload_spec.rb | 1 + spec/features/forums/message_spec.rb | 4 +++ spec/features/forums/sticky_spec.rb | 4 +-- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/app/views/forums/index.html.erb b/app/views/forums/index.html.erb index 345370933b..3f0e996f51 100644 --- a/app/views/forums/index.html.erb +++ b/app/views/forums/index.html.erb @@ -42,9 +42,7 @@ See docs/COPYRIGHT.rdoc for more details. <% end %> <% if @forums.empty? %> - <%= no_results_box(action_url: new_project_forum_path(@project), - display_action: true, - custom_action_text: t('forums.no_results_content_text')) %> + <%= no_results_box(action_url: new_project_forum_path(@project)) %> <% else %>
    diff --git a/config/locales/en.yml b/config/locales/en.yml index ed1efbaece..615901b2b1 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -125,7 +125,6 @@ en: For more information, visit the Net::LDAP documentation. forums: - no_results_content_text: Create a new forum show: no_results_title_text: There are currently no posts for the forum. diff --git a/spec/controllers/forums_controller_spec.rb b/spec/controllers/forums_controller_spec.rb index cee691b69d..1ab9853930 100644 --- a/spec/controllers/forums_controller_spec.rb +++ b/spec/controllers/forums_controller_spec.rb @@ -49,7 +49,7 @@ describe ForumsController, type: :controller do end expect(response).to be_successful - expect(response).to render_template 'forums/show' + expect(response).to render_template 'forums/index' expect(assigns(:forums)).to be_present expect(assigns(:project)).to be_present end @@ -113,12 +113,11 @@ describe ForumsController, type: :controller do end end - it 'should redirect to the settings page if successful' do + it 'should redirect to the index page if successful' do expect(response) - .to redirect_to controller: '/project_settings', - action: 'show', - id: project, - tab: 'forums' + .to redirect_to controller: '/forums', + action: 'index', + project_id: project.id end it 'have a successful creation flash' do @@ -178,7 +177,6 @@ describe ForumsController, type: :controller do describe '#higher' do let(:move_to) { 'higher' } - let(:redirect_url) { "http://test.host/projects/#{project.id}/settings/forums" } before do post 'move', params: { id: forum_2.id, @@ -190,7 +188,12 @@ describe ForumsController, type: :controller do it do expect(response).to be_redirect end - it do expect(response).to redirect_to(redirect_url) end + it do + expect(response) + .to redirect_to controller: '/forums', + action: 'index', + project_id: project.id + end end end @@ -213,11 +216,10 @@ describe ForumsController, type: :controller do end end - it 'should redirect to the settings page if successful' do - expect(response).to redirect_to controller: '/project_settings', - action: 'show', - id: forum.project, - tab: 'forums' + it 'should redirect to the index page if successful' do + expect(response).to redirect_to controller: '/forums', + action: 'index', + project_id: forum.project_id end it 'have a successful update flash' do diff --git a/spec/features/forums/attachment_upload_spec.rb b/spec/features/forums/attachment_upload_spec.rb index 5e9142201d..8b8a6a06b0 100644 --- a/spec/features/forums/attachment_upload_spec.rb +++ b/spec/features/forums/attachment_upload_spec.rb @@ -52,6 +52,7 @@ describe 'Upload attachment to forum message', js: true do it 'can upload an image to new and existing messages via drag & drop' do index_page.visit! + click_link forum.name create_page = index_page.click_create_message create_page.set_subject 'A new message' diff --git a/spec/features/forums/message_spec.rb b/spec/features/forums/message_spec.rb index ea3fc12213..b5eed42666 100644 --- a/spec/features/forums/message_spec.rb +++ b/spec/features/forums/message_spec.rb @@ -51,6 +51,7 @@ describe 'messages', type: :feature, js: true do scenario 'adding, checking replies, replying' do index_page.visit! + click_link forum.name create_page = index_page.click_create_message @@ -68,6 +69,7 @@ describe 'messages', type: :feature, js: true do show_page.expect_content('There is no message here') index_page.visit! + click_link forum.name index_page.expect_listed(subject: 'The message is', replies: 0) @@ -86,6 +88,7 @@ describe 'messages', type: :feature, js: true do content: 'But, but there should be one') index_page.visit! + click_link forum.name index_page.expect_listed(subject: 'The message is', replies: 1, @@ -109,6 +112,7 @@ describe 'messages', type: :feature, js: true do expect(page).to have_selector('blockquote', text: 'But, but there should be one') index_page.visit! + click_link forum.name index_page.expect_listed(subject: 'The message is', replies: 2, last_message: 'And now to something completely different') diff --git a/spec/features/forums/sticky_spec.rb b/spec/features/forums/sticky_spec.rb index 2a2537fb3e..f0017a7b8f 100644 --- a/spec/features/forums/sticky_spec.rb +++ b/spec/features/forums/sticky_spec.rb @@ -56,7 +56,7 @@ describe 'sticky messages', type: :feature do before do login_as user - visit project_forums_path(forum.project) + visit project_forum_path(forum.project, forum) end def expect_order_of_messages(*order) @@ -75,7 +75,7 @@ describe 'sticky messages', type: :feature do check('message[sticky]') click_button('Save') - visit project_forums_path(forum.project) + visit project_forum_path(forum.project, forum) expect_order_of_messages(message2, message1, message3) end