diff --git a/modules/meeting/app/components/meetings/index/form_component.html.erb b/modules/meeting/app/components/meetings/index/form_component.html.erb index b6243a47c4e3..fa6b771de393 100644 --- a/modules/meeting/app/components/meetings/index/form_component.html.erb +++ b/modules/meeting/app/components/meetings/index/form_component.html.erb @@ -1,27 +1,6 @@ <%= component_wrapper do - primer_form_with( - scope: :meeting, - model: @meeting, - method: form_method, - data: { - turbo: true, - controller: [ - "show-when-value-selected", - "show-when-checked", - @meeting.is_a?(RecurringMeeting) ? "recurring-meetings--form" : nil - ].compact.join(" "), - "recurring-meetings--form-persisted-value": @meeting.persisted? - }, - html: { - id: "meeting-form" - }, - url: { - controller: form_controller, - action: form_action, - project_id: @project - } - ) do |f| + primer_form_with(**form_options) do |f| flex_layout(mb: 3) do |modal_body| if @meeting.errors[:base].present? modal_body.with_row do @@ -35,9 +14,9 @@ end end - if creating_onetime_meeting? && no_preselection? && available_templates.any? + if show_template_selector? modal_body.with_row(mb: 3) do - render(Meeting::TemplateAutocompleter.new(f, project: @project)) + render(Meeting::TemplateAutocompleter.new(f, project: effective_project, disabled: template_selector_disabled?, placeholder: template_selector_placeholder, selected_id: @copy_from&.id)) end end diff --git a/modules/meeting/app/components/meetings/index/form_component.rb b/modules/meeting/app/components/meetings/index/form_component.rb index 6e3e1dfe2ede..0304f44099d5 100644 --- a/modules/meeting/app/components/meetings/index/form_component.rb +++ b/modules/meeting/app/components/meetings/index/form_component.rb @@ -33,13 +33,14 @@ class Index::FormComponent < ApplicationComponent include OpTurbo::Streamable include OpPrimer::ComponentHelpers - def initialize(meeting:, project:, copy_from: nil, template: false) + def initialize(meeting:, project:, copy_from: nil, template: false, template_selected_via_dropdown: false) super @meeting = meeting @project = project @copy_from = copy_from @template = template + @template_selected_via_dropdown = template_selected_via_dropdown end private @@ -70,6 +71,29 @@ def form_action end end + def form_options + { + scope: :meeting, + model: @meeting, + method: form_method, + data: { + turbo: true, + controller: [ + "show-when-value-selected", + "show-when-checked", + @meeting.is_a?(RecurringMeeting) ? "recurring-meetings--form" : nil, + "meetings--form", + use_refresh_on_form_changes? ? "refresh-on-form-changes" : nil + ].compact.join(" "), + "recurring-meetings--form-persisted-value": @meeting.persisted?, + "refresh-on-form-changes-target": "form", + "refresh-on-form-changes-turbo-stream-url-value": fetch_templates_url + }, + html: { id: "meeting-form" }, + url: { controller: form_controller, action: form_action, project_id: @project } + } + end + def creating_onetime_meeting? return false unless EnterpriseToken.allows_to?(:meeting_templates) @@ -77,13 +101,59 @@ def creating_onetime_meeting? end def no_preselection? - !@copy_from + !@copy_from || @template_selected_via_dropdown + end + + def show_template_selector? + return false unless creating_onetime_meeting? && no_preselection? + + if @project.nil? + # Global context - show if user can see any templates + globally_visible_templates.any? + else + # Project context - only show if the project actually has templates + available_templates.any? + end + end + + def template_selector_disabled? + effective_project.nil? || available_templates.empty? + end + + def template_selector_placeholder + if effective_project.nil? + I18n.t(:placeholder_meeting_template_select_project_first) + elsif available_templates.empty? + I18n.t(:placeholder_meeting_template_no_templates_for_project) + end + end + + def use_refresh_on_form_changes? + creating_onetime_meeting? && no_preselection? && @project.nil? + end + + def fetch_templates_url + if @project + fetch_templates_project_meetings_path(@project) + else + fetch_templates_meetings_path + end end def available_templates - return [] unless @project + @available_templates ||= if effective_project + Meeting.templates_visible_in_project(effective_project) + else + Meeting.templates_visible_globally + end + end + + def globally_visible_templates + @globally_visible_templates ||= Meeting.templates_visible_globally + end - @available_templates ||= Meeting.templates_visible_in_project(@project) + def effective_project + @project || @meeting.project end end end diff --git a/modules/meeting/app/controllers/meetings_controller.rb b/modules/meeting/app/controllers/meetings_controller.rb index 182e98c7f40f..0afc2ef48c3f 100644 --- a/modules/meeting/app/controllers/meetings_controller.rb +++ b/modules/meeting/app/controllers/meetings_controller.rb @@ -34,13 +34,13 @@ class MeetingsController < ApplicationController before_action :determine_date_range, only: %i[history] before_action :determine_author, only: %i[history] before_action :build_meeting, only: %i[new new_dialog fetch_timezone] - before_action :find_meeting, except: %i[index new create new_dialog fetch_timezone] + before_action :find_meeting, except: %i[index new create new_dialog fetch_timezone fetch_templates] before_action :redirect_to_project, only: %i[show] before_action :set_activity, only: %i[history] before_action :find_copy_from_meeting, only: %i[create] before_action :convert_params, only: %i[create update] before_action :prevent_series_template_destruction, only: :destroy - before_action :check_for_enterprise_token, only: %i[create new_dialog] + before_action :check_for_enterprise_token, only: %i[create new_dialog fetch_templates] helper :watchers include MeetingsHelper @@ -145,7 +145,8 @@ def create # rubocop:disable Metrics/AbcSize component: Meetings::Index::FormComponent.new( meeting: @meeting, project: @project, - copy_from: @copy_from + copy_from: @copy_from, + template_selected_via_dropdown: params.dig(:meeting, :template_id).present? ), status: :bad_request ) @@ -345,6 +346,17 @@ def fetch_timezone respond_with_turbo_streams end + def fetch_templates + selected_project = Project.visible.find_by(id: params.dig(:meeting, :project_id)) + meeting = Meeting.new(project: selected_project) + + update_via_turbo_stream( + component: Meetings::Index::FormComponent.new(meeting: meeting, project: nil) + ) + + respond_with_turbo_streams + end + def generate_pdf_dialog respond_with_dialog Meetings::Exports::ModalDialogComponent.new( meeting: @meeting, @@ -583,7 +595,8 @@ def find_copy_from_meeting # Check for template selection from form submission template_id = params[:meeting][:template_id] if template_id.present? - @copy_from = Meeting.templates_visible_in_project(@project).find_by(id: template_id) + templates = @project ? Meeting.templates_visible_in_project(@project) : Meeting.templates_visible_globally + @copy_from = templates.find_by(id: template_id) return end diff --git a/modules/meeting/app/forms/meeting/project_autocompleter.rb b/modules/meeting/app/forms/meeting/project_autocompleter.rb index e0488d266bb8..7a1ec462ffdb 100644 --- a/modules/meeting/app/forms/meeting/project_autocompleter.rb +++ b/modules/meeting/app/forms/meeting/project_autocompleter.rb @@ -42,6 +42,7 @@ class Meeting::ProjectAutocompleter < ApplicationForm dropdownPosition: "bottom", appendTo: "#new-meeting-dialog", filters: [{ name: "user_action", operator: "=", values: ["meetings/create"] }], + hiddenFieldAction: "change->refresh-on-form-changes#triggerTurboStream", data: { "test-selector": "project_id" } diff --git a/modules/meeting/app/forms/meeting/template_autocompleter.rb b/modules/meeting/app/forms/meeting/template_autocompleter.rb index 3a4f94ea3a05..e71e74d26ac6 100644 --- a/modules/meeting/app/forms/meeting/template_autocompleter.rb +++ b/modules/meeting/app/forms/meeting/template_autocompleter.rb @@ -38,6 +38,8 @@ class Meeting::TemplateAutocompleter < ApplicationForm decorated: true, defaultData: false, multiple: false, + disabled: @disabled, + placeholder: @placeholder, appendTo: "#new-meeting-dialog", data: { "test-selector": "template_id" @@ -47,20 +49,26 @@ class Meeting::TemplateAutocompleter < ApplicationForm templates.each do |template| select.option( value: template.id, - label: template.title + label: template.title, + selected: template.id == @selected_id ) end end end - def initialize(project:) + def initialize(project:, disabled: false, placeholder: nil, selected_id: nil) super() @project = project + @disabled = disabled + @placeholder = placeholder + @selected_id = selected_id end private def templates + return [] if @disabled + Meeting.templates_visible_in_project(@project).order(:title) end end diff --git a/modules/meeting/app/models/meeting.rb b/modules/meeting/app/models/meeting.rb index 8bc03bce7b72..c69486ff5ca7 100644 --- a/modules/meeting/app/models/meeting.rb +++ b/modules/meeting/app/models/meeting.rb @@ -155,6 +155,18 @@ def self.templates_visible_in_project(project, user = User.current) .or(available_onetime_templates.where(sharing: :system)) end + def self.templates_visible_globally(user = User.current) + accessible = Project.allowed_to(user, :view_meetings).to_a + return none if accessible.empty? + + ancestor_ids = accessible.map(&:ancestors).reduce(:or).select(:id) + + available_onetime_templates + .where(project_id: accessible.map(&:id)) + .or(available_onetime_templates.where(sharing: :descendants, project_id: ancestor_ids)) + .or(available_onetime_templates.where(sharing: :system)) + end + def recurring? recurring_meeting_id.present? end diff --git a/modules/meeting/config/locales/en.yml b/modules/meeting/config/locales/en.yml index cb21d7eae97f..648212da783a 100644 --- a/modules/meeting/config/locales/en.yml +++ b/modules/meeting/config/locales/en.yml @@ -215,6 +215,8 @@ en: caption_meeting_template_select: "Select a template to automatically copy its agenda items" caption_template_project_select: "Please select the project in which to create this meeting template" + placeholder_meeting_template_select_project_first: "Select a project first" + placeholder_meeting_template_no_templates_for_project: "No templates available for this project" meeting: participants: diff --git a/modules/meeting/config/routes.rb b/modules/meeting/config/routes.rb index fecf73d4ee06..757f77196a51 100644 --- a/modules/meeting/config/routes.rb +++ b/modules/meeting/config/routes.rb @@ -42,6 +42,7 @@ get :new_dialog get "menu" => "meetings/menus#show" get :fetch_timezone + get :fetch_templates get "ical/:token", controller: "meetings/ical", action: :index, as: "ical_feed" @@ -58,6 +59,7 @@ get :new_dialog get "menu" => "meetings/menus#show" get :fetch_timezone + get :fetch_templates get "templates", action: :index, controller: "meeting_templates", as: "templates" get "templates/new_dialog", action: :new_dialog, controller: "meeting_templates", as: "new_dialog_template" diff --git a/modules/meeting/lib/open_project/meeting/engine.rb b/modules/meeting/lib/open_project/meeting/engine.rb index da1df3a357ea..6a3d193be2f7 100644 --- a/modules/meeting/lib/open_project/meeting/engine.rb +++ b/modules/meeting/lib/open_project/meeting/engine.rb @@ -53,7 +53,7 @@ class Engine < ::Rails::Engine permissible_on: :project permission :create_meetings, { - meetings: %i[new create copy new_dialog fetch_timezone], + meetings: %i[new create copy new_dialog fetch_timezone fetch_templates], recurring_meetings: %i[new create copy init template_completed], "recurring_meetings/schedule": %i[update_text], "meetings/menus": %i[show], diff --git a/modules/meeting/spec/features/meeting_templates/create_meeting_from_template_spec.rb b/modules/meeting/spec/features/meeting_templates/create_meeting_from_template_spec.rb index fd6aa7e050f7..a98dc1e8b247 100644 --- a/modules/meeting/spec/features/meeting_templates/create_meeting_from_template_spec.rb +++ b/modules/meeting/spec/features/meeting_templates/create_meeting_from_template_spec.rb @@ -199,6 +199,91 @@ end end + describe "creating meeting from template using template selector from global index", with_ee: [:meeting_templates] do + let(:global_meetings_page) { Pages::Meetings::Index.new(project: nil) } + let(:template) { create(:onetime_template, project:, title: "Template") } + + before do + global_meetings_page.visit! + end + + it "shows a template selector when user has access to templates" do + template + + global_meetings_page.click_on "add-meeting-button" + global_meetings_page.click_on "One-time" + + expect(page).to have_dialog("New one-time meeting") + + # Initially disabled + within_dialog "New one-time meeting" do + expect(page).to have_css('[data-test-selector="template_id"]', text: "Select a project first") + end + + # Disabled when a project with no accessible templates is selected + wait_for_turbo_stream do + global_meetings_page.set_project(other_project) + end + + within_dialog "New one-time meeting" do + expect(page).to have_css('[data-test-selector="template_id"]', text: "No templates available for this project") + end + + # Enabled when a project with accessible templates is selected + wait_for_turbo_stream do + global_meetings_page.set_project(project) + end + + within_dialog "New one-time meeting" do + find('[data-test-selector="template_id"]').click + expect(page).to have_text(template.title) + end + end + + it "shows no template selector when user has access to no templates" do + global_meetings_page.click_on "add-meeting-button" + global_meetings_page.click_on "One-time" + + expect(page).to have_dialog("New one-time meeting") + + within_dialog "New one-time meeting" do + expect(page).to have_no_css('[data-test-selector="template_id"]') + end + end + + it "keeps the template selector visible with the template preselected on failed submission" do + template + + global_meetings_page.click_on "add-meeting-button" + global_meetings_page.click_on "One-time" + + expect(page).to have_dialog("New one-time meeting") + + wait_for_turbo_stream do + global_meetings_page.set_project(project) + end + + within_dialog "New one-time meeting" do + select_autocomplete find('[data-test-selector="template_id"]'), + query: template.title, + select_text: template.title, + results_selector: "body" + + # Submit without a title to trigger validation failure + wait_for_turbo_stream do + click_button "Create" + end + end + + within_dialog "New one-time meeting" do + expect(page).to have_text("Title can't be blank.") + + # Template selector must still be visible with the previously selected template preselected + expect(page).to have_css('[data-test-selector="template_id"]', text: template.title) + end + end + end + describe "creating meeting from template using 'Create from template' button" do context "without enterprise token" do let!(:template) { create(:onetime_template, project:, title: "Planning template") } diff --git a/modules/meeting/spec/features/meeting_templates/template_sharing_spec.rb b/modules/meeting/spec/features/meeting_templates/template_sharing_spec.rb index 8442ed846325..b62274d5c936 100644 --- a/modules/meeting/spec/features/meeting_templates/template_sharing_spec.rb +++ b/modules/meeting/spec/features/meeting_templates/template_sharing_spec.rb @@ -156,4 +156,94 @@ end end end + + context "with templates shared via :descendants to a user with child-only access", with_ee: [:meeting_templates] do + shared_let(:parent_project) { create(:project, enabled_module_names: %i[meetings]) } + shared_let(:child_project) { create(:project, enabled_module_names: %i[meetings], parent: parent_project) } + shared_let(:user) do + create(:user, member_with_permissions: { child_project => %i[view_meetings create_meetings] }) + end + shared_let(:descendants_template) do + create(:onetime_template, project: parent_project, title: "Parent template", sharing: :descendants) + end + + let(:child_meetings_page) { Pages::Meetings::Index.new(project: child_project) } + let(:global_meetings_page) { Pages::Meetings::Index.new(project: nil) } + + before { login_as user } + + it "shows the template when creating a meeting from the child project" do + child_meetings_page.visit! + child_meetings_page.click_on "add-meeting-button" + child_meetings_page.click_on "One-time" + + within_dialog "New one-time meeting" do + find('[data-test-selector="template_id"]').click + expect(page).to have_text(descendants_template.title) + end + end + + it "shows the template selector and template when creating from the global index" do + global_meetings_page.visit! + global_meetings_page.click_on "add-meeting-button" + global_meetings_page.click_on "One-time" + + within_dialog "New one-time meeting" do + expect(page).to have_css('[data-test-selector="template_id"]', text: "Select a project first") + end + + global_meetings_page.set_project(child_project) + wait_for_network_idle + + within_dialog "New one-time meeting" do + find('[data-test-selector="template_id"]').click + expect(page).to have_text(descendants_template.title) + end + end + end + + context "with a :system template from a project the user has no access to", with_ee: [:meeting_templates] do + shared_let(:accessible_project) { create(:project, enabled_module_names: %i[meetings]) } + shared_let(:inaccessible_project) { create(:project, enabled_module_names: %i[meetings]) } + shared_let(:user) do + create(:user, member_with_permissions: { accessible_project => %i[view_meetings create_meetings] }) + end + shared_let(:system_template) do + create(:onetime_template, project: inaccessible_project, title: "System template", sharing: :system) + end + + let(:accessible_meetings_page) { Pages::Meetings::Index.new(project: accessible_project) } + let(:global_meetings_page) { Pages::Meetings::Index.new(project: nil) } + + before { login_as user } + + it "shows the system template when creating from an accessible project" do + accessible_meetings_page.visit! + accessible_meetings_page.click_on "add-meeting-button" + accessible_meetings_page.click_on "One-time" + + within_dialog "New one-time meeting" do + find('[data-test-selector="template_id"]').click + expect(page).to have_text(system_template.title) + end + end + + it "shows the template selector and system template when creating from the global index" do + global_meetings_page.visit! + global_meetings_page.click_on "add-meeting-button" + global_meetings_page.click_on "One-time" + + within_dialog "New one-time meeting" do + expect(page).to have_css('[data-test-selector="template_id"]', text: "Select a project first") + end + + global_meetings_page.set_project(accessible_project) + wait_for_network_idle + + within_dialog "New one-time meeting" do + find('[data-test-selector="template_id"]').click + expect(page).to have_text(system_template.title) + end + end + end end diff --git a/modules/meeting/spec/models/meeting_spec.rb b/modules/meeting/spec/models/meeting_spec.rb index 2e3b34d9222d..f56a244efb6c 100644 --- a/modules/meeting/spec/models/meeting_spec.rb +++ b/modules/meeting/spec/models/meeting_spec.rb @@ -242,6 +242,49 @@ end end + describe ".templates_visible_globally" do + shared_let(:parent_project) { create(:project) } + shared_let(:child_project) { create(:project, parent: parent_project) } + shared_let(:unrelated_project) { create(:project) } + + shared_let(:user) { create(:user, member_with_permissions: { child_project => [:view_meetings] }) } + + subject { described_class.templates_visible_globally(user) } + + context "with templates in a project the user has access to" do + shared_let(:template_none) { create(:onetime_template, project: child_project, sharing: :none) } + shared_let(:template_descendants) { create(:onetime_template, project: child_project, sharing: :descendants) } + shared_let(:template_system) { create(:onetime_template, project: child_project, sharing: :system) } + + it { expect(subject).to include(template_none) } + it { expect(subject).to include(template_descendants) } + it { expect(subject).to include(template_system) } + end + + context "with templates in a project the user has no access to" do + shared_let(:template_none) { create(:onetime_template, project: unrelated_project, sharing: :none) } + shared_let(:template_descendants) { create(:onetime_template, project: unrelated_project, sharing: :descendants) } + shared_let(:template_system) { create(:onetime_template, project: unrelated_project, sharing: :system) } + + it { expect(subject).not_to include(template_none) } + it { expect(subject).not_to include(template_descendants) } + it { expect(subject).to include(template_system) } + end + + context "when user has child access only and parent has a :descendants template" do + shared_let(:template_descendants) { create(:onetime_template, project: parent_project, sharing: :descendants) } + shared_let(:template_none) { create(:onetime_template, project: parent_project, sharing: :none) } + + it "includes the :descendants template" do + expect(subject).to include(template_descendants) + end + + it "does not include the :none template from the inaccessible parent" do + expect(subject).not_to include(template_none) + end + end + end + describe "sharing" do context "for a regular meeting" do let(:meeting) { build(:meeting, project:, sharing: :none) }