diff --git a/app/contracts/relations/delete_contract.rb b/app/contracts/relations/delete_contract.rb new file mode 100644 index 0000000000..08b641c6ea --- /dev/null +++ b/app/contracts/relations/delete_contract.rb @@ -0,0 +1,37 @@ +#-- encoding: UTF-8 + +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2017 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2017 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +module Relations + class DeleteContract < ::DeleteContract + delete_permission -> { + user.allowed_to? :manage_work_package_relations, model.from.project + } + end +end diff --git a/app/services/relations/base_service.rb b/app/services/relations/base_service.rb index 8906e8feb6..63c3f84cdb 100644 --- a/app/services/relations/base_service.rb +++ b/app/services/relations/base_service.rb @@ -40,33 +40,33 @@ class Relations::BaseService private - def update_relation(relation, attributes) - relation.attributes = relation.attributes.merge attributes + def update_relation(model, attributes) + model.attributes = model.attributes.merge attributes - success, errors = validate_and_save(relation, user) - success, errors = retry_with_inverse_for_relates(relation, errors) unless success + success, errors = validate_and_save(model, user) + success, errors = retry_with_inverse_for_relates(model, errors) unless success - result = ServiceResult.new success: success, errors: errors, result: relation + result = ServiceResult.new success: success, errors: errors, result: model - if success && relation.follows? - reschedule_result = reschedule(relation) + if success && model.follows? + reschedule_result = reschedule(model) result.merge!(reschedule_result) end result end - def set_defaults(relation) - if Relation::TYPE_FOLLOWS == relation.relation_type - relation.delay ||= 0 + def set_defaults(model) + if Relation::TYPE_FOLLOWS == model.relation_type + model.delay ||= 0 else - relation.delay = nil + model.delay = nil end end - def reschedule(relation) + def reschedule(model) schedule_result = WorkPackages::SetScheduleService - .new(user: user, work_package: relation.to) + .new(user: user, work_package: model.to) .call # The to-work_package will not be altered by the schedule service so @@ -80,12 +80,12 @@ class Relations::BaseService schedule_result end - def retry_with_inverse_for_relates(relation, errors) + def retry_with_inverse_for_relates(model, errors) if errors.symbols_for(:base).include?(:"typed_dag.circular_dependency") && - relation.canonical_type == Relation::TYPE_RELATES - relation.from, relation.to = relation.to, relation.from + model.canonical_type == Relation::TYPE_RELATES + model.from, model.to = model.to, model.from - validate_and_save(relation, user) + validate_and_save(model, user) else [false, errors] end diff --git a/app/services/relations/delete_service.rb b/app/services/relations/delete_service.rb new file mode 100644 index 0000000000..fc0ede0189 --- /dev/null +++ b/app/services/relations/delete_service.rb @@ -0,0 +1,29 @@ +#-- copyright +# OpenProject is a project management system. +# Copyright (C) 2012-2019 the OpenProject Foundation (OPF) +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License version 3. +# +# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows: +# Copyright (C) 2006-2017 Jean-Philippe Lang +# Copyright (C) 2010-2013 the ChiliProject Team +# +# This program is free software; you can redistribute it and/or +# modify it under the terms of the GNU General Public License +# as published by the Free Software Foundation; either version 2 +# of the License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. +# +# See doc/COPYRIGHT.rdoc for more details. +#++ + +class Relations::DeleteService < ::BaseServices::Delete; end diff --git a/app/services/relations/update_service.rb b/app/services/relations/update_service.rb index eb498f243e..46c410e4fe 100644 --- a/app/services/relations/update_service.rb +++ b/app/services/relations/update_service.rb @@ -29,17 +29,17 @@ #++ class Relations::UpdateService < Relations::BaseService - attr_accessor :relation + attr_accessor :model - def initialize(user:, relation:) + def initialize(user:, model:) super(user: user) - self.relation = relation + self.model = model self.contract_class = Relations::UpdateContract end - def call(attributes: {}, send_notifications: true) - in_context(send_notifications) do - update_relation relation, attributes + def call(attributes) + in_context(attributes[:send_notifications]) do + update_relation model, attributes end end end diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 8e469939b6..db8f5b0a01 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -36,7 +36,7 @@ See docs/COPYRIGHT.rdoc for more details. end %> <% users_info = user_limit && content_tag(:div) do %> - <%= t(:label_enterprise_active_users, current: User.active.count, limit: user_limit) %> + <%= t(:label_enterprise_active_users, current: OpenProject::Enterprise.active_user_count, limit: user_limit) %>   <%= t(:button_upgrade) %> <% end %> diff --git a/docs/api/apiv3-doc-dev.apib b/docs/api/apiv3-doc-dev.apib index 438742669e..46e543745d 100644 --- a/docs/api/apiv3-doc-dev.apib +++ b/docs/api/apiv3-doc-dev.apib @@ -3,6 +3,6 @@ FORMAT: 1A # OpenProject API V3 (DEV) You're looking at the current **development** documentation of the OpenProject APIv3. If you're interested in the current -stable version, please go to https://docs.openproject.org/apiv3-doc/ +stable version, please go to https://docs.openproject.org/api/ diff --git a/docs/api/apiv3-doc-stable.apib b/docs/api/apiv3-doc-stable.apib index 4381c7ff98..d8bba063ac 100644 --- a/docs/api/apiv3-doc-stable.apib +++ b/docs/api/apiv3-doc-stable.apib @@ -3,6 +3,6 @@ FORMAT: 1A # OpenProject API V3 (Stable) You're looking at the current **stable** documentation of the OpenProject APIv3. If you're interested in the current -development version, please go to https://docs.openproject.org/apiv3-doc-dev/ +development version, please go to https://docs.openproject.org/api/ diff --git a/frontend/src/app/components/main-menu/main-menu-toggle.service.ts b/frontend/src/app/components/main-menu/main-menu-toggle.service.ts index f9e4e11cdd..68e8d32ca4 100644 --- a/frontend/src/app/components/main-menu/main-menu-toggle.service.ts +++ b/frontend/src/app/components/main-menu/main-menu-toggle.service.ts @@ -95,8 +95,6 @@ export class MainMenuToggleService { if (!this.showNavigation) { // sidebar is hidden -> show menu if (this.deviceService.isMobile) { // mobile version this.setWidth(window.innerWidth); - // On mobile the main menu shall close whenever you click outside the menu. - this.setupAutocloseMainMenu(); } else { // desktop version this.saveWidth(parseInt(window.OpenProject.guardedLocalStorage(this.localStorageKey) as string)); } @@ -169,28 +167,6 @@ export class MainMenuToggleService { this.htmlNode.style.setProperty("--main-menu-width", this.elementWidth + 'px'); } - private setupAutocloseMainMenu():void { - let that = this; - jQuery('#main-menu').off('focusout.main_menu'); - jQuery('#main-menu').on('focusout.main_menu', function (event) { - let originalEvent = event.originalEvent as FocusEvent; - // Check that main menu is not closed and that the `focusout` event is not a click on an element - // that tries to close the menu anyways. - if (!that.showNavigation || document.getElementById('main-menu-toggle') === originalEvent.relatedTarget) { - return; - } - else { - // There might be a time gap between `focusout` and the focussing of the activeElement, thus we need a timeout. - setTimeout(function() { - if (!jQuery.contains(document.getElementById('main-menu')!, originalEvent.relatedTarget as Element)) { - // activeElement is outside of main menu. - that.closeMenu(); - } - }, 0); - } - }); - } - private snapBack():void { if (this.elementWidth <= 10) { this.elementWidth = 0; diff --git a/lib/api/v3/relations/relations_api.rb b/lib/api/v3/relations/relations_api.rb index cf40459e50..83c66522a3 100644 --- a/lib/api/v3/relations/relations_api.rb +++ b/lib/api/v3/relations/relations_api.rb @@ -36,8 +36,6 @@ module API module V3 module Relations class RelationsAPI < ::API::OpenProjectAPI - helpers ::API::V3::Relations::RelationsHelper - resources :relations do get do scope = Relation @@ -50,39 +48,13 @@ module API end route_param :id, type: Integer, desc: 'Relation ID' do - get do - representer.new( - Relation.find_by_id!(params[:id]), - current_user: current_user, - embed_links: true - ) - end - - patch do - rep = parse_representer.new Relation.new, current_user: current_user - relation = rep.from_json request.body.read - attributes = filter_attributes relation - service = ::Relations::UpdateService.new relation: Relation.find_by_id!(params[:id]), - user: current_user - call = service.call attributes: attributes, - send_notifications: (params[:notify] != 'false') - - if call.success? - representer.new call.result, current_user: current_user, embed_links: true - else - fail ::API::Errors::ErrorBase.create_and_merge_errors(call.errors) - end + after_validation do + @relation = Relation.visible.find(params[:id]) end - delete do - project_id = project_id_for_relation params[:id] - project = Project.find project_id - - authorize :manage_work_package_relations, context: project - - Relation.destroy params[:id] - status 204 - end + get &::API::V3::Utilities::Endpoints::Show.new(model: Relation).mount + patch &::API::V3::Utilities::Endpoints::Update.new(model: Relation).mount + delete &::API::V3::Utilities::Endpoints::Delete.new(model: Relation).mount end end end diff --git a/lib/api/v3/relations/relations_helper.rb b/lib/api/v3/relations/relations_helper.rb index 551bab2079..135d39d0f6 100644 --- a/lib/api/v3/relations/relations_helper.rb +++ b/lib/api/v3/relations/relations_helper.rb @@ -30,14 +30,6 @@ module API module V3 module Relations module RelationsHelper - def filter_attributes(relation) - relation - .changes - .map { |k, v| [k, v.last] } - .to_h - .with_indifferent_access - end - def representer ::API::V3::Relations::RelationRepresenter end @@ -45,17 +37,6 @@ module API def parse_representer ::API::V3::Relations::RelationPayloadRepresenter end - - def project_id_for_relation(id) - relations = Relation.table_name - work_packages = WorkPackage.table_name - - Relation - .joins(:from) - .where("#{relations}.id" => id) - .pluck("#{work_packages}.project_id") - .first - end end end end diff --git a/lib/api/v3/users/user_representer.rb b/lib/api/v3/users/user_representer.rb index f3b4a49c21..0dc7ced22d 100644 --- a/lib/api/v3/users/user_representer.rb +++ b/lib/api/v3/users/user_representer.rb @@ -136,8 +136,7 @@ module API property :mail, as: :email, - render_nil: true, - getter: ->(*) { pref.hide_mail ? nil : mail } + cache_if: -> { !represented.pref.hide_mail || current_user_is_admin_or_self } property :avatar, exec_context: :decorator, @@ -147,7 +146,8 @@ module API property :status, getter: ->(*) { status_name }, setter: ->(fragment:, represented:, **) { represented.status = User::STATUSES[fragment.to_sym] }, - render_nil: true + render_nil: true, + cache_if: -> { current_user_is_admin_or_self } property :identity_url, exec_context: :decorator, diff --git a/lib/open_project/static/links.rb b/lib/open_project/static/links.rb index c954670820..e5adfa4505 100644 --- a/lib/open_project/static/links.rb +++ b/lib/open_project/static/links.rb @@ -31,7 +31,6 @@ module OpenProject module Static module Links class << self - def help_link_overridden? OpenProject::Configuration.force_help_link.present? end @@ -143,7 +142,7 @@ module OpenProject label: :label_add_edit_translations }, api_docs: { - href: 'https://www.openproject.org/api', + href: 'https://docs.openproject.org/api', label: :label_api_documentation }, text_formatting: { diff --git a/modules/meeting/spec/controllers/meeting_contents_controller_spec.rb b/modules/meeting/spec/controllers/meeting_contents_controller_spec.rb index f5a6e15d21..f7fa1fcd8a 100644 --- a/modules/meeting/spec/controllers/meeting_contents_controller_spec.rb +++ b/modules/meeting/spec/controllers/meeting_contents_controller_spec.rb @@ -26,9 +26,15 @@ describe MeetingContentsController do shared_let(:author) { FactoryBot.create(:user, member_in_project: project, member_through_role: role) } shared_let(:watcher1) { FactoryBot.create(:user, member_in_project: project, member_through_role: role) } shared_let(:watcher2) { FactoryBot.create(:user, member_in_project: project, member_through_role: role) } - shared_let(:meeting) { FactoryBot.create(:meeting, author: author, project: project) } + shared_let(:meeting) do + User.execute_as author do + FactoryBot.create(:meeting, author: author, project: project) + end + end shared_let(:meeting_agenda) do - FactoryBot.create(:meeting_agenda, meeting: meeting) + User.execute_as author do + FactoryBot.create(:meeting_agenda, meeting: meeting) + end end before(:each) do @@ -43,7 +49,7 @@ describe MeetingContentsController do end shared_examples_for 'delivered by mail' do - before { put action, params: { meeting_id: meeting.id } } + before { put action, params: { meeting_id: meeting.id } } it { expect(ActionMailer::Base.deliveries.count).to eql(mail_count) } end diff --git a/spec/features/work_packages/table/inline_create/create_work_packages_spec.rb b/spec/features/work_packages/table/inline_create/create_work_packages_spec.rb index a93fc6f99b..9975d0b7bc 100644 --- a/spec/features/work_packages/table/inline_create/create_work_packages_spec.rb +++ b/spec/features/work_packages/table/inline_create/create_work_packages_spec.rb @@ -104,6 +104,9 @@ describe 'inline create work package', js: true do wp_table.visit! filters.open filters.add_filter_by cf_list.name, 'is', cf_list.custom_options.second.name, cf_accessor_frontend + + sleep(0.3) + columns.open_modal columns.add(cf_list.name, save_changes: true) diff --git a/spec/lib/api/v3/users/user_representer_spec.rb b/spec/lib/api/v3/users/user_representer_spec.rb index 49649a746a..fde4662aa3 100644 --- a/spec/lib/api/v3/users/user_representer_spec.rb +++ b/spec/lib/api/v3/users/user_representer_spec.rb @@ -51,6 +51,8 @@ describe ::API::V3::Users::UserRepresenter do is_expected.not_to have_json_path('admin') is_expected.not_to have_json_path('updatedAt') is_expected.not_to have_json_path('createdAt') + is_expected.not_to have_json_path('status') + is_expected.not_to have_json_path('email') end end @@ -65,6 +67,8 @@ describe ::API::V3::Users::UserRepresenter do is_expected.to have_json_path('lastName') is_expected.to have_json_path('updatedAt') is_expected.to have_json_path('createdAt') + is_expected.to have_json_path('status') + is_expected.to have_json_path('email') is_expected.not_to have_json_path('admin') end @@ -79,6 +83,9 @@ describe ::API::V3::Users::UserRepresenter do is_expected.to have_json_path('firstName') is_expected.to have_json_path('lastName') is_expected.to have_json_path('name') + is_expected.to have_json_path('status') + is_expected.to have_json_path('email') + is_expected.to have_json_path('admin') end it_behaves_like 'has UTC ISO 8601 date and time' do @@ -95,24 +102,44 @@ describe ::API::V3::Users::UserRepresenter do describe 'email' do let(:user) { FactoryBot.build_stubbed(:user, status: 1, preference: preference) } + shared_examples_for 'shows the users E-Mail address' do + it do + is_expected.to be_json_eql(user.mail.to_json).at_path('email') + end + end + context 'user shows his E-Mail address' do let(:preference) { FactoryBot.build(:user_preference, hide_mail: false) } - it 'shows the users E-Mail address' do - is_expected.to be_json_eql(user.mail.to_json).at_path('email') - end + it_behaves_like 'shows the users E-Mail address' end context 'user hides his E-Mail address' do let(:preference) { FactoryBot.build(:user_preference, hide_mail: true) } it 'does not render the users E-Mail address' do - is_expected.to be_json_eql(nil.to_json).at_path('email') + is_expected + .not_to have_json_path('email') + end + + context 'if an admin inquires' do + let(:current_user) { FactoryBot.build_stubbed(:admin) } + + it_behaves_like 'shows the users E-Mail address' + end + + context 'if the user inquires himself' do + let(:current_user) { user } + + it_behaves_like 'shows the users E-Mail address' end end end describe 'status' do + # as only admin or self can see the status + let(:current_user) { user } + it 'contains the name of the account status' do is_expected.to be_json_eql('active'.to_json).at_path('status') end diff --git a/spec/requests/api/v3/relations/relations_api_spec.rb b/spec/requests/api/v3/relations/relations_api_spec.rb index 6e191ce996..337178fa23 100644 --- a/spec/requests/api/v3/relations/relations_api_spec.rb +++ b/spec/requests/api/v3/relations/relations_api_spec.rb @@ -32,6 +32,7 @@ describe 'API v3 Relation resource', type: :request, content_type: :json do include API::V3::Utilities::PathHelper let(:user) { FactoryBot.create :admin } + let(:current_user) { user } let!(:from) { FactoryBot.create :work_package } let!(:to) { FactoryBot.create :work_package } @@ -65,7 +66,7 @@ describe 'API v3 Relation resource', type: :request, content_type: :json do end before do - login_as user + login_as current_user end describe "creating a relation" do @@ -368,15 +369,45 @@ describe 'API v3 Relation resource', type: :request, content_type: :json do end describe "deleting a relation" do - before do - relation + let(:path) do + api_v3_paths.relation(relation.id) + end - delete api_v3_paths.relation(relation.id) + let(:permissions) { %i[view_work_packages manage_work_package_relations] } + let(:role) { FactoryBot.create(:role, permissions: permissions) } + + let(:current_user) do + FactoryBot.create(:user).tap do |user| + FactoryBot.create(:member, + project: to.project, + user: user, + roles: [role]) + FactoryBot.create(:member, + project: from.project, + user: user, + roles: [role]) + end + end + + before do + delete path end it "should return 204 and destroy the relation" do expect(last_response.status).to eq 204 - expect(Relation.exists?(relation.id)).to eq false + expect(Relation.exists?(relation.id)).to be_falsey + end + + context 'lacking the permission' do + let(:permissions) { %i[view_work_packages] } + + it 'returns 403' do + expect(last_response.status).to eq 403 + end + + it 'leaves the relation' do + expect(Relation.exists?(relation.id)).to be_truthy + end end end @@ -446,4 +477,58 @@ describe 'API v3 Relation resource', type: :request, content_type: :json do .at_path('_embedded/elements/0/id') end end + + describe 'GET /api/v3/relations/:id' do + let(:path) do + api_v3_paths.relation(relation.id) + end + + let(:role) { FactoryBot.create(:role, permissions: [:view_work_packages]) } + + let(:current_user) do + FactoryBot.create(:user).tap do |user| + FactoryBot.create(:member, + project: to.project, + user: user, + roles: [role]) + FactoryBot.create(:member, + project: from.project, + user: user, + roles: [role]) + end + end + + before do + get path + end + + context 'for a relation with visible work packages' do + it 'returns 200' do + expect(last_response.status).to eql 200 + end + + it 'returns the relation' do + expect(last_response.body) + .to be_json_eql API::V3::Relations::RelationRepresenter.new(relation, current_user: current_user, embed_links: true).to_json + end + end + + context 'for a relation with an invisible work package' do + let(:invisible_relation) do + invisible_wp = FactoryBot.create(:work_package) + + FactoryBot.create :relation, + from: from, + to: invisible_wp + end + + let(:path) do + api_v3_paths.relation(invisible_relation.id) + end + + it 'returns 404 NOT FOUND' do + expect(last_response.status).to eql 404 + end + end + end end diff --git a/spec/requests/api/v3/relations_resource_spec.rb b/spec/requests/api/v3/relations_resource_spec.rb index 2582d51a4a..57c6416b4d 100644 --- a/spec/requests/api/v3/relations_resource_spec.rb +++ b/spec/requests/api/v3/relations_resource_spec.rb @@ -36,21 +36,21 @@ describe 'API v3 Relation resource', type: :request do let(:project) { FactoryBot.create(:project_with_types) } let(:current_user) do FactoryBot.create(:user, - member_in_project: project, - member_through_role: role) + member_in_project: project, + member_through_role: role) end let(:permissions) { [] } let(:role) { FactoryBot.create(:role, permissions: permissions) } let(:work_package) do FactoryBot.create(:work_package, - project: project, - type: project.types.first) + project: project, + type: project.types.first) end let(:visible_work_package) do FactoryBot.create(:work_package, - project: project, - type: project.types.first) + project: project, + type: project.types.first) end let(:invisible_work_package) do # will be inside another project @@ -58,13 +58,13 @@ describe 'API v3 Relation resource', type: :request do end let(:visible_relation) do FactoryBot.create(:relation, - from: work_package, - to: visible_work_package) + from: work_package, + to: visible_work_package) end let(:invisible_relation) do FactoryBot.create(:relation, - from: work_package, - to: invisible_work_package) + from: work_package, + to: invisible_work_package) end before do diff --git a/spec/services/relations/update_service_spec.rb b/spec/services/relations/update_service_spec.rb index 23a1a1e832..7bc1ec4f9b 100644 --- a/spec/services/relations/update_service_spec.rb +++ b/spec/services/relations/update_service_spec.rb @@ -50,7 +50,7 @@ describe Relations::UpdateService do start_date: work_package2_start_date) end let(:instance) do - described_class.new(user: user, relation: relation) + described_class.new(user: user, model: relation) end let(:relation) do relation = FactoryBot.build_stubbed(:relation) diff --git a/spec/views/users/index.html.erb_spec.rb b/spec/views/users/index.html.erb_spec.rb index e65d69d704..e004a5423d 100644 --- a/spec/views/users/index.html.erb_spec.rb +++ b/spec/views/users/index.html.erb_spec.rb @@ -33,6 +33,8 @@ describe 'users/index', type: :view do let!(:user) { FactoryBot.create :user, firstname: "Scarlet", lastname: "Scallywag" } before do + User.system # create system user which is active but should not count towards limit + assign(:users, [admin, user]) assign(:status, "all") assign(:groups, Group.all)