From ebc3f6fe18c121877f98b4e38cc14ff02db170a6 Mon Sep 17 00:00:00 2001 From: Henriette Darge Date: Mon, 18 May 2020 13:53:28 +0200 Subject: [PATCH 1/7] Prevent that the backend changes the order of filter attributes (sorted by id). Otherwise there is a visible change in the filter attributes in the frontend. --- .../work_packages/filter/category_filter.rb | 6 +++-- .../work_packages/filter/group_filter.rb | 6 +++-- .../work_packages/filter/priority_filter.rb | 6 +++-- .../work_packages/filter/project_filter.rb | 6 +++-- .../work_packages/filter/role_filter.rb | 6 +++-- .../work_packages/filter/status_filter.rb | 12 +++++----- .../work_packages/filter/subproject_filter.rb | 6 +++-- .../work_packages/filter/type_filter.rb | 6 +++-- .../work_packages/filter/version_filter.rb | 6 +++-- .../backlogs/work_package_filter.rb | 7 ++++-- .../open_project/costs/work_package_filter.rb | 6 ++++- .../table/queries/filter_spec.rb | 22 +++++++++++++++++++ .../components/work_packages/filters.rb | 6 +++++ 13 files changed, 76 insertions(+), 25 deletions(-) diff --git a/app/models/queries/work_packages/filter/category_filter.rb b/app/models/queries/work_packages/filter/category_filter.rb index 36bcd04ef7..2f09b621ef 100644 --- a/app/models/queries/work_packages/filter/category_filter.rb +++ b/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? diff --git a/app/models/queries/work_packages/filter/group_filter.rb b/app/models/queries/work_packages/filter/group_filter.rb index b8e489d2a9..b1ccd42187 100644 --- a/app/models/queries/work_packages/filter/group_filter.rb +++ b/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 diff --git a/app/models/queries/work_packages/filter/priority_filter.rb b/app/models/queries/work_packages/filter/priority_filter.rb index 36a10b6fc7..450e82239b 100644 --- a/app/models/queries/work_packages/filter/priority_filter.rb +++ b/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 diff --git a/app/models/queries/work_packages/filter/project_filter.rb b/app/models/queries/work_packages/filter/project_filter.rb index 1e54e9fd24..c64cca1c39 100644 --- a/app/models/queries/work_packages/filter/project_filter.rb +++ b/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 diff --git a/app/models/queries/work_packages/filter/role_filter.rb b/app/models/queries/work_packages/filter/role_filter.rb index 66bc930283..c241bf136e 100644 --- a/app/models/queries/work_packages/filter/role_filter.rb +++ b/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 diff --git a/app/models/queries/work_packages/filter/status_filter.rb b/app/models/queries/work_packages/filter/status_filter.rb index 1b271be080..2f79bcc799 100644 --- a/app/models/queries/work_packages/filter/status_filter.rb +++ b/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 diff --git a/app/models/queries/work_packages/filter/subproject_filter.rb b/app/models/queries/work_packages/filter/subproject_filter.rb index bf67731c31..d2d8f40090 100644 --- a/app/models/queries/work_packages/filter/subproject_filter.rb +++ b/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 diff --git a/app/models/queries/work_packages/filter/type_filter.rb b/app/models/queries/work_packages/filter/type_filter.rb index f9c31564a8..764f05ab90 100644 --- a/app/models/queries/work_packages/filter/type_filter.rb +++ b/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 diff --git a/app/models/queries/work_packages/filter/version_filter.rb b/app/models/queries/work_packages/filter/version_filter.rb index 1290d7fbdd..83a47765ef 100644 --- a/app/models/queries/work_packages/filter/version_filter.rb +++ b/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 diff --git a/modules/backlogs/lib/open_project/backlogs/work_package_filter.rb b/modules/backlogs/lib/open_project/backlogs/work_package_filter.rb index 837d865370..4420d0da8d 100644 --- a/modules/backlogs/lib/open_project/backlogs/work_package_filter.rb +++ b/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 diff --git a/modules/costs/lib/open_project/costs/work_package_filter.rb b/modules/costs/lib/open_project/costs/work_package_filter.rb index 4997071453..6f30f3854e 100644 --- a/modules/costs/lib/open_project/costs/work_package_filter.rb +++ b/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 diff --git a/spec/features/work_packages/table/queries/filter_spec.rb b/spec/features/work_packages/table/queries/filter_spec.rb index 1544b9d480..a8da770b92 100644 --- a/spec/features/work_packages/table/queries/filter_spec.rb +++ b/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 diff --git a/spec/support/components/work_packages/filters.rb b/spec/support/components/work_packages/filters.rb index c61c9d57d8..57c9978d85 100644 --- a/spec/support/components/work_packages/filters.rb +++ b/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 From 032ea67a082784ee23f72a4c890988531c5701db Mon Sep 17 00:00:00 2001 From: bsatarnejad Date: Fri, 22 May 2020 10:42:50 +0200 Subject: [PATCH 2/7] user can edit wp comment for more than one time --- .../app/components/wp-activity/user/user-activity.component.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frontend/src/app/components/wp-activity/user/user-activity.component.ts b/frontend/src/app/components/wp-activity/user/user-activity.component.ts index 97e51e9edf..87cc5a2658 100644 --- a/frontend/src/app/components/wp-activity/user/user-activity.component.ts +++ b/frontend/src/app/components/wp-activity/user/user-activity.component.ts @@ -172,8 +172,9 @@ export class UserActivityComponent extends WorkPackageCommentFieldHandler implem this.wpLinkedActivities.require(this.workPackage, true); this.wpCacheService.updateWorkPackage(this.workPackage); this.deactivate(true); + this.inFlight = false; }) - .catch(() => this.deactivate(true)); + .catch(() => {this.deactivate(true); this.inFlight = false; }); } public focusEditIcon() { From c94c1951b9fc180184689567c1f0926c38b539e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Mon, 25 May 2020 14:29:20 +0200 Subject: [PATCH 3/7] [33413] Use around hook to reload config in jobs https://community.openproject.com/wp/33413 --- app/workers/application_job.rb | 70 ++++++++++++---------------- spec/workers/application_job_spec.rb | 4 +- 2 files changed, 32 insertions(+), 42 deletions(-) diff --git a/app/workers/application_job.rb b/app/workers/application_job.rb index 542ecd0818..66536d1b49 100644 --- a/app/workers/application_job.rb +++ b/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 diff --git a/spec/workers/application_job_spec.rb b/spec/workers/application_job_spec.rb index 386f47d0d9..336154e82e 100644 --- a/spec/workers/application_job_spec.rb +++ b/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 From 5678e27e47d7248c25ff292c25cdca5e2e95abb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 26 May 2020 13:01:08 +0200 Subject: [PATCH 4/7] [33451] Use baseRoute to navigate to split view https://community.openproject.com/wp/33451 --- .../wp-view-context-menu.directive.ts | 7 +++- .../wp-single-card.component.ts | 4 +- .../components/wp-new/wp-create.component.ts | 3 +- .../routing/split-view-routes.helper.ts | 39 +++++++++++++++++++ .../features/work_packages/navigation_spec.rb | 23 +++++++++++ .../pages/work_packages/work_package_cards.rb | 2 + 6 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 frontend/src/app/modules/work_packages/routing/split-view-routes.helper.ts diff --git a/frontend/src/app/components/op-context-menu/wp-context-menu/wp-view-context-menu.directive.ts b/frontend/src/app/components/op-context-menu/wp-context-menu/wp-view-context-menu.directive.ts index 1dbb541f32..e90a043432 100644 --- a/frontend/src/app/components/op-context-menu/wp-context-menu/wp-view-context-menu.directive.ts +++ b/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; diff --git a/frontend/src/app/components/wp-card-view/wp-single-card/wp-single-card.component.ts b/frontend/src/app/components/wp-card-view/wp-single-card/wp-single-card.component.ts index 1efe05d3eb..ec562cf742 100644 --- a/frontend/src/app/components/wp-card-view/wp-single-card/wp-single-card.component.ts +++ b/frontend/src/app/components/wp-card-view/wp-single-card/wp-single-card.component.ts @@ -18,7 +18,7 @@ import {I18nService} from "core-app/modules/common/i18n/i18n.service"; import {CardHighlightingMode} from "core-components/wp-fast-table/builders/highlighting/highlighting-mode.const"; import {CardViewOrientation} from "core-components/wp-card-view/wp-card-view.component"; import {UntilDestroyedMixin} from "core-app/helpers/angular/until-destroyed.mixin"; - +import {splitViewRoute} from "core-app/modules/work_packages/routing/split-view-routes.helper"; @Component({ selector: 'wp-single-card', @@ -73,7 +73,7 @@ export class WorkPackageSingleCardComponent extends UntilDestroyedMixin implemen let classIdentifier = this.classIdentifier(wp); this.wpTableSelection.setSelection(wp.id!, this.cardView.findRenderedCard(classIdentifier)); this.$state.go( - '.details', + splitViewRoute(this.$state), { workPackageId: wp.id! } ); } diff --git a/frontend/src/app/components/wp-new/wp-create.component.ts b/frontend/src/app/components/wp-new/wp-create.component.ts index 13fbfb3c0f..6755598aea 100644 --- a/frontend/src/app/components/wp-new/wp-create.component.ts +++ b/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; diff --git a/frontend/src/app/modules/work_packages/routing/split-view-routes.helper.ts b/frontend/src/app/modules/work_packages/routing/split-view-routes.helper.ts new file mode 100644 index 0000000000..3e5f4ebfdd --- /dev/null +++ b/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'; +} diff --git a/spec/features/work_packages/navigation_spec.rb b/spec/features/work_packages/navigation_spec.rb index b619693b90..ba809fccd4 100644 --- a/spec/features/work_packages/navigation_spec.rb +++ b/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 diff --git a/spec/support/pages/work_packages/work_package_cards.rb b/spec/support/pages/work_packages/work_package_cards.rb index 2e9124dc7f..e0b95690c7 100644 --- a/spec/support/pages/work_packages/work_package_cards.rb +++ b/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) From 70acb4c80f19797b8fd201da20ff95320d8ab7f3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Oliver=20G=C3=BCnther?= Date: Tue, 26 May 2020 14:03:16 +0200 Subject: [PATCH 5/7] [33445] Ensure user is activated if invited and accessing auth source https://community.openproject.com/wp/33445 --- app/controllers/concerns/auth_source_sso.rb | 11 ++++++++++- spec/controllers/concerns/auth_source_sso_spec.rb | 10 ++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/auth_source_sso.rb b/app/controllers/concerns/auth_source_sso.rb index 171da19819..531d008aa6 100644 --- a/app/controllers/concerns/auth_source_sso.rb +++ b/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 diff --git a/spec/controllers/concerns/auth_source_sso_spec.rb b/spec/controllers/concerns/auth_source_sso_spec.rb index ac6ddf18b0..4aec888762 100644 --- a/spec/controllers/concerns/auth_source_sso_spec.rb +++ b/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 } From c64e61568b6fda7a7964ebda30dc485d0cb31cc2 Mon Sep 17 00:00:00 2001 From: bsatarnejad Date: Wed, 27 May 2020 13:27:19 +0200 Subject: [PATCH 6/7] use finally to avoid redundant code --- .../components/wp-activity/user/user-activity.component.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/components/wp-activity/user/user-activity.component.ts b/frontend/src/app/components/wp-activity/user/user-activity.component.ts index 87cc5a2658..da258caa13 100644 --- a/frontend/src/app/components/wp-activity/user/user-activity.component.ts +++ b/frontend/src/app/components/wp-activity/user/user-activity.component.ts @@ -171,10 +171,10 @@ export class UserActivityComponent extends WorkPackageCommentFieldHandler implem this.updateCommentText(); this.wpLinkedActivities.require(this.workPackage, true); this.wpCacheService.updateWorkPackage(this.workPackage); - this.deactivate(true); - this.inFlight = false; }) - .catch(() => {this.deactivate(true); this.inFlight = false; }); + .finally(() => { + this.deactivate(true); this.inFlight = false; + }); } public focusEditIcon() { From f888ec1a025278c74fc5ab2f97155f7b1cee401d Mon Sep 17 00:00:00 2001 From: ulferts Date: Thu, 28 May 2020 08:36:30 +0200 Subject: [PATCH 7/7] fix recent wp autocompleter broken in 4c575217bb --- .../field-types/te-work-package-edit-field.component.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/modules/fields/edit/field-types/te-work-package-edit-field.component.ts b/frontend/src/app/modules/fields/edit/field-types/te-work-package-edit-field.component.ts index 7e26095004..9772c8cac3 100644 --- a/frontend/src/app/modules/fields/edit/field-types/te-work-package-edit-field.component.ts +++ b/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); } }