From fdf09318cf282aaa18c4acad8426938b12f17acd Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Wed, 5 Nov 2014 15:11:46 +0100 Subject: [PATCH 01/37] Test form representer --- .../form/form_representer_spec.rb | 103 ++++++++++++++++++ .../work_package_form_representer_spec.rb | 61 +++++++++++ .../work_package_schema_representer_spec.rb | 81 ++++++++++++++ 3 files changed, 245 insertions(+) create mode 100644 spec/lib/api/v3/work_packages/form/form_representer_spec.rb create mode 100644 spec/lib/api/v3/work_packages/form/work_package_form_representer_spec.rb create mode 100644 spec/lib/api/v3/work_packages/form/work_package_schema_representer_spec.rb diff --git a/spec/lib/api/v3/work_packages/form/form_representer_spec.rb b/spec/lib/api/v3/work_packages/form/form_representer_spec.rb new file mode 100644 index 0000000000..e5d4c1ece3 --- /dev/null +++ b/spec/lib/api/v3/work_packages/form/form_representer_spec.rb @@ -0,0 +1,103 @@ +#-- 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. +#++ + +require 'spec_helper' + +describe ::API::V3::WorkPackages::Form::FormRepresenter do + let(:work_package) { FactoryGirl.build(:work_package) } + let(:current_user) { + FactoryGirl.build(:user, member_in_project: work_package.project) + } + let(:representer) { described_class.new(work_package, current_user: current_user) } + + context 'generation' do + subject(:generated) { representer.to_json } + + it { is_expected.to be_json_eql('Form'.to_json).at_path('_type') } + + describe '_links' do + it { is_expected.to have_json_path('_links') } + + it { is_expected.to have_json_path('_links/self/href') } + + describe 'validate' do + it { is_expected.to have_json_path('_links/validate/href') } + + it { is_expected.to be_json_eql(:post.to_json).at_path('_links/validate/method') } + end + + describe 'preview markup' do + it { is_expected.to have_json_path('_links/previewMarkup/href') } + + it { is_expected.to be_json_eql(:post.to_json).at_path('_links/previewMarkup/method') } + end + + describe 'commit' do + context 'valid work package' do + before { allow(work_package).to receive(:valid?).and_return(true) } + + it { is_expected.to have_json_path('_links/commit/href') } + + it { is_expected.to be_json_eql(:patch.to_json).at_path('_links/commit/method') } + end + + context 'invalid work package' do + before { allow(work_package).to receive(:valid?).and_return(false) } + + it { is_expected.not_to have_json_path('_links/commit/href') } + end + end + end + + describe 'validation errors' do + context 'w/o errors' do + it { is_expected.to be_json_eql({}.to_json).at_path('_embedded/validationErrors') } + end + + context 'with errors' do + let(:subject_error_message) { 'Subject can\'t be blank!' } + let(:status_error_message) { 'Status can\'t be blank!' } + let(:subject_error) { ::API::Errors::Validation.new(subject_error_message) } + let(:status_error) { ::API::Errors::Validation.new(status_error_message) } + let(:api_subject_error) { ::API::V3::Errors::ErrorRepresenter.new(subject_error) } + let(:api_status_error) { ::API::V3::Errors::ErrorRepresenter.new(status_error) } + let(:errors) { { subject: api_subject_error, status: api_status_error } } + + before do + allow(work_package.errors).to receive(:keys).and_return(errors.keys) + allow(work_package.errors).to receive(:full_message).with(:subject, []) + .and_return(subject_error_message) + allow(work_package.errors).to receive(:full_message).with(:status, []) + .and_return(status_error_message) + end + + it { is_expected.to be_json_eql(errors.to_json).at_path('_embedded/validationErrors') } + end + end + end +end diff --git a/spec/lib/api/v3/work_packages/form/work_package_form_representer_spec.rb b/spec/lib/api/v3/work_packages/form/work_package_form_representer_spec.rb new file mode 100644 index 0000000000..f34cb26ff3 --- /dev/null +++ b/spec/lib/api/v3/work_packages/form/work_package_form_representer_spec.rb @@ -0,0 +1,61 @@ +#-- 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. +#++ + +require 'spec_helper' + +describe ::API::V3::WorkPackages::Form::WorkPackageFormRepresenter do + let(:work_package) { FactoryGirl.build(:work_package) } + let(:representer) { described_class.new(work_package) } + + context 'generation' do + subject(:generated) { representer.to_json } + + it { is_expected.to include_json('WorkPackage'.to_json).at_path('_type') } + + describe 'work_package' do + it { is_expected.to have_json_path('subject') } + it { is_expected.to have_json_path('rawDescription') } + + describe 'lock version' do + it { is_expected.to have_json_path('lockVersion') } + + it { is_expected.to have_json_type(Integer).at_path('lockVersion') } + + it { is_expected.to be_json_eql(work_package.lock_version.to_json).at_path('lockVersion') } + end + end + + describe '_links' do + it { is_expected.to have_json_type(Object).at_path('_links') } + + it 'should link status' do + expect(subject).to have_json_path('_links/status/href') + end + end + end +end diff --git a/spec/lib/api/v3/work_packages/form/work_package_schema_representer_spec.rb b/spec/lib/api/v3/work_packages/form/work_package_schema_representer_spec.rb new file mode 100644 index 0000000000..0217a9b92f --- /dev/null +++ b/spec/lib/api/v3/work_packages/form/work_package_schema_representer_spec.rb @@ -0,0 +1,81 @@ +#-- 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. +#++ + +require 'spec_helper' + +describe ::API::V3::WorkPackages::Form::WorkPackageSchemaRepresenter do + let(:work_package) { FactoryGirl.build(:work_package) } + let(:current_user) { + FactoryGirl.build(:user, member_in_project: work_package.project) + } + let(:representer) { described_class.new(work_package, current_user: current_user) } + + context 'generation' do + subject(:generated) { representer.to_json } + + describe 'schema' do + shared_examples_for 'schema property' do |path, type, required, writable| + it { is_expected.to have_json_path(path) } + + it { is_expected.to be_json_eql(type.to_json).at_path("#{path}/type") } + + it 'has valid required value' do + required_path = "#{path}/required" + + if required.nil? + is_expected.not_to have_json_path(required_path) + else + is_expected.to be_json_eql(required.to_json).at_path(required_path) + end + end + + it 'has valid writable value' do + writable_path = "#{path}/writable" + + if writable.nil? + is_expected.not_to have_json_path(writable_path) + else + is_expected.to be_json_eql(writable.to_json).at_path(writable_path) + end + end + end + + describe '_type' do + it_behaves_like 'schema property', '_type', 'MetaType', true, false + end + + describe 'lock version' do + it_behaves_like 'schema property', 'lockVersion', 'Integer', true, false + end + + describe 'subject' do + it_behaves_like 'schema property', 'subject', 'String' + end + end + end +end From f9784bab4cf104946a67b1cb5518e6fe6c703647 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Wed, 5 Nov 2014 15:55:38 +0100 Subject: [PATCH 02/37] Test WP schema's status property --- .../work_package_schema_representer_spec.rb | 45 +++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/spec/lib/api/v3/work_packages/form/work_package_schema_representer_spec.rb b/spec/lib/api/v3/work_packages/form/work_package_schema_representer_spec.rb index 0217a9b92f..79dc89bab4 100644 --- a/spec/lib/api/v3/work_packages/form/work_package_schema_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/form/work_package_schema_representer_spec.rb @@ -76,6 +76,51 @@ describe ::API::V3::WorkPackages::Form::WorkPackageSchemaRepresenter do describe 'subject' do it_behaves_like 'schema property', 'subject', 'String' end + + describe 'status' do + shared_examples_for 'contains statuses' do + it { is_expected.to have_json_path('status') } + + it { is_expected.to have_json_path('status/_links') } + + it { is_expected.to have_json_path('status/_links/allowedValues') } + + it 'contains valid links to statuses' do + status_links = statuses.map do |status| + { href: "/api/v3/statuses/#{status.id}", title: status.name } + end + + is_expected.to be_json_eql(status_links.to_json).at_path('status/_links/allowedValues') + end + + it { is_expected.to be_json_eql('Status'.to_json).at_path('status/type') } + + it 'embeds statuses' do + embedded_statuses = statuses.map do |status| + { _type: 'Status', id: status.id, name: status.name } + end + + is_expected.to be_json_eql(embedded_statuses.to_json) + .at_path('status/_embedded/allowedValues') + end + end + + context 'w/o allowed statuses' do + before { allow(work_package).to receive(:new_statuses_allowed_to).and_return([]) } + + it_behaves_like 'contains statuses' do + let(:statuses) { [] } + end + end + + context 'with allowed statuses' do + let(:statuses) { FactoryGirl.build_list(:status, 3) } + + before { allow(work_package).to receive(:new_statuses_allowed_to).and_return(statuses) } + + it_behaves_like 'contains statuses' + end + end end end end From e33c6570939fe835da0b67e71dca1e6aa852d98d Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Wed, 5 Nov 2014 14:20:34 +0100 Subject: [PATCH 03/37] Implement WP form representation --- .../v3/work_packages/form/form_representer.rb | 109 ++++++++++++++++++ .../form/work_package_form_representer.rb | 66 +++++++++++ .../form/work_package_schema_representer.rb | 63 ++++++++++ 3 files changed, 238 insertions(+) create mode 100644 lib/api/v3/work_packages/form/form_representer.rb create mode 100644 lib/api/v3/work_packages/form/work_package_form_representer.rb create mode 100644 lib/api/v3/work_packages/form/work_package_schema_representer.rb diff --git a/lib/api/v3/work_packages/form/form_representer.rb b/lib/api/v3/work_packages/form/form_representer.rb new file mode 100644 index 0000000000..cdc4fb24f7 --- /dev/null +++ b/lib/api/v3/work_packages/form/form_representer.rb @@ -0,0 +1,109 @@ +#-- 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. +#++ + +require 'roar/decorator' +require 'roar/json/hal' + +module API + module V3 + module WorkPackages + module Form + class FormRepresenter < Roar::Decorator + include Roar::JSON::HAL + include Roar::Hypermedia + include API::Utilities::UrlHelper + include OpenProject::TextFormatting + + self.as_strategy = ::API::Utilities::CamelCasingStrategy.new + + def initialize(model, options = {}) + @current_user = options[:current_user] + + super(model) + end + + property :_type, exec_context: :decorator, writeable: false + + link :self do + { + href: "#{root_path}api/v3/work_packages/#{represented.id}/form", + } + end + + link :validate do + { + href: "#{root_path}api/v3/work_packages/#{represented.id}/form", + method: :post + } + end + + link :previewMarkup do + { + href: "#{root_path}api/v3/render/textile", + method: :post + } + end + + link :commit do + { + href: "#{root_path}api/v3/work_packages/#{represented.id}", + method: :patch + } if represented.valid? + end + + property :payload, + embedded: true, + decorator: Form::WorkPackageFormRepresenter, + getter: -> (*) { self } + property :schema, + embedded: true, + exec_context: :decorator, + getter: -> (*) { + Form::WorkPackageSchemaRepresenter.new(represented, + current_user: @current_user) + } + property :validation_errors, embedded: true, exec_context: :decorator + + def _type + 'Form' + end + + def validation_errors + errors = represented.errors + + errors.keys.each_with_object({}) do |key, hash| + error = ::API::Errors::Validation.new(errors.full_message(key, errors[key])) + hash[key] = ::API::V3::Errors::ErrorRepresenter.new(error) + end + end + end + end + end + end +end diff --git a/lib/api/v3/work_packages/form/work_package_form_representer.rb b/lib/api/v3/work_packages/form/work_package_form_representer.rb new file mode 100644 index 0000000000..1e59dd34be --- /dev/null +++ b/lib/api/v3/work_packages/form/work_package_form_representer.rb @@ -0,0 +1,66 @@ +#-- 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. +#++ + +require 'roar/decorator' +require 'roar/json/hal' + +module API + module V3 + module WorkPackages + module Form + class WorkPackageFormRepresenter < Roar::Decorator + include Roar::JSON::HAL + include Roar::Hypermedia + include API::Utilities::UrlHelper + + self.as_strategy = ::API::Utilities::CamelCasingStrategy.new + + property :_type, exec_context: :decorator, writeable: false + + link :status do + { + href: "#{root_path}api/v3/statuses/#{represented.status.id}" + } if represented.status + end + + property :lock_version + property :subject, render_nil: true + property :raw_description, + getter: -> (*) { description }, + setter: -> (value, *) { self.description = value }, + render_nil: true + + def _type + 'WorkPackage' + end + end + end + end + end +end diff --git a/lib/api/v3/work_packages/form/work_package_schema_representer.rb b/lib/api/v3/work_packages/form/work_package_schema_representer.rb new file mode 100644 index 0000000000..e85bab13d5 --- /dev/null +++ b/lib/api/v3/work_packages/form/work_package_schema_representer.rb @@ -0,0 +1,63 @@ +#-- 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. +#++ + +require 'roar/decorator' +require 'roar/json/hal' + +module API + module V3 + module WorkPackages + module Form + class WorkPackageSchemaRepresenter < Roar::Decorator + include Roar::JSON::HAL + include Roar::Hypermedia + include API::Utilities::UrlHelper + + self.as_strategy = ::API::Utilities::CamelCasingStrategy.new + + def initialize(model, options = {}) + @current_user = options[:current_user] + + super(model) + end + + property :_type, + getter: -> (*) { { type: 'MetaType', required: true, writable: false } }, + writeable: false + property :lock_version, + getter: -> (*) { { type: 'Integer', required: true, writable: false } }, + writeable: false + property :subject, + getter: -> (*) { { type: 'String' } }, + writeable: false + end + end + end + end +end From a3ec3228dc953875e4c0c5d09f2b9af5e33982ae Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Wed, 5 Nov 2014 14:21:16 +0100 Subject: [PATCH 04/37] Add status to property --- .../form/work_package_schema_representer.rb | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/lib/api/v3/work_packages/form/work_package_schema_representer.rb b/lib/api/v3/work_packages/form/work_package_schema_representer.rb index e85bab13d5..3fc75a0562 100644 --- a/lib/api/v3/work_packages/form/work_package_schema_representer.rb +++ b/lib/api/v3/work_packages/form/work_package_schema_representer.rb @@ -56,6 +56,37 @@ module API property :subject, getter: -> (*) { { type: 'String' } }, writeable: false + property :status, + exec_context: :decorator, + getter: -> (*) { represented.new_statuses_allowed_to(@current_user) } do + include Roar::JSON::HAL + + self.as_strategy = ::API::Utilities::CamelCasingStrategy.new + + property :links_to_allowed_statuses, + as: :_links, + getter: -> (*) { self } do + include API::Utilities::UrlHelper + + self.as_strategy = ::API::Utilities::CamelCasingStrategy.new + + property :allowed_values, exec_context: :decorator + + def allowed_values + represented.map do |status| + { href: "#{root_path}api/v3/statuses/#{status.id}", title: status.name } + end + end + end + + property :type, getter: -> (*) { 'Status' } + + collection :allowed_values, + embedded: true, + class: ::Status, + decorator: ::API::V3::Statuses::StatusRepresenter, + getter: -> (*) { self } + end end end end From 076557f73a5f5ad8279ace05b5ba48fc3bee6948 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Wed, 5 Nov 2014 14:21:58 +0100 Subject: [PATCH 05/37] Add API V3 form endpoint --- lib/api/v3/work_packages/form/form_api.rb | 41 +++++++++++++++++++ lib/api/v3/work_packages/work_packages_api.rb | 1 + 2 files changed, 42 insertions(+) create mode 100644 lib/api/v3/work_packages/form/form_api.rb diff --git a/lib/api/v3/work_packages/form/form_api.rb b/lib/api/v3/work_packages/form/form_api.rb new file mode 100644 index 0000000000..9a7cef6709 --- /dev/null +++ b/lib/api/v3/work_packages/form/form_api.rb @@ -0,0 +1,41 @@ +#-- 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. +#++ + +module API + module V3 + module WorkPackages + module Form + class FormAPI < Grape::API + post '/form' do + FormRepresenter.new(@work_package, current_user: current_user) + end + end + end + end + 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 c21d40e87f..827cb8555e 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -149,6 +149,7 @@ module API mount ::API::V3::WorkPackages::WatchersAPI mount ::API::V3::WorkPackages::StatusesAPI mount ::API::V3::Relations::RelationsAPI + mount ::API::V3::WorkPackages::Form::FormAPI end From cbc680fac5758c78b01ca4645d9dfc15407fe224 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Wed, 5 Nov 2014 15:12:00 +0100 Subject: [PATCH 06/37] Correct describe block description --- spec/lib/api/v3/work_packages/work_package_representer_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 aad4ef1817..0981b65e88 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 @@ -304,7 +304,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do end end - describe 'delete' do + describe 'log_time' do it_behaves_like 'action link' do let(:action) { 'log_time' } let(:permission) { :log_time } From b72d0e22022024644262258eb76a11cc5f108f58 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Thu, 6 Nov 2014 12:49:50 +0100 Subject: [PATCH 07/37] Test API V3 WP forms endpoint --- .../form/work_package_form_resource_spec.rb | 243 ++++++++++++++++++ 1 file changed, 243 insertions(+) create mode 100644 spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb diff --git a/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb b/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb new file mode 100644 index 0000000000..a98304a9d0 --- /dev/null +++ b/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb @@ -0,0 +1,243 @@ +#-- 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. +#++ + +require 'spec_helper' +require 'rack/test' + +describe 'API v3 Work package form resource', type: :request do + include Rack::Test::Methods + include Capybara::RSpecMatchers + + let(:project) { FactoryGirl.create(:project, is_public: false) } + let(:work_package) { FactoryGirl.create(:work_package, project: project) } + let(:authorized_user) { FactoryGirl.create(:user, member_in_project: project) } + let(:unauthorized_user) { FactoryGirl.create(:user) } + + describe '#post' do + let(:post_path) { "/api/v3/work_packages/#{work_package.id}/form" } + let(:valid_params) do + { + _type: 'WorkPackage', + lockVersion: work_package.lock_version + } + end + + subject(:response) { last_response } + + shared_context 'post request' do + before(:each) do + allow(User).to receive(:current).and_return current_user + post post_path, params.to_json, 'CONTENT_TYPE' => 'application/json' + end + end + + context 'user without needed permissions' do + let(:work_package) { FactoryGirl.create(:work_package, id: 42, project: project) } + let(:params) { {} } + + include_context 'post request' do + let(:current_user) { unauthorized_user } + end + + it_behaves_like 'not found', 42, 'WorkPackage' + end + + context 'user with needed permissions' do + let(:params) {} + let(:current_user) { authorized_user } + + context 'non-existing work package' do + let(:post_path) { '/api/v3/work_packages/eeek/form' } + + include_context 'post request' + + it_behaves_like 'not found', 'eeek', 'WorkPackage' + end + + context 'existing work package' do + shared_examples_for 'valid payload' do + it { expect(response.status).to eq(200) } + + it { expect(subject.body).to have_json_path('_embedded/payload') } + + it { expect(subject.body).to have_json_path('_embedded/payload/lockVersion') } + + it { expect(subject.body).to have_json_path('_embedded/payload/subject') } + + it { expect(subject.body).to have_json_path('_embedded/payload/rawDescription') } + end + + shared_examples_for 'valid payload with initial values' do + it { + expect(subject.body).to be_json_eql(work_package.lock_version.to_json) + .at_path('_embedded/payload/lockVersion') + } + + it { + expect(subject.body).to be_json_eql(work_package.subject.to_json) + .at_path('_embedded/payload/subject') + } + + it { + expect(subject.body).to be_json_eql(work_package.description.to_json) + .at_path('_embedded/payload/rawDescription') + } + end + + shared_examples_for 'having no errors' do + it { + expect(subject.body).to be_json_eql({}.to_json) + .at_path('_embedded/validationErrors') + } + end + + shared_examples_for 'having an error' do |property| + it { expect(subject.body).to have_json_path("_embedded/validationErrors/#{property}") } + + describe 'error body' do + let(:error_id) { 'urn:openproject-org:api:v3:errors:PropertyConstraintViolation' } + + let(:error_body) { + parse_json(subject.body)['_embedded']['validationErrors'][property] + } + + it { expect(error_body['errorIdentifier']).to eq(error_id) } + end + end + + describe 'body' do + context 'empty' do + include_context 'post request' + + it_behaves_like 'valid payload' + + it_behaves_like 'valid payload with initial values' + + it_behaves_like 'having no errors' + end + + context 'filled' do + let(:valid_params) do + { + _type: 'WorkPackage', + lockVersion: work_package.lock_version + } + end + + describe 'no change' do + let(:params) { valid_params } + + include_context 'post request' + + it_behaves_like 'valid payload' + + it_behaves_like 'valid payload with initial values' + + it_behaves_like 'having no errors' + end + + describe 'lock version' do + context 'missing lock version' do + let(:params) { valid_params.except(:lockVersion) } + + include_context 'post request' + + it_behaves_like 'update conflict' + end + + context 'stale object' do + let(:params) { valid_params.merge(subject: 'Updated subject') } + + before do + params + + work_package.subject = 'I am the first!' + work_package.save! + + expect(valid_params[:lockVersion]).not_to eq(work_package.lock_version) + end + + include_context 'post request' + + it { expect(response.status).to eq(409) } + + it_behaves_like 'update conflict' + end + end + + describe 'subject' do + include_context 'post request' + + context 'valid subject' do + let(:params) { valid_params.merge(subject: 'Updated subject') } + + it_behaves_like 'valid payload' + + it_behaves_like 'having no errors' + + it 'should respond with updated work package subject' do + expect(subject.body).to be_json_eql('Updated subject'.to_json) + .at_path('_embedded/payload/subject') + end + end + + context 'invalid subject' do + let(:params) { valid_params.merge(subject: nil) } + + it_behaves_like 'valid payload' + + it_behaves_like 'having an error', 'subject' + + it 'should respond with updated work package subject' do + expect(subject.body).to be_json_eql(nil.to_json) + .at_path('_embedded/payload/subject') + end + end + end + + describe 'description' do + let(:path) { '_embedded/payload/rawDescription' } + let(:description) { '*Some text* _describing_ *something*...' } + let(:params) { valid_params.merge(rawDescription: description) } + + include_context 'post request' + + it_behaves_like 'valid payload' + + it_behaves_like 'having no errors' + + it 'should respond with updated work package description' do + expect(subject.body).to be_json_eql(description.to_json).at_path(path) + end + end + end + end + end + end + end +end From 0c0cdb4f1ebab24b196414e581c599e2a2ca5169 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Thu, 6 Nov 2014 12:54:33 +0100 Subject: [PATCH 08/37] Process non-empty form requests --- lib/api/errors/error_base.rb | 4 ++- lib/api/v3/work_packages/form/form_api.rb | 27 ++++++++++++++++++- .../v3/work_packages/work_package_contract.rb | 14 +++++++--- 3 files changed, 40 insertions(+), 5 deletions(-) diff --git a/lib/api/errors/error_base.rb b/lib/api/errors/error_base.rb index 88d7f65565..1b11b48efa 100644 --- a/lib/api/errors/error_base.rb +++ b/lib/api/errors/error_base.rb @@ -33,9 +33,11 @@ module API attr_reader :code, :message, :details, :errors def self.create(errors) - [:error_unauthorized, :error_conflict, :error_readonly].each do |key| + [:error_not_found, :error_unauthorized, :error_conflict, :error_readonly].each do |key| if errors.has_key?(key) case key + when :error_not_found + return ::API::Errors::NotFound.new(errors[key].join(' ')) when :error_unauthorized return ::API::Errors::Unauthorized when :error_conflict diff --git a/lib/api/v3/work_packages/form/form_api.rb b/lib/api/v3/work_packages/form/form_api.rb index 9a7cef6709..2b88a3aa97 100644 --- a/lib/api/v3/work_packages/form/form_api.rb +++ b/lib/api/v3/work_packages/form/form_api.rb @@ -31,8 +31,33 @@ module API module WorkPackages module Form class FormAPI < Grape::API + helpers do + def process_form_request + if form_post_request_body + # enforces availibility validation of lock_version + @representer.represented.lock_version = nil + @representer.from_json(patch_request_body) + + patch_request_valid? + end + + error = ::API::Errors::ErrorBase.create(@representer.represented.errors) + + if error.is_a? ::API::Errors::Validation + status 200 + FormRepresenter.new(@representer.represented, current_user: current_user) + else + fail error + end + end + + def form_post_request_body + env['api.request.body'] + end + end + post '/form' do - FormRepresenter.new(@work_package, current_user: current_user) + process_form_request end end end diff --git a/lib/api/v3/work_packages/work_package_contract.rb b/lib/api/v3/work_packages/work_package_contract.rb index 6189d8115d..37392f7a4d 100644 --- a/lib/api/v3/work_packages/work_package_contract.rb +++ b/lib/api/v3/work_packages/work_package_contract.rb @@ -43,9 +43,10 @@ module API @can = WorkPackagePolicy.new(user) end + validate :user_allowed_to_access validate :user_allowed_to_edit validate :user_allowed_to_edit_parent - validate :lock_version_set + validate :lock_version_valid validate :readonly_attributes_unchanged extend Reform::Form::ActiveModel::ModelValidations @@ -53,6 +54,13 @@ module API private + def user_allowed_to_access + unless ::WorkPackage.visible(@user).exists?(model) + message = "Couldn't find WorkPackage with id=#{model.id}" + errors.add :error_not_found, message + end + end + def user_allowed_to_edit errors.add :error_unauthorized, '' unless @can.allowed?(model, :edit) end @@ -63,8 +71,8 @@ module API end end - def lock_version_set - errors.add :error_conflict, '' if model.lock_version.nil? + def lock_version_valid + errors.add :error_conflict, '' if model.lock_version.nil? || model.lock_version_changed? end def readonly_attributes_unchanged From 0e45e9257f7449a908d2aa737d98e9778a558f22 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Thu, 6 Nov 2014 13:10:41 +0100 Subject: [PATCH 09/37] Fix context description --- spec/requests/api/v3/work_package_resource_spec.rb | 2 +- 1 file changed, 1 insertion(+), 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 5cb53d981d..9f555030f9 100644 --- a/spec/requests/api/v3/work_package_resource_spec.rb +++ b/spec/requests/api/v3/work_package_resource_spec.rb @@ -469,7 +469,7 @@ h4. things we like it_behaves_like 'update conflict' end - context 'state object' do + context 'stale object' do let(:params) { valid_params.merge(subject: 'Updated subject') } before do From af8a81ded8cbeea90c6682468e9d56d1c0c0f405 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Thu, 6 Nov 2014 14:51:37 +0100 Subject: [PATCH 10/37] Extract resource link parsing --- lib/api/v3/render/render_api.rb | 27 +++------- lib/api/v3/utilities/resource_link_parser.rb | 56 ++++++++++++++++++++ 2 files changed, 64 insertions(+), 19 deletions(-) create mode 100644 lib/api/v3/utilities/resource_link_parser.rb diff --git a/lib/api/v3/render/render_api.rb b/lib/api/v3/render/render_api.rb index 89c6f2cccc..abfd5be0e6 100644 --- a/lib/api/v3/render/render_api.rb +++ b/lib/api/v3/render/render_api.rb @@ -45,11 +45,11 @@ module API def context_object if params[:context] context_object = nil - namespace, id = parse_context + context = parse_context - case namespace + case context[:ns] when 'work_packages' - context_object = WorkPackage.visible(current_user).find_by_id(id) + context_object = WorkPackage.visible(current_user).find_by_id(context[:id]) end unless context_object @@ -59,27 +59,16 @@ module API end def parse_context - contexts = API::V3::Root.routes.map do |route| - route_options = route.instance_variable_get(:@options) - match = route_options[:compiled].match(params[:context]) + resourceLinkParser = ::API::V3::Utilities::ResourceLinkParser.new + context = resourceLinkParser.parse(params[:context]) - if match - { - ns: /\/(?\w+)\//.match(route_options[:namespace])[:ns], - id: match[:id] - } - end - end - - contexts.compact!.uniq! { |c| c[:ns] } - - fail API::Errors::InvalidRenderContext.new('No context found.') if contexts.empty? + fail API::Errors::InvalidRenderContext.new('No context found.') if context.nil? - unless SUPPORTED_CONTEXT_NAMESPACES.include? contexts[0][:ns] + unless SUPPORTED_CONTEXT_NAMESPACES.include? context[:ns] fail API::Errors::InvalidRenderContext.new('Unsupported context found.') end - [contexts[0][:ns], contexts[0][:id]] + context end def render(type) diff --git a/lib/api/v3/utilities/resource_link_parser.rb b/lib/api/v3/utilities/resource_link_parser.rb new file mode 100644 index 0000000000..561ba225d8 --- /dev/null +++ b/lib/api/v3/utilities/resource_link_parser.rb @@ -0,0 +1,56 @@ +#-- 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. +#++ + +module API + module V3 + module Utilities + class ResourceLinkParser + def parse(resource_link) + matching_resources = API::V3::Root.routes.map do |route| + route_options = route.instance_variable_get(:@options) + match = route_options[:compiled].match(resource_link) + + if match + { + ns: /\/(?\w+)\//.match(route_options[:namespace])[:ns], + id: match[:id] + } + end + end + + matching_resources.compact!.uniq! { |c| c[:ns] } + + unless matching_resources.empty? + { ns: matching_resources[0][:ns], id: matching_resources[0][:id] } + end + end + end + end + end +end From 487970944d6fc6eb6a454a28417547eeb46631f6 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Thu, 6 Nov 2014 14:51:52 +0100 Subject: [PATCH 11/37] Remove superfluous dependency --- lib/api/v3/render/render_api.rb | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/api/v3/render/render_api.rb b/lib/api/v3/render/render_api.rb index abfd5be0e6..80c40e0132 100644 --- a/lib/api/v3/render/render_api.rb +++ b/lib/api/v3/render/render_api.rb @@ -31,7 +31,6 @@ module API module V3 module Render class RenderAPI < Grape::API - include OpenProject::TextFormatting format :txt resources :render do @@ -59,8 +58,8 @@ module API end def parse_context - resourceLinkParser = ::API::V3::Utilities::ResourceLinkParser.new - context = resourceLinkParser.parse(params[:context]) + resource_link_parser = ::API::V3::Utilities::ResourceLinkParser.new + context = resource_link_parser.parse(params[:context]) fail API::Errors::InvalidRenderContext.new('No context found.') if context.nil? From 08fb322024963b3f4eb79fcc496ac720c4ad6102 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Thu, 6 Nov 2014 15:47:09 +0100 Subject: [PATCH 12/37] Test WP form's status handling --- .../form/work_package_form_resource_spec.rb | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb b/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb index a98304a9d0..c4da15b397 100644 --- a/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb +++ b/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb @@ -235,6 +235,46 @@ describe 'API v3 Work package form resource', type: :request do expect(subject.body).to be_json_eql(description.to_json).at_path(path) end end + + describe 'status' do + let(:path) { '_embedded/payload/_links/status/href' } + let(:target_status) { FactoryGirl.create(:status) } + let(:status_link) { "/api/v3/statuses/#{target_status.id}" } + let(:status_parameter) { { _links: { status: status_link } } } + let(:params) { valid_params.merge(status_parameter) } + + context 'valid status' do + let!(:workflow) { + FactoryGirl.create(:workflow, + type_id: work_package.type.id, + old_status: work_package.status, + new_status: target_status, + role: current_user.memberships[0].roles[0]) + } + + include_context 'post request' + + it_behaves_like 'valid payload' + + it_behaves_like 'having no errors' + + it 'should respond with updated work package status' do + expect(subject.body).to be_json_eql(status_link.to_json).at_path(path) + end + end + + context 'invalid status' do + include_context 'post request' + + it_behaves_like 'valid payload' + + it_behaves_like 'having an error', 'status_id' + + it 'should respond with updated work package status' do + expect(subject.body).to be_json_eql(status_link.to_json).at_path(path) + end + end + end end end end From 2e2d51fd32a79e582cf8ec66810968bc2c5d4653 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Thu, 6 Nov 2014 15:58:04 +0100 Subject: [PATCH 13/37] Implement WP status update on API V3 WP forms --- lib/api/v3/statuses/statuses_api.rb | 6 +++ lib/api/v3/work_packages/form/form_api.rb | 12 ++++- .../work_packages/link_to_object_extractor.rb | 49 +++++++++++++++++++ .../v3/work_packages/work_package_contract.rb | 8 ++- 4 files changed, 73 insertions(+), 2 deletions(-) create mode 100644 lib/api/v3/work_packages/link_to_object_extractor.rb diff --git a/lib/api/v3/statuses/statuses_api.rb b/lib/api/v3/statuses/statuses_api.rb index 51c5405ca7..4a621fc994 100644 --- a/lib/api/v3/statuses/statuses_api.rb +++ b/lib/api/v3/statuses/statuses_api.rb @@ -39,6 +39,12 @@ module API get do StatusCollectionRepresenter.new(@statuses) end + + namespace ':id' do + get do + fail NotImplementedError + end + end end end end diff --git a/lib/api/v3/work_packages/form/form_api.rb b/lib/api/v3/work_packages/form/form_api.rb index 2b88a3aa97..3491395df8 100644 --- a/lib/api/v3/work_packages/form/form_api.rb +++ b/lib/api/v3/work_packages/form/form_api.rb @@ -34,9 +34,19 @@ module API helpers do def process_form_request if form_post_request_body + form_post_request_body_without_links = form_post_request_body + links = form_post_request_body_without_links.delete('_links') + # enforces availibility validation of lock_version @representer.represented.lock_version = nil - @representer.from_json(patch_request_body) + @representer.from_json(form_post_request_body_without_links.to_json) + + if links + link_to_object_extractor = ::API::V3::WorkPackages::LinkToObjectExtractor.new + linked_properties = link_to_object_extractor.parse_links(links) + + @representer.represented.attributes = linked_properties + end patch_request_valid? end diff --git a/lib/api/v3/work_packages/link_to_object_extractor.rb b/lib/api/v3/work_packages/link_to_object_extractor.rb new file mode 100644 index 0000000000..24d5bb3f72 --- /dev/null +++ b/lib/api/v3/work_packages/link_to_object_extractor.rb @@ -0,0 +1,49 @@ +#-- 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. +#++ + +module API + module V3 + module WorkPackages + class LinkToObjectExtractor < ::API::V3::Utilities::ResourceLinkParser + def parse_links(links) + links.keys.each_with_object({}) do |attribute, h| + resource = parse links[attribute] + + if resource + case resource[:ns] + when 'statuses' + h[:status_id] = resource[:id] + end + end + end + end + end + end + end +end diff --git a/lib/api/v3/work_packages/work_package_contract.rb b/lib/api/v3/work_packages/work_package_contract.rb index 37392f7a4d..0cccd39b5b 100644 --- a/lib/api/v3/work_packages/work_package_contract.rb +++ b/lib/api/v3/work_packages/work_package_contract.rb @@ -34,7 +34,13 @@ module API module V3 module WorkPackages class WorkPackageContract < Reform::Contract - WRITEABLE_ATTRIBUTES = ['lock_version', 'subject', 'parent_id', 'description'].freeze + WRITEABLE_ATTRIBUTES = [ + 'lock_version', + 'subject', + 'parent_id', + 'description', + 'status_id' + ].freeze def initialize(object, user) super(object) From 93f6ad25c0ad4915d33a1fe5ec8df3d17515bd10 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 08:26:56 +0100 Subject: [PATCH 14/37] Test that API V3 WP endpoint differs read/write access --- .../api/v3/work_package_resource_spec.rb | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/spec/requests/api/v3/work_package_resource_spec.rb b/spec/requests/api/v3/work_package_resource_spec.rb index 9f555030f9..316f6ca40b 100644 --- a/spec/requests/api/v3/work_package_resource_spec.rb +++ b/spec/requests/api/v3/work_package_resource_spec.rb @@ -210,12 +210,29 @@ h4. things we like end context 'user without needed permissions' do - let(:current_user) { FactoryGirl.create :user } - let(:params) { valid_params } + context 'no permission to see the work package' do + let(:work_package) { FactoryGirl.create(:work_package, id: 42) } + let(:current_user) { FactoryGirl.create :user } + let(:params) { valid_params } - include_context 'patch request' + include_context 'patch request' - it_behaves_like 'unauthorized access' + it_behaves_like 'not found', 42, 'WorkPackage' + end + + context 'no permission to edit the work package' do + let(:role) { FactoryGirl.create(:role, permissions: [:view_work_packages]) } + let(:current_user) { + FactoryGirl.create(:user, + member_in_project: work_package.project, + member_through_role: role) + } + let(:params) { valid_params } + + include_context 'patch request' + + it_behaves_like 'unauthorized access' + end end context 'user with needed permissions' do From 4257188c0f4c87d16eb69bae16edac8f9760109a Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 08:27:45 +0100 Subject: [PATCH 15/37] Fix line length --- spec/requests/api/v3/work_package_resource_spec.rb | 5 ++++- 1 file changed, 4 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 316f6ca40b..5cf5a3580e 100644 --- a/spec/requests/api/v3/work_package_resource_spec.rb +++ b/spec/requests/api/v3/work_package_resource_spec.rb @@ -237,7 +237,10 @@ h4. things we like context 'user with needed permissions' do shared_examples_for 'lock version updated' do - it { expect(subject.body).to be_json_eql(work_package.reload.lock_version).at_path('lockVersion') } + it { + expect(subject.body).to be_json_eql(work_package.reload.lock_version) + .at_path('lockVersion') + } end describe 'notification' do From 1ef1f19780ac8a5f73aa702ada6132cbaec7f06e Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 11:24:32 +0100 Subject: [PATCH 16/37] Test status processing for API V3 WP endpoint --- .../api/v3/work_package_resource_spec.rb | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/spec/requests/api/v3/work_package_resource_spec.rb b/spec/requests/api/v3/work_package_resource_spec.rb index 5cf5a3580e..d33903ab7c 100644 --- a/spec/requests/api/v3/work_package_resource_spec.rb +++ b/spec/requests/api/v3/work_package_resource_spec.rb @@ -366,6 +366,44 @@ h4. things we like end end + context 'status' do + let(:target_status) { FactoryGirl.create(:status) } + let(:status_link) { "/api/v3/statuses/#{target_status.id}" } + let(:status_parameter) { { _links: { status: status_link } } } + let(:params) { valid_params.merge(status_parameter) } + + before { allow(User).to receive(:current).and_return current_user } + + context 'valid status' do + let!(:workflow) { + FactoryGirl.create(:workflow, + type_id: work_package.type.id, + old_status: work_package.status, + new_status: target_status, + role: current_user.memberships[0].roles[0]) + } + + include_context 'patch request' + + it { expect(response.status).to eq(200) } + + it 'should respond with updated work package status' do + expect(subject.body).to be_json_eql(target_status.name.to_json) + .at_path('status') + end + + it_behaves_like 'lock version updated' + end + + context 'invalid status' do + include_context 'patch request' + + it_behaves_like 'constraint violation', + 'Status no valid transition exists from old to new '\ + 'status for the current user roles.' + end + end + describe 'update with read-only attributes' do describe 'single read-only violation' do context 'start date' do From 52bdc90222a138f8f5fad0b700873efb32507e23 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 11:26:37 +0100 Subject: [PATCH 17/37] Extract WP writing from form API --- lib/api/v3/work_packages/form/form_api.rb | 22 ++------------ lib/api/v3/work_packages/work_packages_api.rb | 30 ++++++++++++++----- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/lib/api/v3/work_packages/form/form_api.rb b/lib/api/v3/work_packages/form/form_api.rb index 3491395df8..d2b26edf6a 100644 --- a/lib/api/v3/work_packages/form/form_api.rb +++ b/lib/api/v3/work_packages/form/form_api.rb @@ -33,23 +33,8 @@ module API class FormAPI < Grape::API helpers do def process_form_request - if form_post_request_body - form_post_request_body_without_links = form_post_request_body - links = form_post_request_body_without_links.delete('_links') - - # enforces availibility validation of lock_version - @representer.represented.lock_version = nil - @representer.from_json(form_post_request_body_without_links.to_json) - - if links - link_to_object_extractor = ::API::V3::WorkPackages::LinkToObjectExtractor.new - linked_properties = link_to_object_extractor.parse_links(links) - - @representer.represented.attributes = linked_properties - end - - patch_request_valid? - end + write_work_package_attributes + write_request_valid? error = ::API::Errors::ErrorBase.create(@representer.represented.errors) @@ -61,9 +46,6 @@ module API end end - def form_post_request_body - env['api.request.body'] - end end post '/form' do diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index 827cb8555e..f6692a8f62 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -44,11 +44,29 @@ module API @representer = ::API::V3::WorkPackages::WorkPackageRepresenter.new(work_package, { current_user: current_user }, :activities, :users) end - def patch_request_body - env['api.request.input'] + def write_work_package_attributes + if request_body + request_body_without_links = request_body + links = request_body_without_links.delete('_links') + + # enforces availibility validation of lock_version + @representer.represented.lock_version = nil + @representer.from_json(request_body_without_links.to_json) + + if links + link_to_object_extractor = ::API::V3::WorkPackages::LinkToObjectExtractor.new + linked_properties = link_to_object_extractor.parse_links(links) + + @representer.represented.attributes = linked_properties + end + end + end + + def request_body + env['api.request.body'] end - def patch_request_valid? + def write_request_valid? contract = WorkPackageContract.new(@representer.represented, current_user) # Although the contract triggers the ActiveModel validations on @@ -77,9 +95,7 @@ module API end patch do - @representer.represented.lock_version = nil # enforces availibility validation of lock_version - - @representer.from_json(patch_request_body) + write_work_package_attributes send_notifications = !(params.has_key?(:notify) && params[:notify] == 'false') update_service = UpdateWorkPackageService.new(current_user, @@ -87,7 +103,7 @@ module API nil, send_notifications) - if patch_request_valid? && update_service.save + if write_request_valid? && update_service.save decorate_work_package(@work_package.reload) @representer else From 870a08030bae4b03101ca25efb68f59c11f15b2d Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 11:27:20 +0100 Subject: [PATCH 18/37] Remove superfluous method --- lib/api/v3/work_packages/work_packages_api.rb | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index f6692a8f62..e08e52efc1 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -40,10 +40,6 @@ module API helpers do attr_reader :work_package - def decorate_work_package(work_package) - @representer = ::API::V3::WorkPackages::WorkPackageRepresenter.new(work_package, { current_user: current_user }, :activities, :users) - end - def write_work_package_attributes if request_body request_body_without_links = request_body @@ -86,7 +82,8 @@ module API before do @work_package = WorkPackage.find(params[:id]) - decorate_work_package(@work_package) + @representer = ::API::V3::WorkPackages::WorkPackageRepresenter + .new(work_package, { current_user: current_user }, :activities, :users) end get do @@ -104,7 +101,7 @@ module API send_notifications) if write_request_valid? && update_service.save - decorate_work_package(@work_package.reload) + @representer.represented.reload @representer else fail ::API::Errors::ErrorBase.create(@representer.represented.errors) From cb9dc86a6914e69803654246140ba0e777ecbff6 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 12:37:07 +0100 Subject: [PATCH 19/37] Improve handling of non-existing render contexts --- lib/api/v3/render/render_api.rb | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/api/v3/render/render_api.rb b/lib/api/v3/render/render_api.rb index 80c40e0132..a480d46137 100644 --- a/lib/api/v3/render/render_api.rb +++ b/lib/api/v3/render/render_api.rb @@ -42,17 +42,21 @@ module API end def context_object + begin + try_context_object + rescue ::ActiveRecord::RecordNotFound + fail API::Errors::InvalidRenderContext.new('Context does not exist!') + end + end + + def try_context_object if params[:context] - context_object = nil context = parse_context case context[:ns] when 'work_packages' - context_object = WorkPackage.visible(current_user).find_by_id(context[:id]) - end - - unless context_object - fail API::Errors::InvalidRenderContext.new('Context does not exist!') + WorkPackage.visible(current_user).find(context[:id]) + else end end end From 16bd488e1e64ed7d05f12aa975c9da866da1440e Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 13:21:08 +0100 Subject: [PATCH 20/37] Make helper classes modules --- lib/api/v3/render/render_api.rb | 3 +-- lib/api/v3/utilities/resource_link_parser.rb | 4 ++-- lib/api/v3/work_packages/link_to_object_extractor.rb | 6 +++--- lib/api/v3/work_packages/work_packages_api.rb | 4 ++-- 4 files changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/api/v3/render/render_api.rb b/lib/api/v3/render/render_api.rb index a480d46137..ecf143bac5 100644 --- a/lib/api/v3/render/render_api.rb +++ b/lib/api/v3/render/render_api.rb @@ -62,8 +62,7 @@ module API end def parse_context - resource_link_parser = ::API::V3::Utilities::ResourceLinkParser.new - context = resource_link_parser.parse(params[:context]) + context = ::API::V3::Utilities::ResourceLinkParser.parse(params[:context]) fail API::Errors::InvalidRenderContext.new('No context found.') if context.nil? diff --git a/lib/api/v3/utilities/resource_link_parser.rb b/lib/api/v3/utilities/resource_link_parser.rb index 561ba225d8..80021b993a 100644 --- a/lib/api/v3/utilities/resource_link_parser.rb +++ b/lib/api/v3/utilities/resource_link_parser.rb @@ -30,8 +30,8 @@ module API module V3 module Utilities - class ResourceLinkParser - def parse(resource_link) + module ResourceLinkParser + def self.parse(resource_link) matching_resources = API::V3::Root.routes.map do |route| route_options = route.instance_variable_get(:@options) match = route_options[:compiled].match(resource_link) diff --git a/lib/api/v3/work_packages/link_to_object_extractor.rb b/lib/api/v3/work_packages/link_to_object_extractor.rb index 24d5bb3f72..e1e31366df 100644 --- a/lib/api/v3/work_packages/link_to_object_extractor.rb +++ b/lib/api/v3/work_packages/link_to_object_extractor.rb @@ -30,10 +30,10 @@ module API module V3 module WorkPackages - class LinkToObjectExtractor < ::API::V3::Utilities::ResourceLinkParser - def parse_links(links) + module LinkToObjectExtractor + def self.parse_links(links) links.keys.each_with_object({}) do |attribute, h| - resource = parse links[attribute] + resource = ::API::V3::Utilities::ResourceLinkParser.parse links[attribute] if resource case resource[:ns] diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index e08e52efc1..ae94672b00 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -50,8 +50,8 @@ module API @representer.from_json(request_body_without_links.to_json) if links - link_to_object_extractor = ::API::V3::WorkPackages::LinkToObjectExtractor.new - linked_properties = link_to_object_extractor.parse_links(links) + linked_properties = ::API::V3::WorkPackages::LinkToObjectExtractor + .parse_links(links) @representer.represented.attributes = linked_properties end From a58eef064da411c178aefc16c4222f1d455a2bf8 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 13:24:35 +0100 Subject: [PATCH 21/37] Take first matching route --- lib/api/v3/utilities/resource_link_parser.rb | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/lib/api/v3/utilities/resource_link_parser.rb b/lib/api/v3/utilities/resource_link_parser.rb index 80021b993a..62af69e1d5 100644 --- a/lib/api/v3/utilities/resource_link_parser.rb +++ b/lib/api/v3/utilities/resource_link_parser.rb @@ -32,23 +32,19 @@ module API module Utilities module ResourceLinkParser def self.parse(resource_link) - matching_resources = API::V3::Root.routes.map do |route| + API::V3::Root.routes.each do |route| route_options = route.instance_variable_get(:@options) match = route_options[:compiled].match(resource_link) if match - { + return { ns: /\/(?\w+)\//.match(route_options[:namespace])[:ns], id: match[:id] } end end - matching_resources.compact!.uniq! { |c| c[:ns] } - - unless matching_resources.empty? - { ns: matching_resources[0][:ns], id: matching_resources[0][:id] } - end + nil end end end From 2eb38a1fe10fe642a29b6fe4af730639ee8ff646 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 13:41:31 +0100 Subject: [PATCH 22/37] Rename WP form payload representer --- lib/api/v3/work_packages/form/form_representer.rb | 2 +- ..._form_representer.rb => work_package_payload_representer.rb} | 2 +- ...esenter_spec.rb => work_package_payload_representer_spec.rb} | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) rename lib/api/v3/work_packages/form/{work_package_form_representer.rb => work_package_payload_representer.rb} (97%) rename spec/lib/api/v3/work_packages/form/{work_package_form_representer_spec.rb => work_package_payload_representer_spec.rb} (96%) diff --git a/lib/api/v3/work_packages/form/form_representer.rb b/lib/api/v3/work_packages/form/form_representer.rb index cdc4fb24f7..63c6b9feaf 100644 --- a/lib/api/v3/work_packages/form/form_representer.rb +++ b/lib/api/v3/work_packages/form/form_representer.rb @@ -79,7 +79,7 @@ module API property :payload, embedded: true, - decorator: Form::WorkPackageFormRepresenter, + decorator: Form::WorkPackagePayloadRepresenter, getter: -> (*) { self } property :schema, embedded: true, diff --git a/lib/api/v3/work_packages/form/work_package_form_representer.rb b/lib/api/v3/work_packages/form/work_package_payload_representer.rb similarity index 97% rename from lib/api/v3/work_packages/form/work_package_form_representer.rb rename to lib/api/v3/work_packages/form/work_package_payload_representer.rb index 1e59dd34be..b61942c014 100644 --- a/lib/api/v3/work_packages/form/work_package_form_representer.rb +++ b/lib/api/v3/work_packages/form/work_package_payload_representer.rb @@ -34,7 +34,7 @@ module API module V3 module WorkPackages module Form - class WorkPackageFormRepresenter < Roar::Decorator + class WorkPackagePayloadRepresenter < Roar::Decorator include Roar::JSON::HAL include Roar::Hypermedia include API::Utilities::UrlHelper diff --git a/spec/lib/api/v3/work_packages/form/work_package_form_representer_spec.rb b/spec/lib/api/v3/work_packages/form/work_package_payload_representer_spec.rb similarity index 96% rename from spec/lib/api/v3/work_packages/form/work_package_form_representer_spec.rb rename to spec/lib/api/v3/work_packages/form/work_package_payload_representer_spec.rb index f34cb26ff3..eb2b8632df 100644 --- a/spec/lib/api/v3/work_packages/form/work_package_form_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/form/work_package_payload_representer_spec.rb @@ -28,7 +28,7 @@ require 'spec_helper' -describe ::API::V3::WorkPackages::Form::WorkPackageFormRepresenter do +describe ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter do let(:work_package) { FactoryGirl.build(:work_package) } let(:representer) { described_class.new(work_package) } From 1dd819e2a33c5e587e6f511f3c4f6eee8d4f78a9 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 13:58:15 +0100 Subject: [PATCH 23/37] Remove redundant begin block --- lib/api/v3/render/render_api.rb | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/lib/api/v3/render/render_api.rb b/lib/api/v3/render/render_api.rb index ecf143bac5..56a9ade0c3 100644 --- a/lib/api/v3/render/render_api.rb +++ b/lib/api/v3/render/render_api.rb @@ -42,11 +42,9 @@ module API end def context_object - begin - try_context_object - rescue ::ActiveRecord::RecordNotFound - fail API::Errors::InvalidRenderContext.new('Context does not exist!') - end + try_context_object + rescue ::ActiveRecord::RecordNotFound + fail API::Errors::InvalidRenderContext.new('Context does not exist!') end def try_context_object From bfa4c7a4ff65d36427a53cecef50dba339eaa1a8 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 14:53:05 +0100 Subject: [PATCH 24/37] Fix status specs --- spec/requests/api/v3/work_package_resource_spec.rb | 2 +- .../v3/work_packages/form/work_package_form_resource_spec.rb | 2 +- 2 files 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 d33903ab7c..78c8a8653c 100644 --- a/spec/requests/api/v3/work_package_resource_spec.rb +++ b/spec/requests/api/v3/work_package_resource_spec.rb @@ -369,7 +369,7 @@ h4. things we like context 'status' do let(:target_status) { FactoryGirl.create(:status) } let(:status_link) { "/api/v3/statuses/#{target_status.id}" } - let(:status_parameter) { { _links: { status: status_link } } } + let(:status_parameter) { { _links: { status: { href: status_link } } } } let(:params) { valid_params.merge(status_parameter) } before { allow(User).to receive(:current).and_return current_user } diff --git a/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb b/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb index c4da15b397..56d2c49227 100644 --- a/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb +++ b/spec/requests/api/v3/work_packages/form/work_package_form_resource_spec.rb @@ -240,7 +240,7 @@ describe 'API v3 Work package form resource', type: :request do let(:path) { '_embedded/payload/_links/status/href' } let(:target_status) { FactoryGirl.create(:status) } let(:status_link) { "/api/v3/statuses/#{target_status.id}" } - let(:status_parameter) { { _links: { status: status_link } } } + let(:status_parameter) { { _links: { status: { href: status_link } } } } let(:params) { valid_params.merge(status_parameter) } context 'valid status' do From af6340743997c47a779fb60fd62f153368ef7659 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Fri, 7 Nov 2014 15:12:30 +0100 Subject: [PATCH 25/37] Fix resource link parsing --- lib/api/v3/work_packages/link_to_object_extractor.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/v3/work_packages/link_to_object_extractor.rb b/lib/api/v3/work_packages/link_to_object_extractor.rb index e1e31366df..ab33e528c4 100644 --- a/lib/api/v3/work_packages/link_to_object_extractor.rb +++ b/lib/api/v3/work_packages/link_to_object_extractor.rb @@ -33,7 +33,7 @@ module API module LinkToObjectExtractor def self.parse_links(links) links.keys.each_with_object({}) do |attribute, h| - resource = ::API::V3::Utilities::ResourceLinkParser.parse links[attribute] + resource = ::API::V3::Utilities::ResourceLinkParser.parse links[attribute]['href'] if resource case resource[:ns] From 82f8b9336c57d53c8960def6dde4f5c86d874739 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 09:04:58 +0100 Subject: [PATCH 26/37] Move link extraction to WP payload representer --- ...ork_package_attribute_links_representer.rb | 59 +++++++++++++++++++ .../form/work_package_payload_representer.rb | 41 +++++++++++-- lib/api/v3/work_packages/work_packages_api.rb | 15 ++--- 3 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb diff --git a/lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb b/lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb new file mode 100644 index 0000000000..775720d58e --- /dev/null +++ b/lib/api/v3/work_packages/form/work_package_attribute_links_representer.rb @@ -0,0 +1,59 @@ +#-- 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. +#++ + +require 'roar/decorator' +require 'roar/json/hal' + +module API + module V3 + module WorkPackages + module Form + class WorkPackageAttributeLinksRepresenter < Roar::Decorator + include Roar::JSON::HAL + include Roar::Hypermedia + include API::Utilities::UrlHelper + + self.as_strategy = ::API::Utilities::CamelCasingStrategy.new + + property :status, + exec_context: :decorator, + getter: -> (*) { + { href: "#{root_path}api/v3/statuses/#{represented.status.id}" } + }, + setter: -> (value, *) { + resource = ::API::V3::Utilities::ResourceLinkParser.parse value['href'] + + represented.status_id = resource[:id] if resource[:ns] == 'statuses' + }, + if: -> (*) { represented.status } + end + end + end + end +end 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 b61942c014..ca206459ae 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,17 +37,21 @@ module API class WorkPackagePayloadRepresenter < Roar::Decorator include Roar::JSON::HAL include Roar::Hypermedia - include API::Utilities::UrlHelper self.as_strategy = ::API::Utilities::CamelCasingStrategy.new property :_type, exec_context: :decorator, writeable: false - link :status do - { - href: "#{root_path}api/v3/statuses/#{represented.status.id}" - } if represented.status - end + property :linked_resources, + as: :_links, + exec_context: :decorator, + getter: -> (*) { + work_package_attribute_links_representer represented + }, + setter: -> (value, *) { + representer = work_package_attribute_links_representer represented + representer.from_json(value.to_json) + } property :lock_version property :subject, render_nil: true @@ -55,10 +59,35 @@ module API getter: -> (*) { description }, setter: -> (value, *) { self.description = value }, render_nil: true + property :parent_id, writeable: true + + property :project_id, getter: -> (*) { project.id } + property :start_date, + getter: -> (*) { + start_date.to_datetime.utc.iso8601 unless start_date.nil? + }, + render_nil: true + property :due_date, + getter: -> (*) { + due_date.to_datetime.utc.iso8601 unless due_date.nil? + }, + render_nil: true + property :version_id, + getter: -> (*) { fixed_version.try(:id) }, + setter: -> (value, *) { self.fixed_version_id = value }, + render_nil: true + property :created_at, getter: -> (*) { created_at.utc.iso8601 }, render_nil: true + property :updated_at, getter: -> (*) { updated_at.utc.iso8601 }, render_nil: true def _type 'WorkPackage' end + + private + + def work_package_attribute_links_representer(represented) + ::API::V3::WorkPackages::Form::WorkPackageAttributeLinksRepresenter.new represented + end end 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 ae94672b00..1063e3d712 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -42,19 +42,12 @@ module API def write_work_package_attributes if request_body - request_body_without_links = request_body - links = request_body_without_links.delete('_links') + payload = ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter + .new(@work_package) # enforces availibility validation of lock_version - @representer.represented.lock_version = nil - @representer.from_json(request_body_without_links.to_json) - - if links - linked_properties = ::API::V3::WorkPackages::LinkToObjectExtractor - .parse_links(links) - - @representer.represented.attributes = linked_properties - end + payload.represented.lock_version = nil + payload.from_json(request_body.to_json) end end From d03791e0a00e81a6912b9249cff3c7ff1bc6dc43 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 09:10:30 +0100 Subject: [PATCH 27/37] Move lock version validation enforcement to representer --- .../work_packages/form/work_package_payload_representer.rb | 7 +++++++ lib/api/v3/work_packages/work_packages_api.rb | 2 -- 2 files changed, 7 insertions(+), 2 deletions(-) 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 ca206459ae..462644ac17 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 @@ -40,6 +40,13 @@ module API self.as_strategy = ::API::Utilities::CamelCasingStrategy.new + def initialize(represented) + # enforces availibility validation of lock_version + represented.lock_version = nil + + super(represented) + end + property :_type, exec_context: :decorator, writeable: false property :linked_resources, diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index 1063e3d712..3396432a16 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -45,8 +45,6 @@ module API payload = ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter .new(@work_package) - # enforces availibility validation of lock_version - payload.represented.lock_version = nil payload.from_json(request_body.to_json) end end From 09d40c7c6a57e448190bb8a18eb73c0d6e39dc76 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 10:01:53 +0100 Subject: [PATCH 28/37] Fix specs --- .../api/v3/work_packages/form/form_representer_spec.rb | 6 +++++- .../form/work_package_payload_representer_spec.rb | 8 +++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/spec/lib/api/v3/work_packages/form/form_representer_spec.rb b/spec/lib/api/v3/work_packages/form/form_representer_spec.rb index e5d4c1ece3..0007c0d9cf 100644 --- a/spec/lib/api/v3/work_packages/form/form_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/form/form_representer_spec.rb @@ -29,7 +29,11 @@ require 'spec_helper' describe ::API::V3::WorkPackages::Form::FormRepresenter do - let(:work_package) { FactoryGirl.build(:work_package) } + let(:work_package) { + FactoryGirl.build(:work_package, + created_at: DateTime.now, + updated_at: DateTime.now) + } let(:current_user) { FactoryGirl.build(:user, member_in_project: work_package.project) } diff --git a/spec/lib/api/v3/work_packages/form/work_package_payload_representer_spec.rb b/spec/lib/api/v3/work_packages/form/work_package_payload_representer_spec.rb index eb2b8632df..8830f56369 100644 --- a/spec/lib/api/v3/work_packages/form/work_package_payload_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/form/work_package_payload_representer_spec.rb @@ -29,9 +29,15 @@ require 'spec_helper' describe ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter do - let(:work_package) { FactoryGirl.build(:work_package) } + let(:work_package) { + FactoryGirl.build(:work_package, + created_at: DateTime.now, + updated_at: DateTime.now) + } let(:representer) { described_class.new(work_package) } + before { allow(work_package).to receive(:lock_version).and_return(1) } + context 'generation' do subject(:generated) { representer.to_json } From b3909375683f930fed2e462867eaf7ead8b52c61 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 10:59:22 +0100 Subject: [PATCH 29/37] Disable lock version validation by default --- .../form/work_package_payload_representer.rb | 8 +++++--- lib/api/v3/work_packages/work_packages_api.rb | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) 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 462644ac17..25cf03be7c 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 @@ -40,9 +40,11 @@ module API self.as_strategy = ::API::Utilities::CamelCasingStrategy.new - def initialize(represented) - # enforces availibility validation of lock_version - represented.lock_version = nil + def initialize(represented, options={}) + if options[:enforce_lock_version_validation] + # enforces availibility validation of lock_version + represented.lock_version = nil + end super(represented) end diff --git a/lib/api/v3/work_packages/work_packages_api.rb b/lib/api/v3/work_packages/work_packages_api.rb index 3396432a16..6a221e6529 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -43,7 +43,7 @@ module API def write_work_package_attributes if request_body payload = ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter - .new(@work_package) + .new(@work_package, enforce_lock_version_validation: true) payload.from_json(request_body.to_json) end From 17fe7fe9c07b355245604d6ab611751b559a7492 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 15:05:24 +0100 Subject: [PATCH 30/37] Make error handling more readable --- lib/api/v3/render/render_api.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/api/v3/render/render_api.rb b/lib/api/v3/render/render_api.rb index 56a9ade0c3..8839a5891f 100644 --- a/lib/api/v3/render/render_api.rb +++ b/lib/api/v3/render/render_api.rb @@ -62,13 +62,13 @@ module API def parse_context context = ::API::V3::Utilities::ResourceLinkParser.parse(params[:context]) - fail API::Errors::InvalidRenderContext.new('No context found.') if context.nil? - - unless SUPPORTED_CONTEXT_NAMESPACES.include? context[:ns] + if context.nil? + fail API::Errors::InvalidRenderContext.new('No context found.') + elsif !SUPPORTED_CONTEXT_NAMESPACES.include? context[:ns] fail API::Errors::InvalidRenderContext.new('Unsupported context found.') + else + context end - - context end def render(type) From 6b739ea6344447e2dcd8909d568da17167b0e042 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 15:08:15 +0100 Subject: [PATCH 31/37] Remove empty 'else' block --- lib/api/v3/render/render_api.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/api/v3/render/render_api.rb b/lib/api/v3/render/render_api.rb index 8839a5891f..2ad540d8b9 100644 --- a/lib/api/v3/render/render_api.rb +++ b/lib/api/v3/render/render_api.rb @@ -54,7 +54,6 @@ module API case context[:ns] when 'work_packages' WorkPackage.visible(current_user).find(context[:id]) - else end end end From 42aab91164a6fc1c63bcdcf598f063dafc09248f Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 15:21:52 +0100 Subject: [PATCH 32/37] Test preview link's context --- .../api/v3/work_packages/form/form_representer_spec.rb | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/spec/lib/api/v3/work_packages/form/form_representer_spec.rb b/spec/lib/api/v3/work_packages/form/form_representer_spec.rb index 0007c0d9cf..a53fae88f4 100644 --- a/spec/lib/api/v3/work_packages/form/form_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/form/form_representer_spec.rb @@ -31,6 +31,7 @@ require 'spec_helper' describe ::API::V3::WorkPackages::Form::FormRepresenter do let(:work_package) { FactoryGirl.build(:work_package, + id: 42, created_at: DateTime.now, updated_at: DateTime.now) } @@ -59,6 +60,14 @@ describe ::API::V3::WorkPackages::Form::FormRepresenter do it { is_expected.to have_json_path('_links/previewMarkup/href') } it { is_expected.to be_json_eql(:post.to_json).at_path('_links/previewMarkup/method') } + + it 'contains link to work package' do + body = parse_json(subject) + preview_markup_wp_link = body['_links']['previewMarkup']['href'].split('?')[1] + wp_self_link = body['_links']['commit']['href'] + + expect(preview_markup_wp_link).to eq(wp_self_link) + end end describe 'commit' do From e60d25548e42c2a767aa6f079018783bc87c3a66 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 15:22:24 +0100 Subject: [PATCH 33/37] Add the correct context to the preview link --- lib/api/v3/work_packages/form/form_representer.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/api/v3/work_packages/form/form_representer.rb b/lib/api/v3/work_packages/form/form_representer.rb index 63c6b9feaf..f2e4992745 100644 --- a/lib/api/v3/work_packages/form/form_representer.rb +++ b/lib/api/v3/work_packages/form/form_representer.rb @@ -65,7 +65,8 @@ module API link :previewMarkup do { - href: "#{root_path}api/v3/render/textile", + href: "#{root_path}api/v3/render/textile?"\ + "#{root_path}api/v3/work_packages/#{represented.id}", method: :post } end From 7895e29c7cd1b66e158886fcbdd17aababe7b8a9 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 15:27:11 +0100 Subject: [PATCH 34/37] Test that user w/o permission get no commit link --- .../v3/work_packages/form/form_representer_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/lib/api/v3/work_packages/form/form_representer_spec.rb b/spec/lib/api/v3/work_packages/form/form_representer_spec.rb index a53fae88f4..b51263c648 100644 --- a/spec/lib/api/v3/work_packages/form/form_representer_spec.rb +++ b/spec/lib/api/v3/work_packages/form/form_representer_spec.rb @@ -84,6 +84,19 @@ describe ::API::V3::WorkPackages::Form::FormRepresenter do it { is_expected.not_to have_json_path('_links/commit/href') } end + + context 'user with insufficient permissions' do + let(:role) { FactoryGirl.create(:role, permissions: []) } + let(:current_user) { + FactoryGirl.build(:user, + member_in_project: work_package.project, + member_through_role: role) + } + + before { allow(work_package).to receive(:valid?).and_return(true) } + + it { is_expected.not_to have_json_path('_links/commit/href') } + end end end From de86867331476a076bee38773222b0533cc03337 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 15:27:28 +0100 Subject: [PATCH 35/37] Render commit link for users with permissions only --- lib/api/v3/work_packages/form/form_representer.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/api/v3/work_packages/form/form_representer.rb b/lib/api/v3/work_packages/form/form_representer.rb index f2e4992745..27c93a2a34 100644 --- a/lib/api/v3/work_packages/form/form_representer.rb +++ b/lib/api/v3/work_packages/form/form_representer.rb @@ -75,7 +75,8 @@ module API { href: "#{root_path}api/v3/work_packages/#{represented.id}", method: :patch - } if represented.valid? + } if @current_user.allowed_to?(:edit_work_packages, represented.project) && + represented.valid? end property :payload, From 7091b628afcf78b873f864c2515292a6c9a04c89 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 15:51:17 +0100 Subject: [PATCH 36/37] Test WP update links adhere to the specification --- .../work_package_representer_spec.rb | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) 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 0981b65e88..600ba72eb3 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 @@ -45,7 +45,17 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do } let(:category) { FactoryGirl.build(:category) } let(:project) { work_package.project } - let(:permissions) { %i(view_work_packages view_work_package_watchers add_work_package_watchers delete_work_package_watchers manage_work_package_relations add_work_package_notes) } + let(:permissions) { + [ + :view_work_packages, + :view_work_package_watchers, + :edit_work_packages, + :add_work_package_watchers, + :delete_work_package_watchers, + :manage_work_package_relations, + :add_work_package_notes + ] + } let(:role) { FactoryGirl.create :role, permissions: permissions } before(:each) do @@ -124,6 +134,29 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do expect(subject).to have_json_path('_links/self/title') end + describe 'update links' do + describe 'update by form' do + it { expect(subject).to have_json_path('_links/update/href') } + it { + expect(subject).to be_json_eql("/api/v3/work_packages/#{work_package.id}/form".to_json) + .at_path('_links/update/href') + } + it { expect(subject).to be_json_eql('post'.to_json).at_path('_links/update/method') } + end + + describe 'immediate update' do + it { expect(subject).to have_json_path('_links/updateImmediately/href') } + it { + expect(subject).to be_json_eql("/api/v3/work_packages/#{work_package.id}".to_json) + .at_path('_links/updateImmediately/href') + } + it { + expect(subject).to be_json_eql('patch'.to_json) + .at_path('_links/updateImmediately/method') + } + end + end + describe 'version' do context 'no version set' do it { is_expected.to_not have_json_path('versionViewable') } From 6e372e48484f3445fb05ffcdd75966afa436eba0 Mon Sep 17 00:00:00 2001 From: Hagen Schink Date: Mon, 10 Nov 2014 15:52:16 +0100 Subject: [PATCH 37/37] Implement WP update links according to the specification --- .../javascripts/angular/services/work-package-service.js | 2 +- lib/api/v3/work_packages/work_package_representer.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/angular/services/work-package-service.js b/app/assets/javascripts/angular/services/work-package-service.js index 4187650c7b..48203561c4 100644 --- a/app/assets/javascripts/angular/services/work-package-service.js +++ b/app/assets/javascripts/angular/services/work-package-service.js @@ -134,7 +134,7 @@ module.exports = function($http, PathHelper, WorkPackagesHelper, HALAPIResource, data: JSON.stringify(data), contentType: "application/json; charset=utf-8" }, force: true}; - return workPackage.links.update.fetch(options).then(function(workPackage) { + return workPackage.links.updateImmediately.fetch(options).then(function(workPackage) { return workPackage; }) }, diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index b89845b35c..4270ff1324 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -58,6 +58,14 @@ module API end link :update do + { + href: "#{root_path}api/v3/work_packages/#{represented.id}/form", + method: :post, + title: "Update #{represented.subject}" + } if current_user_allowed_to(:edit_work_packages) + end + + link :updateImmediately do { href: "#{root_path}api/v3/work_packages/#{represented.id}", method: :patch,