allow PATCHing start and due date

- adding new errors
- adding more tests
- extending the specification for error conditions

config/locales/de.yml

@@ -1674,6 +1674,7 @@ de:
       multiple_errors: "Die Integrität mehrerer Eigenschaften wurde verletzt."
       parse_error: "Die Abfrage enthielt kein valides JSON."
       writing_read_only_attributes: "Nur-lesbare Eigeschaften dürfen nicht geschrieben werden."
+      invalid_format: "Ungültiges Format für Eigenschaft '%{property}': Ein Format der Art '%{expected_format}' wurde erwartet, aber '%{actual}' wurde übergeben."
       render:
         context_not_found: "Es wurde kein Kontext gefunden."
         unsupported_context: "Der Kontext wird nicht unterstützt."

config/locales/en.yml

@@ -1665,6 +1665,7 @@ en:
       multiple_errors: "Multiple fields violated their constraints."
       parse_error: "The request body was neither empty, nor did it contain a single JSON object."
       writing_read_only_attributes: "You must not write a read-only attribute."
+      invalid_format: "Invalid format for property '%{property}': Expected format like '%{expected_format}', but got '%{actual}'."
       render:
         context_not_found: "No context found."
         unsupported_context: "Unsupported context found."

doc/apiv3-documentation.apib

@@ -180,6 +180,7 @@ embedded errors or simply state that multiple errors occured.
 * `urn:openproject-org:api:v3:errors:PropertyIsReadOnly` (**HTTP 422**) - The client tried to set or change a property that is not writable
 * `urn:openproject-org:api:v3:errors:PropertyConstraintViolation` (**HTTP 422**) - The client tried to set a property to an invalid value
 * `urn:openproject-org:api:v3:errors:PropertyValueNotAvailableAnymore` (**HTTP 422**) - An unchanged property needs to be changed, because other changes to the resource make it unavailable
+* `urn:openproject-org:api:v3:errors:PropertyFormatError` (**HTTP 422**) - A property value was provided in a format that the server does not understand or accept
 * `urn:openproject-org:api:v3:errors:InternalServerError` (**HTTP 500**) - Default for HTTP 500 when no further information is available
 * `urn:openproject-org:api:v3:errors:MultipleErrors` - Multiple errors occured. See the embedded `errors` array for more details.
 
@@ -2644,6 +2645,7 @@ The value of `lockVersion` is used to implement [optimistic locking](http://en.w
     Returned if:
     * the client tries to modify a read-only property
     * a constraint for a property was violated
+    * a property was provided in an unreadable format
 
     + Body
 

lib/api/errors/property_format_error.rb

@@ -0,0 +1,39 @@
+#-- 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 Errors
+    class PropertyFormatError < ErrorBase
+      def initialize(property, expected_format, actual_value)
+        message = I18n.t('api_v3.errors.invalid_format', property: property, expected_format: expected_format, actual: actual_value)
+        super 422, message
+      end
+    end
+  end
+end

lib/api/v3/errors/error_representer.rb

@@ -66,6 +66,8 @@ module API
             'urn:openproject-org:api:v3:errors:MissingPermission'
           when ::API::Errors::UnwritableProperty
             'urn:openproject-org:api:v3:errors:PropertyIsReadOnly'
+          when ::API::Errors::PropertyFormatError
+            'urn:openproject-org:api:v3:errors:PropertyFormatError'
           when ::API::Errors::Validation
             'urn:openproject-org:api:v3:errors:PropertyConstraintViolation'
           when ::API::Errors::InvalidRenderContext

lib/api/v3/work_packages/form/work_package_payload_representer.rb

@@ -80,12 +80,25 @@ module API
           property :project_id,
                    getter: -> (*) { nil },
                    render_nil: false
+
           property :start_date,
-                   getter: -> (*) { nil },
+                   exec_context: :decorator,
-                   render_nil: false
+                   getter: -> (*) {
+                     represented.start_date.to_date.iso8601 unless represented.start_date.nil?
+                   },
+                   setter: -> (value, *) {
+                     represented.start_date = parse_date_only(value, 'startDate')
+                   },
+                   render_nil: true
           property :due_date,
-                   getter: -> (*) { nil },
+                   exec_context: :decorator,
-                   render_nil: false
+                   getter: -> (*) {
+                     represented.due_date.to_date.iso8601 unless represented.due_date.nil?
+                   },
+                   setter: -> (value, *) {
+                     represented.due_date = parse_date_only(value, 'dueDate')
+                   },
+                   render_nil: true
           property :version_id,
                    getter: -> (*) { nil },
                    setter: -> (value, *) { self.fixed_version_id = value },
@@ -108,6 +121,27 @@ module API
           def description_renderer
             ::API::Utilities::Renderer::TextileRenderer.new(represented.description, represented)
           end
+
+          def parse_date_only(value, property_name)
+            return nil if value.nil?
+
+            begin
+              date_and_time = DateTime.iso8601(value)
+            rescue ArgumentError
+              raise API::Errors::PropertyFormatError.new(property_name, 'YYYY-MM-DD', value)
+            end
+
+            date_only = date_and_time.to_date
+
+            # we only want to accept "timeless" dates, e.g. "2015-01-31",
+            # but not "2015-01-31T01:02:03".
+            # However Date.iso8601 is too generous and would accept that
+            unless date_and_time == date_only
+              raise API::Errors::PropertyFormatError.new(property_name, 'YYYY-MM-DD', value)
+            end
+
+            date_only
+          end
         end
       end
     end

lib/api/v3/work_packages/work_package_contract.rb

@@ -39,6 +39,8 @@ module API
           'subject',
           'parent_id',
           'description',
+          'start_date',
+          'due_date',
           'status_id',
           'assigned_to_id',
           'responsible_id',

spec/factories/work_package_factory.rb

@@ -38,6 +38,8 @@ FactoryGirl.define do
     sequence(:subject) { |n| "WorkPackage No. #{n}" }
     description { |i| "Description for '#{i.subject}'" }
     author factory: :user
+    created_at { DateTime.now }
+    updated_at { DateTime.now }
 
     callback(:after_build) do |work_package, evaluator|
       work_package.type = work_package.project.types.first unless work_package.type

spec/lib/api/v3/support/date_time_examples.rb

@@ -0,0 +1,53 @@
+#-- 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'
+
+shared_examples_for 'has ISO 8601 date only' do
+  let(:iso_date_only_string) { '%d-%02d-%02d' % [ date.year, date.month, date.day ] }
+
+  it 'exists' do
+    is_expected.to have_json_path(json_path)
+  end
+
+  it 'indicates date only as ISO 8601' do
+    is_expected.to be_json_eql(iso_date_only_string.to_json).at_path(json_path)
+  end
+end
+
+shared_examples_for 'has UTC ISO 8601 date and time' do
+  let(:iso_string) { date.utc.iso8601 }
+
+  it 'exists' do
+    is_expected.to have_json_path(json_path)
+  end
+
+  it 'indicates date and time as ISO 8601' do
+    is_expected.to be_json_eql(iso_string.to_json).at_path(json_path)
+  end
+end

spec/lib/api/v3/work_packages/form/work_package_payload_representer_spec.rb

@@ -31,6 +31,8 @@ require 'spec_helper'
 describe ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter do
   let(:work_package) {
     FactoryGirl.build(:work_package,
+                      start_date: Date.today.to_datetime,
+                      due_date: Date.today.to_datetime,
                       created_at: DateTime.now,
                       updated_at: DateTime.now)
   }
@@ -59,6 +61,36 @@ describe ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter do
 
         it { is_expected.to be_json_eql(work_package.lock_version.to_json).at_path('lockVersion') }
       end
+
+      describe 'startDate' do
+        it_behaves_like 'has ISO 8601 date only' do
+          let(:date) { work_package.start_date }
+          let(:json_path) { 'startDate' }
+        end
+
+        context 'no start date' do
+          let(:work_package) { FactoryGirl.build(:work_package, start_date: nil) }
+
+          it 'renders as null' do
+            is_expected.to be_json_eql(nil.to_json).at_path('startDate')
+          end
+        end
+      end
+
+      describe 'dueDate' do
+        it_behaves_like 'has ISO 8601 date only' do
+          let(:date) { work_package.due_date }
+          let(:json_path) { 'dueDate' }
+        end
+
+        context 'no due date' do
+          let(:work_package) { FactoryGirl.build(:work_package, due_date: nil) }
+
+          it 'renders as null' do
+            is_expected.to be_json_eql(nil.to_json).at_path('dueDate')
+          end
+        end
+      end
     end
 
     describe '_links' do
@@ -133,14 +165,95 @@ describe ::API::V3::WorkPackages::Form::WorkPackagePayloadRepresenter do
   end
 
   describe 'parsing' do
+    let(:attributes) { {} }
     let(:links) { {} }
     let(:json) do
-      {
+      copy = attributes.clone
-        _links: links
+      copy[:_links] = links
-      }.to_json
+      copy.to_json
+    end
+
+    subject { representer.from_json(json) }
+
+    describe 'startDate' do
+      let(:attributes) do
+        {
+          startDate: dateString
+        }
+      end
+
+      context 'with an ISO formatted date' do
+        let(:dateString) { '2015-01-31' }
+
+        it 'sets the date' do
+          expect(subject.start_date).to eql(Date.new(2015, 1, 31))
+        end
+      end
+
+      context 'with null' do
+        let(:dateString) { nil }
+
+        it 'sets the date to nil' do
+          expect(subject.start_date).to eql(nil)
+        end
+      end
+
+      context 'with a non ISO formatted date' do
+        let(:dateString) { '31.01.2015' }
+
+        it 'raises an error' do
+          expect{ subject }.to raise_error(API::Errors::PropertyFormatError)
+        end
+      end
+
+      context 'with an ISO formatted date and time' do
+        let(:dateString) { '2015-01-31T13:37:00Z' }
+
+        it 'raises an error' do
+          expect{ subject }.to raise_error(API::Errors::PropertyFormatError)
+        end
+      end
     end
 
-    subject(:parsed) { representer.from_json(json) }
+    describe 'dueDate' do
+      let(:attributes) do
+        {
+          dueDate: dateString
+        }
+      end
+
+      context 'with an ISO formatted date' do
+        let(:dateString) { '2015-01-31' }
+
+        it 'sets the date' do
+          expect(subject.due_date).to eql(Date.new(2015, 1, 31))
+        end
+      end
+
+      context 'with null' do
+        let(:dateString) { nil }
+
+        it 'sets the date to nil' do
+          expect(subject.due_date).to eql(nil)
+        end
+      end
+
+      context 'with a non ISO formatted date' do
+        let(:dateString) { '31.01.2015' }
+
+        it 'raises an error' do
+          expect{ subject }.to raise_error(API::Errors::PropertyFormatError)
+        end
+      end
+
+      context 'with an ISO formatted date and time' do
+        let(:dateString) { '2015-01-31T13:37:00Z' }
+
+        it 'raises an error' do
+          expect{ subject }.to raise_error(API::Errors::PropertyFormatError)
+        end
+      end
+    end
 
     describe 'version' do
       let(:id) { 5 }

spec/lib/api/v3/work_packages/work_package_representer_spec.rb

@@ -74,30 +74,6 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
     it { is_expected.to include_json('WorkPackage'.to_json).at_path('_type') }
 
     describe 'work_package' do
-      shared_examples_for 'ISO 8601 date only' do
-        let(:iso_date_only_string) { '%d-%02d-%02d' % [ date.year, date.month, date.day ] }
-
-        it 'exists' do
-          is_expected.to have_json_path(json_path)
-        end
-
-        it 'indicates date only as ISO 8601' do
-          is_expected.to be_json_eql(iso_date_only_string.to_json).at_path(json_path)
-        end
-      end
-
-      shared_examples_for 'UTC ISO 8601 date and time' do
-        let(:iso_string) { date.utc.iso8601 }
-
-        it 'exists' do
-          is_expected.to have_json_path(json_path)
-        end
-
-        it 'indicates date and time as ISO 8601' do
-          is_expected.to be_json_eql(iso_string.to_json).at_path(json_path)
-        end
-      end
-
       it { is_expected.to have_json_path('id') }
 
       it_behaves_like 'API V3 formattable', 'description' do
@@ -112,28 +88,44 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do
       it { is_expected.to have_json_path('projectName') }
 
       describe 'startDate' do
-        it_behaves_like 'ISO 8601 date only' do
+        it_behaves_like 'has ISO 8601 date only' do
           let(:date) { work_package.start_date }
           let(:json_path) { 'startDate' }
         end
+
+        context 'no start date' do
+          let(:work_package) { FactoryGirl.build(:work_package, start_date: nil) }
+
+          it 'renders as null' do
+            is_expected.to be_json_eql(nil.to_json).at_path('startDate')
+          end
+        end
       end
 
       describe 'dueDate' do
-        it_behaves_like 'ISO 8601 date only' do
+        it_behaves_like 'has ISO 8601 date only' do
           let(:date) { work_package.due_date }
           let(:json_path) { 'dueDate' }
         end
+
+        context 'no due date' do
+          let(:work_package) { FactoryGirl.build(:work_package, due_date: nil) }
+
+          it 'renders as null' do
+            is_expected.to be_json_eql(nil.to_json).at_path('dueDate')
+          end
+        end
       end
 
       describe 'createdAt' do
-        it_behaves_like 'UTC ISO 8601 date and time' do
+        it_behaves_like 'has UTC ISO 8601 date and time' do
           let(:date) { work_package.created_at }
           let(:json_path) { 'createdAt' }
         end
       end
 
       describe 'updatedAt' do
-        it_behaves_like 'UTC ISO 8601 date and time' do
+        it_behaves_like 'has UTC ISO 8601 date and time' do
           let(:date) { work_package.updated_at }
           let(:json_path) { 'updatedAt' }
         end