From ec10b80641ce37ad542be437d787bd12e71fba22 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Tue, 3 Feb 2015 09:07:45 +0100 Subject: [PATCH 01/59] Implemented get categories as specified, except for errors --- lib/api/v3/categories/categories_api.rb | 17 ++++--- .../categories/categories_by_project_api.rb | 50 +++++++++++++++++++ lib/api/v3/categories/category_representer.rb | 21 ++++++++ lib/api/v3/projects/projects_api.rb | 4 +- lib/api/v3/root.rb | 1 + lib/api/v3/utilities/path_helper.rb | 4 ++ 6 files changed, 87 insertions(+), 10 deletions(-) create mode 100644 lib/api/v3/categories/categories_by_project_api.rb diff --git a/lib/api/v3/categories/categories_api.rb b/lib/api/v3/categories/categories_api.rb index 34a3bc1e67..10649b0e49 100644 --- a/lib/api/v3/categories/categories_api.rb +++ b/lib/api/v3/categories/categories_api.rb @@ -32,16 +32,17 @@ module API module Categories class CategoriesAPI < Grape::API resources :categories do - before do - @categories = @project.categories - end - get do - self_link = api_v3_paths.categories(@project.identifier) + namespace ':id' do + + before do + @category = Category.find(params[:id]) + authorize(:view_project, context: @category.project) + end - CategoryCollectionRepresenter.new(@categories, - @categories.count, - self_link) + get do + CategoryRepresenter.new(@category) + end end end end diff --git a/lib/api/v3/categories/categories_by_project_api.rb b/lib/api/v3/categories/categories_by_project_api.rb new file mode 100644 index 0000000000..c25853c81a --- /dev/null +++ b/lib/api/v3/categories/categories_by_project_api.rb @@ -0,0 +1,50 @@ +#-- encoding: UTF-8 +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 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-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 doc/COPYRIGHT.rdoc for more details. +#++ + +module API + module V3 + module Categories + class CategoriesByProjectAPI < Grape::API + resources :categories do + before do + @categories = @project.categories + end + + get do + self_link = api_v3_paths.categories(@project.identifier) + + CategoryCollectionRepresenter.new(@categories, + @categories.count, + self_link) + end + end + end + end + end +end diff --git a/lib/api/v3/categories/category_representer.rb b/lib/api/v3/categories/category_representer.rb index c311296e09..895622e025 100644 --- a/lib/api/v3/categories/category_representer.rb +++ b/lib/api/v3/categories/category_representer.rb @@ -34,6 +34,27 @@ module API module V3 module Categories class CategoryRepresenter < ::API::Decorators::Single + link :self do + { + href: api_v3_paths.categories(represented.id), + title: "#{represented.name}" + } + end + + link :project do + { + href: api_v3_paths.project(represented.project.id), + title: represented.project.name + } + end + + link :user do + { + href: api_v3_paths.user(represented.assigned_to.id), + title: "#{represented.assigned_to.name} - #{represented.assigned_to.login}" + } if represented.assigned_to + end + property :id, render_nil: true property :name, render_nil: true diff --git a/lib/api/v3/projects/projects_api.rb b/lib/api/v3/projects/projects_api.rb index 5f2c6b0f0e..08bb4b7977 100644 --- a/lib/api/v3/projects/projects_api.rb +++ b/lib/api/v3/projects/projects_api.rb @@ -38,16 +38,16 @@ module API namespace ':id' do before do @project = Project.find(params[:id]) + authorize(:view_project, context: @project) end get do - authorize(:view_project, context: @project) ProjectRepresenter.new(@project) end mount API::V3::Projects::AvailableAssigneesAPI mount API::V3::Projects::AvailableResponsiblesAPI - mount API::V3::Categories::CategoriesAPI + mount API::V3::Categories::CategoriesByProjectAPI mount API::V3::Versions::ProjectsVersionsAPI end end diff --git a/lib/api/v3/root.rb b/lib/api/v3/root.rb index 397f055ed3..b7ffcf49da 100644 --- a/lib/api/v3/root.rb +++ b/lib/api/v3/root.rb @@ -38,6 +38,7 @@ module API mount ::API::V3::Activities::ActivitiesAPI mount ::API::V3::Attachments::AttachmentsAPI + mount ::API::V3::Categories::CategoriesAPI mount ::API::V3::Priorities::PrioritiesAPI mount ::API::V3::Projects::ProjectsAPI mount ::API::V3::Queries::QueriesAPI diff --git a/lib/api/v3/utilities/path_helper.rb b/lib/api/v3/utilities/path_helper.rb index 2d8826e2c6..b033bae36f 100644 --- a/lib/api/v3/utilities/path_helper.rb +++ b/lib/api/v3/utilities/path_helper.rb @@ -62,6 +62,10 @@ module API "#{project(project_id)}/categories" end + def self.category(id) + "#{root}/categories/#{id}" + end + def self.preview_textile(link) preview_markup(:textile, link) end From 38c598bc5e48e81343ab16659eda02c68197de49 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Tue, 3 Feb 2015 10:41:50 +0100 Subject: [PATCH 02/59] Partially expanded api specification link verbosity --- doc/apiv3-documentation.apib | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/doc/apiv3-documentation.apib b/doc/apiv3-documentation.apib index 2adba69c0c..7ad7eb0144 100644 --- a/doc/apiv3-documentation.apib +++ b/doc/apiv3-documentation.apib @@ -599,9 +599,18 @@ Updates an activity's comment and, on success, returns the updated activity. "elements": [ { "_links": { - "self": { "href": "/api/v3/categories/10" }, - "project": { "href": "/api/v3/projects/11" }, - "defaultAssignee": { "href": "/api/v3/users/42" } + "self": { + "href": "/api/v3/categories/10", + "title": "Category with assignee" + }, + "project": { + "href": "/api/v3/projects/11", + "title": "Example project" + }, + "defaultAssignee": { + "href": "/api/v3/users/42", + "title": "John Sheppard" + } }, "_type": "Category", "id": 10, @@ -654,9 +663,18 @@ Updates an activity's comment and, on success, returns the updated activity. { "_links": { - "self": { "href": "/api/v3/categories/10" }, - "project": { "href": "/api/v3/projects/11" }, - "defaultAssignee": { "href": "/api/v3/users/42" } + "self": { + "href": "/api/v3/categories/10", + "title": "Category with assignee" + }, + "project": { + "href": "/api/v3/projects/11", + "title": "Example project" + }, + "defaultAssignee": { + "href": "/api/v3/users/42", + "title": "John Sheppard" + } }, "_type": "Category", "id": 10, From 1e27f6c46f96b7c62ced7aca08d056a80cf03d6b Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Wed, 4 Feb 2015 10:35:16 +0100 Subject: [PATCH 03/59] Removed unnecessary string interpolation --- lib/api/v3/categories/category_representer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/v3/categories/category_representer.rb b/lib/api/v3/categories/category_representer.rb index 895622e025..49edffaa5e 100644 --- a/lib/api/v3/categories/category_representer.rb +++ b/lib/api/v3/categories/category_representer.rb @@ -51,7 +51,7 @@ module API link :user do { href: api_v3_paths.user(represented.assigned_to.id), - title: "#{represented.assigned_to.name} - #{represented.assigned_to.login}" + title: represented.assigned_to.name } if represented.assigned_to end From 6e5869974c72e3a430a129e50525832ddce0faa3 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Wed, 4 Feb 2015 10:41:31 +0100 Subject: [PATCH 04/59] Added optional test for title to self link in project representer --- spec/lib/api/v3/projects/project_representer_spec.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/lib/api/v3/projects/project_representer_spec.rb b/spec/lib/api/v3/projects/project_representer_spec.rb index 4c9b795a4d..fff98709a3 100644 --- a/spec/lib/api/v3/projects/project_representer_spec.rb +++ b/spec/lib/api/v3/projects/project_representer_spec.rb @@ -52,6 +52,9 @@ describe ::API::V3::Projects::ProjectRepresenter do it 'should link to self' do expect(subject).to have_json_path('_links/self/href') end + it 'should have a title for link to self' do + expect(subject).to have_json_path('_links/self/title') + end describe 'categories' do it { should have_json_path('_links/categories') } From bb671041d3d6221f3e4dfa3c7ccd14466aefb807 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Wed, 4 Feb 2015 10:51:11 +0100 Subject: [PATCH 05/59] Added test for path to single category --- spec/lib/api/v3/utilities/path_helper_spec.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/lib/api/v3/utilities/path_helper_spec.rb b/spec/lib/api/v3/utilities/path_helper_spec.rb index 968091cc5f..1cb1dd9d2e 100644 --- a/spec/lib/api/v3/utilities/path_helper_spec.rb +++ b/spec/lib/api/v3/utilities/path_helper_spec.rb @@ -89,6 +89,14 @@ describe ::API::V3::Utilities::PathHelper do it { is_expected.to match(/^\/api\/v3\/projects\/42\/categories/) } end + describe '#category' do + subject { helper.category 42 } + + it_behaves_like 'api v3 path' + + it { is_expected.to match(/^\/api\/v3\/categories\/42/) } + end + describe '#preview_textile' do subject { helper.preview_textile '/api/v3/work_packages/42' } From 38363530e2d887ab068f3ea8140b17fb9152f992 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Wed, 4 Feb 2015 11:00:20 +0100 Subject: [PATCH 06/59] Added category representer specs for implementation --- .../categories/category_representer_spec.rb | 46 ++++++++++++++++--- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/spec/lib/api/v3/categories/category_representer_spec.rb b/spec/lib/api/v3/categories/category_representer_spec.rb index 2eb065fb3f..3be69db115 100644 --- a/spec/lib/api/v3/categories/category_representer_spec.rb +++ b/spec/lib/api/v3/categories/category_representer_spec.rb @@ -30,21 +30,53 @@ require 'spec_helper' describe ::API::V3::Categories::CategoryRepresenter do let(:category) { FactoryGirl.build(:category) } + let(:user) { FactoryGirl.build(:user) } let(:representer) { described_class.new(category) } context 'generation' do subject(:generated) { representer.to_json } - it { should include_json('Category'.to_json).at_path('_type') } + shared_examples_for 'category has core values' do + it { is_expected.to include_json('Category'.to_json).at_path('_type') } - xit { should have_json_type(Object).at_path('_links') } - xit 'should link to self' do - expect(subject).to have_json_path('_links/self/href') + it { is_expected.to have_json_type(Object).at_path('_links') } + it 'should link to self' do + expect(subject).to have_json_path('_links/self/href') + end + it 'should display its name as title in self' do + expect(subject).to have_json_path('_links/self/title') + end + it 'should link to its project' do + expect(subject).to have_json_path('_links/project/href') + end + it 'should display its project title' do + expect(subject).to have_json_path('_links/project/title') + end + + it { is_expected.to have_json_path('id') } + it { is_expected.to have_json_path('name') } end - describe 'category' do - it { should have_json_path('id') } - it { should have_json_path('name') } + context 'default assignee not set' do + it_behaves_like 'category has core values' + + it 'should not link to an assignee' do + expect(subject).to_not have_json_path('_links/user') + end + end + + context 'default assignee set' do + let(:category) { + FactoryGirl.build(:category, assigned_to: user) + } + it_behaves_like 'category has core values' + + it 'should link to its default assignee' do + expect(subject).to have_json_path('_links/user/href') + end + it 'should display the name of its default assignee' do + expect(subject).to have_json_path('_links/user/title') + end end end end From 21c50fd41abc7d30b1d3a615d52877b62e7ce64b Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Wed, 4 Feb 2015 13:30:20 +0100 Subject: [PATCH 07/59] Accessing a project without permission now returns 404 instead of 403 --- lib/api/root.rb | 15 ++++++++++++--- lib/api/v3/categories/categories_api.rb | 4 +++- lib/api/v3/projects/projects_api.rb | 5 ++++- lib/api/v3/work_packages/work_packages_api.rb | 7 ++++++- spec/requests/api/v3/project_resource_spec.rb | 5 ++++- spec/requests/api/v3/support/api_helper.rb | 2 +- 6 files changed, 30 insertions(+), 8 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 1deccdda7b..3c5c274ab9 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -70,12 +70,21 @@ module API raise API::Errors::Unauthenticated if current_user.nil? || current_user.anonymous? if Setting.login_required? end - def authorize(permission, context: nil, global: false, user: current_user, allow: true) + def authorize(permission, context: nil, global: false, user: current_user, &block) is_authorized = AuthorizationService.new(permission, context: context, global: global, user: user).call - raise API::Errors::Unauthorized unless is_authorized && allow - is_authorized + + return true if is_authorized + + if block_given? + yield block + else + raise API::Errors::Unauthorized + end + + false end + def running_in_test_env? Rails.env.test? && ENV['CAPYBARA_DISABLE_TEST_AUTH_PROTECTION'] != 'true' end diff --git a/lib/api/v3/categories/categories_api.rb b/lib/api/v3/categories/categories_api.rb index 10649b0e49..7adfe75990 100644 --- a/lib/api/v3/categories/categories_api.rb +++ b/lib/api/v3/categories/categories_api.rb @@ -37,7 +37,9 @@ module API before do @category = Category.find(params[:id]) - authorize(:view_project, context: @category.project) + authorize(:view_project, context: @category.project) do + raise API::Errors::NotFound.new(I18n.t('api_v3.errors.code_404', type: I18n.t('activerecord.models.category'), id: params[:id])) + end end get do diff --git a/lib/api/v3/projects/projects_api.rb b/lib/api/v3/projects/projects_api.rb index 08bb4b7977..ef917b46c0 100644 --- a/lib/api/v3/projects/projects_api.rb +++ b/lib/api/v3/projects/projects_api.rb @@ -38,7 +38,10 @@ module API namespace ':id' do before do @project = Project.find(params[:id]) - authorize(:view_project, context: @project) + + authorize(:view_project, context: @project) do + raise API::Errors::NotFound.new(I18n.t('api_v3.errors.code_404', type: I18n.t('activerecord.models.project'), id: params[:id])) + end end get do diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index 02d4898d5b..406dea9d3a 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -121,7 +121,12 @@ module API requires :comment, type: String end post do - authorize({ controller: :journals, action: :new }, context: @work_package.project) + authorize({ controller: :journals, action: :new }, + context: @work_package.project) do + raise API::Errors::NotFound.new(I18n.t('api_v3.errors.code_404', + type: I18n.t('activerecord.models.work_package'), + id: params[:id])) + end @work_package.journal_notes = params[:comment] diff --git a/spec/requests/api/v3/project_resource_spec.rb b/spec/requests/api/v3/project_resource_spec.rb index 76b6c9a7c0..725b146c5f 100644 --- a/spec/requests/api/v3/project_resource_spec.rb +++ b/spec/requests/api/v3/project_resource_spec.rb @@ -71,7 +71,10 @@ describe 'API v3 Project resource' do let(:another_project) { FactoryGirl.create(:project, is_public: false) } let(:get_path) { "/api/v3/projects/#{another_project.id}" } - it_behaves_like 'unauthorized access' + it_behaves_like 'not found' do + let(:id) { "#{another_project.id}" } + let(:type) { 'Project' } + end end end end diff --git a/spec/requests/api/v3/support/api_helper.rb b/spec/requests/api/v3/support/api_helper.rb index dcbe4a534f..8520c7aeed 100644 --- a/spec/requests/api/v3/support/api_helper.rb +++ b/spec/requests/api/v3/support/api_helper.rb @@ -27,7 +27,7 @@ #++ shared_examples_for 'safeguarded API' do - it { expect(response.response_code).to eq(403) } + it { expect(response.response_code).to eq(404) } end shared_examples_for 'valid activity request' do From ea34900d6edf794c5deaa045879b487b2d23bc69 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Wed, 4 Feb 2015 13:30:56 +0100 Subject: [PATCH 08/59] Added specs for category endpoint --- .../requests/api/v3/category_resource_spec.rb | 89 ++++++++++++++++--- 1 file changed, 75 insertions(+), 14 deletions(-) diff --git a/spec/requests/api/v3/category_resource_spec.rb b/spec/requests/api/v3/category_resource_spec.rb index ae9025fdd3..24f24669b3 100644 --- a/spec/requests/api/v3/category_resource_spec.rb +++ b/spec/requests/api/v3/category_resource_spec.rb @@ -32,30 +32,91 @@ require 'rack/test' describe 'API v3 Category resource' do include Rack::Test::Methods - let(:current_user) { FactoryGirl.create(:user) } - let(:role) { FactoryGirl.create(:role, permissions: []) } - let(:project) { FactoryGirl.create(:project, is_public: false) } - let(:categories) { FactoryGirl.create_list(:category, 3, project: project) } - let(:other_categories) { FactoryGirl.create_list(:category, 2) } + let(:role) { FactoryGirl.create(:role, permissions: [:view_project]) } + let(:private_project) { FactoryGirl.create(:project, is_public: false) } + let(:public_project) { FactoryGirl.create(:project, is_public: true) } + let(:anonymous_user) { FactoryGirl.create(:user) } + let(:privileged_user) do + FactoryGirl.create(:user, + member_in_project: private_project, + member_through_role: role) + end + + let!(:categories) { FactoryGirl.create_list(:category, 3, project: private_project) } + let!(:other_categories) { FactoryGirl.create_list(:category, 2, project: public_project) } + let!(:user_categories) do + FactoryGirl.create_list(:category, + 2, + project: private_project, + assigned_to: privileged_user) + end - describe '#get' do + describe 'categories by project' do subject(:response) { last_response } context 'logged in user' do - let(:get_path) { "/api/v3/projects/#{project.id}/categories" } + let(:get_path) { "/api/v3/projects/#{private_project.id}/categories" } before do - allow(User).to receive(:current).and_return current_user - member = FactoryGirl.build(:member, user: current_user, project: project) - member.role_ids = [role.id] - member.save! + allow(User).to receive(:current).and_return privileged_user + + get get_path + end + + it_behaves_like 'API V3 collection response', 5, 5, 'Category' + end - categories - other_categories + context 'not logged in user' do + let(:get_path) { "/api/v3/projects/#{private_project.id}/categories" } + before do + allow(User).to receive(:current).and_return anonymous_user get get_path end - it_behaves_like 'API V3 collection response', 3, 3, 'Category' + it_behaves_like 'not found', + let(:id) { "#{private_project.id}" } + let(:type) { 'Project' } + end + end + + describe 'categories/:id' do + subject(:response) { last_response } + + context 'logged in user' do + let(:get_path) { "/api/v3/categories/#{other_categories.first.id}" } + before do + allow(User).to receive(:current).and_return privileged_user + + get get_path + end + + context 'valid priority id' do + it 'should return HTTP 200' do + expect(response.status).to eql(200) + end + end + + context 'invalid priority id' do + let(:get_path) { '/api/v3/categories/bogus' } + it_behaves_like 'not found' do + let(:id) { 'bogus' } + let(:type) { 'Category' } + end + end + end + + context 'not logged in user' do + let(:get_path) { '/api/v3/categories/bogus' } + before do + allow(User).to receive(:current).and_return anonymous_user + + get get_path + end + + it_behaves_like 'not found' do + let(:id) { 'bogus' } + let(:type) { 'Category' } + end end end end From 65c39db9cf68191d8d485459b3739c1b959a2dbd Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Wed, 4 Feb 2015 14:26:06 +0100 Subject: [PATCH 09/59] Some minor style fixes --- lib/api/root.rb | 10 +++-- lib/api/v3/categories/categories_api.rb | 7 ++-- lib/api/v3/projects/projects_api.rb | 5 ++- lib/api/v3/work_packages/work_packages_api.rb | 41 ++++++++++--------- .../requests/api/v3/category_resource_spec.rb | 3 +- 5 files changed, 39 insertions(+), 27 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 3c5c274ab9..892c1ae071 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -67,11 +67,16 @@ module API end def authenticate - raise API::Errors::Unauthenticated if current_user.nil? || current_user.anonymous? if Setting.login_required? + if Setting.login_required? && (current_user.nil? || current_user.anonymous?) + raise API::Errors::Unauthenticated + end end def authorize(permission, context: nil, global: false, user: current_user, &block) - is_authorized = AuthorizationService.new(permission, context: context, global: global, user: user).call + is_authorized = AuthorizationService.new(permission, + context: context, + global: global, + user: user).call return true if is_authorized @@ -84,7 +89,6 @@ module API false end - def running_in_test_env? Rails.env.test? && ENV['CAPYBARA_DISABLE_TEST_AUTH_PROTECTION'] != 'true' end diff --git a/lib/api/v3/categories/categories_api.rb b/lib/api/v3/categories/categories_api.rb index 7adfe75990..379bbeae54 100644 --- a/lib/api/v3/categories/categories_api.rb +++ b/lib/api/v3/categories/categories_api.rb @@ -32,13 +32,14 @@ module API module Categories class CategoriesAPI < Grape::API resources :categories do - namespace ':id' do - before do @category = Category.find(params[:id]) authorize(:view_project, context: @category.project) do - raise API::Errors::NotFound.new(I18n.t('api_v3.errors.code_404', type: I18n.t('activerecord.models.category'), id: params[:id])) + raise API::Errors::NotFound.new( + I18n.t('api_v3.errors.code_404', + type: I18n.t('activerecord.models.category'), + id: params[:id])) end end diff --git a/lib/api/v3/projects/projects_api.rb b/lib/api/v3/projects/projects_api.rb index ef917b46c0..536b9d08de 100644 --- a/lib/api/v3/projects/projects_api.rb +++ b/lib/api/v3/projects/projects_api.rb @@ -40,7 +40,10 @@ module API @project = Project.find(params[:id]) authorize(:view_project, context: @project) do - raise API::Errors::NotFound.new(I18n.t('api_v3.errors.code_404', type: I18n.t('activerecord.models.project'), id: params[:id])) + raise API::Errors::NotFound.new( + I18n.t('api_v3.errors.code_404', + type: I18n.t('activerecord.models.project'), + id: params[:id])) end end diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index 406dea9d3a..19b70cf1e4 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -31,19 +31,18 @@ module API module WorkPackages class WorkPackagesAPI < Grape::API resources :work_packages do - params do requires :id, desc: 'Work package id' end namespace ':id' do - helpers do attr_reader :work_package def write_work_package_attributes if request_body - payload = ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter - .new(@work_package, enforce_lock_version_validation: true) + payload = ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter.new( + @work_package, + enforce_lock_version_validation: true) begin payload.from_json(request_body.to_json) @@ -60,17 +59,22 @@ module API def write_request_valid? contract = WorkPackageContract.new(@representer.represented, current_user) - # We need to merge the contract errors with the model errors in - # order to have them available at one place. - unless contract.validate & @representer.represented.valid? - contract.errors.keys.each do |key| - contract.errors[key].each do |message| + contract_valid = contract.validate + represented_valid = @representer.represented.valid? + + if contract_valid && represented_valid + true + else + # We need to merge the contract errors with the model errors in + # order to have them available at one place. + contract.errors.each do |key, messages| + messages.each do |message| @representer.represented.errors.add(key, message) end end - end - @representer.represented.errors.count == 0 + false + end end end @@ -104,11 +108,12 @@ module API end resource :activities do - helpers do def save_work_package(work_package) if work_package.save - representer = ::API::V3::Activities::ActivityRepresenter.new(work_package.journals.last, current_user: current_user) + representer = ::API::V3::Activities::ActivityRepresenter.new( + work_package.journals.last, + current_user: current_user) representer else @@ -123,24 +128,22 @@ module API post do authorize({ controller: :journals, action: :new }, context: @work_package.project) do - raise API::Errors::NotFound.new(I18n.t('api_v3.errors.code_404', - type: I18n.t('activerecord.models.work_package'), - id: params[:id])) + raise API::Errors::NotFound.new( + I18n.t('api_v3.errors.code_404', + type: I18n.t('activerecord.models.work_package'), + id: params[:id])) end @work_package.journal_notes = params[:comment] save_work_package(@work_package) end - end mount ::API::V3::WorkPackages::WatchersAPI mount ::API::V3::Relations::RelationsAPI mount ::API::V3::WorkPackages::Form::FormAPI - end - end end end diff --git a/spec/requests/api/v3/category_resource_spec.rb b/spec/requests/api/v3/category_resource_spec.rb index 24f24669b3..f8c5958074 100644 --- a/spec/requests/api/v3/category_resource_spec.rb +++ b/spec/requests/api/v3/category_resource_spec.rb @@ -73,9 +73,10 @@ describe 'API v3 Category resource' do get get_path end - it_behaves_like 'not found', + it_behaves_like 'not found' do let(:id) { "#{private_project.id}" } let(:type) { 'Project' } + end end end From dcc33a826e5e839e9b79055af3ef1509ae8742f9 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Thu, 5 Feb 2015 16:41:22 +0100 Subject: [PATCH 10/59] Fixed previous style "improvement" --- lib/api/v3/work_packages/work_packages_api.rb | 23 ++++++++----------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index 19b70cf1e4..66bb3624e7 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -59,22 +59,17 @@ module API def write_request_valid? contract = WorkPackageContract.new(@representer.represented, current_user) - contract_valid = contract.validate - represented_valid = @representer.represented.valid? - - if contract_valid && represented_valid - true - else - # We need to merge the contract errors with the model errors in - # order to have them available at one place. - contract.errors.each do |key, messages| - messages.each do |message| - @representer.represented.errors.add(key, message) - end - end + return true if contract.validate && @representer.represented.valid? - false + # We need to merge the contract errors with the model errors in + # order to have them available at one place. + contract.errors.keys.each do |key| + contract.errors[key].each do |message| + @representer.represented.errors.add(key, message) + end end + + false end end From bd1ffaee104481d871c16aad647e1940d8f259d5 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Fri, 6 Feb 2015 10:42:12 +0100 Subject: [PATCH 11/59] Fix authorize usage --- lib/api/root.rb | 8 ++++++++ lib/api/v3/queries/queries_api.rb | 16 ++++++++-------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 892c1ae071..45ed36bac8 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -89,6 +89,14 @@ module API false end + def authorize_by_with_raise(&block) + unless yield + raise API::Errors::Unauthorized + else + true + end + end + def running_in_test_env? Rails.env.test? && ENV['CAPYBARA_DISABLE_TEST_AUTH_PROTECTION'] != 'true' end diff --git a/lib/api/v3/queries/queries_api.rb b/lib/api/v3/queries/queries_api.rb index d7f71fe2b6..8db952691d 100644 --- a/lib/api/v3/queries/queries_api.rb +++ b/lib/api/v3/queries/queries_api.rb @@ -45,16 +45,16 @@ module API end helpers do - def allowed_to_manage_stars? - # TODO: find a better way - action = env['api.endpoint'].options[:path].first - QueryPolicy.new(current_user).allowed?(@query, action) + def authorize_by_policy(action) + authorize_by_with_raise do + QueryPolicy.new(current_user).allowed?(@query, action) + end end end patch :star do - # TODO Replace by QueryPolicy - authorize({ controller: :queries, action: :star }, context: @query.project, allow: allowed_to_manage_stars?) + authorize_by_policy(:star) + # Query name is not user-visible, but apparently used as CSS class. WTF. # Normalizing the query name can result in conflicts and empty names in case all # characters are filtered out. A random name doesn't have these problems. @@ -66,8 +66,8 @@ module API end patch :unstar do - # TODO Replace by QueryPolicy - authorize({ controller: :queries, action: :unstar }, context: @query.project, allow: allowed_to_manage_stars?) + authorize_by_policy(:unstar) + query_menu_item = @query.query_menu_item return @representer if @query.query_menu_item.nil? query_menu_item.destroy From d895b82db69b0668c9867a107c15e4c565aee7c8 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Fri, 6 Feb 2015 10:56:04 +0100 Subject: [PATCH 12/59] Some minor style fixes --- lib/api/root.rb | 8 ++++---- lib/api/v3/queries/queries_api.rb | 3 --- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 45ed36bac8..2923886d41 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -89,11 +89,11 @@ module API false end - def authorize_by_with_raise(&block) - unless yield - raise API::Errors::Unauthorized - else + def authorize_by_with_raise(&_block) + if yield true + else + raise API::Errors::Unauthorized end end diff --git a/lib/api/v3/queries/queries_api.rb b/lib/api/v3/queries/queries_api.rb index 8db952691d..19141c2fcd 100644 --- a/lib/api/v3/queries/queries_api.rb +++ b/lib/api/v3/queries/queries_api.rb @@ -33,12 +33,10 @@ module API module Queries class QueriesAPI < Grape::API resources :queries do - params do requires :id, desc: 'Query id' end namespace ':id' do - before do @query = Query.find(params[:id]) @representer = ::API::V3::Queries::QueryRepresenter.new(@query) @@ -75,7 +73,6 @@ module API @representer end end - end end end From 52dc6db8b73f3c934fa13d697e455f7ced92f617 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Fri, 6 Feb 2015 15:23:39 +0100 Subject: [PATCH 13/59] Another style "improvement" fix --- lib/api/v3/work_packages/work_packages_api.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index 66bb3624e7..bcd9649534 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -59,7 +59,10 @@ module API def write_request_valid? contract = WorkPackageContract.new(@representer.represented, current_user) - return true if contract.validate && @representer.represented.valid? + contract_valid = contract.validate + represented_valid = @representer.represented.valid? + + return true if contract_valid && represented_valid # We need to merge the contract errors with the model errors in # order to have them available at one place. From d99e654624dd4a2a0284d5226c8c8627e7bd93ea Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Mon, 22 Dec 2014 15:40:23 +0200 Subject: [PATCH 14/59] switch to 0xf013/ui-select --- frontend/bower.json | 2 +- .../templates/components/inplace_editor/editable/select2.html | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/bower.json b/frontend/bower.json index 47f030d8e0..40070d1861 100644 --- a/frontend/bower.json +++ b/frontend/bower.json @@ -27,7 +27,7 @@ "hyperagent": "manwithtwowatches/hyperagent#v0.4.2", "lodash": "~2.4.1", "foundation-apps": "~1.0.2", - "ui-select": "angular-ui/ui-select#~0.9.5" + "ui-select": "0xf013/ui-select#add-combobox-acessibility" }, "devDependencies": { "mocha": "~1.14.0", diff --git a/frontend/public/templates/components/inplace_editor/editable/select2.html b/frontend/public/templates/components/inplace_editor/editable/select2.html index cddbb55957..24e3f89874 100644 --- a/frontend/public/templates/components/inplace_editor/editable/select2.html +++ b/frontend/public/templates/components/inplace_editor/editable/select2.html @@ -2,6 +2,7 @@ name="value" ng-disabled="isBusy" ng-model="dataObject.value" + title="{{ editTitle }}" theme="select2"> {{$select.selected.name}} Date: Mon, 22 Dec 2014 17:16:46 +0200 Subject: [PATCH 15/59] added some aria tags to inplace editor --- .../templates/components/inplace_editor.html | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/frontend/public/templates/components/inplace_editor.html b/frontend/public/templates/components/inplace_editor.html index 308335abac..e59d1d5f55 100644 --- a/frontend/public/templates/components/inplace_editor.html +++ b/frontend/public/templates/components/inplace_editor.html @@ -1,6 +1,22 @@ -
-
- +
+
+ + + + + + + {{entity.links.version.props.title}} + + + + {{readValue}} + + + + + + @@ -19,7 +35,7 @@
- +
From 9b7378c9d1dbf900951498823c4c8bcdfcc6180e Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Mon, 22 Dec 2014 17:23:15 +0200 Subject: [PATCH 16/59] inject attribute variable into buttons translation --- config/locales/js-de.yml | 6 +++--- config/locales/js-en.yml | 8 ++++---- frontend/app/ui_components/inplace-editor-directive.js | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/config/locales/js-de.yml b/config/locales/js-de.yml index e726ffce3d..06d7057034 100644 --- a/config/locales/js-de.yml +++ b/config/locales/js-de.yml @@ -389,9 +389,9 @@ de: delete: Beziehung löschen inplace: button_edit: "%{attribute} bearbeiten" - button_save: "Speichern" - button_save_and_send: "Speichern mit E-Mail-Benachrichtigung" - button_cancel: "Abbrechen" + button_save: "%{attribute} Speichern" + button_save_and_send: "%{attribute} Speichern mit E-Mail-Benachrichtigung" + button_cancel: "%{attribute} Abbrechen" link_formatting_help: "Textformatierung" btn_preview_enable: "Vorschau" btn_preview_disable: "Vorschau deaktivieren" diff --git a/config/locales/js-en.yml b/config/locales/js-en.yml index f25228e59e..c7b4c9adf4 100644 --- a/config/locales/js-en.yml +++ b/config/locales/js-en.yml @@ -391,10 +391,10 @@ en: empty: No relation exists delete: Delete relation inplace: - button_edit: "Edit %{attribute}" - button_save: "Save" - button_save_and_send: "Save and send email" - button_cancel: "Cancel" + button_edit: "%{attribute}: Edit" + button_save: "%{attribute}: Save" + button_save_and_send: "%{attribute}: Save and send email" + button_cancel: "%{attribute}: Cancel" link_formatting_help: "Text formatting" btn_preview_enable: "Preview" btn_preview_disable: "Disable preview" diff --git a/frontend/app/ui_components/inplace-editor-directive.js b/frontend/app/ui_components/inplace-editor-directive.js index 97bf7d5753..00b5e44eb5 100644 --- a/frontend/app/ui_components/inplace-editor-directive.js +++ b/frontend/app/ui_components/inplace-editor-directive.js @@ -85,9 +85,9 @@ module.exports = function( $scope.isBusy = false; $scope.readValue = ''; $scope.editTitle = I18n.t('js.inplace.button_edit', { attribute: $scope.attributeTitle }); - $scope.saveTitle = I18n.t('js.inplace.button_save'); - $scope.saveAndSendTitle = I18n.t('js.inplace.button_save_and_send'); - $scope.cancelTitle = I18n.t('js.inplace.button_cancel'); + $scope.saveTitle = I18n.t('js.inplace.button_save', { attribute: $scope.attributeTitle }); + $scope.saveAndSendTitle = I18n.t('js.inplace.button_save_and_send', { attribute: $scope.attributeTitle }); + $scope.cancelTitle = I18n.t('js.inplace.button_cancel', { attribute: $scope.attributeTitle }); $scope.error = null; $scope.options = []; From 23e14e3c7b4f2cc7c062c2ae00230d6beb89a5a3 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Tue, 23 Dec 2014 14:51:30 +0200 Subject: [PATCH 17/59] set title for text and textarea --- .../templates/components/inplace_editor/editable/text.html | 1 + .../components/inplace_editor/editable/wiki_textarea.html | 1 + 2 files changed, 2 insertions(+) diff --git a/frontend/public/templates/components/inplace_editor/editable/text.html b/frontend/public/templates/components/inplace_editor/editable/text.html index 1a0a5c4b95..e00d0fc601 100644 --- a/frontend/public/templates/components/inplace_editor/editable/text.html +++ b/frontend/public/templates/components/inplace_editor/editable/text.html @@ -2,4 +2,5 @@ name="value" type="text" ng-disabled="isBusy" + title="{{ editTitle }}" ng-model="dataObject.value" /> diff --git a/frontend/public/templates/components/inplace_editor/editable/wiki_textarea.html b/frontend/public/templates/components/inplace_editor/editable/wiki_textarea.html index 3ebf97bfc6..47a7e594d4 100644 --- a/frontend/public/templates/components/inplace_editor/editable/wiki_textarea.html +++ b/frontend/public/templates/components/inplace_editor/editable/wiki_textarea.html @@ -5,5 +5,6 @@ name="value" ng-disabled="isBusy" ng-model="dataObject.value" + title="{{ editTitle }}" data-wp_autocomplete_url="{{ autocompletePath }}"> From 2540cb0e75567524875c12144622a4b14ad0db38 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Tue, 23 Dec 2014 15:17:59 +0200 Subject: [PATCH 18/59] change typo in bower.json br branch name --- frontend/bower.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/bower.json b/frontend/bower.json index 40070d1861..972a36c3cc 100644 --- a/frontend/bower.json +++ b/frontend/bower.json @@ -27,7 +27,7 @@ "hyperagent": "manwithtwowatches/hyperagent#v0.4.2", "lodash": "~2.4.1", "foundation-apps": "~1.0.2", - "ui-select": "0xf013/ui-select#add-combobox-acessibility" + "ui-select": "0xf013/ui-select#add-combobox-accessibility" }, "devDependencies": { "mocha": "~1.14.0", From fb00dec239a854077c7e1649dba2161e16e18214 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Fri, 23 Jan 2015 13:47:43 +0200 Subject: [PATCH 19/59] switch to latest accessible build; add missing overview titles --- .../app/services/overview-tab-inplace-editor-config.js | 7 +++++-- frontend/bower.json | 2 +- frontend/public/templates/work_packages/tabs/overview.html | 1 + 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/frontend/app/services/overview-tab-inplace-editor-config.js b/frontend/app/services/overview-tab-inplace-editor-config.js index aa0bcf04c3..92d5f43786 100644 --- a/frontend/app/services/overview-tab-inplace-editor-config.js +++ b/frontend/app/services/overview-tab-inplace-editor-config.js @@ -74,19 +74,22 @@ module.exports = function() { embedded: false, placeholder: '-', displayStrategy: 'user', + attributeTitle: I18n.t('js.work_packages.properties.assignee') }, responsible: { type: 'select2', attribute: 'responsible', embedded: false, placeholder: '-', - displayStrategy: 'user' + displayStrategy: 'user', + attributeTitle: I18n.t('js.work_packages.properties.responsible') }, status: { type: 'select2', attribute: 'status.name', embedded: true, - placeholder: '-' + placeholder: '-', + attributeTitle: I18n.t('js.work_packages.properties.status') }, versionName: { type: 'select2', diff --git a/frontend/bower.json b/frontend/bower.json index 972a36c3cc..493be1c63d 100644 --- a/frontend/bower.json +++ b/frontend/bower.json @@ -27,7 +27,7 @@ "hyperagent": "manwithtwowatches/hyperagent#v0.4.2", "lodash": "~2.4.1", "foundation-apps": "~1.0.2", - "ui-select": "0xf013/ui-select#add-combobox-accessibility" + "ui-select": "0xF013/ui-select#b98f2d0808c77b78519cae527d662b2c1e374aae" }, "devDependencies": { "mocha": "~1.14.0", diff --git a/frontend/public/templates/work_packages/tabs/overview.html b/frontend/public/templates/work_packages/tabs/overview.html index ce495274b8..192a3d028a 100644 --- a/frontend/public/templates/work_packages/tabs/overview.html +++ b/frontend/public/templates/work_packages/tabs/overview.html @@ -54,6 +54,7 @@ ined-entity="workPackage" ined-attribute="{{inplaceProperties[propertyData.property].attribute}}" ined-attribute-embedded="inplaceProperties[propertyData.property].embedded" + ined-attribute-title="{{ inplaceProperties[propertyData.property].attributeTitle }}" placeholder="{{inplaceProperties[propertyData.property].placeholder}}">
From d1c4ced43c6774fa2f3d59516e98f572d23ff591 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Wed, 28 Jan 2015 15:29:01 +0200 Subject: [PATCH 20/59] fix german translation missing semicolon --- config/locales/js-de.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/locales/js-de.yml b/config/locales/js-de.yml index 06d7057034..5d79ac564a 100644 --- a/config/locales/js-de.yml +++ b/config/locales/js-de.yml @@ -389,7 +389,7 @@ de: delete: Beziehung löschen inplace: button_edit: "%{attribute} bearbeiten" - button_save: "%{attribute} Speichern" + button_save: "%{attribute}: Speichern" button_save_and_send: "%{attribute} Speichern mit E-Mail-Benachrichtigung" button_cancel: "%{attribute} Abbrechen" link_formatting_help: "Textformatierung" From 1e24e69fc9f1c3cdca3207da6da7db4e615b6ac0 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Thu, 29 Jan 2015 16:56:42 +0200 Subject: [PATCH 21/59] fix first item always selected; added aria-label for null option --- config/locales/js-de.yml | 2 ++ config/locales/js-en.yml | 2 ++ frontend/app/ui_components/inplace-editor-directive.js | 2 ++ .../components/inplace_editor/editable/select2.html | 5 +++-- 4 files changed, 9 insertions(+), 2 deletions(-) diff --git a/config/locales/js-de.yml b/config/locales/js-de.yml index 5d79ac564a..35527f8b03 100644 --- a/config/locales/js-de.yml +++ b/config/locales/js-de.yml @@ -333,6 +333,7 @@ de: type: "Typ" updatedAt: "Aktualisiert" versionName: "Version" + version: "Version" query: column_names: "Spalten" group_by: "Gruppiere Ergebnisse nach" @@ -395,6 +396,7 @@ de: link_formatting_help: "Textformatierung" btn_preview_enable: "Vorschau" btn_preview_disable: "Vorschau deaktivieren" + null_value_label: "Kein Wert" error_could_not_resolve_version_name: "Versionsbezeichner konnte nicht aufgelöst werden" error_could_not_resolve_user_name: "Benutzername konnte nicht aufgelöst werden" diff --git a/config/locales/js-en.yml b/config/locales/js-en.yml index c7b4c9adf4..9e0fab9b0d 100644 --- a/config/locales/js-en.yml +++ b/config/locales/js-en.yml @@ -336,6 +336,7 @@ en: type: "Type" updatedAt: "Updated on" versionName: "Version" + version: "Version" query: column_names: "Columns" group_by: "Group results by" @@ -398,6 +399,7 @@ en: link_formatting_help: "Text formatting" btn_preview_enable: "Preview" btn_preview_disable: "Disable preview" + null_value_label: "No value" error_could_not_resolve_version_name: "Couldn't resolve version name" error_could_not_resolve_user_name: "Couldn't resolve user name" diff --git a/frontend/app/ui_components/inplace-editor-directive.js b/frontend/app/ui_components/inplace-editor-directive.js index 00b5e44eb5..c84088c612 100644 --- a/frontend/app/ui_components/inplace-editor-directive.js +++ b/frontend/app/ui_components/inplace-editor-directive.js @@ -104,6 +104,8 @@ module.exports = function( $scope.acceptErrors = acceptErrors; $scope.pathHelper = PathHelper; + $scope.nullValueLabel = I18n.t('js.inplace.null_value_label'); + activate(); function activate() { diff --git a/frontend/public/templates/components/inplace_editor/editable/select2.html b/frontend/public/templates/components/inplace_editor/editable/select2.html index 24e3f89874..e4257fa563 100644 --- a/frontend/public/templates/components/inplace_editor/editable/select2.html +++ b/frontend/public/templates/components/inplace_editor/editable/select2.html @@ -3,10 +3,11 @@ ng-disabled="isBusy" ng-model="dataObject.value" title="{{ editTitle }}" + reset-search-input="true" theme="select2"> - {{$select.selected.name}} + {{ $select.selected.name }} -
+
From 7c48e6f1fcaa1d069054409872d1871fa293af9f Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Thu, 29 Jan 2015 17:03:08 +0200 Subject: [PATCH 22/59] remove scope digest cancellation in order to enable ui-select to run its activation code --- frontend/app/ui_components/inplace-editor-dispatcher.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/app/ui_components/inplace-editor-dispatcher.js b/frontend/app/ui_components/inplace-editor-dispatcher.js index b63638bdfd..69e59fa537 100644 --- a/frontend/app/ui_components/inplace-editor-dispatcher.js +++ b/frontend/app/ui_components/inplace-editor-dispatcher.js @@ -192,7 +192,7 @@ module.exports = function($sce, $http, $timeout, AutoCompleteHelper, TextileServ scope.$on('focusSelect2', function() { $timeout(function() { element.find('.select2-choice').trigger('click'); - }, 0, false); + }); }); }, startEditing: setOptions, From d879c512ac1e5253dd5e69cbc841466d5641ea9a Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Fri, 30 Jan 2015 12:36:57 +0200 Subject: [PATCH 23/59] switch to a new ui.select commit that deals with activedescendant --- frontend/bower.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/bower.json b/frontend/bower.json index 493be1c63d..2a59609416 100644 --- a/frontend/bower.json +++ b/frontend/bower.json @@ -27,7 +27,7 @@ "hyperagent": "manwithtwowatches/hyperagent#v0.4.2", "lodash": "~2.4.1", "foundation-apps": "~1.0.2", - "ui-select": "0xF013/ui-select#b98f2d0808c77b78519cae527d662b2c1e374aae" + "ui-select": "0xF013/ui-select#2e139ea3f2cd3936396037e36d7803ba884de9aa" }, "devDependencies": { "mocha": "~1.14.0", From 1a1d554aa368e5c0c76ca06d39906ce2b4220cf3 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Fri, 6 Feb 2015 16:50:52 +0200 Subject: [PATCH 24/59] fix enter key and multis --- frontend/bower.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/bower.json b/frontend/bower.json index 2a59609416..43d2280a40 100644 --- a/frontend/bower.json +++ b/frontend/bower.json @@ -27,7 +27,7 @@ "hyperagent": "manwithtwowatches/hyperagent#v0.4.2", "lodash": "~2.4.1", "foundation-apps": "~1.0.2", - "ui-select": "0xF013/ui-select#2e139ea3f2cd3936396037e36d7803ba884de9aa" + "ui-select": "0xF013/ui-select#1a67dea0f6076e8f33354bf573bf5482539f289f" }, "devDependencies": { "mocha": "~1.14.0", From bf90240874fc7f08a107424332dd6ebfa36468e8 Mon Sep 17 00:00:00 2001 From: Florian Kraft Date: Fri, 6 Feb 2015 16:47:57 +0100 Subject: [PATCH 25/59] Refactor ReportingsController to be more readable --- app/controllers/reportings_controller.rb | 44 +++++++++--------------- 1 file changed, 16 insertions(+), 28 deletions(-) diff --git a/app/controllers/reportings_controller.rb b/app/controllers/reportings_controller.rb index 7a9a2fb251..44176de0fe 100644 --- a/app/controllers/reportings_controller.rb +++ b/app/controllers/reportings_controller.rb @@ -35,6 +35,11 @@ class ReportingsController < ApplicationController before_filter :find_project_by_project_id before_filter :authorize + before_filter :find_reporting_via_source, only: [:show, :edit, :update, :confirm_destroy, :destroy] + before_filter :build_reporting, only: :create + + before_filter :check_visibility!, except: [:create, :index, :new, :available_projects] + accept_key_auth :index, :show menu_item :reportings @@ -174,9 +179,6 @@ class ReportingsController < ApplicationController end def show - @reporting = @project.reportings_via_source.find(params[:id]) - check_visibility - respond_to do |format| format.html end @@ -197,18 +199,7 @@ class ReportingsController < ApplicationController end def create - @reporting = @project.reportings_via_source.build - @reporting.reporting_to_project_id = params['reporting']['reporting_to_project_id'] - - if @reporting.reporting_to_project.nil? - flash.now[:error] = l('timelines.reporting_could_not_be_saved') - render action: :new, status: :unprocessable_entity - return - end - - check_visibility - - if @reporting.save + if @reporting.reporting_to_project.present? && @reporting.project.visible? && @reporting.save flash[:notice] = l(:notice_successful_create) redirect_to project_reportings_path else @@ -218,18 +209,12 @@ class ReportingsController < ApplicationController end def edit - @reporting = @project.reportings_via_source.find(params[:id]) - check_visibility - respond_to do |format| format.html end end def update - @reporting = @project.reportings_via_source.find(params[:id]) - check_visibility - if @reporting.update_attributes(params[:reporting]) flash[:notice] = l(:notice_successful_update) redirect_to project_reportings_path @@ -240,26 +225,29 @@ class ReportingsController < ApplicationController end def confirm_destroy - @reporting = @project.reportings_via_source.find(params[:id]) - check_visibility - respond_to do |format| format.html end end def destroy - @reporting = @project.reportings_via_source.find(params[:id]) - check_visibility @reporting.destroy - flash[:notice] = l(:notice_successful_delete) redirect_to project_reportings_path end protected - def check_visibility + def find_reporting_via_source + @reporting = @project.reportings_via_source.find(params[:id]) + end + + def build_reporting + @reporting = @project.reportings_via_source.build + @reporting.reporting_to_project_id = params['reporting']['reporting_to_project_id'] + end + + def check_visibility! raise ActiveRecord::RecordNotFound unless @reporting.visible? end From c24bf1969a753266f804a49454bc1f155fa199b0 Mon Sep 17 00:00:00 2001 From: Florian Kraft Date: Fri, 6 Feb 2015 16:57:55 +0100 Subject: [PATCH 26/59] rename method --- app/controllers/reportings_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/reportings_controller.rb b/app/controllers/reportings_controller.rb index 44176de0fe..a5f93b3a01 100644 --- a/app/controllers/reportings_controller.rb +++ b/app/controllers/reportings_controller.rb @@ -35,7 +35,7 @@ class ReportingsController < ApplicationController before_filter :find_project_by_project_id before_filter :authorize - before_filter :find_reporting_via_source, only: [:show, :edit, :update, :confirm_destroy, :destroy] + before_filter :find_reporting, only: [:show, :edit, :update, :confirm_destroy, :destroy] before_filter :build_reporting, only: :create before_filter :check_visibility!, except: [:create, :index, :new, :available_projects] @@ -238,7 +238,7 @@ class ReportingsController < ApplicationController protected - def find_reporting_via_source + def find_reporting @reporting = @project.reportings_via_source.find(params[:id]) end From 83a42d82b958a1797df36a2a94392a23128cb6ec Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Fri, 6 Feb 2015 17:05:57 +0100 Subject: [PATCH 27/59] fix auto loading of DateTimeFormatter during dev Apparantly the rails autoloader gets confused by trying to load a class from an included module. We can help it, by explicitly referencing (thus loading) the fully qualified class name first... I wish there was a better solution, but I couldn't find one... --- lib/api/decorators/single.rb | 5 +++++ lib/api/v3/activities/activity_representer.rb | 3 ++- lib/api/v3/attachments/attachment_representer.rb | 1 - lib/api/v3/projects/project_representer.rb | 2 -- lib/api/v3/users/user_representer.rb | 2 -- lib/api/v3/versions/version_representer.rb | 2 -- .../work_packages/form/work_package_payload_representer.rb | 1 - lib/api/v3/work_packages/work_package_representer.rb | 2 -- spec/lib/api/v3/utilities/date_time_formatter_spec.rb | 1 - 9 files changed, 7 insertions(+), 12 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index 70d293f6b1..2703d7c044 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -30,12 +30,17 @@ require 'roar/decorator' require 'roar/json/hal' +# we need to help the rails autoloader, otherwise it MIGHT fail to resolve the DateTimeFormatter +# during development mode +API::V3::Utilities::DateTimeFormatter + module API module Decorators class Single < Roar::Decorator include Roar::JSON::HAL include Roar::Hypermedia include API::V3::Utilities::PathHelper + include API::V3::Utilities attr_reader :context class_attribute :as_strategy diff --git a/lib/api/v3/activities/activity_representer.rb b/lib/api/v3/activities/activity_representer.rb index 1e368540c3..17a25924b1 100644 --- a/lib/api/v3/activities/activity_representer.rb +++ b/lib/api/v3/activities/activity_representer.rb @@ -29,7 +29,8 @@ require 'roar/decorator' require 'roar/json/hal' -require 'api/v3/utilities/date_time_formatter' + +API::V3::Utilities::DateTimeFormatter module API module V3 diff --git a/lib/api/v3/attachments/attachment_representer.rb b/lib/api/v3/attachments/attachment_representer.rb index 6c1726e29d..83cc23ec76 100644 --- a/lib/api/v3/attachments/attachment_representer.rb +++ b/lib/api/v3/attachments/attachment_representer.rb @@ -29,7 +29,6 @@ require 'roar/decorator' require 'roar/json/hal' -require 'api/v3/utilities/date_time_formatter' module API module V3 diff --git a/lib/api/v3/projects/project_representer.rb b/lib/api/v3/projects/project_representer.rb index 28d4b02838..ed718277b0 100644 --- a/lib/api/v3/projects/project_representer.rb +++ b/lib/api/v3/projects/project_representer.rb @@ -29,13 +29,11 @@ require 'roar/decorator' require 'roar/json/hal' -require 'api/v3/utilities/date_time_formatter' module API module V3 module Projects class ProjectRepresenter < ::API::Decorators::Single - include API::V3::Utilities link :self do { diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index 92b83bd09b..7b6e37cafa 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -29,14 +29,12 @@ require 'roar/decorator' require 'roar/json/hal' -require 'api/v3/utilities/date_time_formatter' module API module V3 module Users class UserRepresenter < ::API::Decorators::Single include AvatarHelper - include API::V3::Utilities link :self do { diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index 405ee2dd38..2f9fc497fa 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -29,13 +29,11 @@ require 'roar/decorator' require 'roar/json/hal' -require 'api/v3/utilities/date_time_formatter' module API module V3 module Versions class VersionRepresenter < ::API::Decorators::Single - include API::V3::Utilities link :self do { diff --git a/lib/api/v3/work_packages/form/work_package_payload_representer.rb b/lib/api/v3/work_packages/form/work_package_payload_representer.rb index 21567f920b..9db97c3727 100644 --- a/lib/api/v3/work_packages/form/work_package_payload_representer.rb +++ b/lib/api/v3/work_packages/form/work_package_payload_representer.rb @@ -29,7 +29,6 @@ require 'roar/decorator' require 'roar/json/hal' -require 'api/v3/utilities/date_time_formatter' module API module V3 diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 2451cf0cbb..3849c1ffff 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -29,13 +29,11 @@ require 'roar/decorator' require 'roar/json/hal' -require 'api/v3/utilities/date_time_formatter' module API module V3 module WorkPackages class WorkPackageRepresenter < ::API::Decorators::Single - include API::V3::Utilities link :self do { diff --git a/spec/lib/api/v3/utilities/date_time_formatter_spec.rb b/spec/lib/api/v3/utilities/date_time_formatter_spec.rb index 08f1ac6910..7f49839c4a 100644 --- a/spec/lib/api/v3/utilities/date_time_formatter_spec.rb +++ b/spec/lib/api/v3/utilities/date_time_formatter_spec.rb @@ -27,7 +27,6 @@ #++ require 'spec_helper' -require 'api/v3/utilities/date_time_formatter' describe :DateTimeFormatter do subject { ::API::V3::Utilities::DateTimeFormatter } From b45713d7af5dfbe1744d39aff987a331146c4d8d Mon Sep 17 00:00:00 2001 From: Florian Kraft Date: Mon, 9 Feb 2015 09:40:58 +0100 Subject: [PATCH 28/59] fix the api reportings controller --- app/controllers/reportings_controller.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/reportings_controller.rb b/app/controllers/reportings_controller.rb index a5f93b3a01..04729255d2 100644 --- a/app/controllers/reportings_controller.rb +++ b/app/controllers/reportings_controller.rb @@ -38,7 +38,7 @@ class ReportingsController < ApplicationController before_filter :find_reporting, only: [:show, :edit, :update, :confirm_destroy, :destroy] before_filter :build_reporting, only: :create - before_filter :check_visibility!, except: [:create, :index, :new, :available_projects] + before_filter :check_visibility, except: [:create, :index, :new, :available_projects] accept_key_auth :index, :show @@ -247,7 +247,7 @@ class ReportingsController < ApplicationController @reporting.reporting_to_project_id = params['reporting']['reporting_to_project_id'] end - def check_visibility! + def check_visibility raise ActiveRecord::RecordNotFound unless @reporting.visible? end From 256ee76b36edcbdb7737c688a7992c2938e9da2a Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Mon, 9 Feb 2015 14:34:51 +0200 Subject: [PATCH 29/59] fix verical packages being filtered out in timelines --- frontend/app/timelines/services/timeline-loader-service.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frontend/app/timelines/services/timeline-loader-service.js b/frontend/app/timelines/services/timeline-loader-service.js index 3855152b50..1589dc89bf 100644 --- a/frontend/app/timelines/services/timeline-loader-service.js +++ b/frontend/app/timelines/services/timeline-loader-service.js @@ -854,8 +854,10 @@ module.exports = function($q, FilterQueryStringBuilder, Color, HistoricalPlannin TimelineLoader.prototype.registerPlanningElementsByID = function (ids) { this.inChunks(ids, function (planningElementIdsOfPacket, i) { - var projectPrefix = PathHelper.staticBase + - this.options.api_prefix; + var projectPrefix = this.globalPrefix + + PathHelper.projectsPath() + + "/" + + this.options.project_id; // load current planning elements. this.loader.register( @@ -865,7 +867,6 @@ module.exports = function($q, FilterQueryStringBuilder, Color, HistoricalPlannin planningElementIdsOfPacket.join(',')}, { storeIn: PlanningElement.identifier } ); - // load historical planning elements. // TODO: load historical PEs here! if (this.options.target_time) { From 6d04d171d826b3cf45d97ba9dc5b9edc5f984dfa Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Mon, 9 Feb 2015 13:39:01 +0100 Subject: [PATCH 30/59] Fix category link --- lib/api/v3/categories/category_representer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/v3/categories/category_representer.rb b/lib/api/v3/categories/category_representer.rb index 49edffaa5e..f738299bb9 100644 --- a/lib/api/v3/categories/category_representer.rb +++ b/lib/api/v3/categories/category_representer.rb @@ -36,7 +36,7 @@ module API class CategoryRepresenter < ::API::Decorators::Single link :self do { - href: api_v3_paths.categories(represented.id), + href: api_v3_paths.category(represented.id), title: "#{represented.name}" } end From 234417ec1c376802e5a84c33b871f8c6781ef7a8 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Mon, 9 Feb 2015 14:42:38 +0100 Subject: [PATCH 31/59] Added category to work package representer --- doc/apiv3-documentation.apib | 4 ++++ lib/api/v3/work_packages/work_package_representer.rb | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/doc/apiv3-documentation.apib b/doc/apiv3-documentation.apib index 7ad7eb0144..48f8be7904 100644 --- a/doc/apiv3-documentation.apib +++ b/doc/apiv3-documentation.apib @@ -2518,6 +2518,10 @@ Note that due to sharing this might be more than the versions *defined* by that "href": "/api/v3/work_packages/1298", "title": "nisi eligendi officiis eos delectus quis voluptas dolores" }, + "category": { + "href": "/api/v3/categories/1298", + "title": "eligend isi" + }, "children": [ { "href": "/api/v3/work_packages/1529", diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index b0830d59d7..f4e8be5f19 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -202,6 +202,13 @@ module API } if current_user_allowed_to(:view_time_entries) end + link :category do + { + href: api_v3_paths.category(represented.category.id), + title: represented.category.name + } unless represented.category.nil? + end + link :version do { href: api_v3_paths.version(represented.fixed_version.id), From 34f357f492cba1a4d4fda0d0647690e58ed3d3c9 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Mon, 9 Feb 2015 16:59:20 +0100 Subject: [PATCH 32/59] use formatter from member instead of immediately - allows to DI easier - more elegant solution around autoloading issues --- lib/api/decorators/single.rb | 9 ++++---- lib/api/v3/projects/project_representer.rb | 6 +++-- lib/api/v3/users/user_representer.rb | 6 +++-- lib/api/v3/versions/version_representer.rb | 16 ++++++++++---- .../form/work_package_payload_representer.rb | 13 ++++++----- .../work_packages/work_package_representer.rb | 22 ++++++++++++------- 6 files changed, 46 insertions(+), 26 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index 2703d7c044..ded3fa5661 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -30,17 +30,12 @@ require 'roar/decorator' require 'roar/json/hal' -# we need to help the rails autoloader, otherwise it MIGHT fail to resolve the DateTimeFormatter -# during development mode -API::V3::Utilities::DateTimeFormatter - module API module Decorators class Single < Roar::Decorator include Roar::JSON::HAL include Roar::Hypermedia include API::V3::Utilities::PathHelper - include API::V3::Utilities attr_reader :context class_attribute :as_strategy @@ -58,6 +53,10 @@ module API private + def datetime_formatter + API::V3::Utilities::DateTimeFormatter + end + def _type; end end end diff --git a/lib/api/v3/projects/project_representer.rb b/lib/api/v3/projects/project_representer.rb index ed718277b0..feaa079b6b 100644 --- a/lib/api/v3/projects/project_representer.rb +++ b/lib/api/v3/projects/project_representer.rb @@ -59,10 +59,12 @@ module API property :created_on, as: 'createdAt', - getter: -> (*) { DateTimeFormatter::format_datetime(created_on) } + exec_context: :decorator, + getter: -> (*) { datetime_formatter.format_datetime(represented.created_on) } property :updated_on, as: 'updatedAt', - getter: -> (*) { DateTimeFormatter::format_datetime(updated_on) } + exec_context: :decorator, + getter: -> (*) { datetime_formatter.format_datetime(represented.updated_on) } property :type, getter: -> (*) { project_type.try(:name) }, render_nil: true diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index 7b6e37cafa..e55740cbe0 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -87,10 +87,12 @@ module API exec_context: :decorator property :created_on, as: 'createdAt', - getter: -> (*) { DateTimeFormatter::format_datetime(created_on) } + exec_context: :decorator, + getter: -> (*) { datetime_formatter.format_datetime(represented.created_on) } property :updated_on, as: 'updatedAt', - getter: -> (*) { DateTimeFormatter::format_datetime(updated_on) } + exec_context: :decorator, + getter: -> (*) { datetime_formatter.format_datetime(represented.updated_on) } property :status, getter: -> (*) { status_name }, render_nil: true def _type diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index 2f9fc497fa..2f5f8d5e19 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -69,19 +69,27 @@ module API render_nil: true property :start_date, - getter: -> (*) { DateTimeFormatter::format_date(start_date, allow_nil: true) }, + exec_context: :decorator, + getter: -> (*) { + datetime_formatter.format_date(represented.start_date, allow_nil: true) + }, render_nil: true property :due_date, as: 'endDate', - getter: -> (*) { DateTimeFormatter::format_date(due_date, allow_nil: true) }, + exec_context: :decorator, + getter: -> (*) { + datetime_formatter.format_date(represented.due_date, allow_nil: true) + }, render_nil: true property :status, render_nil: true property :created_on, as: 'createdAt', - getter: -> (*) { DateTimeFormatter::format_datetime(created_on) } + exec_context: :decorator, + getter: -> (*) { datetime_formatter.format_datetime(represented.created_on) } property :updated_on, as: 'updatedAt', - getter: -> (*) { DateTimeFormatter::format_datetime(updated_on) } + exec_context: :decorator, + getter: -> (*) { datetime_formatter.format_datetime(represented.updated_on) } def _type 'Version' diff --git a/lib/api/v3/work_packages/form/work_package_payload_representer.rb b/lib/api/v3/work_packages/form/work_package_payload_representer.rb index 9db97c3727..7d3fe4b48a 100644 --- a/lib/api/v3/work_packages/form/work_package_payload_representer.rb +++ b/lib/api/v3/work_packages/form/work_package_payload_representer.rb @@ -37,7 +37,6 @@ module API class WorkPackagePayloadRepresenter < Roar::Decorator include Roar::JSON::HAL include Roar::Hypermedia - include API::V3::Utilities self.as_strategy = ::API::Utilities::CamelCasingStrategy.new @@ -85,10 +84,10 @@ module API property :start_date, exec_context: :decorator, getter: -> (*) { - DateTimeFormatter::format_date(represented.start_date, allow_nil: true) + datetime_formatter.format_date(represented.start_date, allow_nil: true) }, setter: -> (value, *) { - represented.start_date = DateTimeFormatter::parse_date(value, + represented.start_date = datetime_formatter.parse_date(value, 'startDate', allow_nil: true) }, @@ -96,10 +95,10 @@ module API property :due_date, exec_context: :decorator, getter: -> (*) { - DateTimeFormatter::format_date(represented.due_date, allow_nil: true) + datetime_formatter.format_date(represented.due_date, allow_nil: true) }, setter: -> (value, *) { - represented.due_date = DateTimeFormatter::parse_date(value, + represented.due_date = datetime_formatter.parse_date(value, 'dueDate', allow_nil: true) }, @@ -119,6 +118,10 @@ module API private + def datetime_formatter + API::V3::Utilities::DateTimeFormatter + end + def work_package_attribute_links_representer(represented) ::API::V3::WorkPackages::Form::WorkPackageAttributeLinksRepresenter.new represented end diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 3849c1ffff..edc6388b90 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -240,29 +240,31 @@ module API render_nil: true property :start_date, + exec_context: :decorator, getter: -> (*) do - DateTimeFormatter::format_date(start_date, allow_nil: true) + datetime_formatter.format_date(represented.start_date, allow_nil: true) end, render_nil: true property :due_date, + exec_context: :decorator, getter: -> (*) do - DateTimeFormatter::format_date(due_date, allow_nil: true) + datetime_formatter.format_date(represented.due_date, allow_nil: true) end, render_nil: true property :estimated_time, + exec_context: :decorator, getter: -> (*) do - DateTimeFormatter::format_duration_from_hours(represented.estimated_hours, + datetime_formatter.format_duration_from_hours(represented.estimated_hours, allow_nil: true) end, - exec_context: :decorator, render_nil: true, writeable: false property :spent_time, + exec_context: :decorator, getter: -> (*) do - DateTimeFormatter::format_duration_from_hours(represented.spent_hours) + datetime_formatter.format_duration_from_hours(represented.spent_hours) end, writeable: false, - exec_context: :decorator, if: -> (_) { current_user_allowed_to(:view_time_entries) } property :percentage_done, render_nil: true, @@ -277,8 +279,12 @@ module API property :project_id, getter: -> (*) { project.id } property :project_name, getter: -> (*) { project.try(:name) } property :parent_id, writeable: true - property :created_at, getter: -> (*) { DateTimeFormatter::format_datetime(created_at) } - property :updated_at, getter: -> (*) { DateTimeFormatter::format_datetime(updated_at) } + property :created_at, + exec_context: :decorator, + getter: -> (*) { datetime_formatter.format_datetime(represented.created_at) } + property :updated_at, + exec_context: :decorator, + getter: -> (*) { datetime_formatter.format_datetime(represented.updated_at) } collection :custom_properties, exec_context: :decorator, render_nil: true From 19a8e31833dad179f75da065431cd2958a78d5ba Mon Sep 17 00:00:00 2001 From: Solotchi Veaceslav Date: Thu, 5 Feb 2015 14:15:23 +0200 Subject: [PATCH 33/59] Spec tests --- .../plugins/module_handler_spec.rb | 60 +++++++++++++++ spec/lib/redmine/access_control_spec.rb | 75 +++++++++++++++++++ 2 files changed, 135 insertions(+) create mode 100644 spec/lib/open_project/plugins/module_handler_spec.rb create mode 100644 spec/lib/redmine/access_control_spec.rb diff --git a/spec/lib/open_project/plugins/module_handler_spec.rb b/spec/lib/open_project/plugins/module_handler_spec.rb new file mode 100644 index 0000000000..52c5957c6c --- /dev/null +++ b/spec/lib/open_project/plugins/module_handler_spec.rb @@ -0,0 +1,60 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 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-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 doc/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' +describe OpenProject::Plugins::ModuleHandler do + before(:all) do + module_permissions = Redmine::AccessControl.modules_permissions(['repository']) + @repository_permissions = module_permissions.select do |permission| + permission.project_module == :repository + end + + disabled_modules = OpenProject::Plugins::ModuleHandler.disable_modules('repository') + OpenProject::Plugins::ModuleHandler.disable(disabled_modules) + end + + after(:all) do + Redmine::AccessControl.map do |mapper| + mapper.project_module :repository do |map| + @repository_permissions.map do |permission| + options = { project_module: permission.project_module, + public: permission.public?, + require: permission.require_loggedin? } + + map.permission(permission.name, permission.actions, options) + end + end + end + end + + context '#disable' do + it 'should disable repository module' do + expect(Redmine::AccessControl.available_project_modules).to_not include(:repository) + end + end +end diff --git a/spec/lib/redmine/access_control_spec.rb b/spec/lib/redmine/access_control_spec.rb new file mode 100644 index 0000000000..096f3589c3 --- /dev/null +++ b/spec/lib/redmine/access_control_spec.rb @@ -0,0 +1,75 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 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-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 doc/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' +describe Redmine::AccessControl do + let(:global_permissions) { Redmine::AccessControl.permissions } + let(:public_permissions) { Redmine::AccessControl.public_permissions } + let(:members_only_permissions) { Redmine::AccessControl.members_only_permissions } + let(:loggedin_only_permissions) { Redmine::AccessControl.loggedin_only_permissions } + + after(:all) do + Redmine::AccessControl.map do |mapper| + mapper.project_module :repository do |map| + @repository_permissions.map do |permission| + options = { project_module: permission.project_module, + public: permission.public?, + require: permission.require_loggedin? } + + map.permission(permission.name, permission.actions, options) + end + end + end + end + + before(:all) do + module_permissions = Redmine::AccessControl.modules_permissions(['repository']) + @repository_permissions = module_permissions.select do |permission| + permission.project_module == :repository + end + Redmine::AccessControl.remove_modules_permissions(:repository) + end + + describe 'remove module permissions' do + context 'remove from global permissions' do + it { expect(global_permissions).to_not include(@repository_permissions) } + end + + context 'remove from public permissions' do + it { expect(public_permissions).to_not include(@repository_permissions) } + end + + context 'remove from members only permissions' do + it { expect(members_only_permissions).to_not include(@repository_permissions) } + end + + context 'remove from loggedin only permissions' do + it { expect(loggedin_only_permissions).to_not include(@repository_permissions) } + end + end +end From d0780ee63341d16e6ff644675ebd000574c7e5ba Mon Sep 17 00:00:00 2001 From: Solotchi Veaceslav Date: Thu, 5 Feb 2015 14:16:07 +0200 Subject: [PATCH 34/59] Disable default modules --- config/configuration.yml.example | 11 +++++ config/initializers/module_handler.rb | 33 ++++++++++++++ lib/open_project/configuration.rb | 5 ++- lib/open_project/plugins/module_handler.rb | 50 ++++++++++++++++++++++ lib/redmine/access_control.rb | 18 ++++++++ 5 files changed, 116 insertions(+), 1 deletion(-) create mode 100644 config/initializers/module_handler.rb create mode 100644 lib/open_project/plugins/module_handler.rb diff --git a/config/configuration.yml.example b/config/configuration.yml.example index c8d226dac2..282fe6eb8d 100644 --- a/config/configuration.yml.example +++ b/config/configuration.yml.example @@ -94,6 +94,17 @@ # # === More configuration options # +# Disable default module: +# +# By default user may choose which modules can be disabled, +# they should be listed as an array in yml format more information +# regarding yml format you can find here: +# http://symfony.com/doc/current/components/yaml/yaml_format.html +# +# disabled_modules: +# - module_name_1 +# - module_name_2 +# # See following page: # # http://guides.rubyonrails.org/action_mailer_basics.html#action-mailer-configuration diff --git a/config/initializers/module_handler.rb b/config/initializers/module_handler.rb new file mode 100644 index 0000000000..418b8051c8 --- /dev/null +++ b/config/initializers/module_handler.rb @@ -0,0 +1,33 @@ +#-- encoding: UTF-8 +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2014 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-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 doc/COPYRIGHT.rdoc for more details. +#++ + +unless OpenProject::Configuration['disabled_modules'].empty? + to_disable = OpenProject::Configuration['disabled_modules'] + OpenProject::Plugins::ModuleHandler.disable_modules(to_disable) +end diff --git a/lib/open_project/configuration.rb b/lib/open_project/configuration.rb index 1fad3a81e2..a7f024f832 100644 --- a/lib/open_project/configuration.rb +++ b/lib/open_project/configuration.rb @@ -74,7 +74,10 @@ module OpenProject 'disable_password_login' => false, 'omniauth_direct_login_provider' => nil, - 'disable_password_choice' => false + 'disable_password_choice' => false, + + # allow to disable default modules + 'disabled_modules' => [] } @config = nil diff --git a/lib/open_project/plugins/module_handler.rb b/lib/open_project/plugins/module_handler.rb new file mode 100644 index 0000000000..d864265610 --- /dev/null +++ b/lib/open_project/plugins/module_handler.rb @@ -0,0 +1,50 @@ +#-- encoding: UTF-8 +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 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-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 doc/COPYRIGHT.rdoc for more details. +#++project_module + +module OpenProject::Plugins + class ModuleHandler + @@disabled_modules = [] + + class << self + def disable_modules(module_names) + @@disabled_modules += Array(module_names).map(&:to_sym) + end + + def disable(disabled_modules) + disabled_modules.map do |module_name| + Redmine::AccessControl.remove_modules_permissions(module_name) + end + end + end + + OpenProject::Application.config.to_prepare do + OpenProject::Plugins::ModuleHandler.disable(@@disabled_modules) + end + end +end diff --git a/lib/redmine/access_control.rb b/lib/redmine/access_control.rb index 1f6411b3e9..6c1af2173f 100644 --- a/lib/redmine/access_control.rb +++ b/lib/redmine/access_control.rb @@ -76,6 +76,24 @@ module Redmine def modules_permissions(modules) @permissions.select { |p| p.project_module.nil? || modules.include?(p.project_module.to_s) } end + + def remove_modules_permissions(module_name) + permissions = @permissions + + module_permissions = permissions.select { |p| p.project_module.to_s == module_name.to_s } + + reset + + @permissions = permissions - module_permissions + end + + def reset + @permissions = nil + @available_project_modules = nil + @public_permissions = nil + @members_only_permissions = nil + @loggedin_only_permissions = nil + end end class Mapper From 40d6dba16ac303eb7ae5a0b77cdd37a2642941fa Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Tue, 10 Feb 2015 16:14:20 +0100 Subject: [PATCH 35/59] Added missing tests for category link --- .../work_package_representer_spec.rb | 41 +++++++++++++++---- 1 file changed, 34 insertions(+), 7 deletions(-) diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index 7c7052b3fa..6906c776c7 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -41,11 +41,9 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do id: 42, created_at: DateTime.now, updated_at: DateTime.now, - category: category, done_ratio: 50, estimated_hours: 6.0) } - let(:category) { FactoryGirl.build(:category) } let(:project) { work_package.project } let(:permissions) { [ @@ -287,6 +285,40 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do end end + describe 'category' do + let(:embedded_path) { '_embedded/category' } + let(:href_path) { '_links/category/href' } + + context 'no category set' do + it 'has no category linked' do + is_expected.to_not have_json_path(href_path) + end + + it 'has no category embedded' do + is_expected.to_not have_json_path(embedded_path) + end + end + + context 'category set' do + let!(:category) { FactoryGirl.create :category, project: project } + let(:expected_url) { api_v3_paths.category(category.id).to_json } + + before do + work_package.category = category + end + + it 'has a link to the category' do + is_expected.to be_json_eql(expected_url).at_path(href_path) + end + + it 'has the category embedded' do + is_expected.to have_json_type(Hash).at_path('_embedded/category') + is_expected.to be_json_eql('Category'.to_json).at_path("#{embedded_path}/_type") + is_expected.to be_json_eql(category.name.to_json).at_path("#{embedded_path}/name") + end + end + end + context 'when the user has the permission to view work packages' do context 'and the user is not watching the work package' do it 'should have a link to watch' do @@ -537,11 +569,6 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do it { is_expected.not_to have_json_path('_embedded/watchers') } end end - - describe 'category' do - it { is_expected.to have_json_type(Hash).at_path('_embedded/category') } - it { is_expected.to be_json_eql(%{Category}.to_json).at_path('_embedded/category/_type') } - end end end end From 856e09bca596b0081b25b1a8934689ad143ba6f7 Mon Sep 17 00:00:00 2001 From: Alexander Bach Date: Tue, 10 Feb 2015 16:18:02 +0100 Subject: [PATCH 36/59] Rubocopify --- .../work_package_representer_spec.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index 6906c776c7..1d38b3d197 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -31,7 +31,7 @@ require 'spec_helper' describe ::API::V3::WorkPackages::WorkPackageRepresenter do include ::API::V3::Utilities::PathHelper - let(:member) { FactoryGirl.create(:user, member_in_project: project, member_through_role: role) } + let(:member) { FactoryGirl.create(:user, member_in_project: project, member_through_role: role) } let(:current_user) { member } let(:representer) { described_class.new(work_package, current_user: current_user) } @@ -136,14 +136,13 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do allow(user).to receive(:allowed_to?).and_return(false) allow(user).to receive(:allowed_to?).with(:view_time_entries, anything) - .and_return(true) + .and_return(true) end context 'no view_time_entries permission' do before do allow(user).to receive(:allowed_to?).with(:view_time_entries, anything) - .and_return(false) - + .and_return(false) end it { is_expected.to_not have_json_path('spentTime') } @@ -457,7 +456,9 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do project: project, parent_id: forbidden_work_package.id) } - let!(:forbidden_work_package) { FactoryGirl.create(:work_package, project: forbidden_project) } + let!(:forbidden_work_package) { + FactoryGirl.create(:work_package, project: forbidden_project) + } it { expect(subject).to_not have_json_path('_links/parent') } end @@ -481,7 +482,9 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do it { expect(subject).to have_json_size(1).at_path('_links/children') } - it { expect(parse_json(subject)['_links']['children'][0]['title']).to eq(child.subject) } + it { + expect(parse_json(subject)['_links']['children'][0]['title']).to eq(child.subject) + } end end end From 0570457822ad3250f80c28091ce87bae0feef394 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 5 Feb 2015 09:22:02 +0100 Subject: [PATCH 37/59] change links to nothing in work package - now a link exists that has a null href -> instead of hiding the whole link - refactored generation of links to be more uniform --- lib/api/decorators/single.rb | 14 +++ .../work_packages/work_package_representer.rb | 56 ++------- spec/factories/user_factory.rb | 4 +- spec/factories/work_package_factory.rb | 4 +- .../work_package_representer_spec.rb | 108 +++++++++++++----- 5 files changed, 106 insertions(+), 80 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index ded3fa5661..6b2bef1f69 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -51,6 +51,20 @@ module API exec_context: :decorator, render_nil: false + def self.linked_property(property, + path: property, + backing_field: property, + title_getter: -> (*) { represented.send(backing_field).name }, + visible_condition: -> (*) { true }) + link property do + value = represented.send(backing_field) + link_object = { href: (api_v3_paths.send(path, value.id) if value) } + link_object[:title] = self.instance_eval(&title_getter) if value + + link_object if self.instance_eval(&visible_condition) + end + end + private def datetime_formatter diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 5fa2d7fb49..71d454499a 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -90,33 +90,11 @@ module API } if current_user_allowed_to(:move_work_packages) end - link :status do - { - href: api_v3_paths.status(represented.status_id), - title: represented.status.name - } - end - - link :author do - { - href: api_v3_paths.user(represented.author.id), - title: "#{represented.author.name} - #{represented.author.login}" - } unless represented.author.nil? - end - - link :responsible do - { - href: api_v3_paths.user(represented.responsible.id), - title: "#{represented.responsible.name} - #{represented.responsible.login}" - } unless represented.responsible.nil? - end + linked_property :status - link :assignee do - { - href: api_v3_paths.user(represented.assigned_to.id), - title: "#{represented.assigned_to.name} - #{represented.assigned_to.login}" - } unless represented.assigned_to.nil? - end + linked_property :author, path: :user + linked_property :responsible, path: :user + linked_property :assignee, path: :user, backing_field: :assigned_to link :availableWatchers do { @@ -202,27 +180,15 @@ module API } if current_user_allowed_to(:view_time_entries) end - link :category do - { - href: api_v3_paths.category(represented.category.id), - title: represented.category.name - } unless represented.category.nil? - end + linked_property :category - link :version do - { - href: api_v3_paths.version(represented.fixed_version.id), - title: "#{represented.fixed_version.to_s_for_project(represented.project)}" - } if represented.fixed_version && - version_policy.allowed?(represented.fixed_version, :show) - end + linked_property :version, + backing_field: :fixed_version, + title_getter: -> (*) { + represented.fixed_version.to_s_for_project(represented.project) + } - link :priority do - { - href: api_v3_paths.priority(represented.priority.id), - title: represented.priority.name - } - end + linked_property :priority links :children do visible_children.map do |child| diff --git a/spec/factories/user_factory.rb b/spec/factories/user_factory.rb index 576088bfe1..45e49625e1 100644 --- a/spec/factories/user_factory.rb +++ b/spec/factories/user_factory.rb @@ -39,8 +39,8 @@ FactoryGirl.define do sequence(:mail) { |n| "bob#{n}.bobbit@bob.com" } password 'adminADMIN!' password_confirmation 'adminADMIN!' - created_on { Time.now } - updated_on { Time.now } + created_on Time.now + updated_on Time.now mail_notification(Redmine::VERSION::MAJOR > 0 ? 'all' : true) diff --git a/spec/factories/work_package_factory.rb b/spec/factories/work_package_factory.rb index db7ab4fa5a..a5cb3de679 100644 --- a/spec/factories/work_package_factory.rb +++ b/spec/factories/work_package_factory.rb @@ -38,8 +38,8 @@ FactoryGirl.define do sequence(:subject) { |n| "WorkPackage No. #{n}" } description { |i| "Description for '#{i.subject}'" } author factory: :user - created_at { Time.now } - updated_at { Time.now } + created_at Time.now + updated_at Time.now callback(:after_build) do |work_package, evaluator| work_package.type = work_package.project.types.first unless work_package.type diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index 5a984dab57..610875a676 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -233,6 +233,19 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do end describe '_links' do + shared_examples_for 'has a titled link' do + it { is_expected.to be_json_eql(href.to_json).at_path("_links/#{link}/href") } + it { is_expected.to be_json_eql(title.to_json).at_path("_links/#{link}/title") } + end + + shared_examples_for 'has an empty link' do + it { is_expected.to be_json_eql(nil.to_json).at_path("_links/#{link}/href") } + + it 'has no embedded resource' do + is_expected.to_not have_json_path("_embedded/#{link}") + end + end + it { is_expected.to have_json_type(Object).at_path('_links') } it 'should link to self' do @@ -240,6 +253,12 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do expect(subject).to have_json_path('_links/self/title') end + it_behaves_like 'has a titled link' do + let(:link) { 'self' } + let(:href) { "/api/v3/work_packages/#{work_package.id}" } + let(:title) { work_package.subject } + end + describe 'update links' do describe 'update by form' do it { expect(subject).to have_json_path('_links/update/href') } @@ -264,12 +283,59 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do end describe 'status' do - let(:link) { "/api/v3/statuses/#{work_package.status_id}".to_json } - let(:title) { "#{work_package.status.name}".to_json } + it_behaves_like 'has a titled link' do + let(:link) { 'status' } + let(:href) { "/api/v3/statuses/#{work_package.status_id}" } + let(:title) { work_package.status.name } + end + end + + describe 'author' do + it_behaves_like 'has a titled link' do + let(:link) { 'author' } + let(:href) { "/api/v3/users/#{work_package.author.id}" } + let(:title) { work_package.author.name } + end + end + + describe 'assignee' do + context 'assignee is set' do + let(:work_package) { + FactoryGirl.build(:work_package, assigned_to: FactoryGirl.build(:user)) + } + + it_behaves_like 'has a titled link' do + let(:link) { 'assignee' } + let(:href) { "/api/v3/users/#{work_package.assigned_to.id}" } + let(:title) { work_package.assigned_to.name } + end + end + + context 'assignee is not set' do + it_behaves_like 'has an empty link' do + let(:link) { 'assignee' } + end + end + end - it { is_expected.to be_json_eql(link).at_path('_links/status/href') } + describe 'responsible' do + context 'responsible is set' do + let(:work_package) { + FactoryGirl.build(:work_package, responsible: FactoryGirl.build(:user)) + } - it { is_expected.to be_json_eql(title).at_path('_links/status/title') } + it_behaves_like 'has a titled link' do + let(:link) { 'responsible' } + let(:href) { "/api/v3/users/#{work_package.responsible.id}" } + let(:title) { work_package.responsible.name } + end + end + + context 'responsible is not set' do + it_behaves_like 'has an empty link' do + let(:link) { 'responsible' } + end + end end describe 'version' do @@ -277,48 +343,28 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do let(:href_path) { '_links/version/href' } context 'no version set' do - it 'has no version linked' do - is_expected.to_not have_json_path(href_path) - end - - it 'has no version embedded' do - is_expected.to_not have_json_path(embedded_path) + it_behaves_like 'has an empty link' do + let(:link) { 'version' } end end context 'version set' do let!(:version) { FactoryGirl.create :version, project: project } - let(:expected_url) { api_v3_paths.version(version.id).to_json } before do work_package.fixed_version = version end - it 'has a link to the version' do - is_expected.to be_json_eql(expected_url).at_path(href_path) + it_behaves_like 'has a titled link' do + let(:link) { 'version' } + let(:href) { api_v3_paths.version(version.id) } + let(:title) { version.to_s_for_project(project) } end it 'has the version embedded' do is_expected.to be_json_eql('Version'.to_json).at_path("#{embedded_path}/_type") is_expected.to be_json_eql(version.name.to_json).at_path("#{embedded_path}/name") end - - context ' but is not accessible due to permissions' do - before do - policy = double('VersionPolicy') - allow(policy).to receive(:allowed?).with(version, :show).and_return(false) - representer.instance_variable_set(:@version_policy, policy) - end - - it 'has no version linked' do - is_expected.to_not have_json_path(href_path) - end - - it 'has the version embedded as the user has the view work package permission' do - is_expected.to be_json_eql('Version'.to_json).at_path("#{embedded_path}/_type") - is_expected.to be_json_eql(version.name.to_json).at_path("#{embedded_path}/name") - end - end end end @@ -509,7 +555,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do allow(Setting).to receive(:cross_project_work_package_relations?).and_return(true) end - context 'parent' do + describe 'parent' do let(:work_package) { FactoryGirl.create(:work_package, project: project, From 51081271aff1f72a1541d74c2c0c511ccf57a003 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 5 Feb 2015 10:03:13 +0100 Subject: [PATCH 38/59] also use new link helper in version representer --- lib/api/v3/versions/version_representer.rb | 12 ++--- .../v3/support/{action_link.rb => links.rb} | 26 +++++++++ .../v3/versions/version_representer_spec.rb | 53 +++++++++++-------- .../work_package_representer_spec.rb | 13 ----- 4 files changed, 61 insertions(+), 43 deletions(-) rename spec/lib/api/v3/support/{action_link.rb => links.rb} (68%) diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index 2f5f8d5e19..1cba4c214a 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -38,16 +38,14 @@ module API link :self do { href: api_v3_paths.version(represented.id), - title: "#{represented.name}" + title: represented.name } end - link :definingProject do - { - href: api_v3_paths.project(represented.project.id), - title: represented.project.name - } if represented.project.visible?(current_user) - end + linked_property :definingProject, + path: :project, + backing_field: :project, + visible_condition: -> (*) { represented.project.visible?(current_user) } link :availableInProjects do { diff --git a/spec/lib/api/v3/support/action_link.rb b/spec/lib/api/v3/support/links.rb similarity index 68% rename from spec/lib/api/v3/support/action_link.rb rename to spec/lib/api/v3/support/links.rb index 5609d8766d..f3092a3949 100644 --- a/spec/lib/api/v3/support/action_link.rb +++ b/spec/lib/api/v3/support/links.rb @@ -48,3 +48,29 @@ shared_examples_for 'action link' do it { expect(subject).to have_json_path("_links/#{action}/href") } end end + +shared_examples_for 'has a titled link' do + it { is_expected.to be_json_eql(href.to_json).at_path("_links/#{link}/href") } + it { is_expected.to be_json_eql(title.to_json).at_path("_links/#{link}/title") } +end + +shared_examples_for 'has an untitled link' do + it { is_expected.to be_json_eql(href.to_json).at_path("_links/#{link}/href") } + it { is_expected.to_not have_json_path("_links/#{link}/title") } +end + +shared_examples_for 'has an empty link' do + it { is_expected.to be_json_eql(nil.to_json).at_path("_links/#{link}/href") } + + it 'has no embedded resource' do + is_expected.to_not have_json_path("_embedded/#{link}") + end +end + +shared_examples_for 'has no link' do + it { is_expected.to_not have_json_path("_links/#{link}") } + + it 'has no embedded resource' do + is_expected.to_not have_json_path("_embedded/#{link}") + end +end diff --git a/spec/lib/api/v3/versions/version_representer_spec.rb b/spec/lib/api/v3/versions/version_representer_spec.rb index 3cb509d035..b67727f967 100644 --- a/spec/lib/api/v3/versions/version_representer_spec.rb +++ b/spec/lib/api/v3/versions/version_representer_spec.rb @@ -41,40 +41,47 @@ describe ::API::V3::Versions::VersionRepresenter do it { should include_json('Version'.to_json).at_path('_type') } - context 'links' do + describe 'links' do it { should have_json_type(Object).at_path('_links') } - it 'to self' do - path = api_v3_paths.version(version.id) - - expect(subject).to be_json_eql(path.to_json).at_path('_links/self/href') + describe 'to self' do + it_behaves_like 'has a titled link' do + let(:link) { 'self' } + let(:href) { api_v3_paths.version(version.id) } + let(:title) { version.name } + end end - context 'to the defining project' do - let(:path) { api_v3_paths.project(version.project.id) } - - it 'exists if the user has the permission to see the project' do - allow(version.project).to receive(:visible?).with(user).and_return(true) - - subject = representer.to_json - - expect(subject).to be_json_eql(path.to_json).at_path('_links/definingProject/href') + describe 'to the defining project' do + context 'if the user has the permission to see the project' do + before do + allow(version.project).to receive(:visible?).with(user).and_return(true) + end + + it_behaves_like 'has a titled link' do + let(:link) { 'definingProject' } + let(:href) { api_v3_paths.project(version.project.id) } + let(:title) { version.project.name } + end end - it 'does not exist if the user lacks the permission to see the project' do - allow(version.project).to receive(:visible?).with(user).and_return(false) - - subject = representer.to_json + context 'if the user lacks the permission to see the project' do + before do + allow(version.project).to receive(:visible?).with(user).and_return(false) + end - expect(subject).to_not have_json_path('_links/definingProject/href') + it_behaves_like 'has no link' do + let(:link) { 'definingProject' } + end end end - it 'to available projects' do - path = api_v3_paths.versions_projects(version.id) - - expect(subject).to be_json_eql(path.to_json).at_path('_links/availableInProjects/href') + describe 'to available projects' do + it_behaves_like 'has an untitled link' do + let(:link) { 'availableInProjects' } + let(:href) { api_v3_paths.versions_projects(version.id) } + end end end diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index 610875a676..e098f7eabd 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -233,19 +233,6 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do end describe '_links' do - shared_examples_for 'has a titled link' do - it { is_expected.to be_json_eql(href.to_json).at_path("_links/#{link}/href") } - it { is_expected.to be_json_eql(title.to_json).at_path("_links/#{link}/title") } - end - - shared_examples_for 'has an empty link' do - it { is_expected.to be_json_eql(nil.to_json).at_path("_links/#{link}/href") } - - it 'has no embedded resource' do - is_expected.to_not have_json_path("_embedded/#{link}") - end - end - it { is_expected.to have_json_type(Object).at_path('_links') } it 'should link to self' do From 327c53cb94d276c4a1da378677a78d956271e73d Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 5 Feb 2015 10:54:07 +0100 Subject: [PATCH 39/59] extract helper for self links --- lib/api/decorators/single.rb | 9 +++++++++ lib/api/v3/priorities/priority_representer.rb | 8 ++------ lib/api/v3/projects/project_representer.rb | 7 +------ lib/api/v3/statuses/status_representer.rb | 8 ++------ lib/api/v3/users/user_representer.rb | 7 +------ lib/api/v3/versions/version_representer.rb | 7 +------ lib/api/v3/work_packages/work_package_representer.rb | 7 +------ spec/lib/api/v3/statuses/status_representer_spec.rb | 12 +++++------- .../work_packages/work_package_representer_spec.rb | 5 ----- 9 files changed, 22 insertions(+), 48 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index 6b2bef1f69..caa14f9bcb 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -51,6 +51,15 @@ module API exec_context: :decorator, render_nil: false + def self.self_link(path, title_getter: -> (*) { represented.name }) + link :self do + link_object = { href: api_v3_paths.send(path, represented.id) } + link_object[:title] = self.instance_eval(&title_getter) + + link_object + end + end + def self.linked_property(property, path: property, backing_field: property, diff --git a/lib/api/v3/priorities/priority_representer.rb b/lib/api/v3/priorities/priority_representer.rb index cef69d0450..7a5dcd9690 100644 --- a/lib/api/v3/priorities/priority_representer.rb +++ b/lib/api/v3/priorities/priority_representer.rb @@ -34,12 +34,8 @@ module API module V3 module Priorities class PriorityRepresenter < ::API::Decorators::Single - link :self do - { - href: api_v3_paths.priority(represented.id), - title: represented.name - } - end + + self_link :priority property :id, render_nil: true property :name diff --git a/lib/api/v3/projects/project_representer.rb b/lib/api/v3/projects/project_representer.rb index feaa079b6b..d754c4d5d4 100644 --- a/lib/api/v3/projects/project_representer.rb +++ b/lib/api/v3/projects/project_representer.rb @@ -35,12 +35,7 @@ module API module Projects class ProjectRepresenter < ::API::Decorators::Single - link :self do - { - href: api_v3_paths.project(represented.id), - title: "#{represented.name}" - } - end + self_link :project link 'categories' do { href: api_v3_paths.categories(represented.id) } diff --git a/lib/api/v3/statuses/status_representer.rb b/lib/api/v3/statuses/status_representer.rb index a6c80f8185..37a5df074d 100644 --- a/lib/api/v3/statuses/status_representer.rb +++ b/lib/api/v3/statuses/status_representer.rb @@ -31,12 +31,8 @@ module API module V3 module Statuses class StatusRepresenter < ::API::Decorators::Single - link :self do - { - href: api_v3_paths.status(represented.id), - title: "#{represented.name}" - } - end + + self_link :status property :id, render_nil: true property :name diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index e55740cbe0..9f2e35c965 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -36,12 +36,7 @@ module API class UserRepresenter < ::API::Decorators::Single include AvatarHelper - link :self do - { - href: api_v3_paths.user(represented.id), - title: "#{represented.name} - #{represented.login}" - } - end + self_link :user link :lock do { diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index 1cba4c214a..56101920f8 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -35,12 +35,7 @@ module API module Versions class VersionRepresenter < ::API::Decorators::Single - link :self do - { - href: api_v3_paths.version(represented.id), - title: represented.name - } - end + self_link :version linked_property :definingProject, path: :project, diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 71d454499a..91a25303b7 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -35,12 +35,7 @@ module API module WorkPackages class WorkPackageRepresenter < ::API::Decorators::Single - link :self do - { - href: api_v3_paths.work_package(represented.id), - title: represented.subject - } - end + self_link :work_package, title_getter: -> (*) { represented.subject } link :update do { diff --git a/spec/lib/api/v3/statuses/status_representer_spec.rb b/spec/lib/api/v3/statuses/status_representer_spec.rb index 5a49953182..570c07b50c 100644 --- a/spec/lib/api/v3/statuses/status_representer_spec.rb +++ b/spec/lib/api/v3/statuses/status_representer_spec.rb @@ -61,13 +61,11 @@ describe ::API::V3::Statuses::StatusRepresenter do it { is_expected.to have_json_type(Object).at_path('_links') } describe 'self' do - let(:href) { "/api/v3/statuses/#{status.id}".to_json } - - it { is_expected.to have_json_path('_links/self/href') } - it { is_expected.to have_json_path('_links/self/title') } - - it { is_expected.to be_json_eql(href).at_path('_links/self/href') } - it { is_expected.to be_json_eql(status.name.to_json).at_path('_links/self/title') } + it_behaves_like 'has a titled link' do + let(:link) { 'self' } + let(:href) { "/api/v3/statuses/#{status.id}" } + let(:title) { status.name } + end end end end diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index e098f7eabd..1e6ccfd3aa 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -235,11 +235,6 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do describe '_links' do it { is_expected.to have_json_type(Object).at_path('_links') } - it 'should link to self' do - expect(subject).to have_json_path('_links/self/href') - expect(subject).to have_json_path('_links/self/title') - end - it_behaves_like 'has a titled link' do let(:link) { 'self' } let(:href) { "/api/v3/work_packages/#{work_package.id}" } From 7f1401f40c10af5bba19a99cab6b6579d4f90ac2 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 5 Feb 2015 15:36:10 +0100 Subject: [PATCH 40/59] remove unneccessary selfs --- lib/api/decorators/single.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index caa14f9bcb..bfc150188a 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -54,7 +54,7 @@ module API def self.self_link(path, title_getter: -> (*) { represented.name }) link :self do link_object = { href: api_v3_paths.send(path, represented.id) } - link_object[:title] = self.instance_eval(&title_getter) + link_object[:title] = instance_eval(&title_getter) link_object end @@ -68,9 +68,9 @@ module API link property do value = represented.send(backing_field) link_object = { href: (api_v3_paths.send(path, value.id) if value) } - link_object[:title] = self.instance_eval(&title_getter) if value + link_object[:title] = instance_eval(&title_getter) if value - link_object if self.instance_eval(&visible_condition) + link_object if instance_eval(&visible_condition) end end From 5f67b07e95bd1f6b0fd0879c730cc6337ad718de Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 5 Feb 2015 16:56:48 +0100 Subject: [PATCH 41/59] fix integration tests --- spec/requests/api/v3/work_package_resource_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/requests/api/v3/work_package_resource_spec.rb b/spec/requests/api/v3/work_package_resource_spec.rb index 26ab355452..6e624dd655 100644 --- a/spec/requests/api/v3/work_package_resource_spec.rb +++ b/spec/requests/api/v3/work_package_resource_spec.rb @@ -502,14 +502,14 @@ h4. things we like it { expect(response.status).to eq(200) } - it { expect(response.body).not_to have_json_path("_links/#{property}") } + it { expect(response.body).to be_json_eql(nil.to_json).at_path("_links/#{property}/href") } it_behaves_like 'lock version updated' end describe 'valid' do shared_examples_for 'valid user assignment' do - let(:title) { "#{assigned_user.name} - #{assigned_user.login}".to_json } + let(:title) { "#{assigned_user.name}".to_json } it { expect(response.status).to eq(200) } From e142f5144f9d095b5126b5ab6c188904097cc48f Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 5 Feb 2015 17:03:50 +0100 Subject: [PATCH 42/59] just don't say anything hound! --- spec/requests/api/v3/work_package_resource_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/requests/api/v3/work_package_resource_spec.rb b/spec/requests/api/v3/work_package_resource_spec.rb index 6e624dd655..cb100b8b9e 100644 --- a/spec/requests/api/v3/work_package_resource_spec.rb +++ b/spec/requests/api/v3/work_package_resource_spec.rb @@ -494,6 +494,7 @@ h4. things we like shared_examples_for 'handling people' do |property| let(:user_parameter) { { _links: { property => { href: user_href } } } } + let(:href_path) { "_links/#{property}/href" } describe 'nil' do let(:user_href) { nil } @@ -502,7 +503,7 @@ h4. things we like it { expect(response.status).to eq(200) } - it { expect(response.body).to be_json_eql(nil.to_json).at_path("_links/#{property}/href") } + it { expect(response.body).to be_json_eql(nil.to_json).at_path(href_path) } it_behaves_like 'lock version updated' end From a60ce1948df5467e5031019162b840f7e52a1538 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Fri, 6 Feb 2015 11:17:56 +0100 Subject: [PATCH 43/59] rename visible_condition to show_if --- lib/api/decorators/single.rb | 4 ++-- lib/api/v3/versions/version_representer.rb | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index bfc150188a..c295a70124 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -64,13 +64,13 @@ module API path: property, backing_field: property, title_getter: -> (*) { represented.send(backing_field).name }, - visible_condition: -> (*) { true }) + show_if: -> (*) { true }) link property do value = represented.send(backing_field) link_object = { href: (api_v3_paths.send(path, value.id) if value) } link_object[:title] = instance_eval(&title_getter) if value - link_object if instance_eval(&visible_condition) + link_object if instance_eval(&show_if) end end diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index 56101920f8..8148c1081d 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -40,7 +40,7 @@ module API linked_property :definingProject, path: :project, backing_field: :project, - visible_condition: -> (*) { represented.project.visible?(current_user) } + show_if: -> (*) { represented.project.visible?(current_user) } link :availableInProjects do { From f7134df8f9ff6b198e9f3d5a3bfb3cb2e902bb9c Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Fri, 6 Feb 2015 11:50:07 +0100 Subject: [PATCH 44/59] also apply visibility conditions to parent - parent link only invisible if present and not visible to user - no parent rendered as null link - tests for all three cases (none, visible, invisible) --- .../work_packages/work_package_representer.rb | 10 ++--- .../work_package_representer_spec.rb | 41 +++++++++++++++---- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 91a25303b7..700eaf84c1 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -160,12 +160,10 @@ module API } if current_user_allowed_to(:add_work_package_notes) end - link :parent do - { - href: api_v3_paths.work_package(represented.parent.id), - title: represented.parent.subject - } unless represented.parent.nil? || !represented.parent.visible? - end + linked_property :parent, + path: :work_package, + title_getter: -> (*) { represented.parent.subject }, + show_if: -> (*) { represented.parent.nil? || represented.parent.visible? } link :timeEntries do { diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index 1e6ccfd3aa..ee7b517e94 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -538,16 +538,41 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do end describe 'parent' do - let(:work_package) { - FactoryGirl.create(:work_package, - project: project, - parent_id: forbidden_work_package.id) - } - let!(:forbidden_work_package) do - FactoryGirl.create(:work_package, project: forbidden_project) + let(:visible_parent) { FactoryGirl.create(:work_package, project: project) } + let(:invisible_parent) { FactoryGirl.create(:work_package, project: forbidden_project) } + let(:work_package) { FactoryGirl.create(:work_package, project: project) } + + context 'no parent' do + it_behaves_like 'has an empty link' do + let(:link) { 'parent' } + end end - it { expect(subject).to_not have_json_path('_links/parent') } + context 'parent is visible' do + let(:work_package) { + FactoryGirl.create(:work_package, + project: project, + parent_id: visible_parent.id) + } + + it_behaves_like 'has a titled link' do + let(:link) { 'parent' } + let(:href) { api_v3_paths.work_package(visible_parent.id) } + let(:title) { visible_parent.subject } + end + end + + context 'parent not visible' do + let(:work_package) { + FactoryGirl.create(:work_package, + project: project, + parent_id: invisible_parent.id) + } + + it_behaves_like 'has no link' do + let(:link) { 'parent' } + end + end end context 'children' do From 40a727c54f458c6c802253fd567eef43328b8162 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Fri, 6 Feb 2015 15:27:31 +0100 Subject: [PATCH 45/59] fix parameter indentation --- lib/api/decorators/single.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index c295a70124..fff8d3f3d3 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -61,10 +61,10 @@ module API end def self.linked_property(property, - path: property, - backing_field: property, - title_getter: -> (*) { represented.send(backing_field).name }, - show_if: -> (*) { true }) + path: property, + backing_field: property, + title_getter: -> (*) { represented.send(backing_field).name }, + show_if: -> (*) { true }) link property do value = represented.send(backing_field) link_object = { href: (api_v3_paths.send(path, value.id) if value) } From 4f7836dda588c2e3ae36fb86ea5abe8873810b82 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 11 Feb 2015 13:08:41 +0100 Subject: [PATCH 46/59] fix and expand category specs --- .../work_package_representer_spec.rb | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index ee7b517e94..9fb36f34d1 100644 --- a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb @@ -355,25 +355,22 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do let(:href_path) { '_links/category/href' } context 'no category set' do - it 'has no category linked' do - is_expected.to_not have_json_path(href_path) - end - - it 'has no category embedded' do - is_expected.to_not have_json_path(embedded_path) + it_behaves_like 'has an empty link' do + let(:link) { 'category' } end end context 'category set' do let!(:category) { FactoryGirl.create :category, project: project } - let(:expected_url) { api_v3_paths.category(category.id).to_json } before do work_package.category = category end - it 'has a link to the category' do - is_expected.to be_json_eql(expected_url).at_path(href_path) + it_behaves_like 'has a titled link' do + let(:link) { 'category' } + let(:href) { api_v3_paths.category(category.id) } + let(:title) { category.name } end it 'has the category embedded' do From 31e8d58ec513f394c15aed66503193fb9d67b45e Mon Sep 17 00:00:00 2001 From: netfighter Date: Mon, 9 Feb 2015 17:00:24 +0200 Subject: [PATCH 47/59] Specs for Hide specified admin menu items --- .../menu_items/admin_menu_item_spec.rb | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 spec/features/menu_items/admin_menu_item_spec.rb diff --git a/spec/features/menu_items/admin_menu_item_spec.rb b/spec/features/menu_items/admin_menu_item_spec.rb new file mode 100644 index 0000000000..e82f8e816b --- /dev/null +++ b/spec/features/menu_items/admin_menu_item_spec.rb @@ -0,0 +1,68 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 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-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 doc/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +feature 'Admin menu items' do + let(:user) { FactoryGirl.create :admin } + + before do + User.stub(:current).and_return user + end + + after do + OpenProject::Configuration['hidden_menu_items'] = [] + end + + describe 'displaying all the menu items' do + it 'hides the specified admin menu items' do + visit admin_path + + expect(page).to have_selector('a', text: I18n.t('label_user_plural')) + expect(page).to have_selector('a', text: I18n.t('label_project_plural')) + expect(page).to have_selector('a', text: I18n.t('label_role_plural')) + expect(page).to have_selector('a', text: I18n.t('label_type_plural')) + end + end + + describe 'hiding menu items' do + before do + OpenProject::Configuration['hidden_menu_items'] = { 'admin_menu' => ['roles', 'types'] } + end + + it 'hides the specified admin menu items' do + visit admin_path + + expect(page).to have_selector('a', text: I18n.t('label_user_plural')) + expect(page).to have_selector('a', text: I18n.t('label_project_plural')) + + expect(page).not_to have_selector('a', text: I18n.t('label_role_plural')) + expect(page).not_to have_selector('a', text: I18n.t('label_type_plural')) + end + end +end From c3976607f236bf56490800488a8df185f7ea2c00 Mon Sep 17 00:00:00 2001 From: netfighter Date: Fri, 6 Feb 2015 18:45:53 +0200 Subject: [PATCH 48/59] Hide specified admin menu items --- lib/open_project/configuration.rb | 4 ++-- lib/redmine/menu_manager/menu_helper.rb | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/lib/open_project/configuration.rb b/lib/open_project/configuration.rb index a7f024f832..51b52091b2 100644 --- a/lib/open_project/configuration.rb +++ b/lib/open_project/configuration.rb @@ -75,9 +75,9 @@ module OpenProject 'omniauth_direct_login_provider' => nil, 'disable_password_choice' => false, - # allow to disable default modules - 'disabled_modules' => [] + 'disabled_modules' => [], + 'hidden_menu_items' => [] } @config = nil diff --git a/lib/redmine/menu_manager/menu_helper.rb b/lib/redmine/menu_manager/menu_helper.rb index 9ee2a284a5..24acc99376 100644 --- a/lib/redmine/menu_manager/menu_helper.rb +++ b/lib/redmine/menu_manager/menu_helper.rb @@ -220,7 +220,7 @@ module Redmine::MenuManager::MenuHelper def menu_items_for(menu, project=nil) items = [] Redmine::MenuManager.items(menu).root.children.each do |node| - if allowed_node?(node, User.current, project) + if allowed_node?(node, User.current, project) && visible_node?(menu, node) if block_given? yield node else @@ -265,4 +265,13 @@ module Redmine::MenuManager::MenuHelper return true end end + + def visible_node?(menu, node) + if OpenProject::Configuration['hidden_menu_items'].length > 0 + hidden_nodes = OpenProject::Configuration['hidden_menu_items'][menu.to_s] || [] + !hidden_nodes.include? node.name.to_s + else + true + end + end end From 33812ccf01fa2dbe005b7069cf0cfea0f5f5c10e Mon Sep 17 00:00:00 2001 From: netfighter Date: Tue, 10 Feb 2015 11:06:13 +0200 Subject: [PATCH 49/59] Add configuration example for the hidden menu items --- config/configuration.yml.example | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/config/configuration.yml.example b/config/configuration.yml.example index 282fe6eb8d..9696aa4a23 100644 --- a/config/configuration.yml.example +++ b/config/configuration.yml.example @@ -108,6 +108,30 @@ # See following page: # # http://guides.rubyonrails.org/action_mailer_basics.html#action-mailer-configuration +# +# Hide menu items: +# +# By default user may choose which menu items can be disabled, +# they should be listed as an array in yml format more information +# regarding yml format you can find here: +# http://symfony.com/doc/current/components/yaml/yaml_format.html +# +# production: +# hidden_menu_items: +# admin_menu: +# - roles +# - types +# - statuses +# - workflows +# - enumerations +# - settings +# - ldap_authentication +# - colors +# - project_types +# - export_card_configurations +# - plugins +# - info +# # default configuration options for all environments From a33107c36770b78285f24a5ca944827f3aa30313 Mon Sep 17 00:00:00 2001 From: netfighter Date: Wed, 11 Feb 2015 12:28:17 +0200 Subject: [PATCH 50/59] Add documentation for the hidden menu items configuration --- doc/CONFIGURATION.md | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index 7a620c981c..1933708d5d 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -147,6 +147,31 @@ In the case of fog you only have to configure everything under `fog`, however. D to `fog` just yet. Instead leave it as `file`. This is because the current attachments storage is used as the source for the migration. +#### Hide menu items + +By default user may choose which menu items can be disabled, +they should be listed as an array in yml format. +More information regarding yml format you can find here: +http://symfony.com/doc/current/components/yaml/yaml_format.html + +``` +production: + hidden_menu_items: + admin_menu: + - roles + - types + - statuses + - workflows + - enumerations + - settings + - ldap_authentication + - colors + - project_types + - export_card_configurations + - plugins + - info +``` + ## Email configuration * `email_delivery_method`: The way emails should be delivered. Possible values: `smtp` or `sendmail` @@ -168,3 +193,5 @@ for the migration. * `cache_memcache_server`: The memcache server host and IP (default: `127.0.0.1:11211`) * `cache_expires_in`: Expiration time for memcache entries (default: `0`, no expiry) * `cache_namespace`: Namespace for cache keys, useful when multiple applications use a single memcache server (default: none) + + From b5117e84eea9749a6cf70a88b1c3bfdcf96e8e3e Mon Sep 17 00:00:00 2001 From: netfighter Date: Wed, 11 Feb 2015 13:31:05 +0200 Subject: [PATCH 51/59] Set an empty hash as the default value of the hidden menu items --- lib/open_project/configuration.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/open_project/configuration.rb b/lib/open_project/configuration.rb index 51b52091b2..667d13368d 100644 --- a/lib/open_project/configuration.rb +++ b/lib/open_project/configuration.rb @@ -77,7 +77,7 @@ module OpenProject 'disable_password_choice' => false, # allow to disable default modules 'disabled_modules' => [], - 'hidden_menu_items' => [] + 'hidden_menu_items' => {} } @config = nil From be232776c70ecee71880e924d90c1dd132a8746a Mon Sep 17 00:00:00 2001 From: netfighter Date: Wed, 11 Feb 2015 14:22:33 +0200 Subject: [PATCH 52/59] Allow configuration override using environment variables --- doc/CONFIGURATION.md | 4 ++++ lib/open_project/configuration/helpers.rb | 12 ++++++++++++ lib/redmine/menu_manager/menu_helper.rb | 5 +++-- 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index 1933708d5d..d158ae4a78 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -172,6 +172,10 @@ production: - info ``` +Or it can be overridden by by an environment variable: + + OPENPROJECT_HIDDEN__MENU__ITEMS_ADMIN__MENU='roles types' + ## Email configuration * `email_delivery_method`: The way emails should be delivered. Possible values: `smtp` or `sendmail` diff --git a/lib/open_project/configuration/helpers.rb b/lib/open_project/configuration/helpers.rb index 829606cc13..5aaa856c18 100644 --- a/lib/open_project/configuration/helpers.rb +++ b/lib/open_project/configuration/helpers.rb @@ -77,6 +77,18 @@ module OpenProject } end + # overrides default getter in OpenProject::Configuration + def hidden_menu_items + menus = self['hidden_menu_items'].map do |label, nodes| + if nodes =~ / / + [label, nodes.split(' ')] + else + [label, nodes] + end + end + Hash[menus] + end + private def true?(value) diff --git a/lib/redmine/menu_manager/menu_helper.rb b/lib/redmine/menu_manager/menu_helper.rb index 24acc99376..502aca7525 100644 --- a/lib/redmine/menu_manager/menu_helper.rb +++ b/lib/redmine/menu_manager/menu_helper.rb @@ -267,8 +267,9 @@ module Redmine::MenuManager::MenuHelper end def visible_node?(menu, node) - if OpenProject::Configuration['hidden_menu_items'].length > 0 - hidden_nodes = OpenProject::Configuration['hidden_menu_items'][menu.to_s] || [] + @hidden_menu_items ||= OpenProject::Configuration.hidden_menu_items + if @hidden_menu_items.length > 0 + hidden_nodes = @hidden_menu_items[menu.to_s] || [] !hidden_nodes.include? node.name.to_s else true From 5e9bd9232c7859488a8d39fa40d5f4763d16cb0e Mon Sep 17 00:00:00 2001 From: netfighter Date: Wed, 11 Feb 2015 14:46:19 +0200 Subject: [PATCH 53/59] Fix configuration.yml.example --- config/configuration.yml.example | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/config/configuration.yml.example b/config/configuration.yml.example index 9696aa4a23..d08930116c 100644 --- a/config/configuration.yml.example +++ b/config/configuration.yml.example @@ -94,10 +94,14 @@ # # === More configuration options # +# See following page: +# +# http://guides.rubyonrails.org/action_mailer_basics.html#action-mailer-configuration +# # Disable default module: # -# By default user may choose which modules can be disabled, -# they should be listed as an array in yml format more information +# By default user may choose which modules can be disabled, +# they should be listed as an array in yml format more information # regarding yml format you can find here: # http://symfony.com/doc/current/components/yaml/yaml_format.html # @@ -105,10 +109,6 @@ # - module_name_1 # - module_name_2 # -# See following page: -# -# http://guides.rubyonrails.org/action_mailer_basics.html#action-mailer-configuration -# # Hide menu items: # # By default user may choose which menu items can be disabled, From 153586d6788e842743347804f7e22229399d882d Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Wed, 11 Feb 2015 16:24:54 +0200 Subject: [PATCH 54/59] fix timeline not loaded when group other is activated in filters --- .../javascripts/angular/timelines/models/mixins/ui.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/angular/timelines/models/mixins/ui.js b/app/assets/javascripts/angular/timelines/models/mixins/ui.js index 231f629f7e..fab38e5509 100644 --- a/app/assets/javascripts/angular/timelines/models/mixins/ui.js +++ b/app/assets/javascripts/angular/timelines/models/mixins/ui.js @@ -614,8 +614,10 @@ angular.module('openproject.timelines.models') bustVerticalOffsetCache: function(tree) { tree.iterateWithChildren(function(node) { var currentElement = node.getDOMElement(); - currentElement.removeAttr("data-vertical-offset"); - currentElement.removeAttr("data-vertical-bottom-offset"); + if (currentElement) { + currentElement.removeAttr("data-vertical-offset"); + currentElement.removeAttr("data-vertical-bottom-offset"); + } }); }, rebuildForeground: function(tree) { @@ -661,6 +663,9 @@ angular.module('openproject.timelines.models') tree.iterateWithChildren(function(node, indent, index) { var currentElement = node.getDOMElement(); + if (!currentElement) { + return; + } var currentOffset = timeline.getRelativeVerticalOffset(currentElement); var previousElement, previousEnd, groupHeight; var groupingChanged = false; From 2ab5a49b0d5672f936d0ae226a251030e7971cd9 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Wed, 11 Feb 2015 16:58:40 +0000 Subject: [PATCH 55/59] env override for hidden menu items; + docs --- doc/CONFIGURATION.md | 76 ++++++++++++++++------- lib/open_project/configuration.rb | 4 +- lib/open_project/configuration/helpers.rb | 34 ++++++---- 3 files changed, 79 insertions(+), 35 deletions(-) diff --git a/doc/CONFIGURATION.md b/doc/CONFIGURATION.md index d158ae4a78..703fafb6f1 100644 --- a/doc/CONFIGURATION.md +++ b/doc/CONFIGURATION.md @@ -83,6 +83,8 @@ storage config above like this: * [`omniauth_direct_login_provider`](#omniauth-direct-login-provider) (default: nil) * [`disable_password_login`](#disable-password-login) (default: false) * [`attachments_storage`](#attachments-storage) (default: file) +* [`hidden_menu_items`](#hidden-menu-items) (default: {}) +* [`disabled_modules`](#disabled-modules) (default: []) ### disable password login @@ -147,34 +149,64 @@ In the case of fog you only have to configure everything under `fog`, however. D to `fog` just yet. Instead leave it as `file`. This is because the current attachments storage is used as the source for the migration. -#### Hide menu items +### hidden menu items -By default user may choose which menu items can be disabled, -they should be listed as an array in yml format. -More information regarding yml format you can find here: -http://symfony.com/doc/current/components/yaml/yaml_format.html +*default: {}* + +You can disable specific menu items in the menu sidebar for each main menu (such as Administration and Projects). +The following example disables all menu items except 'Users', 'Groups' and 'Custom fields' under 'Administration': + +``` +hidden_menu_items: + admin_menu: + - roles + - types + - statuses + - workflows + - enumerations + - settings + - ldap_authentication + - colors + - project_types + - export_card_configurations + - plugins + - info +``` + +The configuration can be overridden through environment variables. +You have to define one variable for each menu. +For instance 'Roles' and 'Types' under 'Administration' can be disabled by defining the following variable: + +``` +OPENPROJECT_HIDDEN__MENU__ITEMS_ADMIN__MENU='roles types' +``` + +### disabled modules + +*default: []* + +Modules may be disabled through the configuration. +Just give a list of the module names either as an array or as a string with values separated by spaces. + +**Array example:** ``` -production: - hidden_menu_items: - admin_menu: - - roles - - types - - statuses - - workflows - - enumerations - - settings - - ldap_authentication - - colors - - project_types - - export_card_configurations - - plugins - - info +disabled_modules: + - backlogs + - meetings ``` -Or it can be overridden by by an environment variable: +**String example:** - OPENPROJECT_HIDDEN__MENU__ITEMS_ADMIN__MENU='roles types' +``` +disabled_modules: backlogs meetings +``` + +The option to use a string is mostly relevant for when you want to override the disabled modules via ENV variables: + +``` +OPENPROJECT_DISABLED__MODULES='backlogs meetings' +``` ## Email configuration diff --git a/lib/open_project/configuration.rb b/lib/open_project/configuration.rb index 667d13368d..9fcd77ec26 100644 --- a/lib/open_project/configuration.rb +++ b/lib/open_project/configuration.rb @@ -75,8 +75,8 @@ module OpenProject 'omniauth_direct_login_provider' => nil, 'disable_password_choice' => false, - # allow to disable default modules - 'disabled_modules' => [], + + 'disabled_modules' => [], # allow to disable default modules 'hidden_menu_items' => {} } diff --git a/lib/open_project/configuration/helpers.rb b/lib/open_project/configuration/helpers.rb index 5aaa856c18..e54ee53da0 100644 --- a/lib/open_project/configuration/helpers.rb +++ b/lib/open_project/configuration/helpers.rb @@ -70,6 +70,18 @@ module OpenProject available_file_uploaders[OpenProject::Configuration.attachments_storage.to_sym] end + def hidden_menu_items + menus = self['hidden_menu_items'].map do |label, nodes| + [label, array(nodes)] + end + + Hash[menus] + end + + def disabled_modules + array self['disabled_modules'] + end + def available_file_uploaders { fog: ::FogFileUploader, @@ -77,20 +89,20 @@ module OpenProject } end - # overrides default getter in OpenProject::Configuration - def hidden_menu_items - menus = self['hidden_menu_items'].map do |label, nodes| - if nodes =~ / / - [label, nodes.split(' ')] - else - [label, nodes] - end + private + + ## + # Yields the given configuration value as an array. + # Either the value already is an array or a string with values separated by spaces. + # In the latter case the string will be split and the values returned as an array. + def array(value) + if value =~ / / + value.split ' ' + else + value end - Hash[menus] end - private - def true?(value) ['true', true].include? value # check string to accommodate ENV override end From 7c2a686657a01edde63e3274efb603ef0170b4f8 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 12 Feb 2015 13:19:57 +0100 Subject: [PATCH 56/59] make self_link even more awesome --- lib/api/decorators/single.rb | 3 ++- lib/api/v3/priorities/priority_representer.rb | 2 +- lib/api/v3/projects/project_representer.rb | 2 +- lib/api/v3/statuses/status_representer.rb | 2 +- lib/api/v3/users/user_representer.rb | 2 +- lib/api/v3/versions/version_representer.rb | 2 +- lib/api/v3/work_packages/work_package_representer.rb | 2 +- 7 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index fff8d3f3d3..853b31dc6e 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -51,8 +51,9 @@ module API exec_context: :decorator, render_nil: false - def self.self_link(path, title_getter: -> (*) { represented.name }) + def self.self_link(path: nil, title_getter: -> (*) { represented.name }) link :self do + path = _type.underscore unless path link_object = { href: api_v3_paths.send(path, represented.id) } link_object[:title] = instance_eval(&title_getter) diff --git a/lib/api/v3/priorities/priority_representer.rb b/lib/api/v3/priorities/priority_representer.rb index 7a5dcd9690..27c2278e3d 100644 --- a/lib/api/v3/priorities/priority_representer.rb +++ b/lib/api/v3/priorities/priority_representer.rb @@ -35,7 +35,7 @@ module API module Priorities class PriorityRepresenter < ::API::Decorators::Single - self_link :priority + self_link property :id, render_nil: true property :name diff --git a/lib/api/v3/projects/project_representer.rb b/lib/api/v3/projects/project_representer.rb index d754c4d5d4..606e12dc81 100644 --- a/lib/api/v3/projects/project_representer.rb +++ b/lib/api/v3/projects/project_representer.rb @@ -35,7 +35,7 @@ module API module Projects class ProjectRepresenter < ::API::Decorators::Single - self_link :project + self_link link 'categories' do { href: api_v3_paths.categories(represented.id) } diff --git a/lib/api/v3/statuses/status_representer.rb b/lib/api/v3/statuses/status_representer.rb index 37a5df074d..3a40f2d7ac 100644 --- a/lib/api/v3/statuses/status_representer.rb +++ b/lib/api/v3/statuses/status_representer.rb @@ -32,7 +32,7 @@ module API module Statuses class StatusRepresenter < ::API::Decorators::Single - self_link :status + self_link property :id, render_nil: true property :name diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index 9f2e35c965..86be96a101 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -36,7 +36,7 @@ module API class UserRepresenter < ::API::Decorators::Single include AvatarHelper - self_link :user + self_link link :lock do { diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index 8148c1081d..f6d343f7a6 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -35,7 +35,7 @@ module API module Versions class VersionRepresenter < ::API::Decorators::Single - self_link :version + self_link linked_property :definingProject, path: :project, diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 700eaf84c1..34a1f2dd6f 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -35,7 +35,7 @@ module API module WorkPackages class WorkPackageRepresenter < ::API::Decorators::Single - self_link :work_package, title_getter: -> (*) { represented.subject } + self_link title_getter: -> (*) { represented.subject } link :update do { From 2f012a09dd16473896157bf010fd417d0bf34a89 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 12 Feb 2015 13:44:29 +0100 Subject: [PATCH 57/59] use show_if as guard clause --- lib/api/decorators/single.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index 853b31dc6e..87a607f90a 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -67,11 +67,13 @@ module API title_getter: -> (*) { represented.send(backing_field).name }, show_if: -> (*) { true }) link property do + next unless instance_eval(&show_if) + value = represented.send(backing_field) link_object = { href: (api_v3_paths.send(path, value.id) if value) } link_object[:title] = instance_eval(&title_getter) if value - link_object if instance_eval(&show_if) + link_object end end From 7b72bcdc5c5b8913d5472aef493b36d10f535a1d Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 12 Feb 2015 13:44:59 +0100 Subject: [PATCH 58/59] rename parameter --- lib/api/decorators/single.rb | 6 +++--- lib/api/v3/versions/version_representer.rb | 2 +- lib/api/v3/work_packages/work_package_representer.rb | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb index 87a607f90a..d59132bd1a 100644 --- a/lib/api/decorators/single.rb +++ b/lib/api/decorators/single.rb @@ -63,13 +63,13 @@ module API def self.linked_property(property, path: property, - backing_field: property, - title_getter: -> (*) { represented.send(backing_field).name }, + association: property, + title_getter: -> (*) { represented.send(association).name }, show_if: -> (*) { true }) link property do next unless instance_eval(&show_if) - value = represented.send(backing_field) + value = represented.send(association) link_object = { href: (api_v3_paths.send(path, value.id) if value) } link_object[:title] = instance_eval(&title_getter) if value diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index f6d343f7a6..6b50faadc9 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -39,7 +39,7 @@ module API linked_property :definingProject, path: :project, - backing_field: :project, + association: :project, show_if: -> (*) { represented.project.visible?(current_user) } link :availableInProjects do diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 34a1f2dd6f..b9ffc06cb3 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -89,7 +89,7 @@ module API linked_property :author, path: :user linked_property :responsible, path: :user - linked_property :assignee, path: :user, backing_field: :assigned_to + linked_property :assignee, path: :user, association: :assigned_to link :availableWatchers do { @@ -176,7 +176,7 @@ module API linked_property :category linked_property :version, - backing_field: :fixed_version, + association: :fixed_version, title_getter: -> (*) { represented.fixed_version.to_s_for_project(represented.project) } From 497f084e5d87f6f7194445435036349bfe6966e3 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 12 Feb 2015 15:43:02 +0100 Subject: [PATCH 59/59] fix flickering repository tests --- lib/redmine/access_control.rb | 5 +- .../plugins/module_handler_spec.rb | 23 ++------ spec/lib/redmine/access_control_spec.rb | 58 +++++++++---------- 3 files changed, 36 insertions(+), 50 deletions(-) diff --git a/lib/redmine/access_control.rb b/lib/redmine/access_control.rb index 6c1af2173f..cca393a2ad 100644 --- a/lib/redmine/access_control.rb +++ b/lib/redmine/access_control.rb @@ -82,13 +82,12 @@ module Redmine module_permissions = permissions.select { |p| p.project_module.to_s == module_name.to_s } - reset + clear_caches @permissions = permissions - module_permissions end - def reset - @permissions = nil + def clear_caches @available_project_modules = nil @public_permissions = nil @members_only_permissions = nil diff --git a/spec/lib/open_project/plugins/module_handler_spec.rb b/spec/lib/open_project/plugins/module_handler_spec.rb index 52c5957c6c..edb8a3d125 100644 --- a/spec/lib/open_project/plugins/module_handler_spec.rb +++ b/spec/lib/open_project/plugins/module_handler_spec.rb @@ -28,28 +28,17 @@ require 'spec_helper' describe OpenProject::Plugins::ModuleHandler do - before(:all) do - module_permissions = Redmine::AccessControl.modules_permissions(['repository']) - @repository_permissions = module_permissions.select do |permission| - permission.project_module == :repository - end + let!(:all_former_permissions) { Redmine::AccessControl.permissions } + before do disabled_modules = OpenProject::Plugins::ModuleHandler.disable_modules('repository') OpenProject::Plugins::ModuleHandler.disable(disabled_modules) end - after(:all) do - Redmine::AccessControl.map do |mapper| - mapper.project_module :repository do |map| - @repository_permissions.map do |permission| - options = { project_module: permission.project_module, - public: permission.public?, - require: permission.require_loggedin? } - - map.permission(permission.name, permission.actions, options) - end - end - end + after do + raise 'Test outdated' unless Redmine::AccessControl.instance_variable_defined?(:@permissions) + Redmine::AccessControl.instance_variable_set(:@permissions, all_former_permissions) + Redmine::AccessControl.clear_caches end context '#disable' do diff --git a/spec/lib/redmine/access_control_spec.rb b/spec/lib/redmine/access_control_spec.rb index 096f3589c3..4168170653 100644 --- a/spec/lib/redmine/access_control_spec.rb +++ b/spec/lib/redmine/access_control_spec.rb @@ -28,48 +28,46 @@ require 'spec_helper' describe Redmine::AccessControl do - let(:global_permissions) { Redmine::AccessControl.permissions } - let(:public_permissions) { Redmine::AccessControl.public_permissions } - let(:members_only_permissions) { Redmine::AccessControl.members_only_permissions } - let(:loggedin_only_permissions) { Redmine::AccessControl.loggedin_only_permissions } + describe '.remove_modules_permissions' do + let!(:all_former_permissions) { Redmine::AccessControl.permissions } + let!(:former_repository_permissions) do + module_permissions = Redmine::AccessControl.modules_permissions(['repository']) - after(:all) do - Redmine::AccessControl.map do |mapper| - mapper.project_module :repository do |map| - @repository_permissions.map do |permission| - options = { project_module: permission.project_module, - public: permission.public?, - require: permission.require_loggedin? } - - map.permission(permission.name, permission.actions, options) - end + module_permissions.select do |permission| + permission.project_module == :repository end end - end - before(:all) do - module_permissions = Redmine::AccessControl.modules_permissions(['repository']) - @repository_permissions = module_permissions.select do |permission| - permission.project_module == :repository + subject { Redmine::AccessControl } + + before do + Redmine::AccessControl.remove_modules_permissions(:repository) + end + + after do + raise 'Test outdated' unless Redmine::AccessControl.instance_variable_defined?(:@permissions) + Redmine::AccessControl.instance_variable_set(:@permissions, all_former_permissions) + Redmine::AccessControl.clear_caches + end + + it 'removes from global permissions' do + expect(subject.permissions).to_not include(former_repository_permissions) end - Redmine::AccessControl.remove_modules_permissions(:repository) - end - describe 'remove module permissions' do - context 'remove from global permissions' do - it { expect(global_permissions).to_not include(@repository_permissions) } + it 'removes from public permissions' do + expect(subject.public_permissions).to_not include(former_repository_permissions) end - context 'remove from public permissions' do - it { expect(public_permissions).to_not include(@repository_permissions) } + it 'removes from members only permissions' do + expect(subject.members_only_permissions).to_not include(former_repository_permissions) end - context 'remove from members only permissions' do - it { expect(members_only_permissions).to_not include(@repository_permissions) } + it 'removes from loggedin only permissions' do + expect(subject.loggedin_only_permissions).to_not include(former_repository_permissions) end - context 'remove from loggedin only permissions' do - it { expect(loggedin_only_permissions).to_not include(@repository_permissions) } + it 'should disable repository module' do + expect(subject.available_project_modules).to_not include(:repository) end end end