grid update form api

pull/6834/head
Jens Ulferts 6 years ago
parent bc0f97192b
commit 52046c519a
No known key found for this signature in database
GPG Key ID: 3CAA4B1182CF5308
  1. 50
      app/contracts/grids/base_contract.rb
  2. 5
      app/contracts/grids/create_contract.rb
  3. 2
      config/locales/en.yml
  4. 14
      lib/api/v3/grids/create_form_representer.rb
  5. 78
      lib/api/v3/grids/form_representer.rb
  6. 2
      lib/api/v3/grids/grids_api.rb
  7. 69
      lib/api/v3/grids/update_form_api.rb
  8. 53
      lib/api/v3/grids/update_form_representer.rb
  9. 4
      lib/api/v3/utilities/path_helper.rb
  10. 237
      spec/contracts/grids/shared_examples.rb
  11. 6
      spec/lib/api/v3/utilities/path_helper_spec.rb
  12. 12
      spec/requests/api/v3/grids/grids_create_form_resource_spec.rb
  13. 18
      spec/requests/api/v3/grids/grids_resource_spec.rb
  14. 177
      spec/requests/api/v3/grids/grids_update_form_resource_spec.rb

@ -33,8 +33,6 @@ require 'model_contract'
module Grids
class BaseContract < ::ModelContract
include OpenProject::StaticRouting::UrlHelpers
# TODO: validate widgets are in array of allowed widgets
# validate widgets do not collide
attribute :row_count do
validate_positive_integer(:row_count)
end
@ -49,6 +47,8 @@ module Grids
validate_registered_subclass
validate_registered_widgets
validate_widget_collisions
validate_widgets_within
validate_widgets_start_before_end
super
end
@ -76,7 +76,7 @@ module Grids
def validate_registered_widgets
return unless Grids::Configuration.registered_grid?(model.class)
model.widgets.each do |widget|
undestroyed_widgets.each do |widget|
next if Grids::Configuration.allowed_widget?(model.class, widget.identifier)
errors.add(:widgets, :inclusion)
@ -84,13 +84,10 @@ module Grids
end
def validate_widget_collisions
model.widgets.each do |widget|
overlaps = model
.widgets
undestroyed_widgets.each do |widget|
overlaps = undestroyed_widgets
.any? do |other_widget|
widget != other_widget &&
!widget.marked_for_destruction? &&
!other_widget.marked_for_destruction? &&
widgets_overlap?(widget, other_widget)
end
@ -100,6 +97,14 @@ module Grids
end
end
def validate_widgets_within
undestroyed_widgets.each do |widget|
next unless outside?(widget)
errors.add(:widgets, :outside)
end
end
def validate_positive_integer(attribute)
value = model.send(attribute)
@ -110,6 +115,16 @@ module Grids
end
end
def validate_widgets_start_before_end
undestroyed_widgets.each do |widget|
if widget.start_row >= widget.end_row ||
widget.start_column >= widget.end_column
errors.add(:widgets, :end_before_start)
end
end
end
def widgets_overlap?(widget, other_widget)
point_in_widget_area(widget, other_widget.start_row, other_widget.start_column) ||
point_in_widget_area(widget, other_widget.start_row, other_widget.end_column) ||
@ -121,5 +136,24 @@ module Grids
widget.start_row < row && widget.end_row > row &&
widget.start_column < column && widget.end_column > column
end
def outside?(widget)
outside_row(widget.start_row) ||
outside_row(widget.end_row) ||
outside_column(widget.start_column) ||
outside_column(widget.end_column)
end
def outside_row(number)
number > model.row_count + 1 || number < 1
end
def outside_column(number)
number > model.column_count + 1 || number < 1
end
def undestroyed_widgets
model.widgets.reject(&:marked_for_destruction?)
end
end
end

@ -32,11 +32,9 @@ require 'grids/base_contract'
module Grids
class CreateContract < BaseContract
# TODO: generalize
attribute :user_id,
writeable: -> { model.is_a?(MyPageGrid) }
writeable: -> { model.class.reflect_on_association(:user) }
# TODO: prevent that value from being set
attribute :type
# TODO tests and check if it should be here
@ -47,7 +45,6 @@ module Grids
end
end
# TODO tests
def writable?(attribute)
attribute == :page || super
end

@ -508,6 +508,8 @@ en:
unreadable: "can't be read. Are you sure it is a support token?"
grid:
overlaps: 'overlap.'
outside: 'is outside of the grid.'
end_before_start: 'end value needs to be larger than the start value.'
parse_schema_filter_params_service:
attributes:
base:

@ -36,17 +36,17 @@ module API
api_v3_paths.create_grid_form
end
#def resource_url
# api_v3_paths.queries
#end
#def commit_action
# :create
#end
def resource_url
api_v3_paths.grids
end
def commit_method
:post
end
def contract_class
::Grids::CreateContract
end
end
end
end

@ -39,46 +39,33 @@ module API
}
end
#link :validate do
# {
# href: form_url,
# method: :post
# }
#end
#link :commit do
# if allow_commit?
# {
# href: resource_url,
# method: commit_method
# }
# end
#end
link :validate do
{
href: form_url,
method: :post
}
end
#link :create_new do
# if allow_create_as_new?
# {
# href: api_v3_paths.queries,
# method: :post
# }
# end
#end
link :commit do
next unless @errors.empty?
#def commit_action
# raise NotImplementedError, "subclass responsibility"
#end
{
href: resource_url,
method: commit_method
}
end
#def commit_method
# raise NotImplementedError, "subclass responsibility"
#end
def commit_method
raise NotImplementedError, "subclass responsibility"
end
#def form_url
# raise NotImplementedError, "subclass responsibility"
#end
def form_url
raise NotImplementedError, "subclass responsibility"
end
#def resource_url
# raise NotImplementedError, "subclass responsibility"
#end
def resource_url
raise NotImplementedError, "subclass responsibility"
end
def payload_representer
GridPayloadRepresenter
@ -86,13 +73,6 @@ module API
end
def schema_representer
# TODO: spec this out
contract_class = if represented.new_record?
::Grids::CreateContract
else
::Grids::UpdateContract
end
contract = contract_class.new(represented, current_user)
API::V3::Grids::Schemas::GridSchemaRepresenter.new(contract,
@ -100,17 +80,9 @@ module API
current_user: current_user)
end
#def allow_commit?
# @errors.empty? && represented.name.present? && allow_save?
#end
#def allow_save?
# QueryPolicy.new(current_user).allowed? represented, commit_action
#end
#def allow_create_as_new?
# QueryPolicy.new(current_user).allowed? represented, :create_new
#end
def contract_class
raise NotImplementedError, "subclass responsibility"
end
end
end
end

@ -108,6 +108,8 @@ module API
fail ::API::Errors::ErrorBase.create_and_merge_errors(call.errors)
end
end
mount UpdateFormAPI
end
end
end

@ -0,0 +1,69 @@
#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2017 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++
module API
module V3
module Grids
class UpdateFormAPI < ::API::OpenProjectAPI
resource :form do
helpers do
include API::V3::Utilities::FormHelper
end
post do
params = API::V3::ParseResourceParamsService
.new(current_user, representer: GridPayloadRepresenter)
.call(request_body)
.result
if params[:page]
params[:type] = ::Grids::Configuration.grid_for_page(params.delete(:page)).to_s
end
call = ::Grids::SetAttributesService
.new(user: current_user,
grid: @grid,
contract_class: ::Grids::UpdateContract)
.call(params)
api_errors = ::API::Errors::ErrorBase.create_errors(call.errors)
if only_validation_errors(api_errors)
status 200
UpdateFormRepresenter.new(call.result,
errors: api_errors,
current_user: current_user)
else
fail ::API::Errors::MultipleErrors.create_if_many(api_errors)
end
end
end
end
end
end
end

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

@ -147,6 +147,10 @@ module API
"#{grids}/#{id}"
end
def self.grid_form(id)
"#{grid(id)}/form"
end
def self.help_texts
"#{root}/help_texts"
end

@ -102,6 +102,18 @@ shared_examples_for 'shared grid contract attributes' do
it_behaves_like 'validates positive integer'
end
context 'row_count less than 1' do
before do
grid.row_count = 0
end
it 'notes the error' do
instance.validate
expect(instance.errors.details[:row_count])
.to match_array [{ error: :greater_than, count: 0 }]
end
end
end
describe 'column_count' do
@ -111,6 +123,18 @@ shared_examples_for 'shared grid contract attributes' do
it_behaves_like 'validates positive integer'
end
context 'row_count less than 1' do
before do
grid.column_count = 0
end
it 'notes the error' do
instance.validate
expect(instance.errors.details[:column_count])
.to match_array [{ error: :greater_than, count: 0 }]
end
end
end
describe 'widgets' do
@ -119,9 +143,9 @@ shared_examples_for 'shared grid contract attributes' do
let(:value) do
[
GridWidget.new(start_row: 1,
end_row: 1,
end_row: 4,
start_column: 2,
end_column: 2,
end_column: 5,
identifier: 'work_packages_assigned')
]
end
@ -130,9 +154,9 @@ shared_examples_for 'shared grid contract attributes' do
context 'invalid identifier' do
before do
grid.widgets.build(start_row: 1,
end_row: 1,
end_row: 4,
start_column: 2,
end_column: 2,
end_column: 5,
identifier: 'bogus_identifier')
end
@ -173,45 +197,186 @@ shared_examples_for 'shared grid contract attributes' do
.to match_array [{ error: :overlaps }, { error: :overlaps }]
end
end
end
context 'widgets having the same start column as another\'s end column' do
before do
grid.widgets.build(start_row: 1,
end_row: 3,
start_column: 1,
end_column: 3,
identifier: 'work_packages_assigned')
grid.widgets.build(start_row: 1,
end_row: 3,
start_column: 3,
end_column: 4,
identifier: 'work_packages_created')
context 'widgets having the same start column as another\'s end column' do
before do
grid.widgets.build(start_row: 1,
end_row: 3,
start_column: 1,
end_column: 3,
identifier: 'work_packages_assigned')
grid.widgets.build(start_row: 1,
end_row: 3,
start_column: 3,
end_column: 4,
identifier: 'work_packages_created')
end
it 'is valid' do
expect(instance.validate)
.to be_truthy
end
end
it 'is valid' do
expect(instance.validate)
.to be_truthy
context 'widgets having the same start row as another\'s end row' do
before do
grid.widgets.build(start_row: 1,
end_row: 3,
start_column: 1,
end_column: 3,
identifier: 'work_packages_assigned')
grid.widgets.build(start_row: 3,
end_row: 4,
start_column: 1,
end_column: 3,
identifier: 'work_packages_created')
end
it 'is valid' do
expect(instance.validate)
.to be_truthy
end
end
end
context 'widgets having the same start row as another\'s end row' do
before do
grid.widgets.build(start_row: 1,
end_row: 3,
start_column: 1,
end_column: 3,
identifier: 'work_packages_assigned')
grid.widgets.build(start_row: 3,
end_row: 4,
start_column: 1,
end_column: 3,
identifier: 'work_packages_created')
context 'widgets being outside (max) of the grid' do
before do
grid.widgets.build(start_row: 1,
end_row: grid.row_count + 2,
start_column: 1,
end_column: 3,
identifier: 'work_packages_assigned')
end
it 'is invalid' do
expect(instance.validate)
.to be_falsey
end
it 'notes the error' do
instance.validate
expect(instance.errors.details[:widgets])
.to match_array [{ error: :outside }]
end
end
it 'is valid' do
expect(instance.validate)
.to be_truthy
context 'widgets being outside (min) of the grid' do
before do
grid.widgets.build(start_row: 1,
end_row: 2,
start_column: -1,
end_column: 3,
identifier: 'work_packages_assigned')
end
it 'is invalid' do
expect(instance.validate)
.to be_falsey
end
it 'notes the error' do
instance.validate
expect(instance.errors.details[:widgets])
.to match_array [{ error: :outside }]
end
end
context 'widgets spanning the whole grid' do
before do
grid.widgets.build(start_row: 1,
end_row: grid.row_count + 1,
start_column: 1,
end_column: grid.column_count + 1,
identifier: 'work_packages_assigned')
end
it 'is valid' do
expect(instance.validate)
.to be_truthy
end
end
context 'widgets having start after end column' do
before do
grid.widgets.build(start_row: 1,
end_row: 2,
start_column: 4,
end_column: 3,
identifier: 'work_packages_assigned')
end
it 'is invalid' do
expect(instance.validate)
.to be_falsey
end
it 'notes the error' do
instance.validate
expect(instance.errors.details[:widgets])
.to match_array [{ error: :end_before_start }]
end
end
context 'widgets having start after end row' do
before do
grid.widgets.build(start_row: 4,
end_row: 2,
start_column: 1,
end_column: 3,
identifier: 'work_packages_assigned')
end
it 'is invalid' do
expect(instance.validate)
.to be_falsey
end
it 'notes the error' do
instance.validate
expect(instance.errors.details[:widgets])
.to match_array [{ error: :end_before_start }]
end
end
context 'widgets having start equals end column' do
before do
grid.widgets.build(start_row: 1,
end_row: 2,
start_column: 4,
end_column: 3,
identifier: 'work_packages_assigned')
end
it 'is invalid' do
expect(instance.validate)
.to be_falsey
end
it 'notes the error' do
instance.validate
expect(instance.errors.details[:widgets])
.to match_array [{ error: :end_before_start }]
end
end
context 'widgets having start equals end row' do
before do
grid.widgets.build(start_row: 2,
end_row: 2,
start_column: 1,
end_column: 3,
identifier: 'work_packages_assigned')
end
it 'is invalid' do
expect(instance.validate)
.to be_falsey
end
it 'notes the error' do
instance.validate
expect(instance.errors.details[:widgets])
.to match_array [{ error: :end_before_start }]
end
end
end

@ -208,6 +208,12 @@ describe ::API::V3::Utilities::PathHelper do
it_behaves_like 'api v3 path', '/grids/42'
end
describe '#grid_form' do
subject { helper.grid_form(42) }
it_behaves_like 'api v3 path', '/grids/42/form'
end
describe '#message' do
subject { helper.message(42) }

@ -46,7 +46,6 @@ describe "POST /api/v3/grids/form", type: :request, content_type: :json do
end
describe '#post' do
# TODO: spec errors
before do
post path, params.to_json, 'CONTENT_TYPE' => 'application/json'
end
@ -91,6 +90,11 @@ describe "POST /api/v3/grids/form", type: :request, content_type: :json do
.at_path('_embedded/validationErrors/page/message')
end
it 'does not have a commit link' do
expect(subject.body)
.not_to have_json_path('_links/commit')
end
context 'with /my/page for the page value' do
let(:params) do
{
@ -158,6 +162,12 @@ describe "POST /api/v3/grids/form", type: :request, content_type: :json do
.to be_json_eql({}.to_json)
.at_path('_embedded/validationErrors')
end
it 'has a commit link' do
expect(subject.body)
.to be_json_eql(api_v3_paths.grids.to_json)
.at_path('_links/commit/href')
end
end
context 'with an unsupported widget identifier' do

@ -255,9 +255,13 @@ describe 'API v3 Grids resource', type: :request, content_type: :json do
.to be_json_eql('Error'.to_json)
.at_path('_type')
expect(subject.body)
.to be_json_eql("Widgets is outside of the grid.".to_json)
.at_path('_embedded/errors/0/message')
expect(subject.body)
.to be_json_eql("Number of rows must be greater than 0.".to_json)
.at_path('message')
.at_path('_embedded/errors/1/message')
end
it 'does not persist the changes to widgets' do
@ -398,12 +402,16 @@ describe 'API v3 Grids resource', type: :request, content_type: :json do
.at_path('_type')
expect(subject.body)
.to be_json_eql("Number of rows must be greater than 0.".to_json)
.to be_json_eql("Widgets is outside of the grid.".to_json)
.at_path('_embedded/errors/0/message')
expect(subject.body)
.to be_json_eql("Number of columns must be greater than 0.".to_json)
.to be_json_eql("Number of rows must be greater than 0.".to_json)
.at_path('_embedded/errors/1/message')
expect(subject.body)
.to be_json_eql("Number of columns must be greater than 0.".to_json)
.at_path('_embedded/errors/2/message')
end
end
@ -414,8 +422,8 @@ describe 'API v3 Grids resource', type: :request, content_type: :json do
"columnCount": 5,
"widgets": [{
"identifier": "work_packages_assigned",
"startRow": 4,
"endRow": 8,
"startRow": 2,
"endRow": 4,
"startColumn": 2,
"endColumn": 5
}]

@ -0,0 +1,177 @@
#-- copyright
# OpenProject is a project management system.
# Copyright (C) 2012-2018 the OpenProject Foundation (OPF)
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2017 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See docs/COPYRIGHT.rdoc for more details.
#++
require 'spec_helper'
require 'rack/test'
describe "PATCH /api/v3/grids/:id/form", type: :request, content_type: :json do
include Rack::Test::Methods
include API::V3::Utilities::PathHelper
shared_let(:current_user) do
FactoryBot.create(:user)
end
let(:grid) do
grid = MyPageGrid.new_default(current_user)
grid.save!
grid
end
let(:path) { api_v3_paths.grid_form(grid.id) }
let(:params) { {} }
subject(:response) { last_response }
before do
login_as(current_user)
end
describe '#post' do
before do
post path, params.to_json, 'CONTENT_TYPE' => 'application/json'
end
it 'returns 200 OK' do
expect(subject.status)
.to eql 200
end
it 'is of type form' do
expect(subject.body)
.to be_json_eql("Form".to_json)
.at_path('_type')
end
it 'contains a Schema disallowing setting page' do
expect(subject.body)
.to be_json_eql("Schema".to_json)
.at_path('_embedded/schema/_type')
expect(subject.body)
.to be_json_eql(false.to_json)
.at_path('_embedded/schema/page/writable')
end
it 'contains the current data in the payload' do
expected = {
"rowCount": 4,
"columnCount": 5,
"widgets": [
{
"_type": "GridWidget",
"identifier": "work_packages_assigned",
"startRow": 4,
"endRow": 5,
"startColumn": 1,
"endColumn": 2
},
{
"_type": "GridWidget",
"identifier": "work_packages_created",
"startRow": 1,
"endRow": 2,
"startColumn": 1,
"endColumn": 2
},
{
"_type": "GridWidget",
"identifier": "work_packages_watched",
"startRow": 2,
"endRow": 4,
"startColumn": 4,
"endColumn": 5
},
{
"_type": "GridWidget",
"identifier": "work_packages_calendar",
"startRow": 1,
"endRow": 2,
"startColumn": 4,
"endColumn": 6
}
],
"_links": {
"page": {
"href": "/my/page",
"type": "text/html"
}
}
}
expect(subject.body)
.to be_json_eql(expected.to_json)
.at_path('_embedded/payload')
end
it 'has a commit link' do
expect(subject.body)
.to be_json_eql(api_v3_paths.grid(grid.id).to_json)
.at_path('_links/commit/href')
end
context 'with some value for the page value' do
let(:params) do
{
'_links': {
'page': {
'href': '/some/path'
}
}
}
end
it 'has a validation error on page as the value is not writeable' do
expect(subject.body)
.to be_json_eql("You must not write a read-only attribute.".to_json)
.at_path('_embedded/validationErrors/page/message')
end
end
context 'with an unsupported widget identifier' do
let(:params) do
{
"widgets": [
{
"_type": "GridWidget",
"identifier": "bogus_identifier",
"startRow": 4,
"endRow": 5,
"startColumn": 1,
"endColumn": 2
}
]
}
end
it 'has a validationError on widget' do
expect(subject.body)
.to be_json_eql("Widgets is not set to one of the allowed values.".to_json)
.at_path('_embedded/validationErrors/widgets/message')
end
end
end
end
Loading…
Cancel
Save