Merge branch 'release/10.6' into dev

app/controllers/concerns/auth_source_sso.rb

@@ -123,13 +123,16 @@ module AuthSourceSSO
   end
 
   def sso_login_failed?(user)
-    user.nil? || user.new_record? || !user.active?
+    user.nil? || user.new_record? || !(user.active? || user.invited?)
   end
 
   def handle_sso_for!(user, login)
     if sso_login_failed?(user)
       handle_sso_failure!({ user: user, login: login })
     else # valid user
+      # If a user is invited, ensure it gets activated
+      activate_user_if_invited! user
+
       handle_sso_success user
     end
   end
@@ -141,6 +144,12 @@ module AuthSourceSSO
     user
   end
 
+  def activate_user_if_invited!(user)
+    return unless user.invited?
+
+    user.activate!
+  end
+
   def perform_post_logout(prev_session)
     if prev_session[:user_from_auth_header] && header_slo_url.present?
       redirect_to header_slo_url

app/models/queries/work_packages/filter/category_filter.rb

@@ -48,9 +48,11 @@ class Queries::WorkPackages::Filter::CategoryFilter <
   end
 
   def value_objects
-    int_values = values.map(&:to_i)
+    available_categories = all_project_categories.index_by(&:id)
 
-    all_project_categories.select { |c| int_values.include?(c.id) }
+    values
+      .map { |category_id| available_categories[category_id.to_i] }
+      .compact
   end
 
   def ar_object_filter?

app/models/queries/work_packages/filter/group_filter.rb

@@ -53,9 +53,11 @@ class Queries::WorkPackages::Filter::GroupFilter < Queries::WorkPackages::Filter
   end
 
   def value_objects
-    value_ints = values.map(&:to_i)
+    available_groups = all_groups.index_by(&:id)
 
-    all_groups.select { |g| value_ints.include?(g.id) }
+    values
+      .map { |group_id| available_groups[group_id.to_i] }
+      .compact
   end
 
   def where

app/models/queries/work_packages/filter/priority_filter.rb

@@ -50,9 +50,11 @@ class Queries::WorkPackages::Filter::PriorityFilter <
   end
 
   def value_objects
-    value_ints = values.map(&:to_i)
+    available_priorities = priorities.index_by(&:id)
 
-    priorities.select { |p| value_ints.include? p.id }
+    values
+      .map { |priority_id| available_priorities[priority_id.to_i] }
+      .compact
   end
 
   private

app/models/queries/work_packages/filter/project_filter.rb

@@ -58,9 +58,11 @@ class Queries::WorkPackages::Filter::ProjectFilter < Queries::WorkPackages::Filt
   end
 
   def value_objects
-    value_ints = values.map(&:to_i)
+    available_projects = visible_projects.index_by(&:id)
 
-    visible_projects.select { |p| value_ints.include?(p.id) }
+    values
+      .map { |project_id| available_projects[project_id.to_i] }
+      .compact
   end
 
   private

app/models/queries/work_packages/filter/role_filter.rb

@@ -55,9 +55,11 @@ class Queries::WorkPackages::Filter::RoleFilter < Queries::WorkPackages::Filter:
   end
 
   def value_objects
-    value_ints = values.map(&:to_i)
+    available_roles = roles.index_by(&:id)
 
-    roles.select { |r| value_ints.include?(r.id) }
+    values
+      .map { |role_id| available_roles[role_id.to_i] }
+      .compact
   end
 
   def where

app/models/queries/work_packages/filter/status_filter.rb

@@ -30,7 +30,7 @@
 
 class Queries::WorkPackages::Filter::StatusFilter < Queries::WorkPackages::Filter::WorkPackageFilter
   def allowed_values
-    all_statuses.map { |s| [s.name, s.id.to_s] }
+    all_statuses.values.map { |s| [s.name, s.id.to_s] }
   end
 
   def available_operators
@@ -54,13 +54,13 @@ class Queries::WorkPackages::Filter::StatusFilter < Queries::WorkPackages::Filte
   end
 
   def value_objects
-    values_ids = values.map(&:to_i)
-
-    all_statuses.select { |status| values_ids.include?(status.id) }
+    values
+      .map { |status_id| all_statuses[status_id.to_i] }
+      .compact
   end
 
   def allowed_objects
-    all_statuses
+    all_statuses.values
   end
 
   def ar_object_filter?
@@ -73,7 +73,7 @@ class Queries::WorkPackages::Filter::StatusFilter < Queries::WorkPackages::Filte
     key = 'Queries::WorkPackages::Filter::StatusFilter/all_statuses'
 
     RequestStore.fetch(key) do
-      Status.all.to_a
+      Status.all.to_a.index_by(&:id)
     end
   end
 

app/models/queries/work_packages/filter/subproject_filter.rb

@@ -65,9 +65,11 @@ class Queries::WorkPackages::Filter::SubprojectFilter <
   end
 
   def value_objects
-    value_ints = values.map(&:to_i)
+    available_subprojects = visible_subprojects.index_by(&:id)
 
-    visible_subprojects.where(id: value_ints)
+    values
+      .map { |subproject_id| available_subprojects[subproject_id.to_i] }
+      .compact
   end
 
   def where

app/models/queries/work_packages/filter/type_filter.rb

@@ -53,9 +53,11 @@ class Queries::WorkPackages::Filter::TypeFilter <
   end
 
   def value_objects
-    value_ints = values.map(&:to_i)
+    available_types = types.index_by(&:id)
 
-    types.select { |t| value_ints.include?(t.id) }
+    values
+      .map { |type_id| available_types[type_id.to_i] }
+      .compact
   end
 
   private

app/models/queries/work_packages/filter/version_filter.rb

@@ -54,9 +54,11 @@ class Queries::WorkPackages::Filter::VersionFilter <
   end
 
   def value_objects
-    value_ints = values.map(&:to_i)
+    available_versions = versions.index_by(&:id)
 
-    versions.select { |v| value_ints.include?(v.id) }
+    values
+      .map { |version_id| available_versions[version_id.to_i] }
+      .compact
   end
 
   private

app/workers/application_job.rb

@@ -29,6 +29,11 @@
 require 'active_job'
 
 class ApplicationJob < ::ActiveJob::Base
+  around_perform do |_job, block|
+    reload_mailer_configuration!
+    with_clean_request_store { block.call }
+  end
+
   ##
   # Return a priority number on the given payload
   def self.priority_number(prio = :default)
@@ -52,8 +57,31 @@ class ApplicationJob < ::ActiveJob::Base
     end
   end
 
-  def self.inherited(child)
-    child.prepend Setup
+  # Resets the thread local request store.
+  # This should be done, because normal application code expects the RequestStore to be
+  # invalidated between multiple requests and does usually not care whether it is executed
+  # from a request or from a delayed job.
+  # For a delayed job, each job execution is the thing that comes closest to
+  # the concept of a new request.
+  def with_clean_request_store
+    store = RequestStore.store
+
+    begin
+      RequestStore.clear!
+      yield
+    ensure
+      # Reset to previous value
+      RequestStore.clear!
+      RequestStore.store.merge! store
+    end
+  end
+
+  # Reloads the thread local ActionMailer configuration.
+  # Since the email configuration is now done in the web app, we need to
+  # make sure that any changes to the configuration is correctly picked up
+  # by the background jobs at runtime.
+  def reload_mailer_configuration!
+    OpenProject::Configuration.reload_mailer_configuration!
   end
 
   # Delayed jobs can have a status:
@@ -63,42 +91,4 @@ class ApplicationJob < ::ActiveJob::Base
   def status_reference
     nil
   end
-
-  module Setup
-    def perform(*args)
-      before_perform!
-      with_clean_request_store { super }
-    end
-
-    def before_perform!
-      reload_mailer_configuration!
-    end
-
-    # Resets the thread local request store.
-    # This should be done, because normal application code expects the RequestStore to be
-    # invalidated between multiple requests and does usually not care whether it is executed
-    # from a request or from a delayed job.
-    # For a delayed job, each job execution is the thing that comes closest to
-    # the concept of a new request.
-    def with_clean_request_store
-      store = RequestStore.store
-
-      begin
-        RequestStore.clear!
-        yield
-      ensure
-        # Reset to previous value
-        RequestStore.clear!
-        RequestStore.store.merge! store
-      end
-    end
-
-    # Reloads the thread local ActionMailer configuration.
-    # Since the email configuration is now done in the web app, we need to
-    # make sure that any changes to the configuration is correctly picked up
-    # by the background jobs at runtime.
-    def reload_mailer_configuration!
-      OpenProject::Configuration.reload_mailer_configuration!
-    end
-  end
 end

frontend/src/app/components/op-context-menu/wp-context-menu/wp-view-context-menu.directive.ts

@@ -16,6 +16,7 @@ import {WpDestroyModal} from "core-components/modals/wp-destroy-modal/wp-destroy
 import {StateService} from "@uirouter/core";
 import {InjectField} from "core-app/helpers/angular/inject-field.decorator";
 import {TimeEntryCreateService} from "core-app/modules/time_entries/create/create.service";
+import {splitViewRoute} from "core-app/modules/work_packages/routing/split-view-routes.helper";
 
 export class WorkPackageViewContextMenu extends OpContextMenuHandler {
 
@@ -185,7 +186,9 @@ export class WorkPackageViewContextMenu extends OpContextMenuHandler {
           disabled: false,
           icon: 'icon-view-split',
           class: 'detailsViewMenuItem',
-          href: this.$state.href(this.baseRoute + '.details.overview', { workPackageId: this.workPackageId }),
+          href: this.$state.href(
+            splitViewRoute(this.$state) + '.overview',
+            { workPackageId: this.workPackageId }),
           linkText: I18n.t('js.button_open_details'),
           onClick: ($event:JQuery.TriggeredEvent) => {
             if (LinkHandling.isClickedWithModifier($event)) {
@@ -193,7 +196,7 @@ export class WorkPackageViewContextMenu extends OpContextMenuHandler {
             }
 
             this.$state.go(
-              this.baseRoute + '.details.overview',
+              splitViewRoute(this.$state) + '.overview',
               { workPackageId: this.workPackageId }
             );
             return true;

frontend/src/app/components/wp-activity/user/user-activity.component.ts

@@ -171,9 +171,10 @@ export class UserActivityComponent extends WorkPackageCommentFieldHandler implem
         this.updateCommentText();
         this.wpLinkedActivities.require(this.workPackage, true);
         this.wpCacheService.updateWorkPackage(this.workPackage);
-        this.deactivate(true);
       })
-      .catch(() => this.deactivate(true));
+      .finally(() => {
+        this.deactivate(true); this.inFlight = false;
+      });
   }
 
   public focusEditIcon() {

frontend/src/app/components/wp-card-view/wp-single-card/wp-single-card.component.ts

@@ -19,7 +19,7 @@ import {CardHighlightingMode} from "core-components/wp-fast-table/builders/highl
 import {CardViewOrientation} from "core-components/wp-card-view/wp-card-view.component";
 import {UntilDestroyedMixin} from "core-app/helpers/angular/until-destroyed.mixin";
 import {WorkPackageViewFocusService} from "core-app/modules/work_packages/routing/wp-view-base/view-services/wp-view-focus.service";
-
+import {splitViewRoute} from "core-app/modules/work_packages/routing/split-view-routes.helper";
 
 @Component({
   selector: 'wp-single-card',
@@ -76,7 +76,7 @@ export class WorkPackageSingleCardComponent extends UntilDestroyedMixin implemen
     this.wpTableSelection.setSelection(wp.id!, this.cardView.findRenderedCard(classIdentifier));
     this.wpTableFocus.updateFocus(wp.id!);
     this.$state.go(
-      this.$state.current.data.baseRoute + '.details',
+      splitViewRoute(this.$state),
       { workPackageId: wp.id! }
     );
   }

frontend/src/app/components/wp-new/wp-create.component.ts

@@ -45,10 +45,11 @@ import {EditFormComponent} from "core-app/modules/fields/edit/edit-form/edit-for
 import {WorkPackageNotificationService} from "core-app/modules/work_packages/notifications/work-package-notification.service";
 import * as URI from 'urijs';
 import {UntilDestroyedMixin} from "core-app/helpers/angular/until-destroyed.mixin";
+import {splitViewRoute} from "core-app/modules/work_packages/routing/split-view-routes.helper";
 
 @Directive()
 export class WorkPackageCreateComponent extends UntilDestroyedMixin implements OnInit {
-  public successState:string = this.$state.current.data.baseRoute + '.details';
+  public successState:string = splitViewRoute(this.$state);
   public cancelState:string = this.$state.current.data.baseRoute;
   public newWorkPackage:WorkPackageResource;
   public parentWorkPackage:WorkPackageResource;

frontend/src/app/modules/fields/edit/field-types/te-work-package-edit-field.component.ts

@@ -82,7 +82,7 @@ export class TimeEntryWorkPackageEditFieldComponent extends WorkPackageEditField
   // associated with the time entries so that we have the most recent work packages the user logged time on.
   // As a worst case, the user logged RECENT_TIME_ENTRIES_MAGIC_NUMBER times on one work package so we can not guarantee to actually have
   // a fixed number returned.
-  protected allowedValuesFetch(query?:string) {
+  protected loadAllowedValues(query?:string) {
     if (!this.recentWorkPackageIds) {
       return this
         .timeEntryDm
@@ -93,10 +93,10 @@ export class TimeEntryWorkPackageEditFieldComponent extends WorkPackageEditField
             .map((timeEntry) => timeEntry.workPackage.idFromLink)
             .filter((v, i, a) => a.indexOf(v) === i);
 
-          return super.loadAllowedValues(query);
+          return this.fetchAllowedValueQuery(query);
         });
     } else {
-      return super.loadAllowedValues(query);
+      return this.fetchAllowedValueQuery(query);
     }
   }
 

frontend/src/app/modules/work_packages/routing/split-view-routes.helper.ts

@@ -0,0 +1,39 @@
+// -- copyright
+// OpenProject is an open source project management software.
+// Copyright (C) 2012-2020 the OpenProject GmbH
+//
+// 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 docs/COPYRIGHT.rdoc for more details.
+// ++
+
+import {StateService} from "@uirouter/angular";
+
+/**
+ * Returns the path to the split view based on the current route
+ *
+ * @param state State service
+ */
+export function splitViewRoute(state:StateService):string {
+  const baseRoute = state.current.data.baseRoute || '';
+  return baseRoute + '.details';
+}

modules/backlogs/lib/open_project/backlogs/work_package_filter.rb

@@ -72,8 +72,11 @@ module OpenProject::Backlogs
     end
 
     def value_objects
-      allowed_values
-        .select { |av| values.include?(av.last) }
+      available_backlog_types = allowed_values.index_by(&:last)
+
+      values
+        .map { |backlog_type_id| available_backlog_types[backlog_type_id] }
+        .compact
         .map { |value| BacklogsType.new(*value) }
     end
 

modules/costs/lib/open_project/costs/work_package_filter.rb

@@ -59,7 +59,11 @@ module OpenProject::Costs
     end
 
     def value_objects
-      cost_objects.where(id: values)
+      available_cost_objects = cost_objects.index_by(&:id)
+
+      values
+        .map { |cost_object_id| available_cost_objects[cost_object_id.to_i] }
+        .compact
     end
 
     def human_name

spec/controllers/concerns/auth_source_sso_spec.rb

@@ -109,6 +109,16 @@ describe MyController, type: :controller do
       end
     end
 
+    context 'when the user is invited' do
+      let!(:user) {
+        FactoryBot.create :user, login: login, status: Principal::STATUSES[:invited], auth_source_id: auth_source.id
+      }
+
+      it "should log in given user and activate it" do
+        expect(response.body.squish).to have_content("Username   h.wurst")
+        expect(user.reload).to be_active
+      end
+    end
 
     context "with no auth source sso configured" do
       let(:sso_config) { nil }

spec/features/work_packages/navigation_spec.rb

@@ -260,4 +260,27 @@ RSpec.feature 'Work package navigation', js: true, selenium: true do
       expect(page).to have_selector('.work-package--attachments--filename', text: 'attachment-first.pdf', wait: 10)
     end
   end
+
+  context 'two work packages with card view' do
+    let!(:work_package) { FactoryBot.create :work_package, project: project }
+    let!(:work_package2) { FactoryBot.create :work_package, project: project }
+    let(:display_representation) { ::Components::WorkPackages::DisplayRepresentation.new }
+    let(:wp_table) { ::Pages::WorkPackagesTable.new(project) }
+    let(:cards) { ::Pages::WorkPackageCards.new(project) }
+
+    it 'can move between card details using info icon (Regression #33451)' do
+      wp_table.visit!
+      wp_table.expect_work_package_listed work_package, work_package2
+      display_representation.switch_to_card_layout
+      cards.expect_work_package_listed work_package, work_package2
+
+      # move to first details
+      split = cards.open_full_screen_by_details work_package
+      split.expect_subject
+
+      # move to second details
+      split2 = cards.open_full_screen_by_details work_package2
+      split2.expect_subject
+    end
+  end
 end

spec/features/work_packages/table/queries/filter_spec.rb

@@ -488,4 +488,26 @@ describe 'filter work packages', js: true do
       end
     end
   end
+
+  describe 'keep the filter attribute order (Regression #33136)' do
+    let(:version1) { FactoryBot.create :version, project: project, name: 'Version 1', id: 1 }
+    let(:version2) { FactoryBot.create :version, project: project, name: 'Version 2', id: 2 }
+
+    it do
+      wp_table.visit!
+      loading_indicator_saveguard
+
+      filters.open
+      filters.add_filter_by 'Version', 'is', [version2.name, version1.name]
+      loading_indicator_saveguard
+
+      sleep(3)
+
+      filters.expect_filter_by 'Version', 'is', [version1.name]
+      filters.expect_filter_by 'Version', 'is', [version2.name]
+
+      # Order should stay unchanged
+      filters.expect_filter_order('Version', [version2.name, version1.name])
+    end
+  end
 end

spec/support/components/work_packages/filters.rb

@@ -123,6 +123,12 @@ module Components
         end
       end
 
+      def expect_filter_order(name, values, selector = nil)
+        id = selector || name.downcase
+
+        expect(page.all("#values-#{id} .ng-value-label").map(&:text)).to eq(values)
+      end
+
       def remove_filter(field)
         find("#filter_#{field} .advanced-filters--remove-filter-icon").click
       end

spec/support/pages/work_packages/work_package_cards.rb

@@ -76,6 +76,8 @@ module Pages
       scroll_to_element(element)
       element.hover
       element.find('.wp-card--details-button').click
+
+      ::Pages::SplitWorkPackage.new(work_package, project)
     end
 
     def select_work_package(work_package)

spec/workers/application_job_spec.rb

@@ -47,10 +47,10 @@ describe ApplicationJob do
       end)
 
       RequestStore[:test_value] = 'my value'
-      expect { job.perform }.not_to change { RequestStore[:test_value] }
+      expect { job.perform_now }.not_to change { RequestStore[:test_value] }
 
       RequestStore[:test_value] = 'my value2'
-      expect { job.perform }.not_to change { RequestStore[:test_value] }
+      expect { job.perform_now }.not_to change { RequestStore[:test_value] }
 
       expect(RequestStore[:test_value]).to eq 'my value2'
     end