From 8d5382f03e4f57e700d66c56accf97c661ca1a7a Mon Sep 17 00:00:00 2001 From: Aaron Contreras Date: Mon, 5 Jun 2023 18:05:44 -0500 Subject: [PATCH] Extend `meeting#new` form to allow creating "global" meetings The `meeting#new` form toggles as necessary between the project-scoped version and the global version. **TODO**: When a project is selected, I believe because @project is present, that the sidebar and top-menu project dropdown get populated with the project data as if it were now scoped. Some work needs to go in to perhaps modifying the instance variable name so that this doesn't get toggled but I'll ask if there's a pattern for this already in mind before going down that rabbit-hole. --- app/controllers/application_controller.rb | 4 +- .../refresh-on-form-changes.controller.ts | 27 +++- .../meetings/add_button_component.html.erb | 10 ++ .../meetings/add_button_component.rb | 78 ++++++++++ .../app/controllers/base_controller.rb | 77 ++++++++++ .../app/controllers/meetings_controller.rb | 86 ++--------- .../projects/meetings_controller.rb | 109 +++++++++++++ .../meeting/app/helpers/meetings_helper.rb | 8 + .../meeting/app/views/meetings/edit.html.erb | 12 +- .../meeting/app/views/meetings/index.html.erb | 23 +-- .../meeting/app/views/meetings/new.html.erb | 9 +- .../meeting/app/views/meetings/show.html.erb | 10 +- .../views/projects/meetings/index.html.erb | 30 ++++ .../app/views/projects/meetings/new.html.erb | 30 ++++ .../{ => shared}/meetings/_form.html.erb | 19 ++- .../app/views/shared/meetings/_index.html.erb | 40 +++++ .../app/views/shared/meetings/_new.html.erb | 43 ++++++ modules/meeting/config/locales/en.yml | 4 + modules/meeting/config/routes.rb | 4 +- .../lib/open_project/meeting/engine.rb | 8 +- .../controllers/meetings_controller_spec.rb | 69 +-------- .../projects/meetings_controller_spec.rb | 144 ++++++++++++++++++ .../spec/features/meetings_index_spec.rb | 4 +- .../spec/features/meetings_new_spec.rb | 132 ++++++++++++---- .../spec/support/pages/meetings/new.rb | 4 + 25 files changed, 767 insertions(+), 217 deletions(-) create mode 100644 modules/meeting/app/components/meetings/add_button_component.html.erb create mode 100644 modules/meeting/app/components/meetings/add_button_component.rb create mode 100644 modules/meeting/app/controllers/base_controller.rb create mode 100644 modules/meeting/app/controllers/projects/meetings_controller.rb create mode 100644 modules/meeting/app/views/projects/meetings/index.html.erb create mode 100644 modules/meeting/app/views/projects/meetings/new.html.erb rename modules/meeting/app/views/{ => shared}/meetings/_form.html.erb (89%) create mode 100644 modules/meeting/app/views/shared/meetings/_index.html.erb create mode 100644 modules/meeting/app/views/shared/meetings/_new.html.erb create mode 100644 modules/meeting/spec/controllers/projects/meetings_controller_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index eb315c387683..c5525dfec624 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -249,11 +249,11 @@ def authorize_global # * a parameter-like Hash (eg. { controller: '/projects', action: 'edit' }) # * a permission Symbol (eg. :edit_project) def do_authorize(action, global: false) - context = @project || @projects + context = @current_project || @project || @projects is_authorized = User.current.allowed_to?(action, context, global:) unless is_authorized - if @project&.archived? + if (@current_project || @project)&.archived? render_403 message: :notice_not_authorized_archived_project else deny_access diff --git a/frontend/src/stimulus/controllers/refresh-on-form-changes.controller.ts b/frontend/src/stimulus/controllers/refresh-on-form-changes.controller.ts index 43ad1ec3de4b..4b55ba7a53bb 100644 --- a/frontend/src/stimulus/controllers/refresh-on-form-changes.controller.ts +++ b/frontend/src/stimulus/controllers/refresh-on-form-changes.controller.ts @@ -37,17 +37,42 @@ export default class RefreshOnFormChangesController extends ApplicationControlle static values = { refreshUrl: String, + preserveParams: Boolean, }; declare readonly formTarget:HTMLFormElement; declare refreshUrlValue:string; + declare preserveParamsValue:boolean; triggerReload():void { // without the cast to undefined, the URLSearchParams constructor will // not accept the FormData object. const formData = new FormData(this.formTarget) as unknown as undefined; - const serializedFormData = new URLSearchParams(formData).toString(); + const formParams = new URLSearchParams(formData); + const currentParams = new URLSearchParams(window.location.search); + const mergedParams = this.mergeQueryParams(currentParams, formParams); + + const serializedFormData = mergedParams.toString(); + window.location.href = `${this.refreshUrlValue}?${serializedFormData}`; } + + // Merge the form's submitted params onto the currently present + // query string parameters on the URL. + // + // This preserves scope-dependent form data that would painfully + // get reset in some reload scenarios when calling triggerReload + // from a dependent form field. + private mergeQueryParams(currentParams:URLSearchParams, newParams:URLSearchParams) { + if (!this.preserveParamsValue) { + return newParams; + } + + newParams.forEach((value, key) => { + currentParams.set(key, value); + }); + + return currentParams; + } } diff --git a/modules/meeting/app/components/meetings/add_button_component.html.erb b/modules/meeting/app/components/meetings/add_button_component.html.erb new file mode 100644 index 000000000000..9ba9cfb898b7 --- /dev/null +++ b/modules/meeting/app/components/meetings/add_button_component.html.erb @@ -0,0 +1,10 @@ +
  • + + <%= icon %> + <%= label %> + +
    • diff --git a/modules/meeting/app/components/meetings/add_button_component.rb b/modules/meeting/app/components/meetings/add_button_component.rb new file mode 100644 index 000000000000..91df6c1a3698 --- /dev/null +++ b/modules/meeting/app/components/meetings/add_button_component.rb @@ -0,0 +1,78 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2023 the OpenProject GmbH +# +# 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-2013 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 COPYRIGHT and LICENSE files for more details. +# ++ +# + +module Meetings + class AddButtonComponent < ::RailsComponent + options :current_project + + def render? + if current_project + User.current.allowed_to?(:create_meetings, current_project) + else + User.current.allowed_to_globally?(:create_meetings) + end + end + + def li_css_class + 'toolbar-item' + end + + def dynamic_path + polymorphic_path([:new, current_project, :meeting]) + end + + def id + 'add-meeting-button' + end + + def title + I18n.t(:label_meeting_new) + end + + def aria_label + I18n.t(:label_meeting_new) + end + + def link_css_class + 'button -alt-highlight' + end + + def label + content_tag(:span, + I18n.t(:label_meeting), + class: 'button--text') + end + + def icon + helpers.op_icon('button--icon icon-add') + end + end +end diff --git a/modules/meeting/app/controllers/base_controller.rb b/modules/meeting/app/controllers/base_controller.rb new file mode 100644 index 000000000000..5359f005b2f9 --- /dev/null +++ b/modules/meeting/app/controllers/base_controller.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +# -- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2023 the OpenProject GmbH +# +# 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-2013 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 COPYRIGHT and LICENSE files for more details. +# ++ +# + +class BaseController < ApplicationController + around_action :set_time_zone + + helper :watchers + helper :meeting_contents + include WatchersHelper + include PaginationHelper + include SortHelper + + private + + def set_time_zone(&) + zone = User.current.time_zone + if zone.nil? + localzone = Time.current.utc_offset + localzone -= 3600 if Time.current.dst? + zone = ::ActiveSupport::TimeZone[localzone] + end + + Time.use_zone(zone, &) + end + + def convert_params + if params.key?(:meeting) + # We do some preprocessing of `meeting_params` that we will store in this + # instance variable. + @converted_params = meeting_params.to_h + + @converted_params[:duration] = @converted_params[:duration].to_hours + # Force defaults on participants + @converted_params[:participants_attributes] ||= {} + @converted_params[:participants_attributes].each { |p| p.reverse_merge! attended: false, invited: false } + end + end + + def populate_meeting_with_converted_params + @meeting.participants.clear # Start with a clean set of participants + @meeting.participants_attributes = @converted_params.delete(:participants_attributes) + @meeting.attributes = @converted_params + end + + def meeting_params + params.require(:meeting).permit(:title, :location, :start_time, :duration, :start_date, :start_time_hour, + participants_attributes: %i[email name invited attended user user_id meeting id]) + end +end diff --git a/modules/meeting/app/controllers/meetings_controller.rb b/modules/meeting/app/controllers/meetings_controller.rb index 6656d0490e20..70fbb60784b2 100644 --- a/modules/meeting/app/controllers/meetings_controller.rb +++ b/modules/meeting/app/controllers/meetings_controller.rb @@ -26,14 +26,13 @@ # See COPYRIGHT and LICENSE files for more details. #++ -class MeetingsController < ApplicationController - around_action :set_time_zone - before_action :find_optional_project, only: %i[index new create] +class MeetingsController < BaseController + before_action :find_optional_current_project, only: %i[new create] + before_action :convert_params, only: %i[new create update] before_action :build_meeting, only: %i[new create] before_action :find_meeting, except: %i[index new create] - before_action :convert_params, only: %i[create update] - before_action :authorize, except: [:index] - before_action :authorize_global, only: :index + before_action :authorize, except: %i[index new] + before_action :authorize_global, only: %i[index new] helper :watchers helper :meeting_contents @@ -41,47 +40,16 @@ class MeetingsController < ApplicationController include PaginationHelper include SortHelper - menu_item :new_meeting, only: %i[new create] - def index - @meetings = @project ? @project.meetings : global_upcoming_meetings + @meetings = global_upcoming_meetings end def show params[:tab] ||= 'minutes' if @meeting.agenda.present? && @meeting.agenda.locked? end - def create - @meeting.participants.clear # Start with a clean set of participants - @meeting.participants_attributes = @converted_params.delete(:participants_attributes) - @meeting.attributes = @converted_params - if params[:copied_from_meeting_id].present? && params[:copied_meeting_agenda_text].present? - @meeting.agenda = MeetingAgenda.new( - text: params[:copied_meeting_agenda_text], - journal_notes: I18n.t('meeting.copied', id: params[:copied_from_meeting_id]) - ) - @meeting.agenda.author = User.current - end - if @meeting.save - text = I18n.t(:notice_successful_create) - if User.current.time_zone.nil? - link = I18n.t(:notice_timezone_missing, zone: Time.zone) - text += " #{view_context.link_to(link, { controller: '/my', action: :account }, class: 'link_to_profile')}" - end - flash[:notice] = text.html_safe - - redirect_to action: 'show', id: @meeting - else - render template: 'meetings/new', project_id: @project - end - end - def new; end - current_menu_item :new do - :meetings - end - def copy params[:copied_from_meeting_id] = @meeting.id params[:copied_meeting_agenda_text] = @meeting.agenda.text if @meeting.agenda.present? @@ -92,7 +60,7 @@ def copy def destroy @meeting.destroy flash[:notice] = I18n.t(:notice_successful_delete) - redirect_to action: 'index', project_id: @project + redirect_to project_meetings_path(@project) end def edit; end @@ -110,32 +78,26 @@ def update private - def set_time_zone(&) - zone = User.current.time_zone - if zone.nil? - localzone = Time.current.utc_offset - localzone -= 3600 if Time.current.dst? - zone = ::ActiveSupport::TimeZone[localzone] - end - - Time.use_zone(zone, &) - end - def build_meeting @meeting = Meeting.new - @meeting.project = @project + populate_meeting_with_converted_params if @converted_params + @meeting.project = @current_project @meeting.author = User.current end - def find_optional_project - return true unless params[:project_id] + def find_optional_current_project + return true if project_id.blank? - @project = Project.find(params[:project_id]) + @current_project = Project.find(project_id) authorize rescue ActiveRecord::RecordNotFound render_404 end + def project_id + @project_id ||= params[:project_id] || params.dig(:meeting, :project_id) + end + def global_upcoming_meetings projects = Project.allowed_to(User.current, :view_meetings) @@ -150,20 +112,4 @@ def find_meeting rescue ActiveRecord::RecordNotFound render_404 end - - def convert_params - # We do some preprocessing of `meeting_params` that we will store in this - # instance variable. - @converted_params = meeting_params.to_h - - @converted_params[:duration] = @converted_params[:duration].to_hours - # Force defaults on participants - @converted_params[:participants_attributes] ||= {} - @converted_params[:participants_attributes].each { |p| p.reverse_merge! attended: false, invited: false } - end - - def meeting_params - params.require(:meeting).permit(:title, :location, :start_time, :duration, :start_date, :start_time_hour, - participants_attributes: %i[email name invited attended user user_id meeting id]) - end end diff --git a/modules/meeting/app/controllers/projects/meetings_controller.rb b/modules/meeting/app/controllers/projects/meetings_controller.rb new file mode 100644 index 000000000000..afe224a57900 --- /dev/null +++ b/modules/meeting/app/controllers/projects/meetings_controller.rb @@ -0,0 +1,109 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2023 the OpenProject GmbH +# +# 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +class Projects::MeetingsController < BaseController + before_action :find_project, only: %i[index new create] + before_action :build_meeting, only: %i[new create] + before_action :convert_params, only: %i[create update] + before_action :authorize + + def index + @meetings = @project.meetings + end + + def create + populate_meeting_with_converted_params + build_copied_agenda if copied_params_present? + if @meeting.save + set_successful_flash_message + redirect_to meeting_path(@meeting) + else + render failed_create_partial, project_id: @project + end + end + + def new; end + + current_menu_item [:new, :create, :index] do + :meetings + end + + private + + def build_meeting + @meeting = Meeting.new + @meeting.project = @project + @meeting.author = User.current + end + + def build_copied_agenda + @meeting.agenda = MeetingAgenda.new( + text: params[:copied_meeting_agenda_text], + journal_notes: I18n.t('meeting.copied', id: params[:copied_from_meeting_id]) + ) + @meeting.agenda.author = User.current + end + + def set_successful_flash_message + text = I18n.t(:notice_successful_create) + if User.current.time_zone.nil? + link = I18n.t(:notice_timezone_missing, zone: Time.zone) + text += " #{view_context.link_to(link, { controller: '/my', action: :account }, class: 'link_to_profile')}" + end + # rubocop:disable Rails/OutputSafety + flash[:notice] = text.html_safe + # rubocop:enable Rails/OutputSafety + end + + def copied_params_present? + params[:copied_from_meeting_id].present? && params[:copied_meeting_agenda_text].present? + end + + def find_project + @project = Project.find(params[:project_id]) + authorize + rescue ActiveRecord::RecordNotFound + render_404 + end + + def meeting_params + super.merge(params.require(:meeting).permit(:project_id)) + end + + def failed_create_partial + if coming_from_global_form? + 'meetings/new' + else + 'projects/meetings/new' + end + end + + def coming_from_global_form? + params[:meeting].key?(:project_id) + end +end diff --git a/modules/meeting/app/helpers/meetings_helper.rb b/modules/meeting/app/helpers/meetings_helper.rb index debd096f36e7..e6f4862bbbaa 100644 --- a/modules/meeting/app/helpers/meetings_helper.rb +++ b/modules/meeting/app/helpers/meetings_helper.rb @@ -72,4 +72,12 @@ def render_journal_details(journal, header_label = :label_updated_time_by, _mode content_tag('div', "#{header}#{details}".html_safe, id: "change-#{journal.id}", class: 'journal') end + + def create_meeting_url_for(project) + project ? project_meetings_path(project) : '' + end + + def global_meeting_form? + request.path == new_meeting_path + end end diff --git a/modules/meeting/app/views/meetings/edit.html.erb b/modules/meeting/app/views/meetings/edit.html.erb index 2773a0cf8398..0882dbbcb1d2 100644 --- a/modules/meeting/app/views/meetings/edit.html.erb +++ b/modules/meeting/app/views/meetings/edit.html.erb @@ -30,9 +30,11 @@ See COPYRIGHT and LICENSE files for more details. <% html_title "#{t(:label_meeting_edit)}: #{@meeting.title}" %> <%= toolbar title: "#{t(:label_meeting)} ##{@meeting.id}" %> -<%= labelled_tabular_form_for @meeting, :url => {:controller => '/meetings', :action => 'update'}, :html => {:id => 'meeting-form', :method => :put} do |f| -%> - <%= render :partial => 'form', :locals => {:f => f} %> -<%= styled_button_tag t(:button_save), class: '-highlight -with-icon icon-checkmark' %> -<%= link_to t(:button_cancel), { :action => 'show', :id => @meeting }, - class: 'button -with-icon icon-cancel' %> +<%= labelled_tabular_form_for @meeting, + url: { controller: '/meetings', action: 'update'}, + html: { id: 'meeting-form', method: :put } do |f| -%> + <%= render partial: 'shared/meetings/form', locals: { f:, meeting: @meeting, project: @project } %> + <%= styled_button_tag t(:button_save), class: '-highlight -with-icon icon-checkmark' %> + <%= link_to t(:button_cancel), { action: 'show', id: @meeting }, + class: 'button -with-icon icon-cancel' %> <% end if @project %> diff --git a/modules/meeting/app/views/meetings/index.html.erb b/modules/meeting/app/views/meetings/index.html.erb index bf9d94998e23..10992f1c1d22 100644 --- a/modules/meeting/app/views/meetings/index.html.erb +++ b/modules/meeting/app/views/meetings/index.html.erb @@ -27,25 +27,4 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<% html_title t(:label_meeting_plural) %> - -<%= toolbar title: t(:label_meeting_plural) do %> - <% if authorize_for(:meetings, :new) %> -
  • - - <%= op_icon('button--icon icon-add') %> - <%= t(:label_meeting) %> - -
    • - <% end %> -<% end %> - -<% if @meetings.empty? -%> - <%= no_results_box %> -<% else -%> -<%= render Meetings::TableComponent.new(rows: @meetings, current_project: @project) %> -<% end -%> +<%= render partial: 'shared/meetings/index', locals: { project: nil, meetings: @meetings } %> diff --git a/modules/meeting/app/views/meetings/new.html.erb b/modules/meeting/app/views/meetings/new.html.erb index ee5dd0bb5c29..296708c7a49f 100644 --- a/modules/meeting/app/views/meetings/new.html.erb +++ b/modules/meeting/app/views/meetings/new.html.erb @@ -27,11 +27,4 @@ See COPYRIGHT and LICENSE files for more details. ++#%> -<% html_title t(:label_meeting_new) %> -<%= toolbar title: t(:label_meeting_new) %> -<%= labelled_tabular_form_for @meeting, :url => {:controller => '/meetings', :action => 'create', :project_id => @project}, :html => {:id => 'meeting-form'} do |f| -%> - <%= render :partial => 'form', :locals => {:f => f} %> - <%= styled_button_tag t(:button_create), class: '-highlight' %> - <%= link_to t(:button_cancel), { :action => 'index', :project_id => @project }, - class: 'button' %> -<% end if @project %> +<%= render partial: 'shared/meetings/new', locals: { meeting: @meeting, project: (@current_project || @project) } %> diff --git a/modules/meeting/app/views/meetings/show.html.erb b/modules/meeting/app/views/meetings/show.html.erb index 6624b5c8bda6..51a45fbcdfae 100644 --- a/modules/meeting/app/views/meetings/show.html.erb +++ b/modules/meeting/app/views/meetings/show.html.erb @@ -40,7 +40,7 @@ See COPYRIGHT and LICENSE files for more details. <% end %> <% if authorize_for(:meetings, :edit) %>
  • - <%= link_to({:controller => '/meetings', :action => 'edit', :id => @meeting}, class: 'button',:accesskey => accesskey(:edit)) do%> + <%= link_to({ controller: '/meetings', action: 'edit', id: @meeting}, class: 'button', accesskey: accesskey(:edit)) do%> <%= op_icon('button--icon icon-edit') %> <%= t(:button_edit) %> <% end %> @@ -48,7 +48,7 @@ See COPYRIGHT and LICENSE files for more details. <% end %> <% if authorize_for(:meetings, :copy) %>
  • - <%= link_to({:controller => '/meetings', :action => 'copy', :id => @meeting}, class: 'button') do %> + <%= link_to({ controller: '/meetings', action: 'copy', id: @meeting }, class: 'button') do %> <%= op_icon('button--icon icon-copy') %> <%= t(:button_copy) %> <% end %> @@ -56,7 +56,7 @@ See COPYRIGHT and LICENSE files for more details. <% end %> <% if authorize_for(:meetings, :destroy) %>
  • - <%= link_to({controller: '/meetings', action: 'destroy', id: @meeting}, + <%= link_to({ controller: '/meetings', action: 'destroy', id: @meeting }, class: 'button', method: :delete, data: { confirm: t(:text_are_you_sure) }) do %> @@ -88,8 +88,8 @@ See COPYRIGHT and LICENSE files for more details. -<%= render_tabs [{:name => 'agenda', :action => :create_meeting_agendas, :partial => 'meeting_contents/show', :path => meeting_agenda_path(@meeting), :label => :label_meeting_agenda, :content => @meeting.agenda || @meeting.build_agenda, :content_type => "meeting_agenda"}, - {:name => 'minutes', :action => :create_meeting_minutes, :partial => 'meeting_contents/show', :path => meeting_minutes_path(@meeting), :label => :label_meeting_minutes, :content => @meeting.minutes || @meeting.build_minutes, :content_type => "meeting_minutes"}] %> +<%= render_tabs [{ name: 'agenda', action: :create_meeting_agendas, partial: 'meeting_contents/show', path: meeting_agenda_path(@meeting), label: :label_meeting_agenda, content: @meeting.agenda || @meeting.build_agenda, content_type: "meeting_agenda" }, + { name: 'minutes', action: :create_meeting_minutes, partial: 'meeting_contents/show', path: meeting_minutes_path(@meeting), label: :label_meeting_minutes, content: @meeting.minutes || @meeting.build_minutes, content_type: "meeting_minutes" }] %> <% if @meeting.journals.changing.present? %>
    diff --git a/modules/meeting/app/views/projects/meetings/index.html.erb b/modules/meeting/app/views/projects/meetings/index.html.erb new file mode 100644 index 000000000000..3a4f692a29d3 --- /dev/null +++ b/modules/meeting/app/views/projects/meetings/index.html.erb @@ -0,0 +1,30 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) 2012-2023 the OpenProject GmbH + +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-2013 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 COPYRIGHT and LICENSE files for more details. + +++#%> + +<%= render partial: 'shared/meetings/index', locals: { project: @project, meetings: @meetings } %> diff --git a/modules/meeting/app/views/projects/meetings/new.html.erb b/modules/meeting/app/views/projects/meetings/new.html.erb new file mode 100644 index 000000000000..da14e91e8736 --- /dev/null +++ b/modules/meeting/app/views/projects/meetings/new.html.erb @@ -0,0 +1,30 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) 2012-2023 the OpenProject GmbH + +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-2013 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 COPYRIGHT and LICENSE files for more details. + +++#%> + +<%= render partial: 'shared/meetings/new', locals: { meeting: @meeting, project: @project } %> diff --git a/modules/meeting/app/views/meetings/_form.html.erb b/modules/meeting/app/views/shared/meetings/_form.html.erb similarity index 89% rename from modules/meeting/app/views/meetings/_form.html.erb rename to modules/meeting/app/views/shared/meetings/_form.html.erb index 371ec4d76aee..ad5fb27f5a83 100644 --- a/modules/meeting/app/views/meetings/_form.html.erb +++ b/modules/meeting/app/views/shared/meetings/_form.html.erb @@ -35,6 +35,16 @@ See COPYRIGHT and LICENSE files for more details. <%= f.text_field :title, :required => true, :size => 60, container_class: '-wide' %>
    + <% if global_meeting_form? %> +
    + <%= f.select :project_id, + Project.visible.filter { _1.module_enabled?('meetings') }.map { [_1.name, _1.id] }, + {include_blank: t(:project_selection_placeholder), + container_class: '-wide'}, + 'data-action': 'change->refresh-on-form-changes#triggerReload' %> +
    + <% end %> +
    <%= f.text_field :location, :size => 60, container_class: '-wide' %>
    @@ -86,7 +96,8 @@ See COPYRIGHT and LICENSE files for more details. -
    + <% if project %> +
    @@ -98,11 +109,11 @@ See COPYRIGHT and LICENSE files for more details. - <% @meeting.all_changeable_participants.sort.each do |user| -%> + <% meeting.all_changeable_participants.sort.each do |user| -%> <%= hidden_field_tag "meeting[participants_attributes][][user_id]", user.id %> - <% if @meeting.participants.present? && participant = @meeting.participants.detect{|p| p.user_id == user.id} -%> + <% if meeting.participants.present? && participant = meeting.participants.detect{|p| p.user_id == user.id} -%> <%= hidden_field_tag "meeting[participants_attributes][][id]", participant.id %>
    <%=h user %> <%= label_tag "checkbox_invited_#{user.id}", user.name + " " + t(:description_invite), :class => "hidden-for-sighted" %> @@ -128,6 +139,8 @@ See COPYRIGHT and LICENSE files for more details.
    + <% end %> + <%= hidden_field_tag "copied_from_meeting_id", params[:copied_from_meeting_id] if params[:copied_from_meeting_id].present? %> <%= hidden_field_tag "copied_meeting_agenda_text", params[:copied_meeting_agenda_text] if params[:copied_meeting_agenda_text].present? %> diff --git a/modules/meeting/app/views/shared/meetings/_index.html.erb b/modules/meeting/app/views/shared/meetings/_index.html.erb new file mode 100644 index 000000000000..4df673a42114 --- /dev/null +++ b/modules/meeting/app/views/shared/meetings/_index.html.erb @@ -0,0 +1,40 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) 2012-2023 the OpenProject GmbH + +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-2013 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 COPYRIGHT and LICENSE files for more details. + +++#%> + +<% html_title t(:label_meeting_plural) %> + +<%= toolbar title: t(:label_meeting_plural) do %> + <%= render Meetings::AddButtonComponent.new(current_project: project) %> +<% end %> + +<% if meetings.empty? -%> + <%= no_results_box %> +<% else -%> +<%= render Meetings::TableComponent.new(rows: meetings, current_project: project) %> +<% end -%> diff --git a/modules/meeting/app/views/shared/meetings/_new.html.erb b/modules/meeting/app/views/shared/meetings/_new.html.erb new file mode 100644 index 000000000000..0c0438126cf6 --- /dev/null +++ b/modules/meeting/app/views/shared/meetings/_new.html.erb @@ -0,0 +1,43 @@ +<%#-- copyright +OpenProject is an open source project management software. +Copyright (C) 2012-2023 the OpenProject GmbH + +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-2013 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 COPYRIGHT and LICENSE files for more details. + +++#%> + +<% html_title t(:label_meeting_new) %> +<%= toolbar title: t(:label_meeting_new) %> +<%= labelled_tabular_form_for meeting, + url: create_meeting_url_for(project), + html: { id: 'meeting-form', + data: { 'controller': 'refresh-on-form-changes', + 'refresh-on-form-changes-target': 'form', + 'refresh-on-form-changes-refresh-url-value': new_meeting_url, + 'refresh-on-form-changes-preserve-params-value': true } } do |f| -%> + <%= render partial: 'shared/meetings/form', locals: { f:, meeting:, project: } %> + <%= styled_button_tag t(:button_create), class: '-highlight' %> + <%= link_to t(:button_cancel), { action: 'index', project_id: project }, + class: 'button' %> +<% end %> diff --git a/modules/meeting/config/locales/en.yml b/modules/meeting/config/locales/en.yml index 56a80aace60e..04fd9f9e618e 100644 --- a/modules/meeting/config/locales/en.yml +++ b/modules/meeting/config/locales/en.yml @@ -36,6 +36,7 @@ en: participants: "Participants" participants_attended: "Attendees" participants_invited: "Invitees" + project: "Project" start_time: "Time" start_time_hour: "Starting time" errors: @@ -79,6 +80,7 @@ en: label_time_zone: "Time zone" label_start_date: "Start date" + meeting: copied: "Copied from Meeting #%{id}" @@ -99,6 +101,8 @@ en: project_module_meetings: "Meetings" + project_selection_placeholder: "Select project" + text_duration_in_hours: "Duration in hours" text_in_hours: "in hours" text_meeting_agenda_for_meeting: 'agenda for the meeting "%{meeting}"' diff --git a/modules/meeting/config/routes.rb b/modules/meeting/config/routes.rb index 95e17f9ff84e..a53717316ad4 100644 --- a/modules/meeting/config/routes.rb +++ b/modules/meeting/config/routes.rb @@ -28,10 +28,10 @@ OpenProject::Application.routes.draw do resources :projects, only: %i[] do - resources :meetings, only: %i[new create index] + resources :meetings, only: %i[new create index], module: :projects end - resources :meetings, except: %i[new create] do + resources :meetings, except: %i[create] do resource :agenda, controller: 'meeting_agendas', only: [:update] do member do get :history diff --git a/modules/meeting/lib/open_project/meeting/engine.rb b/modules/meeting/lib/open_project/meeting/engine.rb index 98ff2c5043f4..626000cc5356 100644 --- a/modules/meeting/lib/open_project/meeting/engine.rb +++ b/modules/meeting/lib/open_project/meeting/engine.rb @@ -38,9 +38,9 @@ class Engine < ::Rails::Engine author_url: 'https://www.openproject.org', bundled: true do project_module :meetings do - permission :view_meetings, meetings: %i[index show], meeting_agendas: %i[history show diff], - meeting_minutes: %i[history show diff] - permission :create_meetings, { meetings: %i[new create copy] }, require: :member + permission :view_meetings, 'projects/meetings': %i[index show], meetings: %i[index show], + meeting_agendas: %i[history show diff], meeting_minutes: %i[history show diff] + permission :create_meetings, { 'projects/meetings': %i[new create], meetings: %i[new copy] }, require: :member permission :edit_meetings, { meetings: %i[edit update] }, require: :member permission :delete_meetings, { meetings: [:destroy] }, require: :member permission :meetings_send_invite, { meetings: [:icalendar] }, require: :member @@ -57,7 +57,7 @@ class Engine < ::Rails::Engine end menu :project_menu, - :meetings, { controller: '/meetings', action: 'index' }, + :meetings, { controller: '/projects/meetings', action: 'index' }, caption: :project_module_meetings, after: :wiki, before: :members, diff --git a/modules/meeting/spec/controllers/meetings_controller_spec.rb b/modules/meeting/spec/controllers/meetings_controller_spec.rb index e08f617abe8c..10e9673d15b2 100644 --- a/modules/meeting/spec/controllers/meetings_controller_spec.rb +++ b/modules/meeting/spec/controllers/meetings_controller_spec.rb @@ -54,23 +54,12 @@ end describe 'html' do - context 'when requesting meetings globally' do - before do - get 'index' - end - - it { expect(response).to be_successful } - it { expect(assigns(:meetings)).to match_array meetings } + before do + get 'index' end - context 'when requesting meetings scoped to a project ID' do - before do - get 'index', params: { project_id: project.id } - end - - it { expect(response).to be_successful } - it { expect(assigns(:meetings)).to match_array meetings[0..1] } - end + it { expect(response).to be_successful } + it { expect(assigns(:meetings)).to match_array meetings } end end @@ -117,55 +106,5 @@ it { expect(assigns(:meeting)).to eql meeting } end end - - describe 'create' do - render_views - - before do - allow(Project).to receive(:find).and_return(project) - post :create, - params: { - project_id: project.id, - meeting: { - title: 'Foobar', - duration: '1.0' - }.merge(params) - } - end - - describe 'invalid start_date' do - let(:params) do - { - start_date: '-', - start_time_hour: '10:00' - } - end - - it 'renders an error' do - expect(response).to have_http_status :ok - expect(response).to render_template :new - expect(response.body) - .to have_selector '#errorExplanation li', - text: "Start date #{I18n.t('activerecord.errors.messages.not_an_iso_date')}" - end - end - - describe 'invalid start_time_hour' do - let(:params) do - { - start_date: '2015-06-01', - start_time_hour: '-' - } - end - - it 'renders an error' do - expect(response).to have_http_status :ok - expect(response).to render_template :new - expect(response.body) - .to have_selector '#errorExplanation li', - text: "Starting time #{I18n.t('activerecord.errors.messages.invalid_time_format')}" - end - end - end end end diff --git a/modules/meeting/spec/controllers/projects/meetings_controller_spec.rb b/modules/meeting/spec/controllers/projects/meetings_controller_spec.rb new file mode 100644 index 000000000000..e49529ea9587 --- /dev/null +++ b/modules/meeting/spec/controllers/projects/meetings_controller_spec.rb @@ -0,0 +1,144 @@ +#-- copyright +# OpenProject is an open source project management software. +# Copyright (C) 2012-2023 the OpenProject GmbH +# +# 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-2013 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 COPYRIGHT and LICENSE files for more details. +#++ + +require "#{File.dirname(__FILE__)}/../../spec_helper" + +RSpec.describe Projects::MeetingsController do + let(:user) { create(:admin) } + let(:project) { create(:project) } + let(:other_project) { create(:project) } + + before do + allow(User).to receive(:current).and_return user + + allow(Project).to receive(:find).and_return(project) + + allow(controller).to receive(:authorize) + allow(controller).to receive(:authorize_global) + allow(controller).to receive(:check_if_login_required) + end + + describe 'GET' do + describe 'index' do + let(:meetings) do + [ + create(:meeting, project:), + create(:meeting, project:), + create(:meeting, project: other_project) + ] + end + + describe 'html' do + before do + get :index, params: { project_id: project.id } + end + + it { expect(response).to be_successful } + it { expect(assigns(:meetings)).to match_array meetings[0..1] } + end + end + + describe 'new' do + let(:meeting) { Meeting.new(project:) } + + before do + allow(Project).to receive(:find).and_return(project) + allow(Meeting).to receive(:new).and_return(meeting) + end + + describe 'html' do + before do + get :new, params: { project_id: project.id } + end + + it { expect(response).to be_successful } + it { expect(assigns(:meeting)).to eql meeting } + end + end + end + + describe 'POST' do + describe 'create' do + render_views + + before do + allow(Project).to receive(:find).and_return(project) + + post :create, + params: { + project_id: project.id, + meeting: { + title: 'Foobar', + duration: '1.0' + }.merge(params) + } + end + + let(:params) do + { + start_date: '2015-06-01', + start_time_hour: '10:00' + } + end + + context 'with an invalid start_date' do + let(:params) do + { + start_date: '-', + start_time_hour: '10:00' + } + end + + it 'renders an error' do + expect(response).to have_http_status :ok + expect(response).to render_template :new + expect(response.body) + .to have_selector '#errorExplanation li', + text: "Start date #{I18n.t('activerecord.errors.messages.not_an_iso_date')}" + end + end + + describe 'with an invalid start_time_hour' do + let(:params) do + { + start_date: '2015-06-01', + start_time_hour: '-' + } + end + + it 'renders an error' do + expect(response).to have_http_status :ok + expect(response).to render_template :new + expect(response.body) + .to have_selector '#errorExplanation li', + text: "Starting time #{I18n.t('activerecord.errors.messages.invalid_time_format')}" + end + end + end + end +end diff --git a/modules/meeting/spec/features/meetings_index_spec.rb b/modules/meeting/spec/features/meetings_index_spec.rb index f2ba4630234a..58c0f766725a 100644 --- a/modules/meeting/spec/features/meetings_index_spec.rb +++ b/modules/meeting/spec/features/meetings_index_spec.rb @@ -78,10 +78,10 @@ context 'when the user is allowed to create meetings' do let(:permissions) { %i(view_meetings create_meetings) } - it 'does not show a create button' do + it 'shows a create button' do meetings_page.navigate_by_modules_menu - meetings_page.expect_no_create_new_button + meetings_page.expect_create_new_button end end end diff --git a/modules/meeting/spec/features/meetings_new_spec.rb b/modules/meeting/spec/features/meetings_new_spec.rb index 103d2a07bfb7..fdc61c99a966 100644 --- a/modules/meeting/spec/features/meetings_new_spec.rb +++ b/modules/meeting/spec/features/meetings_new_spec.rb @@ -32,7 +32,6 @@ RSpec.describe 'Meetings new', js: true do let(:project) { create(:project, enabled_module_names: %w[meetings]) } - let(:index_page) { Pages::Meetings::Index.new(project:) } let(:time_zone) { 'utc' } let(:user) do create(:user, @@ -60,62 +59,139 @@ login_as(current_user) end - context 'with permission to create meetings' do + context 'when creating a meeting from the global create page' do before do other_user + project end - ['CET', 'UTC', '', 'Pacific Time (US & Canada)'].each do |zone| - let(:time_zone) { zone } + let(:index_page) { Pages::Meetings::Index.new(project: nil) } - it "allows creating a project and handles errors in time zone #{zone}" do + context 'with permission to create meetings' do + ['CET', 'UTC', '', 'Pacific Time (US & Canada)'].each do |zone| + let(:time_zone) { zone } + + it "allows creating a project and handles errors in time zone #{zone}" do + index_page.visit! + + new_page = index_page.click_create_new + + new_page.set_title 'Some title' + new_page.set_project project + + # Setting the project reloads the page + # causing a StaleElementReferenceError + # if the execution is too quick. + SeleniumHubWaiter.wait + + new_page.set_start_date '2013-03-28' + new_page.set_start_time '13:30' + new_page.set_duration '1.5' + new_page.invite(other_user) + + show_page = new_page.click_create + + show_page.expect_toast(message: 'Successful creation') + + show_page.expect_invited(user, other_user) + + show_page.expect_date_time "03/28/2013 01:30 PM - 03:00 PM" + end + end + end + + context 'without permission to create meetings' do + let(:permissions) { %i[view_meetings] } + + it 'shows no edit link' do + index_page.visit! + + index_page.expect_no_create_new_button + end + end + + context 'as an admin' do + let(:current_user) { admin } + + it 'allows creating meeting in a project without members' do index_page.visit! new_page = index_page.click_create_new new_page.set_title 'Some title' - new_page.set_start_date '2013-03-28' - new_page.set_start_time '13:30' - new_page.set_duration '1.5' - new_page.invite(other_user) + + new_page.set_project project show_page = new_page.click_create show_page.expect_toast(message: 'Successful creation') - show_page.expect_invited(user, other_user) - - show_page.expect_date_time "03/28/2013 01:30 PM - 03:00 PM" + # Not sure if that is then intended behaviour but that is what is currently programmed + show_page.expect_invited(admin) end end end - context 'without permission to create meetings' do - let(:permissions) { %i[view_meetings] } + context 'when creating a meeting from the project-specific page' do + let(:index_page) { Pages::Meetings::Index.new(project:) } - it 'shows no edit link' do - index_page.visit! + context 'with permission to create meetings' do + before do + other_user + end - index_page.expect_no_create_new_button + ['CET', 'UTC', '', 'Pacific Time (US & Canada)'].each do |zone| + let(:time_zone) { zone } + + it "allows creating a project and handles errors in time zone #{zone}" do + index_page.visit! + + new_page = index_page.click_create_new + + new_page.set_title 'Some title' + new_page.set_start_date '2013-03-28' + new_page.set_start_time '13:30' + new_page.set_duration '1.5' + new_page.invite(other_user) + + show_page = new_page.click_create + + show_page.expect_toast(message: 'Successful creation') + + show_page.expect_invited(user, other_user) + + show_page.expect_date_time "03/28/2013 01:30 PM - 03:00 PM" + end + end end - end - context 'as an admin' do - let(:current_user) { admin } + context 'without permission to create meetings' do + let(:permissions) { %i[view_meetings] } - it 'allows creating meeting in a project without members' do - index_page.visit! + it 'shows no edit link' do + index_page.visit! - new_page = index_page.click_create_new + index_page.expect_no_create_new_button + end + end + + context 'as an admin' do + let(:current_user) { admin } + + it 'allows creating meeting in a project without members' do + index_page.visit! + + new_page = index_page.click_create_new - new_page.set_title 'Some title' + new_page.set_title 'Some title' - show_page = new_page.click_create + show_page = new_page.click_create - show_page.expect_toast(message: 'Successful creation') + show_page.expect_toast(message: 'Successful creation') - # Not sure if that is then intended behaviour but that is what is currently programmed - show_page.expect_invited(admin) + # Not sure if that is then intended behaviour but that is what is currently programmed + show_page.expect_invited(admin) + end end end end diff --git a/modules/meeting/spec/support/pages/meetings/new.rb b/modules/meeting/spec/support/pages/meetings/new.rb index 5448a66759b0..a632914888c5 100644 --- a/modules/meeting/spec/support/pages/meetings/new.rb +++ b/modules/meeting/spec/support/pages/meetings/new.rb @@ -47,6 +47,10 @@ def set_title(text) fill_in 'Title', with: text end + def set_project(project) + select project.name, from: 'Project' + end + def set_start_date(date) find_by_id('meeting_start_date').click datepicker = Components::BasicDatepicker.new