Merge branch 'release/10.2' into dev

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

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

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

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

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) %>
   &nbsp;
   <a href="<%= OpenProject::Enterprise.upgrade_path %>" class="display-inline button -tiny -highlight" title="<%= t(:title_enterprise_upgrade) %>"><%= t(:button_upgrade) %></a>
 <% end %>

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/
 
 <!-- include(apiv3/index.apib) -->

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/
 
 <!-- include(apiv3/index.apib) -->

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;

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

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

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,

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: {

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

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)
 

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

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

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

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)

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)