From c80c4b0d8ff5ddf36b8daff8ba95eb777b5b8597 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 14 Jan 2015 13:23:46 +0100 Subject: [PATCH 01/47] fix table alignment [ci skip] --- doc/apiv3-documentation.apib | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/apiv3-documentation.apib b/doc/apiv3-documentation.apib index b9cfad5ed9..cd7586ec22 100644 --- a/doc/apiv3-documentation.apib +++ b/doc/apiv3-documentation.apib @@ -63,7 +63,7 @@ Links to other resources in the API are represented uniformly by so called link | Property | Description | Type | Required | Nullable | Default | |:---------:| ------------------------------------------------------------------------ | ------- |:--------:|:--------:| ------- | -| href | URL to the referenced resource (might be relative) | String | ✓ | ✓ | | +| href | URL to the referenced resource (might be relative) | String | ✓ | ✓ | | | title | Representative label for the resource | String | | | | | templated | If true the `href` contains parts that need to be replaced by the client | Boolean | | | false | | method | The HTTP verb to use when requesting the resource | String | | | GET | From b894ffeed644551f665e987a867088b84ca98f5b Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 14 Jan 2015 13:26:13 +0100 Subject: [PATCH 02/47] harmonize user spec - add linked properties - specify status and its values - also add delete action link to actions [ci skip] --- doc/apiv3-documentation.apib | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/doc/apiv3-documentation.apib b/doc/apiv3-documentation.apib index cd7586ec22..e2d8aee603 100644 --- a/doc/apiv3-documentation.apib +++ b/doc/apiv3-documentation.apib @@ -1943,10 +1943,16 @@ This endpoint lists the types that are *available* in a given project. # Group Users ## Actions: -| Link | Description | Condition | -|:-------------------:| -------------------------------------------------------------------- | ----------------------------------------- | -| lock | Restrict the user from logging in and performing any actions | not locked; **Permission**: Administrator | -| unlock | Allow a locked user to login and act again | locked; **Permission**: Administrator | +| Link | Description | Condition | +|:-------------------:| -------------------------------------------------------------------- | ------------------------------------------ | +| lock | Restrict the user from logging in and performing any actions | not locked; **Permission**: Administrator | +| unlock | Allow a locked user to login and act again | locked; **Permission**: Administrator | +| delete | Permanently remove a user from the instance | **Permission**: Administrator, self-delete | + +## Linked Properties: +| Link | Description | Type | Constraints | Supported operations | +|:---------:|-------------------------------------------- | ------------- | --------------------- | -------------------- | +| self | This user | User | not null | READ | ## Properties: | Property | Description | Type | Constraints | Supported operations | @@ -1958,9 +1964,15 @@ This endpoint lists the types that are *available* in a given project. | name | User's full name, formatting depends on instance settings | String | | READ | | mail | User's email | String | | READ | | avatar | URL to user's avatar | String | | READ | +| status | The current activation status of the user (see below) | String | | READ | | createdAt | Time of creation | DateTime | | READ | | updatedAt | Time of the most recent change to the user | DateTime | | READ | +The `status` of a user can be one of: + +* `active` - the user can act normally +* `registered` - the user just registered to the instance, activation by an administrator is required +* `locked` - the user was locked and can't login ## User [/api/v3/users/{id}] @@ -1991,9 +2003,9 @@ This endpoint lists the types that are *available* in a given project. "lastName": "Sheppard", "mail": "shep@mail.com", "avatar": "https://gravatar/avatar", + "status": "active", "createdAt": "2014-05-21T08:51:20Z", - "updatedAt": "2014-05-21T08:51:20Z", - "status": "active" + "updatedAt": "2014-05-21T08:51:20Z" } ## View user [GET] From 2e2889ff0638fd57b044e27e39d046f478a7bab3 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 14 Jan 2015 13:37:54 +0100 Subject: [PATCH 03/47] harmonize user examples [ci skip] --- doc/apiv3-documentation.apib | 365 +++++++++++++++++------------------ 1 file changed, 175 insertions(+), 190 deletions(-) diff --git a/doc/apiv3-documentation.apib b/doc/apiv3-documentation.apib index e2d8aee603..2396611a31 100644 --- a/doc/apiv3-documentation.apib +++ b/doc/apiv3-documentation.apib @@ -2661,27 +2661,6 @@ For more details and all possible responses see the general specification of [Fo ## Add Watcher [/api/v3/work_packages/{work_package_id}/watchers] -+ Model - + Body - - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/1", - "title": "John Sheppard - j.sheppard" - } - }, - "id": 1, - "login": "j.sheppard", - "firstName": "John", - "lastName": "Sheppard", - "mail": "shep@mail.com", - "avatar": "https://gravatar/avatar", - "createdAt": "2014-05-21T08:51:20Z", - "updatedAt": "2014-05-21T08:51:20Z" - } - ## Add watcher [POST] + Parameters @@ -2695,7 +2674,7 @@ For more details and all possible responses see the general specification of [Fo + Response 200 (application/hal+json) - [Add Watcher][] + [User][] ## Remove Watcher [/api/v3/work_packages/{work_package_id}/watchers/{id}] @@ -2716,65 +2695,67 @@ For more details and all possible responses see the general specification of [Fo "_links": { "self": { "href": "/api/v3/work_packages/14/available_watchers" } }, - "total": 3, - "count": 3, + "total": 2, + "count": 2, "_type": "Collection", "_embedded": { "elements": [ - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/4581", - "title": "Mister X" - } - }, - "id": 4581, - "login": "m@x.org", - "firstName": "Mister", - "lastName": "X", - "name": "Mister X", - "mail": "m@x.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-10-01T16:25:17Z", - "updatedAt": "2013-10-02T09:33:42Z" - }, - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/972", - "title": "Mister Y" - } - }, - "id": 972, - "login": "m@y.org", - "firstName": "Mister", - "lastName": "Y", - "name": "Mister Y", - "mail": "m@y.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-01-11T11:47:06Z", - "updatedAt": "2013-11-18T07:13:56Z" - }, - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/4724", - "title": "Mister Z" - } - }, - "id": 4724, - "login": "m@z.org", - "firstName": "Mister", - "lastName": "Z", - "name": "Mister Z", - "mail": "m@z.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-10-08T09:28:46Z", - "updatedAt": "2013-10-08T09:28:52Z" - }] + { + "_type": "User", + "_links": { + "self": { + "href": "/api/v3/users/1", + "title": "John Sheppard - j.sheppard" + }, + "lock": { + "href": "/api/v3/users/1/lock", + "title": "Set lock on j.sheppard" + "method": "POST" + }, + "delete": { + "href": "/api/v3/users/1", + "title": "Delete j.sheppard" + "method": "DELETE" + } + }, + "id": 1, + "login": "j.sheppard", + "firstName": "John", + "lastName": "Sheppard", + "mail": "shep@mail.com", + "avatar": "https://gravatar/avatar", + "status": "active", + "createdAt": "2014-05-21T08:51:20Z", + "updatedAt": "2014-05-21T08:51:20Z" + }, + { + "_type": "User", + "_links": { + "self": { + "href": "/api/v3/users/2", + "title": "Jim Sheppard - j.sheppard2" + }, + "lock": { + "href": "/api/v3/users/2/lock", + "title": "Set lock on j.sheppard2" + "method": "POST" + }, + "delete": { + "href": "/api/v3/users/2", + "title": "Delete j.sheppard2" + "method": "DELETE" + } + }, + "id": 2, + "login": "j.sheppard2", + "firstName": "Jim", + "lastName": "Sheppard", + "mail": "shep@mail.net", + "avatar": "https://gravatar/avatar", + "status": "active", + "createdAt": "2014-05-21T08:51:20Z", + "updatedAt": "2014-05-21T08:51:20Z" + }] } } @@ -2863,65 +2844,67 @@ updated activity. "templated": true } }, - "total": 3, - "count": 3, + "total": 2, + "count": 2, "_type": "Collection", "_embedded": { "elements": [ - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/4581", - "title": "Mister X" - } - }, - "id": 4581, - "login": "m@x.org", - "firstName": "Mister", - "lastName": "X", - "name": "Mister X", - "mail": "m@x.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-10-01T16:25:17Z", - "updatedAt": "2013-10-02T09:33:42Z" - }, - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/972", - "title": "Mister Y" - } - }, - "id": 972, - "login": "m@y.org", - "firstName": "Mister", - "lastName": "Y", - "name": "Mister Y", - "mail": "m@y.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-01-11T11:47:06Z", - "updatedAt": "2013-11-18T07:13:56Z" - }, - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/4724", - "title": "Mister Z" - } - }, - "id": 4724, - "login": "m@z.org", - "firstName": "Mister", - "lastName": "Z", - "name": "Mister Z", - "mail": "m@z.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-10-08T09:28:46Z", - "updatedAt": "2013-10-08T09:28:52Z" - }] + { + "_type": "User", + "_links": { + "self": { + "href": "/api/v3/users/1", + "title": "John Sheppard - j.sheppard" + }, + "lock": { + "href": "/api/v3/users/1/lock", + "title": "Set lock on j.sheppard" + "method": "POST" + }, + "delete": { + "href": "/api/v3/users/1", + "title": "Delete j.sheppard" + "method": "DELETE" + } + }, + "id": 1, + "login": "j.sheppard", + "firstName": "John", + "lastName": "Sheppard", + "mail": "shep@mail.com", + "avatar": "https://gravatar/avatar", + "status": "active", + "createdAt": "2014-05-21T08:51:20Z", + "updatedAt": "2014-05-21T08:51:20Z" + }, + { + "_type": "User", + "_links": { + "self": { + "href": "/api/v3/users/2", + "title": "Jim Sheppard - j.sheppard2" + }, + "lock": { + "href": "/api/v3/users/2/lock", + "title": "Set lock on j.sheppard2" + "method": "POST" + }, + "delete": { + "href": "/api/v3/users/2", + "title": "Delete j.sheppard2" + "method": "DELETE" + } + }, + "id": 2, + "login": "j.sheppard2", + "firstName": "Jim", + "lastName": "Sheppard", + "mail": "shep@mail.net", + "avatar": "https://gravatar/avatar", + "status": "active", + "createdAt": "2014-05-21T08:51:20Z", + "updatedAt": "2014-05-21T08:51:20Z" + }] } } @@ -2969,65 +2952,67 @@ Gets a list of users that can be assigned to work packages in the given project. "templated": true } }, - "total": 3, - "count": 3, + "total": 2, + "count": 2, "_type": "Collection", "_embedded": { "elements": [ - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/4581", - "title": "Mister X" - } - }, - "id": 4581, - "login": "m@x.org", - "firstName": "Mister", - "lastName": "X", - "name": "Mister X", - "mail": "m@x.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-10-01T16:25:17Z", - "updatedAt": "2013-10-02T09:33:42Z" - }, - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/972", - "title": "Mister Y" - } - }, - "id": 972, - "login": "m@y.org", - "firstName": "Mister", - "lastName": "Y", - "name": "Mister Y", - "mail": "m@y.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-01-11T11:47:06Z", - "updatedAt": "2013-11-18T07:13:56Z" - }, - { - "_type": "User", - "_links": { - "self": { - "href": "/api/v3/users/4724", - "title": "Mister Z" - } - }, - "id": 4724, - "login": "m@z.org", - "firstName": "Mister", - "lastName": "Z", - "name": "Mister Z", - "mail": "m@z.org", - "avatar": "http://gravatar.com/avatar/12345678901234567890123456789012", - "createdAt": "2013-10-08T09:28:46Z", - "updatedAt": "2013-10-08T09:28:52Z" - }] + { + "_type": "User", + "_links": { + "self": { + "href": "/api/v3/users/1", + "title": "John Sheppard - j.sheppard" + }, + "lock": { + "href": "/api/v3/users/1/lock", + "title": "Set lock on j.sheppard" + "method": "POST" + }, + "delete": { + "href": "/api/v3/users/1", + "title": "Delete j.sheppard" + "method": "DELETE" + } + }, + "id": 1, + "login": "j.sheppard", + "firstName": "John", + "lastName": "Sheppard", + "mail": "shep@mail.com", + "avatar": "https://gravatar/avatar", + "status": "active", + "createdAt": "2014-05-21T08:51:20Z", + "updatedAt": "2014-05-21T08:51:20Z" + }, + { + "_type": "User", + "_links": { + "self": { + "href": "/api/v3/users/2", + "title": "Jim Sheppard - j.sheppard2" + }, + "lock": { + "href": "/api/v3/users/2/lock", + "title": "Set lock on j.sheppard2" + "method": "POST" + }, + "delete": { + "href": "/api/v3/users/2", + "title": "Delete j.sheppard2" + "method": "DELETE" + } + }, + "id": 2, + "login": "j.sheppard2", + "firstName": "Jim", + "lastName": "Sheppard", + "mail": "shep@mail.net", + "avatar": "https://gravatar/avatar", + "status": "active", + "createdAt": "2014-05-21T08:51:20Z", + "updatedAt": "2014-05-21T08:51:20Z" + }] } } From c11bd5b8d51d887de5233ce6a1eb3e9bca78b3af Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 14 Jan 2015 13:54:08 +0100 Subject: [PATCH 04/47] actually include delete link how hard can ruby be anyway... I will for certain regret that commit soon ^^ --- lib/api/v3/users/user_representer.rb | 8 ++++++++ spec/lib/api/v3/users/user_representer_spec.rb | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index 56972d377a..d0963a57ef 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -73,6 +73,14 @@ module API method: :delete } if current_user_is_admin && represented.activatable? end + + link :delete do + { + href: api_v3_paths.user(represented.id), + title: "Delete #{represented.login}", + method: :delete + } if DeleteUserService.new.deletion_allowed? represented, current_user + end link :removeWatcher do { diff --git a/spec/lib/api/v3/users/user_representer_spec.rb b/spec/lib/api/v3/users/user_representer_spec.rb index f314704bee..542079c547 100644 --- a/spec/lib/api/v3/users/user_representer_spec.rb +++ b/spec/lib/api/v3/users/user_representer_spec.rb @@ -87,6 +87,10 @@ describe ::API::V3::Users::UserRepresenter do user.lock expect(subject).to have_json_path('_links/unlock/href') end + end + + it 'should link to delete' do + expect(subject).to have_json_path('_links/delete/href') end end end From d1e3f23467584db2ec58dc6418a5e604d19109a5 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 14 Jan 2015 13:57:19 +0100 Subject: [PATCH 05/47] obey style rules --- lib/api/v3/users/user_representer.rb | 4 ++-- spec/lib/api/v3/users/user_representer_spec.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index d0963a57ef..bda487d6bd 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -73,8 +73,8 @@ module API method: :delete } if current_user_is_admin && represented.activatable? end - - link :delete do + + link :delete do { href: api_v3_paths.user(represented.id), title: "Delete #{represented.login}", diff --git a/spec/lib/api/v3/users/user_representer_spec.rb b/spec/lib/api/v3/users/user_representer_spec.rb index 542079c547..03ce3dccd9 100644 --- a/spec/lib/api/v3/users/user_representer_spec.rb +++ b/spec/lib/api/v3/users/user_representer_spec.rb @@ -88,8 +88,8 @@ describe ::API::V3::Users::UserRepresenter do expect(subject).to have_json_path('_links/unlock/href') end end - - it 'should link to delete' do + + it 'should link to delete' do expect(subject).to have_json_path('_links/delete/href') end end From e1c02db9abfdcb05c02ad17fe1144684a1e1acac Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 14 Jan 2015 15:02:43 +0100 Subject: [PATCH 06/47] fix reference to current user --- lib/api/v3/users/user_representer.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index bda487d6bd..7be51116ff 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -79,7 +79,7 @@ module API href: api_v3_paths.user(represented.id), title: "Delete #{represented.login}", method: :delete - } if DeleteUserService.new.deletion_allowed? represented, current_user + } if current_user_can_delete_user(represented) end link :removeWatcher do @@ -115,6 +115,10 @@ module API def current_user_allowed_to(permission, work_package) @current_user && @current_user.allowed_to?(permission, work_package.project) end + + def current_user_can_delete_user(other_user) + @current_user && DeleteUserService.new.deletion_allowed?(other_user, @current_user) + end end end end From c28a581ec45b865f354ff26048f825d6e4d6d02d Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Wed, 14 Jan 2015 15:12:19 +0100 Subject: [PATCH 07/47] test for missing delete link --- spec/lib/api/v3/users/user_representer_spec.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/lib/api/v3/users/user_representer_spec.rb b/spec/lib/api/v3/users/user_representer_spec.rb index 03ce3dccd9..555f89ff55 100644 --- a/spec/lib/api/v3/users/user_representer_spec.rb +++ b/spec/lib/api/v3/users/user_representer_spec.rb @@ -73,6 +73,10 @@ describe ::API::V3::Users::UserRepresenter do expect(subject).to_not have_json_path('_links/lock/href') expect(subject).to_not have_json_path('_links/unlock/href') end + + it 'should not link to delete' do + expect(subject).to_not have_json_path('_links/delete/href') + end end context 'when current_user is admin' do From abe13dc671c3b7be5214b4833c31ab5f77dce77d Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 16 Dec 2014 07:53:23 +0100 Subject: [PATCH 08/47] GET versions/:id endpoint --- lib/api/v3/projects/projects_api.rb | 2 +- lib/api/v3/root.rb | 1 + lib/api/v3/versions/projects_versions_api.rb | 48 ++++++++++++++++++ .../version_collection_representer.rb | 1 + lib/api/v3/versions/versions_api.rb | 16 +++--- spec/requests/api/v3/version_resource_spec.rb | 50 +++++++++++++++++-- 6 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 lib/api/v3/versions/projects_versions_api.rb diff --git a/lib/api/v3/projects/projects_api.rb b/lib/api/v3/projects/projects_api.rb index b1629cfe7f..5f2c6b0f0e 100644 --- a/lib/api/v3/projects/projects_api.rb +++ b/lib/api/v3/projects/projects_api.rb @@ -48,7 +48,7 @@ module API mount API::V3::Projects::AvailableAssigneesAPI mount API::V3::Projects::AvailableResponsiblesAPI mount API::V3::Categories::CategoriesAPI - mount API::V3::Versions::VersionsAPI + mount API::V3::Versions::ProjectsVersionsAPI end end end diff --git a/lib/api/v3/root.rb b/lib/api/v3/root.rb index 4eeeed1f33..397f055ed3 100644 --- a/lib/api/v3/root.rb +++ b/lib/api/v3/root.rb @@ -44,6 +44,7 @@ module API mount ::API::V3::Render::RenderAPI mount ::API::V3::Statuses::StatusesAPI mount ::API::V3::Users::UsersAPI + mount ::API::V3::Versions::VersionsAPI mount ::API::V3::WorkPackages::WorkPackagesAPI get '/' do diff --git a/lib/api/v3/versions/projects_versions_api.rb b/lib/api/v3/versions/projects_versions_api.rb new file mode 100644 index 0000000000..4ca7bb8a86 --- /dev/null +++ b/lib/api/v3/versions/projects_versions_api.rb @@ -0,0 +1,48 @@ +#-- 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 Versions + class ProjectsVersionsAPI < Grape::API + resources :versions do + before do + @versions = @project.shared_versions.all + end + + get do + VersionCollectionRepresenter.new(@versions, + @versions.count, + api_v3_paths.versions(@project.identifier)) + end + end + end + end + end +end diff --git a/lib/api/v3/versions/version_collection_representer.rb b/lib/api/v3/versions/version_collection_representer.rb index 07efc7e4ad..2598b66cd5 100644 --- a/lib/api/v3/versions/version_collection_representer.rb +++ b/lib/api/v3/versions/version_collection_representer.rb @@ -28,6 +28,7 @@ #++ require 'roar/decorator' +require 'roar/json' require 'roar/json/collection' require 'roar/json/hal' diff --git a/lib/api/v3/versions/versions_api.rb b/lib/api/v3/versions/versions_api.rb index f4db6a7821..e5ad8af22e 100644 --- a/lib/api/v3/versions/versions_api.rb +++ b/lib/api/v3/versions/versions_api.rb @@ -32,14 +32,16 @@ module API module Versions class VersionsAPI < Grape::API resources :versions do - before do - @versions = @project.shared_versions.all - end - get do - VersionCollectionRepresenter.new(@versions, - @versions.count, - api_v3_paths.versions(@project.identifier)) + namespace ':id' do + + before do + @version = Version.find(params[:id]) + end + + get do + VersionRepresenter.new(@version) + end end end end diff --git a/spec/requests/api/v3/version_resource_spec.rb b/spec/requests/api/v3/version_resource_spec.rb index 7d939a5031..8d1a0f66a2 100644 --- a/spec/requests/api/v3/version_resource_spec.rb +++ b/spec/requests/api/v3/version_resource_spec.rb @@ -38,11 +38,12 @@ describe 'API v3 Version resource' do let(:versions) { FactoryGirl.create_list(:version, 4, project: project) } let(:other_versions) { FactoryGirl.create_list(:version, 2) } - describe '#get' do - subject(:response) { last_response } + subject(:response) { last_response } + + describe '#get (index)' do + let(:get_path) { "/api/v3/projects/#{project.id}/versions" } context 'logged in user' do - let(:get_path) { "/api/v3/projects/#{project.id}/versions" } before do allow(User).to receive(:current).and_return current_user member = FactoryGirl.build(:member, user: current_user, project: project) @@ -58,4 +59,47 @@ describe 'API v3 Version resource' do it_behaves_like 'API V3 collection response', 4, 4, 'Version' end end + + describe '#get (:id)' do + let(:version_in_project) { FactoryGirl.build(:version, project: project) } + + let(:get_path) { "/api/v3/versions/#{version_in_project.id}" } + + let(:expected_response) do + { + '_type' => 'Version' + } + end + + context 'logged in user with permissions' do + let(:current_user) do + user = FactoryGirl.create(:user, + member_in_project: project, + member_through_role: role) + + allow(User).to receive(:current).and_return user + + user + end + + before do + version_in_project.save! + + get get_path + end + + it 'responds with 200' do + expect(last_response.status).to eq(200) + end + + it 'returns the work package' do + expected = { + _type: 'Version', + name: version_in_project.name + }.to_json + + expect(last_response.body).to be_json_eql(expected) + end + end + end end From 1c8fced609b20edf73c9af36ac18add74d11a66f Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 16 Dec 2014 15:59:43 +0100 Subject: [PATCH 09/47] adds scope of projects on version --- app/models/version.rb | 2 + app/models/version/project_sharing.rb | 157 ++++++++++++++++++++++++++ spec/models/version_spec.rb | 111 ++++++++++++++++++ 3 files changed, 270 insertions(+) create mode 100644 app/models/version/project_sharing.rb diff --git a/app/models/version.rb b/app/models/version.rb index 77dfeac837..45c9176c64 100644 --- a/app/models/version.rb +++ b/app/models/version.rb @@ -31,6 +31,8 @@ class Version < ActiveRecord::Base include Redmine::SafeAttributes extend DeprecatedAlias + include Version::ProjectSharing + after_update :update_issues_from_sharing_change belongs_to :project has_many :fixed_issues, class_name: 'WorkPackage', foreign_key: 'fixed_version_id', dependent: :nullify diff --git a/app/models/version/project_sharing.rb b/app/models/version/project_sharing.rb new file mode 100644 index 0000000000..4cdda41d67 --- /dev/null +++ b/app/models/version/project_sharing.rb @@ -0,0 +1,157 @@ +#-- 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 Version::ProjectSharing + # Returns all projects the version is available in + def projects + Project.scoped.joins(project_sharing_join) + end + + private + + def project_sharing_join + projects = Project.scoped + projects_table = projects.arel_table + versions_table = Version.scoped.arel_table + + sharing_inner_select = project_sharing_select(versions_table) + + join_condition = project_sharing_join_condition(sharing_inner_select, projects_table) + join_on = projects_table.create_on(join_condition) + + projects_table.create_join(sharing_inner_select, join_on) + end + + def project_sharing_select(versions_table) + sharing_select = if sharing == 'tree' + project_sharing_tree_select(versions_table) + else + project_sharing_default_select(versions_table) + end + + sharing_id_condition = versions_table[:id].eq(id) + + sharing_select + .where(sharing_id_condition) + .as('sharing') + end + + def project_sharing_tree_select(versions_table) + hierarchy_table = Project.scoped.arel_table + + roots_table = Project.arel_table.alias('roots') + roots_join_condition = project_sharing_tree_root_join_condition(roots_table, hierarchy_table) + sharing_select = join_project_and_version(hierarchy_table, versions_table) + + sharing_select + .join(roots_table) + .on(roots_join_condition) + + necessary_sharing_fields(sharing_select, + roots_table, + versions_table) + end + + def project_sharing_default_select(versions_table) + hierarchy_table = Project.scoped.arel_table + + sharing_select = join_project_and_version(hierarchy_table, versions_table) + + necessary_sharing_fields(sharing_select, + hierarchy_table, + versions_table) + end + + def necessary_sharing_fields(sharing_select, projects_table, versions_table) + sharing_select + .project(projects_table[:id], + versions_table[:id].as('version_id'), + projects_table[:lft], + projects_table[:rgt], + versions_table[:sharing]) + end + + def join_project_and_version(projects_table, versions_table) + join_condition = projects_table[:id].eq(versions_table[:project_id]) + + projects_table + .join(versions_table) + .on(join_condition) + end + + def project_sharing_tree_root_join_condition(roots_table, hierarchy_table) + roots_table[:lft].lteq(hierarchy_table[:lft]) + .and(roots_table[:rgt].gteq(hierarchy_table[:rgt])) + .and(roots_table[:parent_id].eq(nil)) + end + + def project_sharing_join_condition(sharing_table, projects_table) + case self[:sharing] + when 'tree' + project_sharing_tree_join_condition(sharing_table, projects_table) + when 'descendants' + project_sharing_descendants_join_condition(sharing_table, projects_table) + when 'hierarchy' + project_sharing_hierarchy_join_condition(sharing_table, projects_table) + when 'system' + Arel::Nodes::True.new + else + sharing_table[:id].eq(projects_table[:id]) + end + end + + def project_sharing_tree_join_condition(sharing_table, projects_table) + projects_table[:lft].gteq(sharing_table[:lft]) + .and(projects_table[:rgt].lteq(sharing_table[:rgt])) + end + + def project_sharing_descendants_join_condition(sharing_table, projects_table) + project_sharing_equal_condition(sharing_table, projects_table) + .or(project_sharing_below_condition(sharing_table, projects_table)) + end + + def project_sharing_hierarchy_join_condition(sharing_table, projects_table) + project_sharing_descendants_join_condition(sharing_table, projects_table) + .or(project_sharing_above_condition(sharing_table, projects_table)) + end + + def project_sharing_equal_condition(sharing_table, projects_table) + sharing_table[:id].eq(projects_table[:id]) + end + + def project_sharing_above_condition(sharing_table, projects_table) + projects_table[:lft].lt(sharing_table[:lft]) + .and(projects_table[:rgt].gt(sharing_table[:rgt])) + end + + def project_sharing_below_condition(sharing_table, projects_table) + projects_table[:lft].gt(sharing_table[:lft]) + .and(projects_table[:rgt].lt(sharing_table[:rgt])) + end +end diff --git a/spec/models/version_spec.rb b/spec/models/version_spec.rb index 9e8e6d0ae3..0fbf6a455f 100644 --- a/spec/models/version_spec.rb +++ b/spec/models/version_spec.rb @@ -81,4 +81,115 @@ describe Version, type: :model do expect(Version.systemwide.all).to be_empty end end + + context '#projects' do + let(:grand_parent_project) do + FactoryGirl.build(:project, name: 'grand_parent_project') + end + let(:parent_project) do + FactoryGirl.build(:project, parent: grand_parent_project, name: 'parent_project') + end + let(:sibling_parent_project) do + FactoryGirl.build(:project, parent: grand_parent_project, name: 'sibling_parent_project') + end + let(:child_project) do + FactoryGirl.build(:project, parent: parent_project, name: 'child_project') + end + let(:sibling_project) do + FactoryGirl.build(:project, parent: parent_project, name: 'sibling_project') + end + let(:unrelated_project) do + FactoryGirl.build(:project, name: 'unrelated_project') + end + + let(:unshared_version) do + FactoryGirl.build(:version, project: parent_project, sharing: 'none') + end + let(:hierarchy_shared_version) do + FactoryGirl.build(:version, project: parent_project, sharing: 'hierarchy') + end + let(:descendants_shared_version) do + FactoryGirl.build(:version, project: parent_project, sharing: 'descendants') + end + let(:system_shared_version) do + FactoryGirl.build(:version, project: parent_project, sharing: 'system') + end + let(:tree_shared_version) do + FactoryGirl.build(:version, project: parent_project, sharing: 'tree') + end + + def save_all_projects + grand_parent_project.save! + parent_project.save! + sibling_parent_project.save! + child_project.save! + sibling_project.save! + unrelated_project.save! + end + + before do + save_all_projects + end + + it 'returns a scope' do + unshared_version.save + + expect(unshared_version.projects).to be_a(ActiveRecord::Relation) + end + + it 'is empty for a new version' do + expect(Version.new.projects).to be_empty + end + + it 'returns project the version is defined in for unshared' do + unshared_version.save + + expect(unshared_version.projects).to match_array([parent_project]) + end + + it 'returns all projects the version is shared with (hierarchy)' do + hierarchy_shared_version.save! + + expect(hierarchy_shared_version.projects).to match_array([grand_parent_project, + parent_project, + child_project, + sibling_project]) + end + + it 'returns all projects the version is shared with (descendants)' do + descendants_shared_version.save! + + expect(descendants_shared_version.projects).to match_array([parent_project, + child_project, + sibling_project]) + end + + it 'returns all projects the version is shared with (tree)' do + tree_shared_version.save! + + expect(tree_shared_version.projects).to match_array([grand_parent_project, + parent_project, + sibling_parent_project, + child_project, + sibling_project]) + end + + it 'returns all projects the version is shared with (system)' do + system_shared_version.save! + + expect(system_shared_version.projects).to match_array([grand_parent_project, + parent_project, + sibling_parent_project, + child_project, + sibling_project, + unrelated_project]) + end + + it 'returns only the projects for the version although there is a system shared version' do + unshared_version.save + system_shared_version.save! + + expect(unshared_version.projects).to match_array([parent_project]) + end + end end From ad65c39a2f1193d37a41bf02bb663b4130dceb10 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 17 Dec 2014 14:51:35 +0100 Subject: [PATCH 10/47] authorization on version endpoint --- lib/api/root.rb | 15 +++++ lib/api/v3/versions/versions_api.rb | 12 ++++ spec/requests/api/v3/version_resource_spec.rb | 63 +++++++++++++++---- 3 files changed, 78 insertions(+), 12 deletions(-) diff --git a/lib/api/root.rb b/lib/api/root.rb index 845057bbb9..65cebec90f 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -75,6 +75,21 @@ module API raise API::Errors::Unauthorized unless is_authorized && allow is_authorized end + + # checks whether the user has + # any of the provided permission in any of the provided + # projects + def authorize_any(permissions, projects, user: current_user) + authorized = permissions.any? do |permission| + allowed_condition = Project.allowed_to_condition(user, permission) + allowed_projects = Project.where(allowed_condition) + + !(allowed_projects & projects).empty? + end + + raise API::Errors::Unauthorized unless authorized + authorized + end end rescue_from ActiveRecord::RecordNotFound do |e| diff --git a/lib/api/v3/versions/versions_api.rb b/lib/api/v3/versions/versions_api.rb index e5ad8af22e..5bea23fa22 100644 --- a/lib/api/v3/versions/versions_api.rb +++ b/lib/api/v3/versions/versions_api.rb @@ -37,6 +37,18 @@ module API before do @version = Version.find(params[:id]) + + authorized_for_version?(@version) + end + + helpers do + def authorized_for_version?(version) + projects = version.projects + + permissions = [:view_work_packages, :manage_versions] + + authorize_any(permissions, projects, user: current_user) + end end get do diff --git a/spec/requests/api/v3/version_resource_spec.rb b/spec/requests/api/v3/version_resource_spec.rb index 8d1a0f66a2..d1c354b2a3 100644 --- a/spec/requests/api/v3/version_resource_spec.rb +++ b/spec/requests/api/v3/version_resource_spec.rb @@ -35,6 +35,7 @@ describe 'API v3 Version resource' do let(:current_user) { FactoryGirl.create(:user) } let(:role) { FactoryGirl.create(:role, permissions: []) } let(:project) { FactoryGirl.create(:project, is_public: false) } + let(:other_project) { FactoryGirl.create(:project, is_public: false) } let(:versions) { FactoryGirl.create_list(:version, 4, project: project) } let(:other_versions) { FactoryGirl.create_list(:version, 2) } @@ -62,28 +63,53 @@ describe 'API v3 Version resource' do describe '#get (:id)' do let(:version_in_project) { FactoryGirl.build(:version, project: project) } + let(:version_in_other_project) do + FactoryGirl.build(:version, project: other_project, + sharing: 'system') + end let(:get_path) { "/api/v3/versions/#{version_in_project.id}" } - let(:expected_response) do - { - '_type' => 'Version' - } + let(:role) { FactoryGirl.create(:role, permissions: [:view_work_packages]) } + + let(:current_user) do + user = FactoryGirl.create(:user, + member_in_project: project, + member_through_role: role) + + allow(User).to receive(:current).and_return user + + user end context 'logged in user with permissions' do - let(:current_user) do - user = FactoryGirl.create(:user, - member_in_project: project, - member_through_role: role) + before do + version_in_project.save! + current_user - allow(User).to receive(:current).and_return user + get get_path + end - user + it 'responds with 200' do + expect(last_response.status).to eq(200) end + it 'returns the work package' do + expected = { + _type: 'Version', + name: version_in_project.name + }.to_json + + expect(last_response.body).to be_json_eql(expected) + end + end + + context 'logged in user with permission on project a version is shared with' do + let(:get_path) { "/api/v3/versions/#{version_in_other_project.id}" } + before do - version_in_project.save! + version_in_other_project.save! + current_user get get_path end @@ -95,11 +121,24 @@ describe 'API v3 Version resource' do it 'returns the work package' do expected = { _type: 'Version', - name: version_in_project.name + name: version_in_other_project.name }.to_json expect(last_response.body).to be_json_eql(expected) end end + + context 'logged in user without permission' do + let(:role) { FactoryGirl.create(:role, permissions: []) } + + before(:each) do + version_in_project.save! + current_user + + get get_path + end + + it_behaves_like 'unauthorized access' + end end end From 7cd45d42bc442728fe6c602eefafa669ac844f94 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 6 Jan 2015 15:09:25 +0100 Subject: [PATCH 11/47] enforce authorization on versions of project api resource --- lib/api/root.rb | 2 + lib/api/v3/versions/projects_versions_api.rb | 2 + .../api/v3/projects/version_resource_spec.rb | 80 +++++++++++++++++++ spec/requests/api/v3/version_resource_spec.rb | 57 ++++--------- 4 files changed, 99 insertions(+), 42 deletions(-) create mode 100644 spec/requests/api/v3/projects/version_resource_spec.rb diff --git a/lib/api/root.rb b/lib/api/root.rb index 65cebec90f..6522da56dc 100644 --- a/lib/api/root.rb +++ b/lib/api/root.rb @@ -80,6 +80,8 @@ module API # any of the provided permission in any of the provided # projects def authorize_any(permissions, projects, user: current_user) + projects = Array(projects) + authorized = permissions.any? do |permission| allowed_condition = Project.allowed_to_condition(user, permission) allowed_projects = Project.where(allowed_condition) diff --git a/lib/api/v3/versions/projects_versions_api.rb b/lib/api/v3/versions/projects_versions_api.rb index 4ca7bb8a86..fea1e7d427 100644 --- a/lib/api/v3/versions/projects_versions_api.rb +++ b/lib/api/v3/versions/projects_versions_api.rb @@ -34,6 +34,8 @@ module API resources :versions do before do @versions = @project.shared_versions.all + + authorize_any [:view_work_packages, :manage_versions], @project end get do diff --git a/spec/requests/api/v3/projects/version_resource_spec.rb b/spec/requests/api/v3/projects/version_resource_spec.rb new file mode 100644 index 0000000000..4c18b26a7b --- /dev/null +++ b/spec/requests/api/v3/projects/version_resource_spec.rb @@ -0,0 +1,80 @@ +#-- 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' +require 'rack/test' + +describe "API v3 project's versions resource" do + include Rack::Test::Methods + + let(:current_user) do + user = FactoryGirl.create(:user, + member_in_project: project, + member_through_role: role) + + allow(User).to receive(:current).and_return user + + user + end + let(:role) { FactoryGirl.create(:role, permissions: [:view_work_packages]) } + let(:project) { FactoryGirl.create(:project, is_public: false) } + let(:other_project) { FactoryGirl.create(:project, is_public: false) } + let(:versions) { FactoryGirl.create_list(:version, 4, project: project) } + let(:other_versions) { FactoryGirl.create_list(:version, 2) } + + subject(:response) { last_response } + + describe '#get (index)' do + let(:get_path) { "/api/v3/projects/#{project.id}/versions" } + + context 'logged in user' do + before do + current_user + + versions + other_versions + + get get_path + end + + it_behaves_like 'API V3 collection response', 4, 4, 'Version' + end + + context 'logged in user without permission' do + let(:role) { FactoryGirl.create(:role, permissions: []) } + + before do + current_user + + get get_path + end + + it_behaves_like 'unauthorized access' + end + end +end diff --git a/spec/requests/api/v3/version_resource_spec.rb b/spec/requests/api/v3/version_resource_spec.rb index d1c354b2a3..5e75ae58f8 100644 --- a/spec/requests/api/v3/version_resource_spec.rb +++ b/spec/requests/api/v3/version_resource_spec.rb @@ -32,56 +32,29 @@ require 'rack/test' describe 'API v3 Version resource' do include Rack::Test::Methods - let(:current_user) { FactoryGirl.create(:user) } - let(:role) { FactoryGirl.create(:role, permissions: []) } + let(:current_user) do + user = FactoryGirl.create(:user, + member_in_project: project, + member_through_role: role) + + allow(User).to receive(:current).and_return user + + user + end + let(:role) { FactoryGirl.create(:role, permissions: [:view_work_packages]) } let(:project) { FactoryGirl.create(:project, is_public: false) } let(:other_project) { FactoryGirl.create(:project, is_public: false) } - let(:versions) { FactoryGirl.create_list(:version, 4, project: project) } - let(:other_versions) { FactoryGirl.create_list(:version, 2) } + let(:version_in_project) { FactoryGirl.build(:version, project: project) } + let(:version_in_other_project) do + FactoryGirl.build(:version, project: other_project, + sharing: 'system') + end subject(:response) { last_response } - describe '#get (index)' do - let(:get_path) { "/api/v3/projects/#{project.id}/versions" } - - context 'logged in user' do - before do - allow(User).to receive(:current).and_return current_user - member = FactoryGirl.build(:member, user: current_user, project: project) - member.role_ids = [role.id] - member.save! - - versions - other_versions - - get get_path - end - - it_behaves_like 'API V3 collection response', 4, 4, 'Version' - end - end - describe '#get (:id)' do - let(:version_in_project) { FactoryGirl.build(:version, project: project) } - let(:version_in_other_project) do - FactoryGirl.build(:version, project: other_project, - sharing: 'system') - end - let(:get_path) { "/api/v3/versions/#{version_in_project.id}" } - let(:role) { FactoryGirl.create(:role, permissions: [:view_work_packages]) } - - let(:current_user) do - user = FactoryGirl.create(:user, - member_in_project: project, - member_through_role: role) - - allow(User).to receive(:current).and_return user - - user - end - context 'logged in user with permissions' do before do version_in_project.save! From 8a5beaa7cfecf93a97ad4c20eae53a554a41563a Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 6 Jan 2015 16:02:21 +0100 Subject: [PATCH 12/47] user providable in visible scope --- app/models/project.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index 7bf8974f26..df0c5f4068 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -117,7 +117,7 @@ class Project < ActiveRecord::Base scope :has_module, lambda { |mod| { conditions: ["#{Project.table_name}.id IN (SELECT em.project_id FROM #{EnabledModule.table_name} em WHERE em.name=?)", mod.to_s] } } scope :active, lambda { |*_args| where(status: STATUS_ACTIVE) } scope :public, lambda { |*_args| where(is_public: true) } - scope :visible, lambda { { conditions: Project.visible_by(User.current) } } + scope :visible, ->(user = User.current) { { conditions: Project.visible_by(user) } } # timelines stuff From 1086d672809249c63c1dd1a582632f7a2714b3a6 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 6 Jan 2015 16:01:25 +0100 Subject: [PATCH 13/47] add projects of version api endpoint --- .../project_collection_representer.rb | 45 +++++++++ lib/api/v3/utilities/path_helper.rb | 4 + lib/api/v3/versions/versions_api.rb | 2 + lib/api/v3/versions/versions_projects_api.rb | 51 ++++++++++ .../project_collection_representer_spec.rb | 41 ++++++++ spec/lib/api/v3/utilities/path_helper_spec.rb | 8 ++ .../api/v3/versions/project_resource_spec.rb | 97 +++++++++++++++++++ 7 files changed, 248 insertions(+) create mode 100644 lib/api/v3/projects/project_collection_representer.rb create mode 100644 lib/api/v3/versions/versions_projects_api.rb create mode 100644 spec/lib/api/v3/projects/project_collection_representer_spec.rb create mode 100644 spec/requests/api/v3/versions/project_resource_spec.rb diff --git a/lib/api/v3/projects/project_collection_representer.rb b/lib/api/v3/projects/project_collection_representer.rb new file mode 100644 index 0000000000..579061e4d5 --- /dev/null +++ b/lib/api/v3/projects/project_collection_representer.rb @@ -0,0 +1,45 @@ +#-- 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. +#++ + +require 'roar/decorator' +require 'roar/json' +require 'roar/json/collection' +require 'roar/json/hal' + +module API + module V3 + module Projects + class ProjectCollectionRepresenter < ::API::Decorators::Collection + def initialize(models, total, self_link) + super(models, total, self_link, ::API::V3::Projects::ProjectRepresenter) + end + end + end + end +end diff --git a/lib/api/v3/utilities/path_helper.rb b/lib/api/v3/utilities/path_helper.rb index b0c707e845..2da55bd889 100644 --- a/lib/api/v3/utilities/path_helper.rb +++ b/lib/api/v3/utilities/path_helper.rb @@ -110,6 +110,10 @@ module API "#{project(project_id)}/versions" end + def self.versions_projects(version_id) + "#{root}/versions/#{version_id}/projects" + end + def self.watcher(id, work_package_id) "#{work_package(work_package_id)}/watchers/#{id}" end diff --git a/lib/api/v3/versions/versions_api.rb b/lib/api/v3/versions/versions_api.rb index 5bea23fa22..60cee0fa0c 100644 --- a/lib/api/v3/versions/versions_api.rb +++ b/lib/api/v3/versions/versions_api.rb @@ -54,6 +54,8 @@ module API get do VersionRepresenter.new(@version) end + + mount API::V3::Versions::VersionsProjectsAPI end end end diff --git a/lib/api/v3/versions/versions_projects_api.rb b/lib/api/v3/versions/versions_projects_api.rb new file mode 100644 index 0000000000..e447a52310 --- /dev/null +++ b/lib/api/v3/versions/versions_projects_api.rb @@ -0,0 +1,51 @@ +#-- 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 Versions + class VersionsProjectsAPI < Grape::API + resources :projects do + before do + @projects = @version.projects.visible(current_user).all + + # Authorization for accessing the version is done in the versions + # endpoint into which this endpoint is embedded. + end + + get do + Projects::ProjectCollectionRepresenter.new(@projects, + @projects.count, + api_v3_paths.versions_projects(@version.id)) + end + end + end + end + end +end diff --git a/spec/lib/api/v3/projects/project_collection_representer_spec.rb b/spec/lib/api/v3/projects/project_collection_representer_spec.rb new file mode 100644 index 0000000000..0a9374e58f --- /dev/null +++ b/spec/lib/api/v3/projects/project_collection_representer_spec.rb @@ -0,0 +1,41 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2015 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2013 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +require 'spec_helper' + +describe ::API::V3::Projects::ProjectCollectionRepresenter do + let(:self_link) { '/api/v3/versions/1/projects' } + let(:projects) { FactoryGirl.build_list(:project, 3) } + let(:representer) { described_class.new(projects, 42, self_link) } + + context 'generation' do + subject(:collection) { representer.to_json } + + it_behaves_like 'API V3 collection decorated', 42, 3, 'versions/1/projects', 'Project' + end +end diff --git a/spec/lib/api/v3/utilities/path_helper_spec.rb b/spec/lib/api/v3/utilities/path_helper_spec.rb index 97d668dd1d..d05846deaf 100644 --- a/spec/lib/api/v3/utilities/path_helper_spec.rb +++ b/spec/lib/api/v3/utilities/path_helper_spec.rb @@ -185,6 +185,14 @@ describe ::API::V3::Utilities::PathHelper do it { is_expected.to match(/^\/api\/v3\/projects\/42\/versions/) } end + describe '#versions_projects' do + subject { helper.versions_projects 42 } + + it_behaves_like 'api v3 path' + + it { is_expected.to match(/^\/api\/v3\/versions\/42\/projects/) } + end + describe 'work packages paths' do shared_examples_for 'api v3 work packages path' do it { is_expected.to match(/^\/api\/v3\/work_packages/) } diff --git a/spec/requests/api/v3/versions/project_resource_spec.rb b/spec/requests/api/v3/versions/project_resource_spec.rb new file mode 100644 index 0000000000..7e997d835a --- /dev/null +++ b/spec/requests/api/v3/versions/project_resource_spec.rb @@ -0,0 +1,97 @@ +#-- 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' +require 'rack/test' + +describe "API v3 version's projects resource" do + include Rack::Test::Methods + + let(:current_user) do + user = FactoryGirl.create(:user, + member_in_project: project, + member_through_role: role) + + allow(User).to receive(:current).and_return user + + user + end + let(:role) { FactoryGirl.create(:role, permissions: [:view_work_packages]) } + let(:role_without_permissions) { FactoryGirl.create(:role, permissions: []) } + let(:project) { FactoryGirl.create(:project, is_public: false) } + let(:project2) { FactoryGirl.create(:project, is_public: false) } + let(:project3) { FactoryGirl.create(:project, is_public: false) } + let(:project4) { FactoryGirl.create(:project, is_public: false) } + let(:version) { FactoryGirl.create(:version, project: project, sharing: 'system') } + + subject(:response) { last_response } + + describe '#get (index)' do + let(:get_path) { "/api/v3/versions/#{version.id}/projects" } + + context 'logged in user with permissions' do + before do + current_user + + # this is to be included + FactoryGirl.create(:member, user: current_user, + project: project2, + roles: [role]) + # this is to be included as the user is a member of the project, the + # lack of permissions is irrelevant. + FactoryGirl.create(:member, user: current_user, + project: project3, + roles: [role_without_permissions]) + # project4 should NOT be included + project4 + + get get_path + end + + it_behaves_like 'API V3 collection response', 3, 3, 'Project' + + it 'includes only the projects which the user can see' do + id_in_response = JSON.parse(response.body)['_embedded']['elements'].map { |p| p['id'] } + + expect(id_in_response).to match_array [project.id, project2.id, project3.id] + end + end + + context 'logged in user without permissions' do + let(:role) { role_without_permissions } + + before do + current_user + + get get_path + end + + it_behaves_like 'unauthorized access' + end + end +end From 423babcb37f749636286366d0794459b411ad963 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Tue, 6 Jan 2015 17:31:26 +0100 Subject: [PATCH 14/47] fully fletched out version representation --- lib/api/v3/utilities/path_helper.rb | 6 +- lib/api/v3/versions/version_representer.rb | 48 ++++++++++++++- spec/lib/api/v3/support/api_v3_formattable.rb | 6 +- spec/lib/api/v3/utilities/path_helper_spec.rb | 8 +++ .../v3/versions/version_representer_spec.rb | 60 ++++++++++++++++--- spec/requests/api/v3/version_resource_spec.rb | 37 +++++------- 6 files changed, 132 insertions(+), 33 deletions(-) diff --git a/lib/api/v3/utilities/path_helper.rb b/lib/api/v3/utilities/path_helper.rb index 2da55bd889..2d8826e2c6 100644 --- a/lib/api/v3/utilities/path_helper.rb +++ b/lib/api/v3/utilities/path_helper.rb @@ -106,12 +106,16 @@ module API "#{user(id)}/lock" end + def self.version(version_id) + "#{root}/versions/#{version_id}" + end + def self.versions(project_id) "#{project(project_id)}/versions" end def self.versions_projects(version_id) - "#{root}/versions/#{version_id}/projects" + "#{version(version_id)}/projects" end def self.watcher(id, work_package_id) diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index 73868cb23a..5ccb1e4d14 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -36,14 +36,58 @@ module API class VersionRepresenter < Roar::Decorator include Roar::JSON::HAL include Roar::Hypermedia - include OpenProject::StaticRouting::UrlHelpers + include API::V3::Utilities::PathHelper self.as_strategy = API::Utilities::CamelCasingStrategy.new + attr_reader :current_user + + def initialize(model, options = {}) + @current_user = options[:current_user] + + super(model) + end + property :_type, exec_context: :decorator + link :self do + { + href: api_v3_paths.version(represented.id), + title: "#{represented.name}" + } + end + + link :definingProject do + { + href: api_v3_paths.project(represented.project.id), + title: "#{represented.project.name}" + } if represented.project.visible?(current_user) + end + + link :availableInProjects do + { + href: api_v3_paths.versions_projects(represented.project.id) + } + end + property :id, render_nil: true - property :name + property :name, render_nil: true + + property :description, + exec_context: :decorator, + getter: -> (*) { + { + format: 'plain', + raw: represented.description, + } + }, + render_nil: true + + property :start_date, render_nil: true + property :due_date, as: 'endDate', render_nil: true + property :status, render_nil: true + property :created_on, as: 'createdAt', render_nil: true + property :updated_on, as: 'updatedAt', render_nil: true def _type 'Version' diff --git a/spec/lib/api/v3/support/api_v3_formattable.rb b/spec/lib/api/v3/support/api_v3_formattable.rb index a4818bc806..6edc5acff3 100644 --- a/spec/lib/api/v3/support/api_v3_formattable.rb +++ b/spec/lib/api/v3/support/api_v3_formattable.rb @@ -35,5 +35,9 @@ shared_examples_for 'API V3 formattable' do |property| it { is_expected.to be_json_eql(raw.to_json).at_path(property + '/raw') } - it { is_expected.to be_json_eql(html.to_json).at_path(property + '/html') } + it do + if defined?(html) + is_expected.to be_json_eql(html.to_json).at_path(property + '/html') + end + end end diff --git a/spec/lib/api/v3/utilities/path_helper_spec.rb b/spec/lib/api/v3/utilities/path_helper_spec.rb index d05846deaf..968091cc5f 100644 --- a/spec/lib/api/v3/utilities/path_helper_spec.rb +++ b/spec/lib/api/v3/utilities/path_helper_spec.rb @@ -177,6 +177,14 @@ describe ::API::V3::Utilities::PathHelper do it { is_expected.to match(/^\/api\/v3\/users\/1/) } end + describe '#version' do + subject { helper.version 42 } + + it_behaves_like 'api v3 path' + + it { is_expected.to match(/^\/api\/v3\/versions\/42/) } + end + describe '#versions' do subject { helper.versions 42 } diff --git a/spec/lib/api/v3/versions/version_representer_spec.rb b/spec/lib/api/v3/versions/version_representer_spec.rb index e8fddd7003..bf4d92c6ae 100644 --- a/spec/lib/api/v3/versions/version_representer_spec.rb +++ b/spec/lib/api/v3/versions/version_representer_spec.rb @@ -29,22 +29,68 @@ require 'spec_helper' describe ::API::V3::Versions::VersionRepresenter do - let(:version) { FactoryGirl.build(:version) } - let(:representer) { described_class.new(version) } + let(:version) { FactoryGirl.build_stubbed(:version) } + let(:user) { FactoryGirl.build_stubbed(:user) } + let(:representer) { described_class.new(version, current_user: user) } + + include API::V3::Utilities::PathHelper context 'generation' do subject(:generated) { representer.to_json } it { should include_json('Version'.to_json).at_path('_type') } - xit { should have_json_type(Object).at_path('_links') } - xit 'should link to self' do - expect(subject).to have_json_path('_links/self/href') + context 'links' do + + it { should have_json_type(Object).at_path('_links') } + + it 'to self' do + path = api_v3_paths.version(version.id) + + expect(subject).to be_json_eql(path.to_json).at_path('_links/self/href') + end + + context 'to the defining project' do + let(:path) { api_v3_paths.project(version.project.id) } + + it 'exists if the user has the permission to see the project' do + allow(version.project).to receive(:visible?).with(user).and_return(true) + + subject = representer.to_json + + expect(subject).to be_json_eql(path.to_json).at_path('_links/definingProject/href') + end + + it 'does not exist if the user lacks the permission to see the project' do + allow(version.project).to receive(:visible?).with(user).and_return(false) + + subject = representer.to_json + + expect(subject).to_not have_json_path('_links/definingProject/href') + end + end + + it 'to available projects' do + path = api_v3_paths.versions_projects(version.project.id) + + expect(subject).to be_json_eql(path.to_json).at_path('_links/availableInProjects/href') + end end describe 'version' do - it { should have_json_path('id') } - it { should have_json_path('name') } + it { is_expected.to be_json_eql(version.id.to_json).at_path('id') } + it { is_expected.to be_json_eql(version.name.to_json).at_path('name') } + + it_behaves_like 'API V3 formattable', 'description' do + let(:format) { 'plain' } + let(:raw) { version.description } + end + + it { is_expected.to be_json_eql(version.start_date.to_json).at_path('startDate') } + it { is_expected.to be_json_eql(version.due_date.to_json).at_path('endDate') } + it { is_expected.to be_json_eql(version.status.to_json).at_path('status') } + it { is_expected.to be_json_eql(version.created_on.to_json).at_path('createdAt') } + it { is_expected.to be_json_eql(version.updated_on.to_json).at_path('updatedAt') } end end end diff --git a/spec/requests/api/v3/version_resource_spec.rb b/spec/requests/api/v3/version_resource_spec.rb index 5e75ae58f8..3ffcf871d1 100644 --- a/spec/requests/api/v3/version_resource_spec.rb +++ b/spec/requests/api/v3/version_resource_spec.rb @@ -55,6 +55,17 @@ describe 'API v3 Version resource' do describe '#get (:id)' do let(:get_path) { "/api/v3/versions/#{version_in_project.id}" } + shared_examples_for 'successful response' do + it 'responds with 200' do + expect(last_response.status).to eq(200) + end + + it 'returns the version' do + expect(last_response.body).to be_json_eql('Version'.to_json).at_path('_type') + expect(last_response.body).to be_json_eql(expected_version.id.to_json).at_path('id') + end + end + context 'logged in user with permissions' do before do version_in_project.save! @@ -63,17 +74,8 @@ describe 'API v3 Version resource' do get get_path end - it 'responds with 200' do - expect(last_response.status).to eq(200) - end - - it 'returns the work package' do - expected = { - _type: 'Version', - name: version_in_project.name - }.to_json - - expect(last_response.body).to be_json_eql(expected) + it_should_behave_like 'successful response' do + let(:expected_version) { version_in_project } end end @@ -87,17 +89,8 @@ describe 'API v3 Version resource' do get get_path end - it 'responds with 200' do - expect(last_response.status).to eq(200) - end - - it 'returns the work package' do - expected = { - _type: 'Version', - name: version_in_other_project.name - }.to_json - - expect(last_response.body).to be_json_eql(expected) + it_should_behave_like 'successful response' do + let(:expected_version) { version_in_other_project } end end From da588b812cf8656221f5ea6767c99c56322bad39 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 7 Jan 2015 13:59:20 +0100 Subject: [PATCH 15/47] harmonize api decorators --- lib/api/decorators/collection.rb | 24 +++++++-- lib/api/decorators/single.rb | 53 +++++++++++++++++++ .../category_collection_representer.rb | 8 +-- lib/api/v3/categories/category_representer.rb | 10 +--- .../priority_collection_representer.rb | 8 +-- lib/api/v3/priorities/priority_representer.rb | 10 +--- .../project_collection_representer.rb | 4 +- lib/api/v3/projects/project_representer.rb | 10 +--- lib/api/v3/queries/query_representer.rb | 10 +--- .../statuses/status_collection_representer.rb | 4 +- lib/api/v3/statuses/status_representer.rb | 13 +---- .../v3/users/user_collection_representer.rb | 4 +- lib/api/v3/users/user_representer.rb | 35 ++++++------ lib/api/v3/versions/projects_versions_api.rb | 3 +- .../version_collection_representer.rb | 4 +- lib/api/v3/versions/version_representer.rb | 24 +++------ lib/api/v3/versions/versions_api.rb | 6 ++- .../v3/work_packages/relation_representer.rb | 30 ++++------- .../work_packages/work_package_representer.rb | 41 ++++++-------- lib/api/v3/work_packages/work_packages_api.rb | 7 +-- .../lib/api/v3/users/user_representer_spec.rb | 2 +- .../version_collection_representer_spec.rb | 4 +- .../v3/versions/version_representer_spec.rb | 3 +- 23 files changed, 151 insertions(+), 166 deletions(-) create mode 100644 lib/api/decorators/single.rb diff --git a/lib/api/decorators/collection.rb b/lib/api/decorators/collection.rb index 721464ced6..b9458b2fce 100644 --- a/lib/api/decorators/collection.rb +++ b/lib/api/decorators/collection.rb @@ -37,14 +37,24 @@ module API include Roar::Hypermedia include API::Utilities::UrlHelper - def initialize(models, total, self_link, decorator) + def initialize(models, total, self_link, context: {}) @total = total @self_link = self_link - @decorator = decorator + @context = context super(models) end + class_attribute :element_decorator_class + + def self.element_decorator(klass) + self.element_decorator_class = klass + end + + def element_decorator + self.class.element_decorator_class + end + as_strategy = API::Utilities::CamelCasingStrategy.new link :self do @@ -56,9 +66,17 @@ module API property :count, getter: -> (*) { empty? ? 0 : count } collection :elements, - getter: -> (*) { represented.map { |model| @decorator.new(model) } }, + getter: -> (*) { + represented.map { |model| + element_decorator.new(model, context) + } + }, exec_context: :decorator, embedded: true + + private + + attr_reader :context end end end diff --git a/lib/api/decorators/single.rb b/lib/api/decorators/single.rb new file mode 100644 index 0000000000..7919f1da61 --- /dev/null +++ b/lib/api/decorators/single.rb @@ -0,0 +1,53 @@ +#-- 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. +#++ + +require 'roar/decorator' +require 'roar/json/hal' + +module API + module Decorators + class Single < Roar::Decorator + include Roar::JSON::HAL + include Roar::Hypermedia + include API::V3::Utilities::PathHelper + + attr_reader :context + class_attribute :as_strategy + self.as_strategy = API::Utilities::CamelCasingStrategy.new + + def initialize(model, context = {}) + @context = context + + super(model) + end + + property :_type, exec_context: :decorator + end + end +end diff --git a/lib/api/v3/categories/category_collection_representer.rb b/lib/api/v3/categories/category_collection_representer.rb index c888af2506..8c135d0755 100644 --- a/lib/api/v3/categories/category_collection_representer.rb +++ b/lib/api/v3/categories/category_collection_representer.rb @@ -27,17 +27,11 @@ # See doc/COPYRIGHT.rdoc for more details. #++ -require 'roar/decorator' -require 'roar/json/collection' -require 'roar/json/hal' - module API module V3 module Categories class CategoryCollectionRepresenter < ::API::Decorators::Collection - def initialize(models, total, self_link) - super(models, total, self_link, ::API::V3::Categories::CategoryRepresenter) - end + element_decorator ::API::V3::Categories::CategoryRepresenter end end end diff --git a/lib/api/v3/categories/category_representer.rb b/lib/api/v3/categories/category_representer.rb index af3816f6dd..c311296e09 100644 --- a/lib/api/v3/categories/category_representer.rb +++ b/lib/api/v3/categories/category_representer.rb @@ -33,15 +33,7 @@ require 'roar/json/hal' module API module V3 module Categories - class CategoryRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include OpenProject::StaticRouting::UrlHelpers - - self.as_strategy = API::Utilities::CamelCasingStrategy.new - - property :_type, exec_context: :decorator - + class CategoryRepresenter < ::API::Decorators::Single property :id, render_nil: true property :name, render_nil: true diff --git a/lib/api/v3/priorities/priority_collection_representer.rb b/lib/api/v3/priorities/priority_collection_representer.rb index dac8b4ccb4..7adc14ff5e 100644 --- a/lib/api/v3/priorities/priority_collection_representer.rb +++ b/lib/api/v3/priorities/priority_collection_representer.rb @@ -27,17 +27,11 @@ # See doc/COPYRIGHT.rdoc for more details. #++ -require 'roar/decorator' -require 'roar/json/collection' -require 'roar/json/hal' - module API module V3 module Priorities class PriorityCollectionRepresenter < ::API::Decorators::Collection - def initialize(models, total, self_link) - super(models, total, self_link, ::API::V3::Priorities::PriorityRepresenter) - end + element_decorator ::API::V3::Priorities::PriorityRepresenter end end end diff --git a/lib/api/v3/priorities/priority_representer.rb b/lib/api/v3/priorities/priority_representer.rb index 3b1e51e59f..f5b771ed12 100644 --- a/lib/api/v3/priorities/priority_representer.rb +++ b/lib/api/v3/priorities/priority_representer.rb @@ -33,15 +33,7 @@ require 'roar/json/hal' module API module V3 module Priorities - class PriorityRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include OpenProject::StaticRouting::UrlHelpers - - self.as_strategy = API::Utilities::CamelCasingStrategy.new - - property :_type, exec_context: :decorator - + class PriorityRepresenter < ::API::Decorators::Single property :id, render_nil: true property :name diff --git a/lib/api/v3/projects/project_collection_representer.rb b/lib/api/v3/projects/project_collection_representer.rb index 579061e4d5..07828dd693 100644 --- a/lib/api/v3/projects/project_collection_representer.rb +++ b/lib/api/v3/projects/project_collection_representer.rb @@ -36,9 +36,7 @@ module API module V3 module Projects class ProjectCollectionRepresenter < ::API::Decorators::Collection - def initialize(models, total, self_link) - super(models, total, self_link, ::API::V3::Projects::ProjectRepresenter) - end + element_decorator ::API::V3::Projects::ProjectRepresenter end end end diff --git a/lib/api/v3/projects/project_representer.rb b/lib/api/v3/projects/project_representer.rb index fd14e0de17..049379da9f 100644 --- a/lib/api/v3/projects/project_representer.rb +++ b/lib/api/v3/projects/project_representer.rb @@ -33,15 +33,7 @@ require 'roar/json/hal' module API module V3 module Projects - class ProjectRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include API::V3::Utilities::PathHelper - - self.as_strategy = API::Utilities::CamelCasingStrategy.new - - property :_type, exec_context: :decorator - + class ProjectRepresenter < ::API::Decorators::Single link :self do { href: api_v3_paths.project(represented.id), diff --git a/lib/api/v3/queries/query_representer.rb b/lib/api/v3/queries/query_representer.rb index 303533e402..d44b6f5ceb 100644 --- a/lib/api/v3/queries/query_representer.rb +++ b/lib/api/v3/queries/query_representer.rb @@ -33,15 +33,7 @@ require 'roar/json/hal' module API module V3 module Queries - class QueryRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include API::V3::Utilities::PathHelper - - self.as_strategy = API::Utilities::CamelCasingStrategy.new - - property :_type, exec_context: :decorator - + class QueryRepresenter < ::API::Decorators::Single link :self do { href: api_v3_paths.query(represented.id), diff --git a/lib/api/v3/statuses/status_collection_representer.rb b/lib/api/v3/statuses/status_collection_representer.rb index 172699ca58..c07e902caf 100644 --- a/lib/api/v3/statuses/status_collection_representer.rb +++ b/lib/api/v3/statuses/status_collection_representer.rb @@ -35,9 +35,7 @@ module API module V3 module Statuses class StatusCollectionRepresenter < ::API::Decorators::Collection - def initialize(models, total, self_link) - super(models, total, self_link, ::API::V3::Statuses::StatusRepresenter) - end + element_decorator ::API::V3::Statuses::StatusRepresenter end end end diff --git a/lib/api/v3/statuses/status_representer.rb b/lib/api/v3/statuses/status_representer.rb index 811f2073f6..a6c80f8185 100644 --- a/lib/api/v3/statuses/status_representer.rb +++ b/lib/api/v3/statuses/status_representer.rb @@ -27,21 +27,10 @@ # See doc/COPYRIGHT.rdoc for more details. #++ -require 'roar/decorator' -require 'roar/json/hal' - module API module V3 module Statuses - class StatusRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include API::V3::Utilities::PathHelper - - self.as_strategy = API::Utilities::CamelCasingStrategy.new - - property :_type, exec_context: :decorator - + class StatusRepresenter < ::API::Decorators::Single link :self do { href: api_v3_paths.status(represented.id), diff --git a/lib/api/v3/users/user_collection_representer.rb b/lib/api/v3/users/user_collection_representer.rb index 37819267ec..5f5d54e16d 100644 --- a/lib/api/v3/users/user_collection_representer.rb +++ b/lib/api/v3/users/user_collection_representer.rb @@ -31,9 +31,7 @@ module API module V3 module Users class UserCollectionRepresenter < ::API::Decorators::Collection - def initialize(models, total, self_link) - super(models, total, self_link, ::API::V3::Users::UserRepresenter) - end + element_decorator ::API::V3::Users::UserRepresenter end end end diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index 56972d377a..fcece739b8 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -33,24 +33,9 @@ require 'roar/json/hal' module API module V3 module Users - class UserRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include API::V3::Utilities::PathHelper + class UserRepresenter < ::API::Decorators::Single include AvatarHelper - self.as_strategy = API::Utilities::CamelCasingStrategy.new - - def initialize(model, options = {}, *expand) - @current_user = options[:current_user] - @work_package = options[:work_package] - @expand = expand - - super(model) - end - - property :_type, exec_context: :decorator - link :self do { href: api_v3_paths.user(represented.id), @@ -76,10 +61,10 @@ module API link :removeWatcher do { - href: api_v3_paths.watcher(represented.id, @work_package.id), + href: api_v3_paths.watcher(represented.id, work_package.id), method: :delete, title: 'Remove watcher' - } if @work_package && current_user_allowed_to(:delete_work_package_watchers, @work_package) + } if work_package && current_user_allowed_to(:delete_work_package_watchers, work_package) end property :id, render_nil: true @@ -101,11 +86,21 @@ module API end def current_user_is_admin - @current_user && @current_user.admin? + current_user && current_user.admin? end def current_user_allowed_to(permission, work_package) - @current_user && @current_user.allowed_to?(permission, work_package.project) + current_user && current_user.allowed_to?(permission, work_package.project) + end + + private + + def current_user + context[:current_user] + end + + def work_package + context[:work_package] end end end diff --git a/lib/api/v3/versions/projects_versions_api.rb b/lib/api/v3/versions/projects_versions_api.rb index fea1e7d427..327cd93084 100644 --- a/lib/api/v3/versions/projects_versions_api.rb +++ b/lib/api/v3/versions/projects_versions_api.rb @@ -41,7 +41,8 @@ module API get do VersionCollectionRepresenter.new(@versions, @versions.count, - api_v3_paths.versions(@project.identifier)) + api_v3_paths.versions(@project.identifier), + context: { current_user: current_user }) end end end diff --git a/lib/api/v3/versions/version_collection_representer.rb b/lib/api/v3/versions/version_collection_representer.rb index 2598b66cd5..6fd7bd1ff7 100644 --- a/lib/api/v3/versions/version_collection_representer.rb +++ b/lib/api/v3/versions/version_collection_representer.rb @@ -36,9 +36,7 @@ module API module V3 module Versions class VersionCollectionRepresenter < ::API::Decorators::Collection - def initialize(models, total, self_link) - super(models, total, self_link, ::API::V3::Versions::VersionRepresenter) - end + element_decorator ::API::V3::Versions::VersionRepresenter end end end diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index 5ccb1e4d14..cc977f4d3f 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -33,23 +33,7 @@ require 'roar/json/hal' module API module V3 module Versions - class VersionRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include API::V3::Utilities::PathHelper - - self.as_strategy = API::Utilities::CamelCasingStrategy.new - - attr_reader :current_user - - def initialize(model, options = {}) - @current_user = options[:current_user] - - super(model) - end - - property :_type, exec_context: :decorator - + class VersionRepresenter < ::API::Decorators::Single link :self do { href: api_v3_paths.version(represented.id), @@ -92,6 +76,12 @@ module API def _type 'Version' end + + private + + def current_user + context[:current_user] + end end end end diff --git a/lib/api/v3/versions/versions_api.rb b/lib/api/v3/versions/versions_api.rb index 60cee0fa0c..d16ea25fc6 100644 --- a/lib/api/v3/versions/versions_api.rb +++ b/lib/api/v3/versions/versions_api.rb @@ -49,10 +49,14 @@ module API authorize_any(permissions, projects, user: current_user) end + + def context + { current_user: current_user } + end end get do - VersionRepresenter.new(@version) + VersionRepresenter.new(@version, context) end mount API::V3::Versions::VersionsProjectsAPI diff --git a/lib/api/v3/work_packages/relation_representer.rb b/lib/api/v3/work_packages/relation_representer.rb index dd7204f420..e1d7354f57 100644 --- a/lib/api/v3/work_packages/relation_representer.rb +++ b/lib/api/v3/work_packages/relation_representer.rb @@ -33,23 +33,7 @@ require 'roar/json/hal' module API module V3 module WorkPackages - class RelationRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include API::V3::Utilities::PathHelper - - self.as_strategy = API::Utilities::CamelCasingStrategy.new - - def initialize(model, options = {}, *expand) - @current_user = options[:current_user] - @work_package = options[:work_package] - @expand = expand - - super(model) - end - - property :_type, exec_context: :decorator - + class RelationRepresenter < ::API::Decorators::Single link :self do { href: api_v3_paths.relation(represented.id) } end @@ -79,11 +63,19 @@ module API private def current_user_allowed_to(permission) - @current_user && @current_user.allowed_to?(permission, represented.from.project) + current_user && current_user.allowed_to?(permission, represented.from.project) end def relation_type - represented.relation_type_for(@work_package).camelize + represented.relation_type_for(work_package).camelize + end + + def work_package + context[:work_package] + end + + def current_user + context[:current_user] end end end diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 5ec9fc703a..86fa767da2 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -33,21 +33,8 @@ require 'roar/json/hal' module API module V3 module WorkPackages - class WorkPackageRepresenter < Roar::Decorator - include Roar::JSON::HAL - include Roar::Hypermedia - include API::V3::Utilities::PathHelper - - self.as_strategy = ::API::Utilities::CamelCasingStrategy.new - - def initialize(model, options = {}, *expand) - @current_user = options[:current_user] - @expand = expand - - super(model) - end - - property :_type, exec_context: :decorator, writeable: false + class WorkPackageRepresenter < ::API::Decorators::Single + include OpenProject::TextFormatting link :self do { @@ -143,20 +130,20 @@ module API { href: api_v3_paths.work_package_watchers(represented.id), method: :post, - data: { user_id: @current_user.id }, + data: { user_id: current_user.id }, title: 'Watch work package' - } if !@current_user.anonymous? && + } if !current_user.anonymous? && current_user_allowed_to(:view_work_packages) && - !represented.watcher_users.include?(@current_user) + !represented.watcher_users.include?(current_user) end link :unwatchChanges do { - href: "#{api_v3_paths.work_package_watchers(represented.id)}/#{@current_user.id}", + href: "#{api_v3_paths.work_package_watchers(represented.id)}/#{current_user.id}", method: :delete, title: 'Unwatch work package' } if current_user_allowed_to(:view_work_packages) && - represented.watcher_users.include?(@current_user) + represented.watcher_users.include?(current_user) end link :addWatcher do @@ -220,7 +207,7 @@ module API href: version_path(represented.fixed_version), type: 'text/html', title: "#{represented.fixed_version.to_s_for_project(represented.project)}" - } if represented.fixed_version && @current_user.allowed_to?({ controller: 'versions', action: 'show' }, represented.fixed_version.project, global: false) + } if represented.fixed_version && current_user.allowed_to?({ controller: 'versions', action: 'show' }, represented.fixed_version.project, global: false) end links :children do @@ -298,18 +285,18 @@ module API end def activities - represented.journals.map { |activity| ::API::V3::Activities::ActivityRepresenter.new(activity, current_user: @current_user) } + represented.journals.map { |activity| ::API::V3::Activities::ActivityRepresenter.new(activity, current_user: current_user) } end def watchers watchers = represented.watcher_users.order(User::USER_FORMATS_STRUCTURE[Setting.user_format]) - watchers.map { |watcher| ::API::V3::Users::UserRepresenter.new(watcher, work_package: represented, current_user: @current_user) } + watchers.map { |watcher| ::API::V3::Users::UserRepresenter.new(watcher, work_package: represented, current_user: current_user) } end def relations relations = represented.relations visible_relations = relations.select { |relation| relation.other_work_package(represented).visible? } - visible_relations.map { |relation| RelationRepresenter.new(relation, work_package: represented, current_user: @current_user) } + visible_relations.map { |relation| RelationRepresenter.new(relation, work_package: represented, current_user: current_user) } end def custom_properties @@ -320,7 +307,7 @@ module API end def current_user_allowed_to(permission) - @current_user && @current_user.allowed_to?(permission, represented.project) + current_user && current_user.allowed_to?(permission, represented.project) end def visible_children @@ -343,6 +330,10 @@ module API { hours: hours.to_i, minutes: minutes } end + + def current_user + context[:current_user] + 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 ee8f91a2e0..02d4898d5b 100644 --- a/lib/api/v3/work_packages/work_packages_api.rb +++ b/lib/api/v3/work_packages/work_packages_api.rb @@ -76,12 +76,13 @@ module API before do @work_package = WorkPackage.find(params[:id]) - @representer = ::API::V3::WorkPackages::WorkPackageRepresenter - .new(work_package, { current_user: current_user }, :activities, :users) + @representer = WorkPackageRepresenter.new(work_package, + current_user: current_user) end get do - authorize({ controller: :work_packages_api, action: :get }, context: @work_package.project) + authorize({ controller: :work_packages_api, action: :get }, + context: @work_package.project) @representer end diff --git a/spec/lib/api/v3/users/user_representer_spec.rb b/spec/lib/api/v3/users/user_representer_spec.rb index f314704bee..1310de7ba4 100644 --- a/spec/lib/api/v3/users/user_representer_spec.rb +++ b/spec/lib/api/v3/users/user_representer_spec.rb @@ -76,7 +76,7 @@ describe ::API::V3::Users::UserRepresenter do end context 'when current_user is admin' do - let(:current_user) { FactoryGirl.create(:admin) } + let(:current_user) { FactoryGirl.build_stubbed(:admin) } it 'should link to lock' do expect(subject).to have_json_path('_links/lock/href') diff --git a/spec/lib/api/v3/versions/version_collection_representer_spec.rb b/spec/lib/api/v3/versions/version_collection_representer_spec.rb index 95b83a3d81..39611dd3b9 100644 --- a/spec/lib/api/v3/versions/version_collection_representer_spec.rb +++ b/spec/lib/api/v3/versions/version_collection_representer_spec.rb @@ -31,7 +31,9 @@ require 'spec_helper' describe ::API::V3::Versions::VersionCollectionRepresenter do let(:self_link) { '/api/v3/projects/1/versions' } let(:versions) { FactoryGirl.build_list(:version, 3) } - let(:representer) { described_class.new(versions, 42, self_link) } + let(:user) { FactoryGirl.build_stubbed(:user) } + let(:context) { { current_user: user } } + let(:representer) { described_class.new(versions, 42, self_link, context: context) } context 'generation' do subject(:collection) { representer.to_json } diff --git a/spec/lib/api/v3/versions/version_representer_spec.rb b/spec/lib/api/v3/versions/version_representer_spec.rb index bf4d92c6ae..107699f330 100644 --- a/spec/lib/api/v3/versions/version_representer_spec.rb +++ b/spec/lib/api/v3/versions/version_representer_spec.rb @@ -31,7 +31,8 @@ require 'spec_helper' describe ::API::V3::Versions::VersionRepresenter do let(:version) { FactoryGirl.build_stubbed(:version) } let(:user) { FactoryGirl.build_stubbed(:user) } - let(:representer) { described_class.new(version, current_user: user) } + let(:context) { { current_user: user } } + let(:representer) { described_class.new(version, context) } include API::V3::Utilities::PathHelper From 30c08cc0037c68fb9a337676fc9a0bea794ee99b Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 7 Jan 2015 14:09:07 +0100 Subject: [PATCH 16/47] use proper api link --- lib/api/v3/work_packages/work_package_representer.rb | 3 +-- .../api/v3/work_packages/work_package_representer_spec.rb | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 86fa767da2..60e890c332 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -204,8 +204,7 @@ module API link :version do { - href: version_path(represented.fixed_version), - type: 'text/html', + href: api_v3_paths.version(represented.fixed_version.id), title: "#{represented.fixed_version.to_s_for_project(represented.project)}" } if represented.fixed_version && current_user.allowed_to?({ controller: 'versions', action: 'show' }, represented.fixed_version.project, global: false) end diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index fc75bcf60a..44d644bca4 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 @@ -29,6 +29,8 @@ require 'spec_helper' describe ::API::V3::WorkPackages::WorkPackageRepresenter do + include ::API::V3::Utilities::PathHelper + let(:member) { FactoryGirl.create(:user, member_in_project: project, member_through_role: role) } let(:current_user) { member } @@ -242,7 +244,7 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do context 'version set' do let!(:version) { FactoryGirl.create :version, project: project } - let(:expected_url) { "/versions/#{version.id}".to_json } + let(:expected_url) { api_v3_paths.version(version.id).to_json } before do work_package.fixed_version = version @@ -252,8 +254,6 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do is_expected.to be_json_eql(expected_url).at_path('_links/version/href') } - it { is_expected.to be_json_eql('text/html'.to_json).at_path('_links/version/type') } - context ' but is not accessible due to permissions' do before do current_user.stub(:allowed_to?).and_call_original From 026e7de0fc370a40ec2fb7f78b727d24bdcd286c Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Wed, 7 Jan 2015 14:15:00 +0100 Subject: [PATCH 17/47] adapt api documentation to include title --- doc/apiv3-documentation.apib | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/apiv3-documentation.apib b/doc/apiv3-documentation.apib index b9cfad5ed9..af1f0c7bf1 100644 --- a/doc/apiv3-documentation.apib +++ b/doc/apiv3-documentation.apib @@ -2450,7 +2450,8 @@ Note that due to sharing this might be more than the versions *defined* by that "title": "New" }, "version": { - "href": "/api/v3/versions/1" + "href": "/api/v3/versions/1", + "title": "Version 1" }, "availableWatchers": { "href": "/api/v3/work_packages/1528/available_watchers", From 09a369982deccdad8ba6796f4d7781ef0a185819 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 8 Jan 2015 14:41:58 +0100 Subject: [PATCH 18/47] embed version into work_package representer --- .../work_packages/work_package_representer.rb | 10 ++++++ .../work_package_representer_spec.rb | 31 ++++++++++++++++--- 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index 60e890c332..cc3d0cc096 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -275,6 +275,12 @@ module API property :category, embedded: true, class: ::Category, decorator: ::API::V3::Categories::CategoryRepresenter, if: -> (*) { !category.nil? } property :activities, embedded: true, exec_context: :decorator + + property :version, + embedded: true, + exec_context: :decorator, + if: ->(*) { represented.fixed_version.present? } + property :watchers, embedded: true, exec_context: :decorator, if: -> (*) { current_user_allowed_to(:view_work_package_watchers) } collection :attachments, embedded: true, class: ::Attachment, decorator: ::API::V3::Attachments::AttachmentRepresenter property :relations, embedded: true, exec_context: :decorator @@ -298,6 +304,10 @@ module API visible_relations.map { |relation| RelationRepresenter.new(relation, work_package: represented, current_user: current_user) } end + def version + Versions::VersionRepresenter.new(represented.fixed_version, current_user: current_user) + end + def custom_properties values = represented.custom_field_values values.map do |v| 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 44d644bca4..43e85b6cfc 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 @@ -238,8 +238,17 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do end describe 'version' do + let(:embedded_path) { '_embedded/version' } + let(:href_path) { '_links/version/href' } + context 'no version set' do - it { is_expected.to_not have_json_path('versionViewable') } + it 'has no version linked' do + is_expected.to_not have_json_path(href_path) + end + + it 'has no version embedded' do + is_expected.to_not have_json_path(embedded_path) + end end context 'version set' do @@ -250,9 +259,14 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do work_package.fixed_version = version end - it { - is_expected.to be_json_eql(expected_url).at_path('_links/version/href') - } + it 'has a link to the version' do + is_expected.to be_json_eql(expected_url).at_path(href_path) + end + + it 'has the version embedded' do + is_expected.to be_json_eql('Version'.to_json).at_path("#{embedded_path}/_type") + is_expected.to be_json_eql(version.name.to_json).at_path("#{embedded_path}/name") + end context ' but is not accessible due to permissions' do before do @@ -260,7 +274,14 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do current_user.stub(:allowed_to?).with({ controller: 'versions', action: 'show' }, project, global: false).and_return(false) end - it { is_expected.to_not have_json_path('_links/version/href') } + it 'has no version linked' do + is_expected.to_not have_json_path(href_path) + end + + it 'has the version embedded as the user has the view work package permission' do + is_expected.to be_json_eql('Version'.to_json).at_path("#{embedded_path}/_type") + is_expected.to be_json_eql(version.name.to_json).at_path("#{embedded_path}/name") + end end end end From f05ec67eb3c65c10be3f71e4268cbfc426f61769 Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 8 Jan 2015 14:58:34 +0100 Subject: [PATCH 19/47] use policy to calculate version link existence --- app/policies/version_policy.rb | 56 +++++++++++++++++++ .../work_packages/work_package_representer.rb | 7 ++- .../work_package_representer_spec.rb | 5 +- 3 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 app/policies/version_policy.rb diff --git a/app/policies/version_policy.rb b/app/policies/version_policy.rb new file mode 100644 index 0000000000..7690e8ff23 --- /dev/null +++ b/app/policies/version_policy.rb @@ -0,0 +1,56 @@ +#-- 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. +#++ + +class VersionPolicy < BasePolicy + private + + def cache + @cache ||= Hash.new do |hash, version| + # copy checks for the move_work_packages permission. This makes + # sense only because the work_packages/moves controller handles + # copying multiple work packages. + hash[version] = { + show: show_allowed?(version) + } + end + end + + def show_allowed?(version) + @show_cache ||= Hash.new do |hash, queried_version| + permissions = [:view_work_packages, :manage_versions] + + hash[queried_version] = permissions.any? do |permission| + allowed_condition = Project.allowed_to_condition(user, permission) + + queried_version.projects.where(allowed_condition).exists? + end + end + + @show_cache[version] + end +end diff --git a/lib/api/v3/work_packages/work_package_representer.rb b/lib/api/v3/work_packages/work_package_representer.rb index cc3d0cc096..b0830d59d7 100644 --- a/lib/api/v3/work_packages/work_package_representer.rb +++ b/lib/api/v3/work_packages/work_package_representer.rb @@ -206,7 +206,8 @@ module API { href: api_v3_paths.version(represented.fixed_version.id), title: "#{represented.fixed_version.to_s_for_project(represented.project)}" - } if represented.fixed_version && current_user.allowed_to?({ controller: 'versions', action: 'show' }, represented.fixed_version.project, global: false) + } if represented.fixed_version && + version_policy.allowed?(represented.fixed_version, :show) end links :children do @@ -343,6 +344,10 @@ module API def current_user context[:current_user] end + + def version_policy + @version_policy ||= ::VersionPolicy.new(current_user) + end end end end diff --git a/spec/lib/api/v3/work_packages/work_package_representer_spec.rb b/spec/lib/api/v3/work_packages/work_package_representer_spec.rb index 43e85b6cfc..7c7052b3fa 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 @@ -270,8 +270,9 @@ describe ::API::V3::WorkPackages::WorkPackageRepresenter do context ' but is not accessible due to permissions' do before do - current_user.stub(:allowed_to?).and_call_original - current_user.stub(:allowed_to?).with({ controller: 'versions', action: 'show' }, project, global: false).and_return(false) + policy = double('VersionPolicy') + allow(policy).to receive(:allowed?).with(version, :show).and_return(false) + representer.instance_variable_set(:@version_policy, policy) end it 'has no version linked' do From 88a1dc289931c70c3434bdf637b94c230187c9ea Mon Sep 17 00:00:00 2001 From: Jens Ulferts Date: Thu, 8 Jan 2015 15:18:39 +0100 Subject: [PATCH 20/47] remove unnecessary interpolation --- lib/api/v3/versions/version_representer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/v3/versions/version_representer.rb b/lib/api/v3/versions/version_representer.rb index cc977f4d3f..7a9be96861 100644 --- a/lib/api/v3/versions/version_representer.rb +++ b/lib/api/v3/versions/version_representer.rb @@ -44,7 +44,7 @@ module API link :definingProject do { href: api_v3_paths.project(represented.project.id), - title: "#{represented.project.name}" + title: represented.project.name } if represented.project.visible?(current_user) end From bb5cc904b19bc4005695e661637c847adf72652e Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 15 Jan 2015 08:59:23 +0100 Subject: [PATCH 21/47] actually call the method I intended apparently I am not able to distinguish a class method from an instance method ^^ --- lib/api/v3/users/user_representer.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index 7be51116ff..b67fe120bf 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -117,7 +117,7 @@ module API end def current_user_can_delete_user(other_user) - @current_user && DeleteUserService.new.deletion_allowed?(other_user, @current_user) + @current_user && DeleteUserService.deletion_allowed?(other_user, @current_user) end end end From 0f3433d6194b3472b4aa4e21c0d31f386470ea81 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 15 Jan 2015 14:36:09 +0100 Subject: [PATCH 22/47] remove param from method --- lib/api/v3/users/user_representer.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index b67fe120bf..c7adaf8959 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -79,7 +79,7 @@ module API href: api_v3_paths.user(represented.id), title: "Delete #{represented.login}", method: :delete - } if current_user_can_delete_user(represented) + } if current_user_can_delete_represented? end link :removeWatcher do @@ -116,8 +116,8 @@ module API @current_user && @current_user.allowed_to?(permission, work_package.project) end - def current_user_can_delete_user(other_user) - @current_user && DeleteUserService.deletion_allowed?(other_user, @current_user) + def current_user_can_delete_represented? + @current_user && DeleteUserService.deletion_allowed?(represented, @current_user) end end end From 14ded43bda658c25fd8f5009cc326874663f8639 Mon Sep 17 00:00:00 2001 From: Jan Sandbrink Date: Thu, 15 Jan 2015 14:36:27 +0100 Subject: [PATCH 23/47] fix tests, use mocking --- .../lib/api/v3/users/user_representer_spec.rb | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/spec/lib/api/v3/users/user_representer_spec.rb b/spec/lib/api/v3/users/user_representer_spec.rb index 555f89ff55..22036fb2d8 100644 --- a/spec/lib/api/v3/users/user_representer_spec.rb +++ b/spec/lib/api/v3/users/user_representer_spec.rb @@ -73,10 +73,6 @@ describe ::API::V3::Users::UserRepresenter do expect(subject).to_not have_json_path('_links/lock/href') expect(subject).to_not have_json_path('_links/unlock/href') end - - it 'should not link to delete' do - expect(subject).to_not have_json_path('_links/delete/href') - end end context 'when current_user is admin' do @@ -92,11 +88,31 @@ describe ::API::V3::Users::UserRepresenter do expect(subject).to have_json_path('_links/unlock/href') end end + end + + context 'when deletion is allowed' do + before do + allow(DeleteUserService).to receive(:deletion_allowed?) + .with(user, current_user) + .and_return(true) + end it 'should link to delete' do expect(subject).to have_json_path('_links/delete/href') end end + + context 'when deletion is not allowed' do + before do + allow(DeleteUserService).to receive(:deletion_allowed?) + .with(user, current_user) + .and_return(false) + end + + it 'should not link to delete' do + expect(subject).to_not have_json_path('_links/delete/href') + end + end end describe 'avatar' do From 2a8202a1cbf8dbf85dcba26910b1db443dce40aa Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 16 Jan 2015 09:38:24 +0000 Subject: [PATCH 24/47] use file column for filename the uploader uses that one too, anyway also this way things don't break when the file is missing --- app/models/attachment.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/attachment.rb b/app/models/attachment.rb index db614088e3..32fcf9fb46 100644 --- a/app/models/attachment.rb +++ b/app/models/attachment.rb @@ -139,7 +139,7 @@ class Attachment < ActiveRecord::Base end def filename - file.file.filename if file.file + attributes['file'] end def file=(file) From a38dfd9a450e1a3dd00ae69a7d50dfdad7c0ecfb Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 16 Jan 2015 13:27:02 +0000 Subject: [PATCH 25/47] restore original disk filename if available --- .../20141215104802_migrate_attachments_to_carrier_wave.rb | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/db/migrate/20141215104802_migrate_attachments_to_carrier_wave.rb b/db/migrate/20141215104802_migrate_attachments_to_carrier_wave.rb index a0d3c1b60f..907eb63f88 100644 --- a/db/migrate/20141215104802_migrate_attachments_to_carrier_wave.rb +++ b/db/migrate/20141215104802_migrate_attachments_to_carrier_wave.rb @@ -100,7 +100,11 @@ class MigrateAttachmentsToCarrierWave < ActiveRecord::Migration FileUtils.move file, old_file attachment.update_column :file, nil attachment.update_column :filename, Pathname(file).basename.to_s - attachment.update_column :disk_filename, Pathname(old_file).basename.to_s + + # keep original disk filename if it was preserved + if attachment.disk_filename.blank? + attachment.update_column :disk_filename, Pathname(old_file).basename.to_s + end FileUtils.rmdir Pathname(file).dirname end From d378de5ecebd5aa49f50d51e30e165c977efd761 Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 16 Jan 2015 13:27:50 +0000 Subject: [PATCH 26/47] set file column for corrupt attachments i.e. attachments whose file could not be found in the original attachments migration --- ...0150116095004_patch_corrupt_attachments.rb | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 db/migrate/20150116095004_patch_corrupt_attachments.rb diff --git a/db/migrate/20150116095004_patch_corrupt_attachments.rb b/db/migrate/20150116095004_patch_corrupt_attachments.rb new file mode 100644 index 0000000000..affaffcc44 --- /dev/null +++ b/db/migrate/20150116095004_patch_corrupt_attachments.rb @@ -0,0 +1,49 @@ +## +# Goes through all attachments looking for ones whose 'file' column, which is the new column +# used by the carrierwave-based attachments, is not set. +# +# For every one of those attachments the migration then sets the 'file' column to +# whatever the value of the legacy column 'filename' is. If that one is empty too +# it falls back to the 'disk_filename' column. This one was not meant to be displayed +# to users but it's better than nothing, especially when trying to identify corrupt attachments. +# +# If *that* column is empty too, the attachment is broken beyond repair and will be dropped. +# +# Note: Just because the 'file' column is restored doesn't mean the actual file exists. +# Rather the 'file' column being empty means precisely that the file is missing. +# By still writing the filename into the file column the attachment can at least +# be displayed, if not downloaded. +# +# Important: The migration is irreversible. +class PatchCorruptAttachments < ActiveRecord::Migration + def up + Attachment.all.each do |attachment| + patch_attachment attachment + end + end + + def down + puts "Can't revert this migration as it would mean breaking valid attachments. \ + Even if we wanted to do that we can't know which ones were broken before \ + to only break those again.".squish + end + + def patch_attachment(attachment) + attributes = attachment.attributes + if attributes['file'].blank? + # fall back to disk filename if necessary + file = attributes['filename'].presence || attributes['disk_filename'].presence + + if file + attachment.update_column :file, file + puts "updated attachment #{attachment.id}'s file column: #{file}" + else + # this really shouldn't happen - but just in case it does, it is more sensible + # to just delete the attachment because it will just break things + puts "could not patch #{attachment.inspect} - missing file name information - \ + it's hopeless ... deleting it".squish + attachment.destroy + end + end + end +end From 4411d172c8aeb6ea902b3e52a78d0a7b8b324fcf Mon Sep 17 00:00:00 2001 From: Markus Kahl Date: Fri, 16 Jan 2015 13:49:51 +0000 Subject: [PATCH 27/47] fixed spec --- spec/controllers/attachments_controller_spec.rb | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/spec/controllers/attachments_controller_spec.rb b/spec/controllers/attachments_controller_spec.rb index a27fc905c4..74ea68431a 100644 --- a/spec/controllers/attachments_controller_spec.rb +++ b/spec/controllers/attachments_controller_spec.rb @@ -105,6 +105,10 @@ describe AttachmentsController, type: :controller do let(:work_package) { FactoryGirl.create :work_package, project: project } let(:uploader) { nil } + ## + # Stubs an attachment instance of the respective uploader. + # It's an anonymous subclass of Attachment and can therefore + # not be saved. let(:attachment) do clazz = Class.new Attachment clazz.mount_uploader :file, uploader @@ -118,6 +122,7 @@ describe AttachmentsController, type: :controller do att = clazz.new container: work_package, author: user, file: file att.id = 42 att.file.store! + att.send :write_attribute, :file, file.original_filename att end From 6fef6760aac90cc8352878ac518d58276f5928e0 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Mon, 29 Dec 2014 18:53:09 +0200 Subject: [PATCH 28/47] begin menu harmonization --- app/assets/stylesheets/layout/_drop_down.sass | 1 - .../has-dropdown-menu-directive.js | 19 +- .../ui_components/with-dropdown-directive.js | 2 +- .../app/work_packages/controllers/index.js | 1 + .../menus/column-context-menu-controller.js} | 0 .../work_packages/controllers/menus/index.js | 116 ++++++++++++ .../menus/options-dropdown-menu-controller.js | 166 ++++++++++++++++++ .../menus/tasks-dropdown-menu-controller.js | 33 ++++ .../work-package-context-menu-controller.js} | 6 +- .../work-packages-list-controller.js | 4 - .../app/work_packages/directives/index.js | 14 -- frontend/app/work_packages/index.js | 49 ------ frontend/bower.json | 2 +- .../public/templates/work_packages.list.html | 68 +------ .../{ => menus}/column_context_menu.html | 32 ++-- .../menus/options_dropdown_menu.html | 48 +++++ .../menus/tasks_dropdown_menu.html | 9 + .../menus/work_package_context_menu.html | 18 ++ .../work_package_context_menu.html | 34 ---- 19 files changed, 426 insertions(+), 196 deletions(-) rename frontend/app/work_packages/{column-context-menu.js => controllers/menus/column-context-menu-controller.js} (100%) create mode 100644 frontend/app/work_packages/controllers/menus/index.js create mode 100644 frontend/app/work_packages/controllers/menus/options-dropdown-menu-controller.js create mode 100644 frontend/app/work_packages/controllers/menus/tasks-dropdown-menu-controller.js rename frontend/app/work_packages/{work-package-context-menu.js => controllers/menus/work-package-context-menu-controller.js} (94%) rename frontend/public/templates/work_packages/{ => menus}/column_context_menu.html (59%) create mode 100644 frontend/public/templates/work_packages/menus/options_dropdown_menu.html create mode 100644 frontend/public/templates/work_packages/menus/tasks_dropdown_menu.html create mode 100644 frontend/public/templates/work_packages/menus/work_package_context_menu.html delete mode 100644 frontend/public/templates/work_packages/work_package_context_menu.html diff --git a/app/assets/stylesheets/layout/_drop_down.sass b/app/assets/stylesheets/layout/_drop_down.sass index 3ace7f33ce..b744560919 100644 --- a/app/assets/stylesheets/layout/_drop_down.sass +++ b/app/assets/stylesheets/layout/_drop_down.sass @@ -43,7 +43,6 @@ .dropdown position: absolute z-index: 9999999 - display: none .dropdown .dropdown-menu, .dropdown .dropdown-panel diff --git a/frontend/app/ui_components/has-dropdown-menu-directive.js b/frontend/app/ui_components/has-dropdown-menu-directive.js index fe269bab24..c7ee5f4f11 100644 --- a/frontend/app/ui_components/has-dropdown-menu-directive.js +++ b/frontend/app/ui_components/has-dropdown-menu-directive.js @@ -79,13 +79,6 @@ module.exports = function($injector, $window, $parse) { menu to the trigger event triggerOnEvent allows for binding the event for opening the menu to "click" */ - // prepare locals, these define properties to be passed on to the context menu scope - var localKeys = attrs.locals.split(',').map(function(local) { - return local.trim(); - }); - angular.forEach(localKeys, function(key) { - locals[key] = scope[key]; - }); function toggle(event) { active() ? close() : open(event); @@ -96,18 +89,25 @@ module.exports = function($injector, $window, $parse) { } function open(event) { + // prepare locals, these define properties to be passed on to the context menu scope + var localKeys = (attrs.locals || "").split(',').map(function(local) { + return local.trim(); + }); + angular.forEach(localKeys, function(key) { + locals[key] = scope[key]; + }); + ctrl.open(); contextMenu.open(event.target, locals) .then(function(element) { menuElement = element; - angular.element(element).trap(); + // angular.element(element).trap(); }); } function close() { ctrl.close(); - contextMenu.close(); } @@ -154,7 +154,6 @@ module.exports = function($injector, $window, $parse) { function handleWindowClickEvent(event) { if (contextMenu.active() && event.button !== 2) { - scope.$apply(function() { close(); }); diff --git a/frontend/app/ui_components/with-dropdown-directive.js b/frontend/app/ui_components/with-dropdown-directive.js index 0a06e06c59..a2eb32d089 100644 --- a/frontend/app/ui_components/with-dropdown-directive.js +++ b/frontend/app/ui_components/with-dropdown-directive.js @@ -82,7 +82,7 @@ module.exports = function ($rootScope, $window, ESC_KEY, FocusHelper) { dropdownId: '@', focusElementId: '@' }, - link: function (scope, element, attributes) { + link1: function (scope, element, attributes) { var dropdown = jQuery("#" + attributes.dropdownId), trigger; diff --git a/frontend/app/work_packages/controllers/index.js b/frontend/app/work_packages/controllers/index.js index 207268adc6..55f531f082 100644 --- a/frontend/app/work_packages/controllers/index.js +++ b/frontend/app/work_packages/controllers/index.js @@ -238,3 +238,4 @@ angular.module('openproject.workPackages.controllers') 'I18n', require('./dialogs/sorting') ]); +require('./menus'); diff --git a/frontend/app/work_packages/column-context-menu.js b/frontend/app/work_packages/controllers/menus/column-context-menu-controller.js similarity index 100% rename from frontend/app/work_packages/column-context-menu.js rename to frontend/app/work_packages/controllers/menus/column-context-menu-controller.js diff --git a/frontend/app/work_packages/controllers/menus/index.js b/frontend/app/work_packages/controllers/menus/index.js new file mode 100644 index 0000000000..20064e77c3 --- /dev/null +++ b/frontend/app/work_packages/controllers/menus/index.js @@ -0,0 +1,116 @@ +//-- 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. +//++ + + +angular.module('openproject.workPackages') + .factory('ColumnContextMenu', [ + 'ngContextMenu', + function(ngContextMenu) { + return ngContextMenu({ + controller: 'ColumnContextMenuController', + controllerAs: 'contextMenu', + templateUrl: '/templates/work_packages/menus/column_context_menu.html', + container: '.work-packages--list-table-area' + }); + } + ]) + .controller('ColumnContextMenuController', [ + '$scope', + 'ColumnContextMenu', + 'I18n', + 'QueryService', + 'WorkPackagesTableHelper', + 'WorkPackagesTableService', + 'columnsModal', + require('./column-context-menu-controller') + ]) + .factory('OptionsDropdownMenu', [ + 'ngContextMenu', + function(ngContextMenu) { + return ngContextMenu({ + controller: 'OptionsDropdownMenuController', + templateUrl: '/templates/work_packages/menus/options_dropdown_menu.html', + container: '#toolbar' + }); + } + ]) + .controller('OptionsDropdownMenuController', [ + '$scope', + 'I18n', + 'columnsModal', + 'exportModal', + 'saveModal', + 'settingsModal', + 'shareModal', + 'sortingModal', + 'groupingModal', + 'QueryService', + 'AuthorisationService', + '$window', + '$state', + '$timeout', require('./options-dropdown-menu-controller') + ]) + .factory('TasksDropdownMenu', [ + 'ngContextMenu', + function(ngContextMenu) { + return ngContextMenu({ + controller: 'TasksDropdownMenuController', + templateUrl: '/templates/work_packages/menus/tasks_dropdown_menu.html', + container: '#toolbar' + }); + } + ]) + .controller('TasksDropdownMenuController', [ + '$scope', + 'PathHelper', require('./tasks-dropdown-menu-controller') + ]) + .constant('PERMITTED_CONTEXT_MENU_ACTIONS', [ + 'edit', 'watch', 'log_time', + 'duplicate', 'move', 'copy', 'delete' + ]) + .factory('WorkPackageContextMenu', [ + 'ngContextMenu', + function(ngContextMenu) { + return ngContextMenu({ + controller: 'WorkPackageContextMenuController', + controllerAs: 'contextMenu', + templateUrl: '/templates/work_packages/menus/work_package_context_menu.html' + }); + } + ]) + .controller('WorkPackageContextMenuController', [ + '$scope', + 'WorkPackagesTableHelper', + 'WorkPackageContextMenuHelper', + 'WorkPackageService', + 'WorkPackagesTableService', + 'I18n', + '$window', + 'PERMITTED_CONTEXT_MENU_ACTIONS', + require('./work-package-context-menu-controller') + ]); diff --git a/frontend/app/work_packages/controllers/menus/options-dropdown-menu-controller.js b/frontend/app/work_packages/controllers/menus/options-dropdown-menu-controller.js new file mode 100644 index 0000000000..41defa67ba --- /dev/null +++ b/frontend/app/work_packages/controllers/menus/options-dropdown-menu-controller.js @@ -0,0 +1,166 @@ +//-- 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.exports = function($scope, I18n, columnsModal, exportModal, saveModal, settingsModal, shareModal, sortingModal, groupingModal, QueryService, AuthorisationService, $window, $state, $timeout){ + // angular.element($window).bind('click', function() { + // $scope.$emit('hideAllDropdowns'); + // }); + + $scope.$watch('query.displaySums', function(newValue, oldValue) { + $timeout(function() { + $scope.displaySumsLabel = (newValue) ? I18n.t('js.toolbar.settings.hide_sums') + : I18n.t('js.toolbar.settings.display_sums'); + }); + }); + + $scope.saveQuery = function(event){ + if (!$scope.query.dirty) { + return; + } + if($scope.query.isNew()){ + if( allowQueryAction(event, 'create') ){ + // $scope.$emit('hideAllDropdowns'); + saveModal.activate(); + } + } else { + if( allowQueryAction(event, 'update') ) { + QueryService.saveQuery() + .then(function(data){ + $scope.$emit('flashMessage', data.status); + $state.go('work-packages.list', + { query_id: $scope.query.id, query_props: null }, + { notify: false }); + }); + } + } + }; + + $scope.deleteQuery = function(event){ + if( allowQueryAction(event, 'delete') && preventNewQueryAction(event) && deleteConfirmed() ){ + QueryService.deleteQuery() + .then(function(data){ + settingsModal.deactivate(); + $scope.$emit('flashMessage', data.status); + $state.go('work-packages.list', + { query_id: null, query_props: null }, + { reload: true }); + }); + } + }; + + // Modals + $scope.showSaveAsModal = function(event){ + if( allowQueryAction(event, 'create') ) { + showExistingQueryModal.call(saveModal, event); + } + }; + + $scope.showShareModal = function(event){ + if (allowQueryAction(event, 'publicize') || allowQueryAction(event, 'star')) { + showExistingQueryModal.call(shareModal, event); + } + }; + + $scope.showSettingsModal = function(event){ + if( allowQueryAction(event, 'update') ) { + showExistingQueryModal.call(settingsModal, event); + } + }; + + $scope.showExportModal = function(event){ + if( allowWorkPackageAction(event, 'export') ) { + showModal.call(exportModal); + } + }; + + $scope.showColumnsModal = function(){ + showModal.call(columnsModal); + }; + + $scope.showGroupingModal = function(){ + showModal.call(groupingModal); + }; + + $scope.showSortingModal = function(){ + showModal.call(sortingModal); + }; + + $scope.toggleDisplaySums = function(){ + // $scope.$emit('hideAllDropdowns'); + $scope.query.displaySums = !$scope.query.displaySums; + + // This eventually calls the resize event handler defined in the + // WorkPackagesTable directive and ensures that the sum row at the + // table footer is properly displayed. + angular.element($window).trigger('resize'); + }; + + function preventNewQueryAction(event){ + if (event && $scope.query.isNew()) { + event.preventDefault(); + event.stopPropagation(); + return false; + } + return true; + } + + function showModal() { + // $scope.$emit('hideAllDropdowns'); + this.activate(); + } + + function showExistingQueryModal(event) { + if( preventNewQueryAction(event) ){ + // $scope.$emit('hideAllDropdowns'); + this.activate(); + } + } + + function allowQueryAction(event, action) { + return allowAction(event, 'query', action); + } + + function allowWorkPackageAction(event, action) { + return allowAction(event, 'work_package', action); + } + + function allowAction(event, modelName, action) { + if(AuthorisationService.can(modelName, action)){ + return true; + } else { + event.preventDefault(); + event.stopPropagation(); + return false; + } + } + + function deleteConfirmed() { + return $window.confirm(I18n.t('js.text_query_destroy_confirmation')); + } +}; + diff --git a/frontend/app/work_packages/controllers/menus/tasks-dropdown-menu-controller.js b/frontend/app/work_packages/controllers/menus/tasks-dropdown-menu-controller.js new file mode 100644 index 0000000000..adee4b4137 --- /dev/null +++ b/frontend/app/work_packages/controllers/menus/tasks-dropdown-menu-controller.js @@ -0,0 +1,33 @@ +//-- 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.exports = function($scope, PathHelper) { + $scope.workPackageNewPath = function(typeId) { + return PathHelper.staticWorkPackageNewWithParametersPath($scope.projectIdentifier, { type_id: typeId }); + }; +} \ No newline at end of file diff --git a/frontend/app/work_packages/work-package-context-menu.js b/frontend/app/work_packages/controllers/menus/work-package-context-menu-controller.js similarity index 94% rename from frontend/app/work_packages/work-package-context-menu.js rename to frontend/app/work_packages/controllers/menus/work-package-context-menu-controller.js index 40fd251c84..5da6c20351 100644 --- a/frontend/app/work_packages/work-package-context-menu.js +++ b/frontend/app/work_packages/controllers/menus/work-package-context-menu-controller.js @@ -30,8 +30,6 @@ module.exports = function($scope, WorkPackagesTableHelper, WorkPackageContextMen $scope.I18n = I18n; - $scope.hideResourceActions = true; - $scope.$watch('row', function() { if (!$scope.row.checked) { WorkPackagesTableService.setCheckedStateForAllRows($scope.rows, false); @@ -41,8 +39,8 @@ module.exports = function($scope, WorkPackagesTableHelper, WorkPackageContextMen $scope.permittedActions = WorkPackageContextMenuHelper.getPermittedActions(getSelectedWorkPackages(), PERMITTED_CONTEXT_MENU_ACTIONS); }); - $scope.isDetailsViewLinkVisible = function() { - return angular.element('#work-package-context-menu li.open').is(':visible'); + $scope.isDetailsViewLinkPresent = function() { + return !!angular.element('#work-package-context-menu li.open').length; }; $scope.triggerContextMenuAction = function(action, link) { diff --git a/frontend/app/work_packages/controllers/work-packages-list-controller.js b/frontend/app/work_packages/controllers/work-packages-list-controller.js index df23219694..1e1d89d48c 100644 --- a/frontend/app/work_packages/controllers/work-packages-list-controller.js +++ b/frontend/app/work_packages/controllers/work-packages-list-controller.js @@ -274,8 +274,4 @@ module.exports = function($scope, $rootScope, $state, $location, latestTab, $state.go(latestTab.getStateName(), { workPackageId: id, query_props: $location.search().query_props }); } }; - - $scope.workPackageNewPath = function(typeId) { - return PathHelper.staticWorkPackageNewWithParametersPath($scope.projectIdentifier, { type_id: typeId }); - }; }; diff --git a/frontend/app/work_packages/directives/index.js b/frontend/app/work_packages/directives/index.js index 54a4f9f46a..fb8fe6fe94 100644 --- a/frontend/app/work_packages/directives/index.js +++ b/frontend/app/work_packages/directives/index.js @@ -28,20 +28,6 @@ angular.module('openproject.workPackages.directives') .directive('langAttribute', require('./lang-attribute-directive')) - .directive('optionsDropdown', ['I18n', - 'columnsModal', - 'exportModal', - 'saveModal', - 'settingsModal', - 'shareModal', - 'sortingModal', - 'groupingModal', - 'QueryService', - 'AuthorisationService', - '$window', - '$state', - '$timeout', require('./options-dropdown-directive') - ]) .directive('queryColumns', [ 'WorkPackagesTableHelper', 'WorkPackagesTableService', diff --git a/frontend/app/work_packages/index.js b/frontend/app/work_packages/index.js index cb553ff904..b729921adf 100644 --- a/frontend/app/work_packages/index.js +++ b/frontend/app/work_packages/index.js @@ -35,52 +35,3 @@ require('./models'); require('./services'); require('./tabs'); require('./view_models'); - -angular.module('openproject.workPackages') - .factory('ColumnContextMenu', [ - 'ngContextMenu', - function(ngContextMenu) { - - return ngContextMenu({ - controller: 'ColumnContextMenuController', - controllerAs: 'contextMenu', - templateUrl: '/templates/work_packages/column_context_menu.html', - container: '.work-packages--list-table-area' - }); - } - ]) - .controller('ColumnContextMenuController', [ - '$scope', - 'ColumnContextMenu', - 'I18n', - 'QueryService', - 'WorkPackagesTableHelper', - 'WorkPackagesTableService', - 'columnsModal', - require('./column-context-menu') - ]) - .constant('PERMITTED_CONTEXT_MENU_ACTIONS', ['edit', 'watch', 'log_time', - 'duplicate', 'move', 'copy', 'delete' - ]) - .factory('WorkPackageContextMenu', [ - 'ngContextMenu', - function(ngContextMenu) { - - return ngContextMenu({ - controller: 'WorkPackageContextMenuController', - controllerAs: 'contextMenu', - templateUrl: '/templates/work_packages/work_package_context_menu.html' - }); - } - ]) - .controller('WorkPackageContextMenuController', [ - '$scope', - 'WorkPackagesTableHelper', - 'WorkPackageContextMenuHelper', - 'WorkPackageService', - 'WorkPackagesTableService', - 'I18n', - '$window', - 'PERMITTED_CONTEXT_MENU_ACTIONS', - require('./work-package-context-menu') - ]); diff --git a/frontend/bower.json b/frontend/bower.json index 4bf834a725..4bf3d6b92b 100644 --- a/frontend/bower.json +++ b/frontend/bower.json @@ -22,7 +22,7 @@ "jquery-migrate": "~1.2.1", "momentjs": "~2.7.0", "moment-timezone": "~0.2.0", - "angular-context-menu": "finnlabs/angular-context-menu#v0.2.0", + "angular-context-menu": "0xF013/angular-context-menu#220a74c6c05eb084b1630420c2d8b5167e4fc024", "angular-busy": "~4.1.1", "hyperagent": "manwithtwowatches/hyperagent#v0.4.2", "lodash": "~2.4.1", diff --git a/frontend/public/templates/work_packages.list.html b/frontend/public/templates/work_packages.list.html index cb2d8f16e7..3cbdc905dc 100644 --- a/frontend/public/templates/work_packages.list.html +++ b/frontend/public/templates/work_packages.list.html @@ -9,8 +9,9 @@
  • @@ -73,65 +76,6 @@ - - - -
    diff --git a/frontend/public/templates/work_packages/column_context_menu.html b/frontend/public/templates/work_packages/menus/column_context_menu.html similarity index 59% rename from frontend/public/templates/work_packages/column_context_menu.html rename to frontend/public/templates/work_packages/menus/column_context_menu.html index 0c176cf546..1f6ba5282b 100644 --- a/frontend/public/templates/work_packages/column_context_menu.html +++ b/frontend/public/templates/work_packages/menus/column_context_menu.html @@ -1,51 +1,51 @@ \ No newline at end of file +
    diff --git a/frontend/public/templates/work_packages/work_package_details_toolbar.html b/frontend/public/templates/work_packages/work_package_details_toolbar.html index e321e52db0..45e6af7b28 100644 --- a/frontend/public/templates/work_packages/work_package_details_toolbar.html +++ b/frontend/public/templates/work_packages/work_package_details_toolbar.html @@ -2,24 +2,10 @@
    - - From d53f105633ba687d4c16399f5797689a5743d18c Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Tue, 13 Jan 2015 14:11:07 +0200 Subject: [PATCH 30/47] fix table headers menu and change menus for more, query search --- .../has-dropdown-menu-directive.js | 8 +- frontend/app/ui_components/index.js | 5 +- .../selectable-title-directive.js | 200 +----------------- .../work_packages/controllers/menus/index.js | 15 ++ .../query-select-dropdown-menu-controller.js | 197 +++++++++++++++++ .../directives/sort-header-directive.js | 9 +- .../components/selectable_title.html | 28 +-- .../menus/query_select_dropdown_menu.html | 22 ++ .../work_packages/work_packages_table.html | 3 +- 9 files changed, 251 insertions(+), 236 deletions(-) create mode 100644 frontend/app/work_packages/controllers/menus/query-select-dropdown-menu-controller.js create mode 100644 frontend/public/templates/work_packages/menus/query_select_dropdown_menu.html diff --git a/frontend/app/ui_components/has-dropdown-menu-directive.js b/frontend/app/ui_components/has-dropdown-menu-directive.js index c464fc810e..e5d434b0f6 100644 --- a/frontend/app/ui_components/has-dropdown-menu-directive.js +++ b/frontend/app/ui_components/has-dropdown-menu-directive.js @@ -26,7 +26,7 @@ // See doc/COPYRIGHT.rdoc for more details. //++ -module.exports = function($injector, $window, $parse) { +module.exports = function($injector, $window, $parse, FocusHelper) { function getCssPositionProperties(dropdown, trigger) { var hOffset = 0, @@ -111,6 +111,12 @@ module.exports = function($injector, $window, $parse) { function close() { ctrl.close(); + if (element.is('th')) { + element.focus(); + } else { + FocusHelper.focusElement(element); + } + contextMenu.close(); } diff --git a/frontend/app/ui_components/index.js b/frontend/app/ui_components/index.js index 61f9c270a2..0591fc8a2b 100644 --- a/frontend/app/ui_components/index.js +++ b/frontend/app/ui_components/index.js @@ -64,6 +64,7 @@ angular.module('openproject.uiComponents') '$injector', '$window', '$parse', + 'FocusHelper', require('./has-dropdown-menu-directive') ]) .service('I18n', [require('./i18n')]) @@ -85,9 +86,7 @@ angular.module('openproject.uiComponents') up: 38, down: 40 }) - .directive('selectableTitle', ['$sce', 'LABEL_MAX_CHARS', 'KEY_CODES', - require('./selectable-title-directive') - ]) + .directive('selectableTitle', [require('./selectable-title-directive')]) .constant('DOUBLE_CLICK_DELAY', 300) // Thanks to http://stackoverflow.com/a/20445344 .directive('singleClick', [ diff --git a/frontend/app/ui_components/selectable-title-directive.js b/frontend/app/ui_components/selectable-title-directive.js index a2ab27bf12..0b85977f81 100644 --- a/frontend/app/ui_components/selectable-title-directive.js +++ b/frontend/app/ui_components/selectable-title-directive.js @@ -27,7 +27,7 @@ //++ // TODO move to UI components -module.exports = function($sce, LABEL_MAX_CHARS, KEY_CODES) { +module.exports = function() { return { restrict: 'E', replace: true, @@ -36,202 +36,6 @@ module.exports = function($sce, LABEL_MAX_CHARS, KEY_CODES) { groups: '=', transitionMethod: '=' }, - templateUrl: '/templates/components/selectable_title.html', - link: function(scope) { - scope.$watch('groups', refreshFilteredGroups); - scope.$watch('selectedId', selectTitle); - - function refreshFilteredGroups() { - if(scope.groups){ - initFilteredModels(); - } - } - - function selectTitle() { - angular.forEach(scope.filteredGroups, function(group) { - if(group.models.length) { - angular.forEach(group.models, function(model){ - model.highlighted = model.id == scope.selectedId; - }); - } - }); - } - - function initFilteredModels() { - scope.filteredGroups = angular.copy(scope.groups); - angular.forEach(scope.filteredGroups, function(group) { - group.models = group.models.map(function(model){ - return { - label: model[0], - labelHtml: $sce.trustAsHtml(truncate(model[0], LABEL_MAX_CHARS)), - id: model[1], - highlighted: false - }; - }); - }); - } - - function labelHtml(label, filterBy) { - filterBy = filterBy.toLowerCase(); - label = truncate(label, LABEL_MAX_CHARS); - if(label.toLowerCase().indexOf(filterBy) >= 0) { - var labelHtml = label.substr(0, label.toLowerCase().indexOf(filterBy)) - + "" + label.substr(label.toLowerCase().indexOf(filterBy), filterBy.length) + "" - + label.substr(label.toLowerCase().indexOf(filterBy) + filterBy.length); - } else { - var labelHtml = label; - } - return $sce.trustAsHtml(labelHtml); - } - - function truncate(text, chars) { - if (text.length > chars) { - return text.substr(0, chars) + "..."; - } - return text; - } - - function modelIndex(models) { - return models.map(function(model){ - return model.id; - }).indexOf(scope.selectedId); - } - - function performSelect() { - scope.transitionMethod(scope.selectedId); - } - - function nextNonEmptyGroup(groups, currentGroupIndex) { - currentGroupIndex = (currentGroupIndex == undefined) ? -1 : currentGroupIndex; - while(currentGroupIndex < groups.length - 1) { - if(groups[currentGroupIndex + 1].models.length) { - return groups[currentGroupIndex + 1]; - } - currentGroupIndex = currentGroupIndex + 1; - } - return null; - } - - function previousNonEmptyGroup(groups, currentGroupIndex) { - while(currentGroupIndex > 0) { - if(groups[currentGroupIndex - 1].models.length) { - return groups[currentGroupIndex - 1]; - } - currentGroupIndex = currentGroupIndex - 1; - } - return null; - } - - function getModelPosition(groups, selectedId) { - for(var group_index = 0; group_index < groups.length; group_index++) { - var models = groups[group_index].models; - var model_index = modelIndex(models); - if(model_index >= 0) { - return { - group: group_index, - model: model_index - }; - } - } - return false; - } - - function selectNext() { - var groups = scope.filteredGroups; - if(!scope.selectedId) { - var nextGroup = nextNonEmptyGroup(groups); - scope.selectedId = nextGroup ? nextGroup.models[0].id : 0; - } else { - var position = getModelPosition(groups, scope.selectedId); - if (!position) return; - var models = groups[position.group].models; - - if(position.model == models.length - 1){ // It is the last in the group - var nextGroup = nextNonEmptyGroup(groups, position.group); - if(nextGroup) { - scope.selectedId = nextGroup.models[0].id; - } - } else { - scope.selectedId = models[position.model + 1].id; - } - } - } - - function selectPrevious() { - var groups = scope.filteredGroups; - if(scope.selectedId) { - var position = getModelPosition(groups, scope.selectedId); - if (!position) return; - var models = groups[position.group].models; - - if(position.model == 0){ // It is the last in the group - var previousGroup = previousNonEmptyGroup(groups, position.group); - if(previousGroup) { - scope.selectedId = previousGroup.models[previousGroup.models.length - 1].id; - } - } else { - scope.selectedId = models[position.model - 1].id; - } - } - } - - function preventDefault(event) { - event.preventDefault(); - event.stopPropagation(); - } - - angular.element('#title-filter').bind('click', function(event) { - preventDefault(event); - }); - - scope.handleSelection = function(event) { - switch(event.which) { - case KEY_CODES.enter: - performSelect(); - preventDefault(event); - break; - case KEY_CODES.down: - selectNext(); - preventDefault(event); - break; - case KEY_CODES.up: - selectPrevious(); - preventDefault(event); - break; - default: - break; - } - }; - - scope.reload = function(modelId, newTitle) { - scope.selectedTitle = newTitle; - scope.reloadMethod(modelId); - scope.$emit('hideAllDropdowns'); - }; - - scope.filterModels = function(filterBy) { - initFilteredModels(); - - scope.selectedId = 0; - angular.forEach(scope.filteredGroups, function(group) { - if(filterBy.length) { - group.filterBy = filterBy; - group.models = group.models.filter(function(model){ - return model.label.toLowerCase().indexOf(filterBy.toLowerCase()) >= 0; - }); - - if(group.models.length) { - angular.forEach(group.models, function(model){ - model['labelHtml'] = labelHtml(model.label, filterBy); - }); - if(!scope.selectedId) { - group.models[0].highlighted = true; - scope.selectedId = group.models[0].id; - } - } - } - }); - }; - } + templateUrl: '/templates/components/selectable_title.html' }; }; diff --git a/frontend/app/work_packages/controllers/menus/index.js b/frontend/app/work_packages/controllers/menus/index.js index 731cc95444..a2303833eb 100644 --- a/frontend/app/work_packages/controllers/menus/index.js +++ b/frontend/app/work_packages/controllers/menus/index.js @@ -122,4 +122,19 @@ angular.module('openproject.workPackages') container: '.work-packages--details-toolbar' }); } + ]) + .factory('QuerySelectDropdownMenu', [ + 'ngContextMenu', + function(ngContextMenu) { + return ngContextMenu({ + templateUrl: '/templates/work_packages/menus/query_select_dropdown_menu.html', + container: '.title-container', + controller: 'QuerySelectDropdownMenuController' + }); + } + ]) + .controller('QuerySelectDropdownMenuController', [ + '$scope', + '$sce', 'LABEL_MAX_CHARS', 'KEY_CODES', + require('./query-select-dropdown-menu-controller') ]); diff --git a/frontend/app/work_packages/controllers/menus/query-select-dropdown-menu-controller.js b/frontend/app/work_packages/controllers/menus/query-select-dropdown-menu-controller.js new file mode 100644 index 0000000000..dbba3e8194 --- /dev/null +++ b/frontend/app/work_packages/controllers/menus/query-select-dropdown-menu-controller.js @@ -0,0 +1,197 @@ +module.exports = function($scope, $sce, LABEL_MAX_CHARS, KEY_CODES) { + var scope = $scope; + scope.$watch('groups', refreshFilteredGroups); + scope.$watch('selectedId', selectTitle); + + function refreshFilteredGroups() { + if(scope.groups){ + initFilteredModels(); + } + } + + function selectTitle() { + angular.forEach(scope.filteredGroups, function(group) { + if(group.models.length) { + angular.forEach(group.models, function(model){ + model.highlighted = model.id == scope.selectedId; + }); + } + }); + } + + function initFilteredModels() { + scope.filteredGroups = angular.copy(scope.groups); + angular.forEach(scope.filteredGroups, function(group) { + group.models = group.models.map(function(model){ + return { + label: model[0], + labelHtml: $sce.trustAsHtml(truncate(model[0], LABEL_MAX_CHARS)), + id: model[1], + highlighted: false + }; + }); + }); + } + + function labelHtml(label, filterBy) { + filterBy = filterBy.toLowerCase(); + label = truncate(label, LABEL_MAX_CHARS); + if(label.toLowerCase().indexOf(filterBy) >= 0) { + var labelHtml = label.substr(0, label.toLowerCase().indexOf(filterBy)) + + "" + label.substr(label.toLowerCase().indexOf(filterBy), filterBy.length) + "" + + label.substr(label.toLowerCase().indexOf(filterBy) + filterBy.length); + } else { + var labelHtml = label; + } + return $sce.trustAsHtml(labelHtml); + } + + function truncate(text, chars) { + if (text.length > chars) { + return text.substr(0, chars) + "..."; + } + return text; + } + + function modelIndex(models) { + return models.map(function(model){ + return model.id; + }).indexOf(scope.selectedId); + } + + function performSelect() { + scope.transitionMethod(scope.selectedId); + } + + function nextNonEmptyGroup(groups, currentGroupIndex) { + currentGroupIndex = (currentGroupIndex == undefined) ? -1 : currentGroupIndex; + while(currentGroupIndex < groups.length - 1) { + if(groups[currentGroupIndex + 1].models.length) { + return groups[currentGroupIndex + 1]; + } + currentGroupIndex = currentGroupIndex + 1; + } + return null; + } + + function previousNonEmptyGroup(groups, currentGroupIndex) { + while(currentGroupIndex > 0) { + if(groups[currentGroupIndex - 1].models.length) { + return groups[currentGroupIndex - 1]; + } + currentGroupIndex = currentGroupIndex - 1; + } + return null; + } + + function getModelPosition(groups, selectedId) { + for(var group_index = 0; group_index < groups.length; group_index++) { + var models = groups[group_index].models; + var model_index = modelIndex(models); + if(model_index >= 0) { + return { + group: group_index, + model: model_index + }; + } + } + return false; + } + + function selectNext() { + var groups = scope.filteredGroups; + if(!scope.selectedId) { + var nextGroup = nextNonEmptyGroup(groups); + scope.selectedId = nextGroup ? nextGroup.models[0].id : 0; + } else { + var position = getModelPosition(groups, scope.selectedId); + if (!position) return; + var models = groups[position.group].models; + + if(position.model == models.length - 1){ // It is the last in the group + var nextGroup = nextNonEmptyGroup(groups, position.group); + if(nextGroup) { + scope.selectedId = nextGroup.models[0].id; + } + } else { + scope.selectedId = models[position.model + 1].id; + } + } + } + + function selectPrevious() { + var groups = scope.filteredGroups; + if(scope.selectedId) { + var position = getModelPosition(groups, scope.selectedId); + if (!position) return; + var models = groups[position.group].models; + + if(position.model == 0){ // It is the last in the group + var previousGroup = previousNonEmptyGroup(groups, position.group); + if(previousGroup) { + scope.selectedId = previousGroup.models[previousGroup.models.length - 1].id; + } + } else { + scope.selectedId = models[position.model - 1].id; + } + } + } + + function preventDefault(event) { + event.preventDefault(); + event.stopPropagation(); + } + + angular.element('#title-filter').bind('click', function(event) { + preventDefault(event); + }); + + scope.handleSelection = function(event) { + switch(event.which) { + case KEY_CODES.enter: + performSelect(); + preventDefault(event); + break; + case KEY_CODES.down: + selectNext(); + preventDefault(event); + break; + case KEY_CODES.up: + selectPrevious(); + preventDefault(event); + break; + default: + break; + } + }; + + scope.reload = function(modelId, newTitle) { + scope.selectedTitle = newTitle; + scope.reloadMethod(modelId); + scope.$emit('hideAllDropdowns'); + }; + + scope.filterModels = function(filterBy) { + initFilteredModels(); + + scope.selectedId = 0; + angular.forEach(scope.filteredGroups, function(group) { + if(filterBy.length) { + group.filterBy = filterBy; + group.models = group.models.filter(function(model){ + return model.label.toLowerCase().indexOf(filterBy.toLowerCase()) >= 0; + }); + + if(group.models.length) { + angular.forEach(group.models, function(model){ + model['labelHtml'] = labelHtml(model.label, filterBy); + }); + if(!scope.selectedId) { + group.models[0].highlighted = true; + scope.selectedId = group.models[0].id; + } + } + } + }); + }; +} \ No newline at end of file diff --git a/frontend/app/work_packages/directives/sort-header-directive.js b/frontend/app/work_packages/directives/sort-header-directive.js index 4aa3297ff3..00e2968c9e 100644 --- a/frontend/app/work_packages/directives/sort-header-directive.js +++ b/frontend/app/work_packages/directives/sort-header-directive.js @@ -38,8 +38,7 @@ module.exports = function(I18n){ sortable: '=', locale: '=' }, - require: 'hasDropdownMenu', - link: function(scope, element, attributes, dropdownMenuCtrl) { + link: function(scope, element, attributes) { scope.$watch('query.sortation.sortElements', function(sortElements){ var latestSortElement = sortElements[0]; @@ -66,12 +65,8 @@ module.exports = function(I18n){ // active-column class setting function setActiveColumnClass() { - element.toggleClass('active-column', !!scope.currentSortDirection || scope.dropDownMenuOpened); + element.toggleClass('active-column', !!scope.currentSortDirection); } - scope.$watch(dropdownMenuCtrl.opened, function(opened) { - scope.dropDownMenuOpened = opened; - setActiveColumnClass(); - }); scope.$watch('currentSortDirection', setActiveColumnClass); } diff --git a/frontend/public/templates/components/selectable_title.html b/frontend/public/templates/components/selectable_title.html index 51cfac6f42..b1ccbd6bbc 100644 --- a/frontend/public/templates/components/selectable_title.html +++ b/frontend/public/templates/components/selectable_title.html @@ -1,36 +1,12 @@

    - + {{ selectedTitle | characters:50 }}

    - - -
    diff --git a/frontend/public/templates/work_packages/menus/query_select_dropdown_menu.html b/frontend/public/templates/work_packages/menus/query_select_dropdown_menu.html new file mode 100644 index 0000000000..29d17bd8d7 --- /dev/null +++ b/frontend/public/templates/work_packages/menus/query_select_dropdown_menu.html @@ -0,0 +1,22 @@ + diff --git a/frontend/public/templates/work_packages/work_packages_table.html b/frontend/public/templates/work_packages/work_packages_table.html index 4d3b15d74d..235d97027b 100644 --- a/frontend/public/templates/work_packages/work_packages_table.html +++ b/frontend/public/templates/work_packages/work_packages_table.html @@ -30,7 +30,8 @@ position-relative-to=".sort-header-outer" locals="columns, column" sortable="true" - query="query"/> + query="query"> + Date: Tue, 13 Jan 2015 14:38:09 +0200 Subject: [PATCH 31/47] disable clicking on inputs propagation to body and causing the dropdown to close --- frontend/app/ui_components/has-dropdown-menu-directive.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/frontend/app/ui_components/has-dropdown-menu-directive.js b/frontend/app/ui_components/has-dropdown-menu-directive.js index e5d434b0f6..c6d57eee37 100644 --- a/frontend/app/ui_components/has-dropdown-menu-directive.js +++ b/frontend/app/ui_components/has-dropdown-menu-directive.js @@ -106,6 +106,13 @@ module.exports = function($injector, $window, $parse, FocusHelper) { .then(function(element) { menuElement = element; angular.element(element).trap(); + menuElement.on('click', function(e) { + // allow inputs to be clickable + // without closing the dropdown + if (angular.element(e.target).is(':input')) { + e.stopPropagation(); + } + }); }); } From 6b47febe0b8899d5d9cc9a3fcb6a47c023ea6926 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Tue, 13 Jan 2015 14:40:18 +0200 Subject: [PATCH 32/47] add final linebreak --- .../controllers/menus/options-dropdown-menu-controller.js | 1 - .../controllers/menus/tasks-dropdown-menu-controller.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/frontend/app/work_packages/controllers/menus/options-dropdown-menu-controller.js b/frontend/app/work_packages/controllers/menus/options-dropdown-menu-controller.js index 41defa67ba..6eac1818d8 100644 --- a/frontend/app/work_packages/controllers/menus/options-dropdown-menu-controller.js +++ b/frontend/app/work_packages/controllers/menus/options-dropdown-menu-controller.js @@ -163,4 +163,3 @@ module.exports = function($scope, I18n, columnsModal, exportModal, saveModal, se return $window.confirm(I18n.t('js.text_query_destroy_confirmation')); } }; - diff --git a/frontend/app/work_packages/controllers/menus/tasks-dropdown-menu-controller.js b/frontend/app/work_packages/controllers/menus/tasks-dropdown-menu-controller.js index adee4b4137..b3bbeed24c 100644 --- a/frontend/app/work_packages/controllers/menus/tasks-dropdown-menu-controller.js +++ b/frontend/app/work_packages/controllers/menus/tasks-dropdown-menu-controller.js @@ -30,4 +30,4 @@ module.exports = function($scope, PathHelper) { $scope.workPackageNewPath = function(typeId) { return PathHelper.staticWorkPackageNewWithParametersPath($scope.projectIdentifier, { type_id: typeId }); }; -} \ No newline at end of file +} From 7d71c71d2423b717e68172442bc2d3801cb6caf8 Mon Sep 17 00:00:00 2001 From: Mihail Maxacov <0xf013@gmail.com> Date: Tue, 13 Jan 2015 16:12:26 +0200 Subject: [PATCH 33/47] remove unused specs and probably improve accessibility --- .../inaccessible-by-tab-directive.js | 2 + frontend/app/ui_components/index.js | 5 - .../ui_components/with-dropdown-directive.js | 126 ------------------ .../menus/column_context_menu.html | 17 +-- .../menus/details_more_dropdown_menu.html | 4 +- .../menus/options_dropdown_menu.html | 18 +-- .../menus/tasks_dropdown_menu.html | 4 +- .../menus/work_package_context_menu.html | 6 +- .../with-dropdown-directive-test.js | 65 --------- 9 files changed, 27 insertions(+), 220 deletions(-) delete mode 100644 frontend/app/ui_components/with-dropdown-directive.js delete mode 100644 frontend/tests/unit/tests/ui_components/with-dropdown-directive-test.js diff --git a/frontend/app/ui_components/inaccessible-by-tab-directive.js b/frontend/app/ui_components/inaccessible-by-tab-directive.js index d172891d3d..38258b788d 100644 --- a/frontend/app/ui_components/inaccessible-by-tab-directive.js +++ b/frontend/app/ui_components/inaccessible-by-tab-directive.js @@ -42,7 +42,9 @@ module.exports = function() { scope.oldTabIndex = currentTabIndex; } element.attr("tabindex", -1); + element.attr('aria-disabled', true); } else { + element.attr('aria-disabled', false); if (scope.oldTabIndex) { element.attr("tabindex", scope.oldTabIndex); } else { diff --git a/frontend/app/ui_components/index.js b/frontend/app/ui_components/index.js index 0591fc8a2b..355f994971 100644 --- a/frontend/app/ui_components/index.js +++ b/frontend/app/ui_components/index.js @@ -105,11 +105,6 @@ angular.module('openproject.uiComponents') .directive('toolbar', require('./toolbar-directive')) .constant('ESC_KEY', 27) .directive('wikiToolbar', [require('./wiki-toolbar-directive')]) - .directive('withDropdown', ['$rootScope', - '$window', - 'ESC_KEY', - 'FocusHelper', require('./with-dropdown-directive') - ]) .directive('zoomSlider', ['I18n', require('./zoom-slider-directive')]) .filter('ancestorsExpanded', require('./filters/ancestors-expanded-filter')) .filter('latestItems', require('./filters/latest-items-filter')); diff --git a/frontend/app/ui_components/with-dropdown-directive.js b/frontend/app/ui_components/with-dropdown-directive.js deleted file mode 100644 index a2eb32d089..0000000000 --- a/frontend/app/ui_components/with-dropdown-directive.js +++ /dev/null @@ -1,126 +0,0 @@ -//-- 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. -//++ - -// TODO move to UI components -module.exports = function ($rootScope, $window, ESC_KEY, FocusHelper) { - - function position(dropdown, trigger) { - var hOffset = 0, - vOffset = 0; - - if( dropdown.length === 0 || !trigger ) return; - - // Styling logic taken from jQuery-dropdown plugin: https://github.com/plapier/jquery-dropdown - // (dual MIT/GPL-Licensed) - - // Position the dropdown relative-to-parent or relative-to-document - if (dropdown.hasClass('dropdown-relative')) { - var leftPosition = dropdown.hasClass('dropdown-anchor-right') ? - trigger.position().left - (dropdown.outerWidth(true) - trigger.outerWidth(true)) - parseInt(trigger.css('margin-right')) + hOffset : - trigger.position().left + parseInt(trigger.css('margin-left')) + hOffset; - - if (dropdown.hasClass('dropdown-up')) { - var dropdownHeight = dropdown.outerHeight(true); - - dropdown.css({ - left: leftPosition, - top: trigger.position().top - dropdownHeight + parseInt(trigger.css('margin-top')) - vOffset - }); - } else { - dropdown.css({ - left: leftPosition, - top: trigger.position().top + trigger.outerHeight(true) - parseInt(trigger.css('margin-top')) + vOffset - }); - } - } else { - dropdown.css({ - left: dropdown.hasClass('dropdown-anchor-right') ? - trigger.offset().left - (dropdown.outerWidth() - trigger.outerWidth()) + hOffset : trigger.offset().left + hOffset, - top: trigger.offset().top + trigger.outerHeight() + vOffset - }); - } - } - - function accessDropdown(dropdown) { - var links = dropdown.find('a'); - - if (links.length > 0) { - angular.element(links[0]).focus(); - } - - angular.element(dropdown).trap(); - } - - return { - restrict: 'EA', - scope: { - dropdownId: '@', - focusElementId: '@' - }, - link1: function (scope, element, attributes) { - var dropdown = jQuery("#" + attributes.dropdownId), - trigger; - - $rootScope.$on('hideAllDropdowns', function(event){ - jQuery('.dropdown').hide(); - }); - - angular.element($window).on('resize', function(event) { - if(dropdown.is(':visible')) { - position(dropdown, trigger); - } - }); - - element.on('click', function (event) { - var showDropdown = dropdown.is(':hidden'); - - trigger = jQuery(this); - - event.preventDefault(); - event.stopPropagation(); - - scope.$emit('hideAllDropdowns'); - if (showDropdown) dropdown.show(); - - position(dropdown, trigger); - accessDropdown(dropdown); - - if(attributes.focusElementId) { - angular.element('#' + attributes.focusElementId).focus(); - } - }); - - angular.element(dropdown).on('keyup', function(event) { - if (event.keyCode === ESC_KEY) { - scope.$emit('hideAllDropdowns'); - FocusHelper.focusElement(element); - } - }); - } - }; - }; diff --git a/frontend/public/templates/work_packages/menus/column_context_menu.html b/frontend/public/templates/work_packages/menus/column_context_menu.html index 1f6ba5282b..04a95a8746 100644 --- a/frontend/public/templates/work_packages/menus/column_context_menu.html +++ b/frontend/public/templates/work_packages/menus/column_context_menu.html @@ -1,51 +1,52 @@