Fix more project settings related specs

app/assets/javascripts/members_form.js

@@ -77,6 +77,8 @@ jQuery(document).ready(function($) {
   // Toggle filter
   $('.toggle-member-filter-link').click(toggleMemberFilter);
 
+
+
   // Toggle editing row
   $('.toggle-membership-button').click(function() {
     var el = $(this);

app/controllers/groups_controller.rb

@@ -122,6 +122,7 @@ class GroupsController < ApplicationController
     @users = User.includes(:memberships).where(id: params[:user_ids])
     @group.users << @users
 
+    I18n.t :notice_successful_update
     redirect_to controller: '/groups', action: 'edit', id: @group, tab: 'users'
   end
 
@@ -129,6 +130,7 @@ class GroupsController < ApplicationController
     @group = Group.includes(:users).find(params[:id])
     @group.users.delete(User.includes(:memberships).find(params[:user_id]))
 
+    I18n.t :notice_successful_update
     redirect_to controller: '/groups', action: 'edit', id: @group, tab: 'users'
   end
 
@@ -143,8 +145,13 @@ class GroupsController < ApplicationController
     @membership = membership_id.present? ? Member.find(membership_id) : Member.new(principal: @group)
 
     service = ::Members::EditMembershipService.new(@membership, save: true, current_user: current_user)
-    service.call(attributes: membership_params[:membership])
+    result = service.call(attributes: membership_params[:membership])
 
+    if result.success?
+      flash[:notice] = I18n.t :notice_successful_update
+    else
+      flash[:error] = reusult.errors.full_mesages.join("\n")
+    end
     redirect_to controller: '/groups', action: 'edit', id: @group, tab: 'memberships'
   end
 
@@ -153,6 +160,8 @@ class GroupsController < ApplicationController
   def destroy_membership
     membership_params = permitted_params.group_membership
     Member.find(membership_params[:membership_id]).destroy
+
+    flash[:notice] = I18n.t :notice_successful_delete
     redirect_to controller: '/groups', action: 'edit', id: @group, tab: 'memberships'
   end
 

app/controllers/repositories_controller.rb

@@ -56,7 +56,7 @@ class RepositoriesController < ApplicationController
     @repository = @project.repository
     update_repository(params.fetch(:repository, {}))
 
-    redirect_to settings_repository_tab_path
+    redirect_to helpers.settings_repository_tab_path
   end
 
   def create
@@ -69,7 +69,7 @@ class RepositoriesController < ApplicationController
       flash[:error] = service.build_error
     end
 
-    redirect_to settings_repository_tab_path
+    redirect_to helpers.settings_repository_tab_path
   end
 
   def committers
@@ -93,7 +93,7 @@ class RepositoriesController < ApplicationController
 
   def destroy_info
     @repository = @project.repository
-    @back_link = settings_repository_tab_path
+    @back_link = helpers.settings_repository_tab_path
   end
 
   def destroy
@@ -103,7 +103,7 @@ class RepositoriesController < ApplicationController
     else
       flash[:error] = repository.errors.full_messages
     end
-    redirect_to settings_repository_tab_path
+    redirect_to helpers.settings_repository_tab_path
   end
 
   def show
@@ -313,9 +313,9 @@ class RepositoriesController < ApplicationController
     @repository.attributes = @repository.class.permitted_params(repo_params)
 
     if @repository.save
-      flash.now[:notice] = l('repositories.update_settings_successful')
+      flash[:notice] = l('repositories.update_settings_successful')
     else
-      flash.now[:error] = @repository.errors.full_messages.join('\n')
+      flash[:error] = @repository.errors.full_messages.join('\n')
     end
   end
 

app/controllers/users/memberships_controller.rb

@@ -35,11 +35,11 @@ class Users::MembershipsController < ApplicationController
   before_action :find_user
 
   def update
-    update_or_create(request.patch?)
+    update_or_create(request.patch?, :notice_successful_update)
   end
 
   def create
-    update_or_create(request.post?)
+    update_or_create(request.post?, :notice_successful_create)
   end
 
   def destroy
@@ -47,6 +47,7 @@ class Users::MembershipsController < ApplicationController
 
     if @membership.deletable? && request.delete?
       @membership.destroy && @membership = nil
+      flash[:notice] = I18n.t(:notice_successful_delete)
     end
 
     redirect_to controller: '/users', action: 'edit', id: @user, tab: 'memberships'
@@ -54,11 +55,16 @@ class Users::MembershipsController < ApplicationController
 
   private
 
-  def update_or_create(save_record)
+  def update_or_create(save_record, message)
     @membership = params[:id].present? ? Member.find(params[:id]) : Member.new(principal: @user)
     service = ::Members::EditMembershipService.new(@membership, save: save_record, current_user: current_user)
-    service.call(attributes: permitted_params.membership)
+    result = service.call(attributes: permitted_params.membership)
 
+    if result.success?
+      flash[:notice] = I18n.t(message)
+    else
+      flash[:error] = result.errors.full_messages.join("\n")
+    end
     redirect_to controller: '/users', action: 'edit', id: @user, tab: 'memberships'
   end
 

app/controllers/watchers_controller.rb

@@ -64,6 +64,6 @@ class WatchersController < ApplicationController
 
   def set_watcher(user, watching)
     @watched.set_watcher(user, watching)
-    redirect_back(home_url)
+    redirect_back(fallback_location: home_url)
   end
 end

app/helpers/projects_helper.rb

@@ -58,19 +58,19 @@ module ProjectsHelper
       {
         name: 'custom_fields',
         action: :edit_project,
-        partial: 'projects_settings/custom_fields',
+        partial: 'project_settings/custom_fields',
         label: :label_custom_field_plural
       },
       {
         name: 'versions',
         action: :manage_versions,
-        partial: 'projects_settings/versions',
+        partial: 'project_settings/versions',
         label: :label_version_plural
       },
       {
         name: 'categories',
         action: :manage_categories,
-        partial: 'projects_settings/categories',
+        partial: 'project_settings/categories',
         label: :label_work_package_category_plural
       },
       {
@@ -82,19 +82,19 @@ module ProjectsHelper
       {
         name: 'boards',
         action: :manage_boards,
-        partial: 'projects_settings/boards',
+        partial: 'project_settings/boards',
         label: :label_board_plural
       },
       {
         name: 'activities',
         action: :manage_project_activities,
-        partial: 'projects_settings/activities',
+        partial: 'project_settings/activities',
         label: :enumeration_activities
       },
       {
         name: 'types',
         action: :manage_types,
-        partial: 'projects_settings/types',
+        partial: 'project_settings/types',
         label: :label_work_package_types
       }
     ]
@@ -169,9 +169,9 @@ module ProjectsHelper
   end
 
   def project_more_menu_settings_item(project)
-    if User.current.allowed_to?({ controller: 'projects', action: 'settings' }, project)
+    if User.current.allowed_to?({ controller: '/project_settings', action: 'show' }, project)
       [t(:label_project_settings),
-       { controller: 'projects', action: 'settings', id: project },
+       { controller: '/project_settings', action: 'show', id: project },
        class: 'icon-context icon-settings',
        title: t(:label_project_settings)]
     end

app/services/members/edit_membership_service.rb

@@ -78,7 +78,8 @@ module Members
     def validate_attributes!(attributes)
       # We need to check for empty roles here because that _implicitly_
       # deletes the membership and causes failures
-      new_roles_are_empty = attributes[:role_ids].empty?
+      new_roles = attributes[:role_ids]
+      new_roles_are_empty = new_roles.nil? || new_roles.empty?
 
       if new_roles_are_empty && !has_inherited_roles?
         member.errors.add :roles, :role_blank

app/views/members/_member_errors.html.erb

@@ -1,30 +0,0 @@
-<%#-- copyright
-OpenProject is a project management system.
-Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
-
-This program is free software; you can redistribute it and/or
-modify it under the terms of the GNU General Public License version 3.
-
-OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
-Copyright (C) 2006-2017 Jean-Philippe Lang
-Copyright (C) 2010-2013 the ChiliProject Team
-
-This program is free software; you can redistribute it and/or
-modify it under the terms of the GNU General Public License
-as published by the Free Software Foundation; either version 2
-of the License, or (at your option) any later version.
-
-This program is distributed in the hope that it will be useful,
-but WITHOUT ANY WARRANTY; without even the implied warranty of
-MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-GNU General Public License for more details.
-
-You should have received a copy of the GNU General Public License
-along with this program; if not, write to the Free Software
-Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-
-See docs/COPYRIGHT.rdoc for more details.
-
-++#%>
-
-<%= error_messages_for :member, object: member %>

app/views/project_settings/_custom_fields.html.erb

@@ -42,7 +42,7 @@ See docs/COPYRIGHT.rdoc for more details.
 
 <% if  @issue_custom_fields.any? %>
   <%= labelled_tabular_form_for @project,
-                                url: { action: 'custom_fields', id: @project },
+                                url: { controller: '/projects', action: 'custom_fields', id: @project },
                                 method: :put,
                                 html: { id: 'modules-form' } do |form| %>
 

app/views/project_settings/_types.html.erb

@@ -29,7 +29,7 @@ See docs/COPYRIGHT.rdoc for more details.
 
 <% if  Type.all.any? %>
   <%= form_for @project,
-               url: { action: 'types', id: @project },
+               url: { controller: '/projects', action: 'types', id: @project },
                method: :patch,
                html: {id: 'types-form'} do |f| %>
     <%= render partial: 'projects/form/types', locals: { f: f, project: @project, withControlls: true } %>

config/initializers/secure_headers.rb

@@ -29,7 +29,7 @@ SecureHeaders::Configuration.default do |config|
     # Form targets can only be self
     form_action: %w('self'),
     # Allow iframe from vimeo (welcome video)
-    frame_src: %w(https://*.vimeo.com),
+    frame_src: %w(https://*.vimeo.com 'self'),
     frame_ancestors: %w('self'),
     # Allow images from anywhere
     img_src: %w(* data:),

spec/controllers/messages_controller_spec.rb

@@ -212,4 +212,18 @@ describe MessagesController, type: :controller do
       let(:preview_params) { { board_id: board.id, id: message.id, message: {} } }
     end
   end
+
+  describe 'quote' do
+    let(:message) { FactoryGirl.create :message, content: 'foo', subject: 'subject', board: board }
+
+    context 'when allowed' do
+      let(:authorized) { true }
+
+      it 'renders the content as json' do
+        get :quote, params: { board_id: board.id, id: message.id }, format: :json
+        expect(response).to be_success
+        expect(response.body).to eq '{"subject":"RE: subject","content":" wrote:\n\u003e foo\n\n"}'
+      end
+    end
+  end
 end

spec/controllers/repositories_controller_spec.rb

@@ -68,26 +68,6 @@ describe RepositoriesController, type: :controller do
       allow(controller).to receive(:authorize).and_return(true)
     end
 
-    shared_examples_for 'successful settings response' do
-      it 'is successful' do
-        expect(response).to be_success
-      end
-
-      it 'renders the template' do
-        expect(response).to render_template 'repositories/settings/repository_form'
-      end
-    end
-
-    context 'with #edit' do
-      before do
-        get :edit,
-            params: { project_id: project.id, scm_vendor: 'subversion' },
-            xhr: true
-      end
-
-      it_behaves_like 'successful settings response'
-    end
-
     context 'with #destroy' do
       before do
         allow(repository).to receive(:destroy).and_return(true)
@@ -104,7 +84,9 @@ describe RepositoriesController, type: :controller do
         put :update, params: { project_id: project.id }, xhr: true
       end
 
-      it_behaves_like 'successful settings response'
+      it 'redirects to settings' do
+        expect(response).to redirect_to(controller: '/project_settings', id: project.identifier, action: 'show', tab: 'repository')
+      end
     end
 
     context 'with #create' do
@@ -115,13 +97,11 @@ describe RepositoriesController, type: :controller do
                scm_vendor: 'subversion',
                scm_type: 'local',
                url: 'file:///tmp/repo.svn/'
-             },
-             xhr: true
+             }
       end
 
-      it 'renders a JS redirect' do
-        path = "\/projects\/#{project.identifier}/settings\/repository"
-        expect(response.body).to match(/window\.location = '#{path}'/)
+      it 'redirects to settings' do
+        expect(response).to redirect_to(controller: '/project_settings', id: project.identifier, action: 'show', tab: 'repository')
       end
     end
   end

spec/controllers/users/memberships_controller_spec.rb

@@ -48,11 +48,10 @@ describe Users::MembershipsController, type: :controller do
                  project_id: project.id,
                  role_ids: [role.id]
                }
-             },
-             format: 'js'
+             }
       end
 
-      expect(response.status).to eql(200)
+      expect(response).to redirect_to(controller: '/users', action: 'edit', id: user.id, tab: 'memberships')
 
       is_member = user.reload.memberships.any? { |m|
         m.project_id == project.id && m.role_ids.include?(role.id)

spec/features/users/user_memberships_spec.rb

@@ -58,7 +58,7 @@ feature 'user memberships through user page', type: :feature, js: true do
     user_page.expect_project(project.name)
     user_page.edit_roles!(member, %w())
 
-    expect(page).to have_selector('#errorExplanation', 'Please choose at least one role.')
+    expect(page).to have_selector('.flash.error', text: 'Please choose at least one role.')
   end
 
   context 'when user has an inherited role' do

spec/models/users/allowed_to_spec.rb

@@ -302,7 +302,7 @@ describe User, 'allowed_to?' do
              w/ requesting a controller and action allowed by multiple permissions
              w/ the project being public
              w/ anonymous being allowed the action' do
-      let(:permission) { { controller: 'projects', action: 'settings' } }
+      let(:permission) { { controller: '/project_settings', action: 'show' } }
 
       before do
         project.is_public = true
@@ -432,7 +432,7 @@ describe User, 'allowed_to?' do
       end
 
       it 'should be true' do
-        expect(user.allowed_to?({ controller: 'projects', action: 'settings' }, project))
+        expect(user.allowed_to?({ controller: 'projects', action: 'show' }, project))
           .to be_truthy
       end
     end
@@ -560,7 +560,7 @@ describe User, 'allowed_to?' do
       end
 
       it 'should be true' do
-        expect(user.allowed_to?({ controller: 'projects', action: 'settings' }, nil, global: true))
+        expect(user.allowed_to?({ controller: '/project_settings', action: 'show' }, nil, global: true))
           .to be_truthy
       end
     end

spec/routing/repositories_routing_spec.rb

@@ -111,15 +111,6 @@ describe RepositoriesController, type: :routing do
     }
   end
 
-  describe 'edit' do
-    it {
-      expect(get('/projects/testproject/repository/edit'))
-        .to route_to(controller: 'repositories',
-                     action: 'edit',
-                     project_id: 'testproject')
-    }
-  end
-
   describe 'create' do
     it {
       expect(post('/projects/testproject/repository/'))

spec/views/projects/settings.html.erb_spec.rb

@@ -28,7 +28,7 @@
 
 require 'spec_helper'
 
-describe 'projects/settings', type: :view do
+describe 'project_settings/show', type: :view do
   let(:project) { FactoryGirl.build_stubbed(:project) }
 
   describe 'project copy permission' do
@@ -84,6 +84,7 @@ describe 'projects/settings', type: :view do
       assign(:project, project)
       allow(project).to receive(:copy_allowed?).and_return(true)
       allow(User).to receive(:current).and_return(non_admin)
+      allow(view).to receive(:render_tabs).and_return('')
       render
     end
 

spec_legacy/functional/messages_controller_spec.rb

@@ -160,11 +160,4 @@ describe MessagesController, type: :controller do
     assert_redirected_to project_board_path('ecookbook', 1)
     assert_nil Message.find_by(id: 1)
   end
-
-  it 'should quote' do
-    session[:user_id] = 2
-    get :quote, params: { board_id: 1, id: 3 }, xhr: true
-    assert_response :success
-    assert_template 'quote'
-  end
 end

spec_legacy/functional/projects_controller_spec.rb

@@ -389,13 +389,6 @@ describe ProjectsController, type: :controller do
     assert_select 'a', content: /Private child/
   end
 
-  it 'should settings' do
-    session[:user_id] = 2 # manager
-    get :settings, params: { id: 1 }
-    assert_response :success
-    assert_template 'settings'
-  end
-
   it 'should update' do
     session[:user_id] = 2 # manager
     put :update,

spec_legacy/unit/lib/redmine/access_control_spec.rb

@@ -54,6 +54,6 @@ describe Redmine::AccessControl do
     assert_equal :edit_project, perm.name
     assert_nil perm.project_module
     assert perm.actions.is_a?(Array)
-    assert perm.actions.include?('projects/settings')
+    assert perm.actions.include?('project_settings/show')
   end
 end