Merge pull request #13323 from opf/bug/fix-403-when-creating-a-meeting-without-a-project

[#48160] Fix 403 when creating a Meeting globally without a Project

modules/meeting/app/controllers/meetings_controller.rb

@@ -32,8 +32,8 @@ class MeetingsController < ApplicationController
   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: %i[index new]
-  before_action :authorize_global, only: %i[index new]
+  before_action :authorize, except: %i[index new create]
+  before_action :authorize_global, only: %i[index new create]
 
   helper :watchers
   helper :meeting_contents

modules/meeting/app/helpers/meetings_helper.rb

@@ -171,7 +171,15 @@ module MeetingsHelper
     content_tag('div', "#{header}#{details}".html_safe, id: "change-#{journal.id}", class: 'journal')
   end
 
-  def global_create_context?
+  def global_meeting_create_context?
+    global_new_meeting_action? || global_create_meeting_action?
+  end
+
+  def global_new_meeting_action?
     request.path == new_meeting_path
   end
+
+  def global_create_meeting_action?
+    request.path == meetings_path && @project.nil?
+  end
 end

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

@@ -35,7 +35,7 @@ See COPYRIGHT and LICENSE files for more details.
     <%= f.text_field :title, :required => true, :size => 60, container_class: '-wide' %>
   </div>
 
-  <% if global_create_context? %>
+  <% if global_meeting_create_context? %>
     <div class="form--field -required">
       <label class="form--label" for="project_id"><%= Meeting.human_attribute_name(:project) %>:</label>
       <div class="form--field-container">

modules/meeting/spec/controllers/meetings_controller_spec.rb

@@ -135,53 +135,90 @@ RSpec.describe MeetingsController do
         it { expect(assigns(:meeting)).to eql meeting }
       end
     end
+  end
 
+  describe 'POST' do
     describe 'create' do
       render_views
 
+      let(:base_params) do
+        {
+          project_id: project&.id,
+          meeting: meeting_params
+        }
+      end
+
+      let(:base_meeting_params) do
+        {
+          title: 'Foobar',
+          duration: '1.0',
+          start_date: '2015-06-01',
+          start_time_hour: '10:00'
+        }
+      end
+
+      let(:params) { base_params }
+      let(:meeting_params) { base_meeting_params }
+
       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)
-             }
+             params:
       end
 
-      describe 'invalid start_date' do
-        let(:params) do
-          {
-            start_date: '-',
-            start_time_hour: '10:00'
-          }
+      context 'with a project_id' do
+        context 'and an invalid start_date with start_time_hour' do
+          let(:meeting_params) do
+            base_meeting_params.merge(start_date: '-')
+          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
 
+        context 'and an invalid start_time_hour with start_date' do
+          let(:meeting_params) do
+            base_meeting_params.merge(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
+
+      context 'with a nil project_id' do
+        let(:project) { nil }
+
         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')}"
+                              text: "Project #{I18n.t('activerecord.errors.messages.blank')}"
         end
       end
 
-      describe 'invalid start_time_hour' do
-        let(:params) do
-          {
-            start_date: '2015-06-01',
-            start_time_hour: '-'
-          }
-        end
+      context 'without a project_id' do
+        let(:params) { base_params.except(:project_id) }
+        let(:project) { nil }
 
         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')}"
+                              text: "Project #{I18n.t('activerecord.errors.messages.blank')}"
         end
       end
     end

modules/meeting/spec/features/meetings_new_spec.rb

@@ -30,7 +30,7 @@ require 'spec_helper'
 
 require_relative '../support/pages/meetings/index'
 
-RSpec.describe 'Meetings new', :js do
+RSpec.describe 'Meetings new', :js, with_cuprite: false do
   shared_let(:project) { create(:project, enabled_module_names: %w[meetings]) }
   shared_let(:admin) { create(:admin) }
   let(:time_zone) { 'utc' }
@@ -64,19 +64,29 @@ RSpec.describe 'Meetings new', :js do
     end
 
     let(:index_page) { Pages::Meetings::Index.new(project: nil) }
+    let(:new_page) { Pages::Meetings::New.new(nil) }
 
     context 'with permission to create meetings' do
-      it 'does not render menus' do
-        index_page.expect_no_main_menu
+      it 'does not render menus', :with_cuprite do
+        new_page.visit!
+        new_page.expect_no_main_menu
+      end
+
+      describe 'clicking on the create new meeting button', :with_cuprite do
+        it 'navigates to the global create form' do
+          index_page.visit!
+          index_page.click_create_new
+          expect(page).to have_current_path(new_page.path)
+        end
       end
 
       ['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}", with_cuprite: false do
-          index_page.visit!
+        it "allows creating a project and handles errors in time zone #{zone}" do
+          new_page.visit!
 
-          new_page = index_page.click_create_new
+          expect_angular_frontend_initialized # Wait for project dropdown to be ready
 
           new_page.set_title 'Some title'
           new_page.set_project project
@@ -95,9 +105,53 @@ RSpec.describe 'Meetings new', :js do
           show_page.expect_date_time "03/28/2013 01:30 PM - 03:00 PM"
         end
       end
+
+      context 'without a title set' do
+        before do
+          new_page.visit!
+
+          # Wait for project dropdown to be initialized
+          expect_angular_frontend_initialized
+
+          new_page.set_project project
+
+          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)
+        end
+
+        it 'renders a validation error' do
+          expect do
+            new_page.click_create
+          end.not_to change(Query, :count)
+
+          # HTML required attribute validation error
+          expect(page).to have_current_path(new_page.path)
+        end
+      end
+
+      context 'without a project set' do
+        before do
+          new_page.visit!
+          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'
+        end
+
+        it 'renders a validation error' do
+          new_page.click_create
+
+          new_page.expect_toast(message: "#{Project.model_name.human} #{I18n.t('activerecord.errors.messages.blank')}",
+                                type: :error)
+
+          new_page.expect_project_dropdown
+        end
+      end
     end
 
-    context 'without permission to create meetings' do
+    context 'without permission to create meetings', :with_cuprite do
       let(:permissions) { %i[view_meetings] }
 
       it 'shows no edit link' do
@@ -107,13 +161,14 @@ RSpec.describe 'Meetings new', :js do
       end
     end
 
-    context 'as an admin' do
+    context 'as an admin', :with_cuprite do
       let(:current_user) { admin }
 
-      it 'allows creating meeting in a project without members', with_cuprite: false do
-        index_page.visit!
+      it 'allows creating meeting in a project without members' do
+        new_page.visit!
+
+        expect_angular_frontend_initialized # Wait for project dropdown to be ready
 
-        new_page = index_page.click_create_new
         new_page.set_title 'Some title'
 
         new_page.set_project project
@@ -125,24 +180,66 @@ RSpec.describe 'Meetings new', :js do
         # Not sure if that is then intended behaviour but that is what is currently programmed
         show_page.expect_invited(admin)
       end
+
+      context 'without a project set' do
+        before do
+          new_page.visit!
+          new_page.set_title 'Some title'
+        end
+
+        it 'renders a validation error' do
+          new_page.click_create
+
+          new_page.expect_toast(message: "#{Project.model_name.human} #{I18n.t('activerecord.errors.messages.blank')}",
+                                type: :error)
+          new_page.expect_project_dropdown
+        end
+      end
+
+      context 'without a title set' do
+        before do
+          new_page.visit!
+
+          # Wait for project dropdown to be initialized
+          expect_angular_frontend_initialized
+
+          new_page.set_project project
+        end
+
+        it 'renders a validation error' do
+          expect do
+            new_page.click_create
+          end.not_to change(Query, :count)
+
+          # HTML required attribute validation error
+          expect(page).to have_current_path(new_page.path)
+        end
+      end
     end
   end
 
   context 'when creating a meeting from the project-specific page' do
     let(:index_page) { Pages::Meetings::Index.new(project:) }
+    let(:new_page) { Pages::Meetings::New.new(project) }
 
     context 'with permission to create meetings' do
       before do
         other_user
       end
 
+      describe 'clicking on the create new meeting button', :with_cuprite do
+        it 'navigates to the project-specific create form' do
+          index_page.visit!
+          index_page.click_create_new
+          expect(page).to have_current_path(new_page.path)
+        end
+      end
+
       ['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}", with_cuprite: false do
-          index_page.visit!
-
-          new_page = index_page.click_create_new
+        it "allows creating a project and handles errors in time zone #{zone}" do
+          new_page.visit!
 
           new_page.set_title 'Some title'
           new_page.set_start_date '2013-03-28'
@@ -159,9 +256,28 @@ RSpec.describe 'Meetings new', :js do
           show_page.expect_date_time "03/28/2013 01:30 PM - 03:00 PM"
         end
       end
+
+      context 'without a title set' do
+        before do
+          new_page.visit!
+          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)
+        end
+
+        it 'renders a validation error' do
+          expect do
+            new_page.click_create
+          end.not_to change(Query, :count)
+
+          # HTML required attribute validation error
+          expect(page).to have_current_path(new_page.path)
+        end
+      end
     end
 
-    context 'without permission to create meetings' do
+    context 'without permission to create meetings', :with_cuprite do
       let(:permissions) { %i[view_meetings] }
 
       it 'shows no edit link' do
@@ -171,7 +287,7 @@ RSpec.describe 'Meetings new', :js do
       end
     end
 
-    context 'as an admin' do
+    context 'as an admin', :with_cuprite do
       let(:current_user) { admin }
       let(:field) do
         TextEditorField.new(page,
@@ -180,9 +296,7 @@ RSpec.describe 'Meetings new', :js do
       end
 
       it 'allows creating meeting in a project without members' do
-        index_page.visit!
-
-        new_page = index_page.click_create_new
+        new_page.visit!
 
         new_page.set_title 'Some title'
 
@@ -194,10 +308,23 @@ RSpec.describe 'Meetings new', :js do
         show_page.expect_invited(admin)
       end
 
-      it 'can save the meeting agenda via cmd+Enter' do
-        index_page.visit!
+      context 'without a title set' do
+        before do
+          new_page.visit!
+        end
 
-        new_page = index_page.click_create_new
+        it 'renders a validation error' do
+          expect do
+            new_page.click_create
+          end.not_to change(Query, :count)
+
+          # HTML required attribute validation error
+          expect(page).to have_current_path(new_page.path)
+        end
+      end
+
+      it 'can save the meeting agenda via cmd+Enter' do
+        new_page.visit!
 
         new_page.set_title 'Some title'
 

modules/meeting/spec/support/pages/meetings/new.rb

@@ -33,6 +33,10 @@ module Pages::Meetings
   class New < Base
     include Components::Autocompleter::NgSelectAutocompleteHelpers
 
+    def expect_no_main_menu
+      expect(page).not_to have_selector '#main-menu'
+    end
+
     def click_create
       click_button 'Create'
 
@@ -49,8 +53,12 @@ module Pages::Meetings
       fill_in 'Title', with: text
     end
 
+    def expect_project_dropdown
+      find "[data-qa-selector='project_id']"
+    end
+
     def set_project(project)
-      select_autocomplete find("[data-qa-selector='project_id'"),
+      select_autocomplete find("[data-qa-selector='project_id']"),
                           query: project.name,
                           results_selector: 'body'
     end