Merge pull request #14861 from opf/feature/copy-meeting-agenda-items

Copy meeting agenda items

modules/meeting/app/controllers/meetings_controller.rb

@@ -31,6 +31,7 @@ class MeetingsController < ApplicationController
   before_action :find_optional_project, only: %i[index new create]
   before_action :build_meeting, only: %i[new create]
   before_action :find_meeting, except: %i[index new create]
+  before_action :find_copy_from_meeting, only: %i[create]
   before_action :convert_params, only: %i[create update update_participants]
   before_action :authorize, except: %i[index new create update_title update_details update_participants change_state]
   before_action :authorize_global, only: %i[index new create update_title update_details update_participants change_state]
@@ -73,13 +74,8 @@ 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
+    copy_meeting_agenda
+
     if @meeting.save
       text = I18n.t(:notice_successful_create)
       if User.current.time_zone.nil?
@@ -90,7 +86,7 @@ def create
 
       redirect_to action: 'show', id: @meeting
     else
-      render template: 'meetings/new', project_id: @project
+      render template: 'meetings/new', project_id: @project, locals: { copy_from: @copy_from }
     end
   end
 
@@ -101,10 +97,9 @@ def new; end
   end
 
   def copy
-    params[:copied_from_meeting_id] = @meeting.id
-    params[:copied_meeting_agenda_text] = @meeting.agenda.text if @meeting.agenda.present?
+    copy_from = @meeting
     @meeting = @meeting.copy(author: User.current)
-    render action: 'new', project_id: @project, locals: { copy: true }
+    render action: 'new', project_id: @project, locals: { copy_from: }
   end
 
   def destroy
@@ -274,7 +269,8 @@ def set_time_zone(&)
   end
 
   def build_meeting
-    @meeting = Meeting.new
+    cls = meeting_type(params.dig(:meeting, :type)).constantize
+    @meeting = cls.new
     @meeting.project = @project
     @meeting.author = User.current
   end
@@ -299,7 +295,6 @@ def convert_params
     # instance variable.
     @converted_params = meeting_params.to_h
 
-    @converted_params[:type] = meeting_type(@converted_params[:type])
     @converted_params[:duration] = @converted_params[:duration].to_hours if @converted_params[:duration].present?
     # Force defaults on participants
     @converted_params[:participants_attributes] ||= {}
@@ -308,7 +303,7 @@ def convert_params
 
   def meeting_params
     if params[:meeting].present?
-      params.require(:meeting).permit(:title, :location, :start_time, :type,
+      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
@@ -330,4 +325,27 @@ def meeting_type(given_type)
       'Meeting'
     end
   end
+
+  def find_copy_from_meeting
+    return unless params[:copied_from_meeting_id]
+
+    @copy_from = Meeting.visible.find(params[:copied_from_meeting_id])
+  rescue ActiveRecord::RecordNotFound
+    render_404
+  end
+
+  def copy_meeting_agenda
+    return unless params[:copy_agenda] == '1' && @copy_from
+
+    if @meeting.is_a?(StructuredMeeting)
+      @meeting.agenda_items_attributes = @copy_from.agenda_items.map(&:copy_attributes)
+    else
+      @meeting.agenda = MeetingAgenda.new(
+        author: current_user,
+        text: @copy_from.agenda&.text,
+        journal_notes: I18n.t('meeting.copied', id: params[:copied_from_meeting_id])
+      )
+      @meeting.agenda.author = current_user
+    end
+  end
 end

modules/meeting/app/models/meeting_agenda_item.rb

@@ -45,7 +45,11 @@ class MeetingAgendaItem < ApplicationRecord
 
   scope :with_includes_to_render, -> { includes(:author, :meeting) }
 
-  validates :meeting_id, presence: true
+  # The primer form depends on meeting_id being validated, even though Rails pattern would suggest
+  # to validate only :meeting. When copying meetings however,
+  # we build meetings and agenda items together, so meeting_id will stay empty.
+  # We can use loaded? to check if the meeting has been provided
+  validates :meeting_id, presence: true, unless: Proc.new { |item| item.association(:meeting).loaded? && item.meeting }
   validates :title, presence: true, if: Proc.new { |item| item.simple? }
   validates :work_package_id, presence: true, if: Proc.new { |item| item.work_package? }, on: :create
   validates :work_package_id,
@@ -85,4 +89,8 @@ def editable?
   def modifiable?
     !(meeting&.closed? || (deleted_work_package? && work_package_id.present?))
   end
+
+  def copy_attributes
+    attributes.except('id', 'meeting_id')
+  end
 end

modules/meeting/app/models/structured_meeting.rb

@@ -27,7 +27,12 @@
 #++
 
 class StructuredMeeting < Meeting
-  has_many :agenda_items, dependent: :destroy, foreign_key: 'meeting_id', class_name: 'MeetingAgendaItem'
+  has_many :agenda_items,
+           dependent: :destroy,
+           foreign_key: 'meeting_id',
+           class_name: 'MeetingAgendaItem',
+           inverse_of: :meeting
+  accepts_nested_attributes_for :agenda_items
 
   # triggered by MeetingAgendaItem#after_create/after_destroy/after_save
   def calculate_agenda_item_time_slots

modules/meeting/app/views/meetings/_form.html.erb

@@ -35,7 +35,24 @@ See COPYRIGHT and LICENSE files for more details.
     <%= f.text_field :title, :required => true, :size => 60, container_class: '-wide' %>
   </div>
 
-  <% if @meeting.new_record? %>
+  <% copy_from = local_assigns[:copy_from] %>
+  <% if copy_from.present? %>
+    <%= f.hidden_field :type, value: copy_from.is_a?(StructuredMeeting) ? 'dynamic' : 'classic' %>
+    <%= hidden_field_tag "copied_from_meeting_id", copy_from.id %>
+    <div class="form--field -required">
+      <%= styled_label_tag 'copy_agenda', t('meeting.copy.agenda') %>
+      <div class="form--field-container">
+        <%= styled_check_box_tag 'copy_agenda',
+                                 1,
+                                 true,
+                                 no_label: true,
+                                 class: 'radio-button' %>
+      </div>
+      <div class="form--field-instructions">
+        <%= t('meeting.copy.agenda_text') %>
+      </div>
+    </div>
+  <% elsif @meeting.new_record? %>
     <div class="form--field -required -align-start">
       <label class="form--label" for="meeting_type"><%= Meeting.human_attribute_name(:type) %></label>
       <div class="form--field-container">
@@ -161,7 +178,4 @@ See COPYRIGHT and LICENSE files for more details.
   </div>
 
   <%= render partial: 'meetings/participants_section' %>
-
-  <%= 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? %>
 </section>

modules/meeting/app/views/meetings/new.html.erb

@@ -26,17 +26,23 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 See COPYRIGHT and LICENSE files for more details.
 
 ++#%>
+<% copy_from = local_assigns[:copy_from] %>
+<% if copy_from %>
+  <% html_title t('meeting.copy.title', title: copy_from.title) %>
+  <%= toolbar title: t('meeting.copy.title', title: copy_from.title) %>
+<% else %>
+  <% html_title t(:label_meeting_new) %>
+  <%= toolbar title: t(:label_meeting_new) %>
+<% end %>
 
-<% html_title t(:label_meeting_new) %>
-<%= toolbar title: t(:label_meeting_new) %>
 <%= labelled_tabular_form_for @meeting,
                               as: :meeting,
                               url: {:controller => '/meetings', :action => 'create', :project_id => @project},
                               :html => {:id => 'meeting-form',
                                         :data => { :controller => 'refresh-on-form-changes',
                                                    'refresh-on-form-changes-target': 'form',
                                                    'refresh-on-form-changes-turbo-stream-url-value': new_meeting_url }} do |f| -%>
-  <%= render :partial => 'form', :locals => { f:, copy: local_assigns[:copy] } %>
+  <%= render :partial => 'form', :locals => { f:, copy_from: } %>
   <%= styled_button_tag t(:button_create), class: '-highlight' %>
   <%= link_to t(:button_cancel), { :action => 'index', :project_id => @project },
         class: 'button' %>

modules/meeting/config/locales/en.yml

@@ -113,6 +113,10 @@ en:
 
 
   meeting:
+    copy:
+      title: "Copy meeting %{title}"
+      agenda: "Copy agenda"
+      agenda_text: "Copy the agenda of the old meeting"
     email:
       open_meeting_link: "Open meeting"
       invited:

modules/meeting/spec/features/meetings_copy_spec.rb

@@ -94,8 +94,6 @@
     expect(page)
       .to have_field 'Time',       with: start_time.strftime("%H:%M")
 
-    choose 'Classic'
-
     click_button "Create"
 
     # Be on the new meeting's page with copied over attributes

modules/meeting/spec/features/structured_meetings/structured_meeting_crud_spec.rb

@@ -251,7 +251,7 @@
     expect(page).to have_css('.flash', text: I18n.t('activerecord.errors.messages.error_conflict'))
   end
 
-  it 'can copy the meeting (empty)' do
+  it 'can copy the meeting' do
     show_page.expect_toast(message: 'Successful creation')
 
     # Can add and edit a single item
@@ -270,7 +270,7 @@
 
     click_button 'Create'
 
-    expect(page).to have_text 'Your meeting is empty'
+    show_page.expect_agenda_item title: 'My agenda item'
     new_meeting = StructuredMeeting.reorder(id: :asc).last
     expect(page).to have_current_path "/meetings/#{new_meeting.id}"
   end

modules/meeting/spec/requests/meetings_spec.rb

@@ -0,0 +1,99 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2024 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 'spec_helper'
+
+RSpec.describe 'Meeting requests',
+               :skip_csrf,
+               type: :rails_request do
+
+  shared_let(:project) { create(:project, enabled_module_names: %i[meetings]) }
+  shared_let(:user) { create(:user, member_with_permissions: { project => %i[view_meetings create_meetings] }) }
+
+  before do
+    login_as user
+  end
+
+  describe 'copy' do
+    let(:meeting) { create(:structured_meeting, project:) }
+    let(:base_params) do
+      {
+        copied_from_meeting_id: meeting.id,
+        project_id: project.id,
+        meeting: {
+          title: 'Copied meeting',
+          type: :dynamic
+        }
+      }
+    end
+    let(:params) { {} }
+
+    subject do
+      post meetings_path(project),
+           params: base_params.merge(params)
+
+      Meeting.find_by(title: 'Copied meeting')
+    end
+
+    context 'when copying agenda items' do
+      let!(:agenda_item) { create(:meeting_agenda_item, meeting:, notes: '**foo**') }
+      let(:params) { { copy_agenda: '1' } }
+
+      it 'copies the agenda items' do
+        subject
+
+        expect(response).to be_redirect
+
+        expect(subject).to be_present
+        expect(subject.agenda_items.count).to eq(1)
+        expect(subject.agenda_items.first.notes).to eq('**foo**')
+      end
+    end
+
+    context 'when copying without additional params' do
+      it 'copies the meeting, but not the agenda' do
+        subject
+
+        expect(response).to be_redirect
+
+        expect(subject).to be_present
+        expect(subject.agenda_items).to be_empty
+      end
+    end
+
+    context 'when meeting is not visible' do
+      let(:other_project) { create(:project) }
+      let(:meeting) { create(:meeting, project: other_project) }
+
+      it 'renders a 404' do
+        subject
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+  end
+end