From a9e515cd156d73a14e8ab947b8467aa5c08c73a6 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 12 Nov 2024 15:16:13 +0100 Subject: [PATCH 01/12] [#59040] Prepare pull request to remove primerised work packages feature flag https://community.openproject.org/work_packages/59040 From ab51b17f5140cefa4574170d99746272e8828c71 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 12 Nov 2024 18:49:14 +0100 Subject: [PATCH 02/12] WIP remove feature flag --- .github/workflows/pullpreview.yml | 1 - config/initializers/feature_decisions.rb | 1 - .../center/state/ian-center.service.ts | 51 ------------------- .../work-package-comment.component.html | 50 +----------------- .../work-package-comment.component.ts | 6 +-- .../overview-tab/overview-tab.component.ts | 6 --- .../overview-tab/overview-tab.html | 12 +---- .../work-package-notification.service.ts | 32 +++--------- .../work_package/activities_spec.rb | 2 +- .../work_package/emoji_reactions_spec.rb | 3 +- .../custom_actions/custom_actions_spec.rb | 31 ++++++----- .../components/work_packages/activities.rb | 26 +++++----- .../work_packages/abstract_work_package.rb | 6 --- 13 files changed, 42 insertions(+), 185 deletions(-) diff --git a/.github/workflows/pullpreview.yml b/.github/workflows/pullpreview.yml index 23364897f05f..aba0a76f525f 100644 --- a/.github/workflows/pullpreview.yml +++ b/.github/workflows/pullpreview.yml @@ -33,7 +33,6 @@ jobs: echo "OPENPROJECT_FEATURE__SHOW__CHANGES__ACTIVE=true" >> .env.pullpreview echo "OPENPROJECT_LOOKBOOK__ENABLED=true" >> .env.pullpreview echo "OPENPROJECT_HSTS=false" >> .env.pullpreview - echo "OPENPROJECT_FEATURE_PRIMERIZED_WORK_PACKAGE_ACTIVITIES_ACTIVE=true" >> .env.pullpreview echo "OPENPROJECT_NOTIFICATIONS_POLLING_INTERVAL=10000" >> .env.pullpreview - name: Boot as BIM edition if: contains(github.ref, 'bim/') || contains(github.head_ref, 'bim/') diff --git a/config/initializers/feature_decisions.rb b/config/initializers/feature_decisions.rb index 4a14e7a891e9..1c8150ac96e0 100644 --- a/config/initializers/feature_decisions.rb +++ b/config/initializers/feature_decisions.rb @@ -39,7 +39,6 @@ # OpenProject::FeatureDecisions.add :some_flag # end -OpenProject::FeatureDecisions.add :primerized_work_package_activities OpenProject::FeatureDecisions.add :built_in_oauth_applications, description: "Allows the display and use of built-in OAuth applications." diff --git a/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts b/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts index 0979826f6503..ef4f799ab2ab 100644 --- a/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts +++ b/frontend/src/app/features/in-app-notifications/center/state/ian-center.service.ts @@ -37,7 +37,6 @@ import { IToast, ToastService } from 'core-app/shared/components/toaster/toast.s import { centerUpdatedInPlace, markNotificationsAsRead, - notificationCountIncreased, notificationCountChanged, notificationsMarkedRead, } from 'core-app/core/state/in-app-notifications/in-app-notifications.actions'; @@ -63,7 +62,6 @@ import { FrameElement } from '@hotwired/turbo'; import { PathHelperService } from 'core-app/core/path-helper/path-helper.service'; import { UrlParamsService } from 'core-app/core/navigation/url-params.service'; import { IanBellService } from 'core-app/features/in-app-notifications/bell/state/ian-bell.service'; -import { ConfigurationService } from 'core-app/core/config/configuration.service'; export interface INotificationPageQueryParameters { filter?:string|null; @@ -196,7 +194,6 @@ export class IanCenterService extends UntilDestroyedMixin { readonly deviceService:DeviceService, readonly pathHelper:PathHelperService, readonly ianBellService:IanBellService, - readonly configurationService:ConfigurationService, ) { super(); this.reload.subscribe(); @@ -276,8 +273,6 @@ export class IanCenterService extends UntilDestroyedMixin { */ @EffectCallback(notificationCountChanged) private handleChangedNotificationCount() { - if (!this.primerizedActivitiesEnabled) return; - // update the UI state for increased AND decreased notifications, not only increased count // decreasing the notification count could happen when the user itself // marks notifications as read in the split view or on another tab @@ -290,52 +285,6 @@ export class IanCenterService extends UntilDestroyedMixin { this.reload.next(false); } - /** - * Check for updates after bell count increased - */ - @EffectCallback(notificationCountIncreased) - private checkForNewNotifications() { - // There is a new concept for primerized work package activities bound to notificationCountChanged - // See @EffectCallback(notificationCountChanged) - if (this.primerizedActivitiesEnabled) return; - - this.onReload.pipe(take(1)).subscribe((collection) => { - const { activeCollection } = this.query.getValue(); - const hasNewNotifications = !collection.ids.reduce( - (allInOldCollection, id) => allInOldCollection && activeCollection.ids.includes(id), - true, - ); - - if (!hasNewNotifications) { - return; - } - - if (this.activeReloadToast) { - this.toastService.remove(this.activeReloadToast); - this.activeReloadToast = null; - } - - this.activeReloadToast = this.toastService.add({ - type: 'info', - icon: 'bell', - message: this.I18n.t('js.notifications.center.new_notifications.message'), - link: { - text: this.I18n.t('js.notifications.center.new_notifications.link_text'), - target: () => { - this.store.update({ activeCollection: collection }); - this.actions$.dispatch(centerUpdatedInPlace({ origin: this.id })); - this.activeReloadToast = null; - }, - }, - }); - }); - this.reload.next(false); - } - - private get primerizedActivitiesEnabled():boolean { - return this.configurationService.activeFeatureFlags.includes('primerizedWorkPackageActivities'); - } - /** * Reload after notifications were successfully marked as read */ diff --git a/frontend/src/app/features/work-packages/components/work-package-comment/work-package-comment.component.html b/frontend/src/app/features/work-packages/components/work-package-comment/work-package-comment.component.html index 3bff5b7611e2..4b5a5faa8087 100644 --- a/frontend/src/app/features/work-packages/components/work-package-comment/work-package-comment.component.html +++ b/frontend/src/app/features/work-packages/components/work-package-comment/work-package-comment.component.html @@ -1,4 +1,4 @@ -
+
@@ -11,51 +11,3 @@
- -
-
- - - -
-
- - -
- -
-
-
- - -
-
diff --git a/frontend/src/app/features/work-packages/components/work-package-comment/work-package-comment.component.ts b/frontend/src/app/features/work-packages/components/work-package-comment/work-package-comment.component.ts index c2876c5c0387..61d3cc5e682d 100644 --- a/frontend/src/app/features/work-packages/components/work-package-comment/work-package-comment.component.ts +++ b/frontend/src/app/features/work-packages/components/work-package-comment/work-package-comment.component.ts @@ -84,13 +84,12 @@ export class WorkPackageCommentComponent extends WorkPackageCommentFieldHandler public showAbove:boolean; - public primerizedActivitiesEnabled:boolean; - public turboFrameSrc:string; public htmlId = 'wp-comment-field'; - constructor(protected elementRef:ElementRef, + constructor( + protected elementRef:ElementRef, protected injector:Injector, protected commentService:CommentService, protected wpLinkedActivities:WorkPackagesActivityService, @@ -111,7 +110,6 @@ export class WorkPackageCommentComponent extends WorkPackageCommentFieldHandler this.canAddComment = !!this.workPackage.addComment; this.showAbove = this.configurationService.commentsSortedInDescendingOrder(); - this.primerizedActivitiesEnabled = this.configurationService.activeFeatureFlags.includes('primerizedWorkPackageActivities'); this.turboFrameSrc = `${this.PathHelper.staticBase}/work_packages/${this.workPackage.id}/activities`; this.commentService.draft$ diff --git a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/overview-tab/overview-tab.component.ts b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/overview-tab/overview-tab.component.ts index 11766a4197b2..ea20e3c598bc 100644 --- a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/overview-tab/overview-tab.component.ts +++ b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/overview-tab/overview-tab.component.ts @@ -32,7 +32,6 @@ import { WorkPackageResource } from 'core-app/features/hal/resources/work-packag import { I18nService } from 'core-app/core/i18n/i18n.service'; import { UntilDestroyedMixin } from 'core-app/shared/helpers/angular/until-destroyed.mixin'; import { ApiV3Service } from 'core-app/core/apiv3/api-v3.service'; -import { ConfigurationService } from 'core-app/core/config/configuration.service'; @Component({ templateUrl: './overview-tab.html', @@ -45,13 +44,10 @@ export class WorkPackageOverviewTabComponent extends UntilDestroyedMixin impleme public tabName = this.I18n.t('js.label_latest_activity'); - public primerizedActivitiesEnabled:boolean; - public constructor( readonly I18n:I18nService, readonly $state:StateService, readonly apiV3Service:ApiV3Service, - readonly configurationService:ConfigurationService, ) { super(); } @@ -59,8 +55,6 @@ export class WorkPackageOverviewTabComponent extends UntilDestroyedMixin impleme ngOnInit() { this.workPackageId = this.workPackage?.id || this.$state.params.workPackageId as string; - this.primerizedActivitiesEnabled = this.configurationService.activeFeatureFlags.includes('primerizedWorkPackageActivities'); - this .apiV3Service .work_packages diff --git a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/overview-tab/overview-tab.html b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/overview-tab/overview-tab.html index 60d4cac5e695..c71afe1cb47c 100644 --- a/frontend/src/app/features/work-packages/components/wp-single-view-tabs/overview-tab/overview-tab.html +++ b/frontend/src/app/features/work-packages/components/wp-single-view-tabs/overview-tab/overview-tab.html @@ -1,12 +1,2 @@ - -
-
-
-

-
-
- - -
+ *ngIf="workPackage"> \ No newline at end of file diff --git a/frontend/src/app/features/work-packages/services/notifications/work-package-notification.service.ts b/frontend/src/app/features/work-packages/services/notifications/work-package-notification.service.ts index a4a19324f13a..7044fb5d3756 100644 --- a/frontend/src/app/features/work-packages/services/notifications/work-package-notification.service.ts +++ b/frontend/src/app/features/work-packages/services/notifications/work-package-notification.service.ts @@ -33,21 +33,15 @@ import { WorkPackageResource } from 'core-app/features/hal/resources/work-packag import { ApiV3Service } from 'core-app/core/apiv3/api-v3.service'; import { HalResource } from 'core-app/features/hal/resources/hal-resource'; import { TurboRequestsService } from 'core-app/core/turbo/turbo-requests.service'; -import { ConfigurationService } from 'core-app/core/config/configuration.service'; @Injectable() export class WorkPackageNotificationService extends HalResourceNotificationService { - primerizedActivitiesEnabled:boolean; - constructor( readonly injector:Injector, readonly apiV3Service:ApiV3Service, readonly turboRequests:TurboRequestsService, - readonly configurationService:ConfigurationService, ) { super(injector); - - this.primerizedActivitiesEnabled = this.configurationService.activeFeatureFlags.includes('primerizedWorkPackageActivities'); } public showSave(resource:HalResource, isCreate = false) { @@ -61,26 +55,12 @@ export class WorkPackageNotificationService extends HalResourceNotificationServi protected showCustomError(errorResource:any, resource:WorkPackageResource):boolean { if (errorResource.errorIdentifier === 'urn:openproject-org:api:v3:errors:UpdateConflict') { - if (this.primerizedActivitiesEnabled) { - // currently we do not have a programmatic way to show the primer flash messages - // so we just do a request to the server to show it - // should be refactored once we have a programmatic way to show the primer flash messages! - void this.turboRequests.request('/work_packages/show_conflict_flash_message?scheme=danger', { - method: 'GET', - }); - } else { - // code from before: - this.ToastService.addError({ - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment, @typescript-eslint/no-unsafe-member-access - message: errorResource.message, - type: 'error', - link: { - text: this.I18n.t('js.hal.error.update_conflict_refresh'), - // eslint-disable-next-line @typescript-eslint/no-misused-promises - target: () => this.apiV3Service.work_packages.id(resource).refresh(), - }, - }); - } + // currently we do not have a programmatic way to show the primer flash messages + // so we just do a request to the server to show it + // should be refactored once we have a programmatic way to show the primer flash messages! + void this.turboRequests.request('/work_packages/show_conflict_flash_message?scheme=danger', { + method: 'GET', + }); return true; } diff --git a/spec/features/activities/work_package/activities_spec.rb b/spec/features/activities/work_package/activities_spec.rb index 5428863f8fde..e9c25256eaad 100644 --- a/spec/features/activities/work_package/activities_spec.rb +++ b/spec/features/activities/work_package/activities_spec.rb @@ -28,7 +28,7 @@ require "spec_helper" -RSpec.describe "Work package activity", :js, :with_cuprite, with_flag: { primerized_work_package_activities: true } do +RSpec.describe "Work package activity", :js, :with_cuprite do let(:project) { create(:project) } let(:admin) { create(:admin) } let(:member_role) do diff --git a/spec/features/activities/work_package/emoji_reactions_spec.rb b/spec/features/activities/work_package/emoji_reactions_spec.rb index ac64940a4291..598a300d3e9d 100644 --- a/spec/features/activities/work_package/emoji_reactions_spec.rb +++ b/spec/features/activities/work_package/emoji_reactions_spec.rb @@ -28,8 +28,7 @@ require "spec_helper" -RSpec.describe "Emoji reactions on work package activity", :js, :with_cuprite, - with_flag: { primerized_work_package_activities: true } do +RSpec.describe "Emoji reactions on work package activity", :js, :with_cuprite do let(:project) { create(:project) } let(:admin) { create(:admin) } let(:member) { create_user_as_project_member } diff --git a/spec/features/work_packages/custom_actions/custom_actions_spec.rb b/spec/features/work_packages/custom_actions/custom_actions_spec.rb index afca1b64aeaa..68c77817e7a3 100644 --- a/spec/features/work_packages/custom_actions/custom_actions_spec.rb +++ b/spec/features/work_packages/custom_actions/custom_actions_spec.rb @@ -129,6 +129,7 @@ cf end let(:index_ca_page) { Pages::Admin::CustomActions::Index.new } + let(:activity_tab) { Components::WorkPackages::Activities.new(work_package) } before do login_as admin @@ -137,7 +138,7 @@ # this big spec just has a different expectation when primerized_work_package_activities is enabled for the very last part # where the a different expectation banner would be expected # IMO not worth the extra computation time for the intermediate state when the feature flag is active - it "viewing workflow buttons", with_flag: { primerized_work_package_activities: false } do + it "viewing workflow buttons" do # create custom action 'Unassign' index_ca_page.visit! @@ -319,24 +320,26 @@ wp_page.click_custom_action("Unassign") wp_page.expect_attributes assignee: "-" - within ".work-package-details-activities-list" do - expect(page) - .to have_css(".op-user-activity .op-user-activity--user-name", - text: user.name, - wait: 10) - end + activity_tab.expect_journal_details_header(text: user.name) + # within ".work-package-details-activities-list" do + # expect(page) + # .to have_css(".op-user-activity .op-user-activity--user-name", + # text: user.name, + # wait: 10) + # end wp_page.click_custom_action("Escalate") wp_page.expect_attributes priority: immediate_priority.name, status: default_status.name, assignee: "-", "customField#{list_custom_field.id}" => selected_list_custom_field_options.map(&:name).join("\n") - within ".work-package-details-activities-list" do - expect(page) - .to have_css(".op-user-activity a.user-mention", - text: other_member_user.name, - wait: 10) - end + # within ".work-package-details-activities-list" do + # expect(page) + # .to have_css(".op-user-activity a.user-mention", + # text: other_member_user.name, + # wait: 10) + # end + activity_tab.expect_journal_details_header(text: other_member_user.name) wp_page.click_custom_action("Close") wp_page.expect_attributes status: closed_status.name, @@ -441,7 +444,7 @@ wp_page.click_custom_action("Escalate", expect_success: false) - wp_page.expect_toast type: :error, message: I18n.t("api_v3.errors.code_409") + wp_page.expect_conflict_error_banner end it "editing a current date custom action (Regression #30949)" do diff --git a/spec/support/components/work_packages/activities.rb b/spec/support/components/work_packages/activities.rb index 822808fea401..8be018b42b6e 100644 --- a/spec/support/components/work_packages/activities.rb +++ b/spec/support/components/work_packages/activities.rb @@ -74,39 +74,39 @@ def expect_journal_changed_attribute(text:) end def expect_no_journal_changed_attribute(text: nil) - expect(page).not_to have_test_selector("op-journal-detail-description", text:) + expect(page).not_to have_test_selector("op-journal-detail-description", text:, wait: 10) end def expect_no_journal_notes(text: nil) - expect(page).not_to have_test_selector("op-journal-notes-body", text:) + expect(page).not_to have_test_selector("op-journal-notes-body", text:, wait: 10) end def expect_journal_details_header(text: nil) - expect(page).to have_test_selector("op-journal-details-header", text:) + expect(page).to have_test_selector("op-journal-details-header", text:, wait: 10) end def expect_no_journal_details_header(text: nil) - expect(page).not_to have_test_selector("op-journal-details-header", text:) + expect(page).not_to have_test_selector("op-journal-details-header", text:, wait: 10) end def expect_journal_notes_header(text: nil) - expect(page).to have_test_selector("op-journal-notes-header", text:) + expect(page).to have_test_selector("op-journal-notes-header", text:, wait: 10) end def expect_no_journal_notes_header(text: nil) - expect(page).not_to have_test_selector("op-journal-notes-header", text:) + expect(page).not_to have_test_selector("op-journal-notes-header", text:, wait: 10) end def expect_journal_notes(text: nil) - expect(page).to have_test_selector("op-journal-notes-body", text:) + expect(page).to have_test_selector("op-journal-notes-body", text:, wait: 10) end def expect_notification_bubble - expect(page).to have_test_selector("op-journal-unread-notification") + expect(page).to have_test_selector("op-journal-unread-notification", wait: 10) end def expect_no_notification_bubble - expect(page).not_to have_test_selector("op-journal-unread-notification") + expect(page).not_to have_test_selector("op-journal-unread-notification", wait: 10) end def expect_journal_container_at_bottom @@ -130,19 +130,19 @@ def expect_journal_container_at_position(position) end def expect_empty_state - expect(page).to have_test_selector("op-wp-journals-container-empty") + expect(page).to have_test_selector("op-wp-journals-container-empty", wait: 10) end def expect_no_empty_state - expect(page).not_to have_test_selector("op-wp-journals-container-empty") + expect(page).not_to have_test_selector("op-wp-journals-container-empty", wait: 10) end def expect_input_field - expect(page).to have_test_selector("op-work-package-journal-form") + expect(page).to have_test_selector("op-work-package-journal-form", wait: 10) end def expect_no_input_field - expect(page).not_to have_test_selector("op-work-package-journal-form") + expect(page).not_to have_test_selector("op-work-package-journal-form", wait: 10) end def open_new_comment_editor diff --git a/spec/support/pages/work_packages/abstract_work_package.rb b/spec/support/pages/work_packages/abstract_work_package.rb index 751b601e3b7e..e9983e406981 100644 --- a/spec/support/pages/work_packages/abstract_work_package.rb +++ b/spec/support/pages/work_packages/abstract_work_package.rb @@ -115,12 +115,6 @@ def open_in_split_view def ensure_page_loaded expect_angular_frontend_initialized - unless OpenProject::FeatureDecisions.primerized_work_package_activities_active? - expect(page).to have_css(".op-user-activity--user-name", - text: work_package.journals.last.user.name, - minimum: 1, - wait: 10) - end end def disable_ajax_requests From 05c6a887d4bda302e5c9e4288b8abd15c6a20c7c Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 17:25:33 +0100 Subject: [PATCH 03/12] fixed custom_actions_spec --- .../custom_actions/custom_actions_spec.rb | 15 ++------------- .../components/work_packages/activities.rb | 8 ++++++++ 2 files changed, 10 insertions(+), 13 deletions(-) diff --git a/spec/features/work_packages/custom_actions/custom_actions_spec.rb b/spec/features/work_packages/custom_actions/custom_actions_spec.rb index 68c77817e7a3..67aceca4f779 100644 --- a/spec/features/work_packages/custom_actions/custom_actions_spec.rb +++ b/spec/features/work_packages/custom_actions/custom_actions_spec.rb @@ -321,25 +321,14 @@ wp_page.click_custom_action("Unassign") wp_page.expect_attributes assignee: "-" activity_tab.expect_journal_details_header(text: user.name) - # within ".work-package-details-activities-list" do - # expect(page) - # .to have_css(".op-user-activity .op-user-activity--user-name", - # text: user.name, - # wait: 10) - # end wp_page.click_custom_action("Escalate") wp_page.expect_attributes priority: immediate_priority.name, status: default_status.name, assignee: "-", "customField#{list_custom_field.id}" => selected_list_custom_field_options.map(&:name).join("\n") - # within ".work-package-details-activities-list" do - # expect(page) - # .to have_css(".op-user-activity a.user-mention", - # text: other_member_user.name, - # wait: 10) - # end - activity_tab.expect_journal_details_header(text: other_member_user.name) + + activity_tab.expect_journal_mention(text: other_member_user.name) wp_page.click_custom_action("Close") wp_page.expect_attributes status: closed_status.name, diff --git a/spec/support/components/work_packages/activities.rb b/spec/support/components/work_packages/activities.rb index a50fbf017f9f..c0c8d0781c39 100644 --- a/spec/support/components/work_packages/activities.rb +++ b/spec/support/components/work_packages/activities.rb @@ -101,6 +101,14 @@ def expect_journal_notes(text: nil) expect(page).to have_test_selector("op-journal-notes-body", text:, wait: 10) end + def expect_journal_mention(text: nil) + expect_journal_notes # wait for the notes to be loaded + + page.within_test_selector("op-journal-notes-body") do + expect(page).to have_css("a.user-mention", text:, wait: 10) + end + end + def expect_notification_bubble expect(page).to have_test_selector("op-journal-unread-notification", wait: 10) end From b4c5cd53048963e1c6292ee90cfa9d0f2b9f7c7a Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 17:55:09 +0100 Subject: [PATCH 04/12] removed obsolete specs --- .../work_packages/tabs/activity_tab_spec.rb | 230 ------------------ 1 file changed, 230 deletions(-) delete mode 100644 spec/features/work_packages/tabs/activity_tab_spec.rb diff --git a/spec/features/work_packages/tabs/activity_tab_spec.rb b/spec/features/work_packages/tabs/activity_tab_spec.rb deleted file mode 100644 index 11b30cc1e1c7..000000000000 --- a/spec/features/work_packages/tabs/activity_tab_spec.rb +++ /dev/null @@ -1,230 +0,0 @@ -#-- copyright -# OpenProject is an open source project management software. -# Copyright (C) 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 COPYRIGHT and LICENSE files for more details. -#++ - -require "spec_helper" - -require "features/work_packages/work_packages_page" -require "support/edit_fields/edit_field" - -RSpec.describe "Activity tab", :js, :with_cuprite do - let(:project) do - create(:project_with_types, - types: [type_with_cf], - work_package_custom_fields: [custom_field], - public: true) - end - - let(:custom_field) { create(:text_wp_custom_field) } - - let(:type_with_cf) do - create(:type, custom_fields: [custom_field]) - end - - let(:creation_time) { 5.days.ago } - let(:subject_change_time) { 3.days.ago } - let(:revision_time) { 2.days.ago } - let(:comment_time) { 1.day.ago } - - let!(:work_package) do - create(:work_package, - project:, - created_at: creation_time, - subject: initial_subject, - journals: { - creation_time => { notes: initial_comment }, - subject_change_time => { subject: "New subject", description: "Some not so long description." }, - comment_time => { notes: "A comment by a different user", user: create(:admin) } - }).tap do |wp| - Journal::CustomizableJournal.create!(journal: wp.journals[1], - custom_field_id: custom_field.id, - value: "* [x] Task 1\n* [ ] Task 2") - end - end - - let(:initial_subject) { "My Subject" } - let(:initial_comment) { "First comment on this wp." } - let(:comments_in_reverse) { false } - let(:activity_tab) { Components::WorkPackages::Activities.new(work_package) } - - let(:creation_journal) do - work_package.journals.reload.first - end - let(:subject_change_journal) { work_package.journals[1] } - let(:comment_journal) { work_package.journals[2] } - - current_user { user } - - before do - allow(user.pref).to receive(:warn_on_leaving_unsaved?).and_return(false) - allow(user.pref).to receive(:comments_sorting).and_return(comments_in_reverse ? "desc" : "asc") - allow(user.pref).to receive(:comments_in_reverse_order?).and_return(comments_in_reverse) - end - - shared_examples "shows activities in order" do - let(:journals) do - journals = [creation_journal, subject_change_journal, comment_journal] - - journals - end - - it "shows activities in ascending order" do - journals.each_with_index do |journal, idx| - actual_index = - if comments_in_reverse - journals.length - idx - else - idx + 1 - end - - date_selector = ".work-package-details-activities-activity:nth-of-type(#{actual_index}) .activity-date" - # Do not use :long format to match the printed date without double spaces - # on the first 9 days of the month - expect(page).to have_selector(date_selector, - text: journal.created_at.to_date.strftime("%B %-d, %Y")) - - activity = page.find("#activity-#{idx + 1}") - - if journal.id != subject_change_journal.id - expect(activity).to have_css(".op-user-activity--user-line", text: journal.user.name) - expect(activity).to have_css(".user-comment > .message", text: journal.notes, visible: :all) - end - - if activity == subject_change_journal - expect(activity).to have_css(".work-package-details-activities-messages .message", - count: 2) - expect(activity).to have_css(".message", - text: "Subject changed from #{initial_subject} " \ - "to #{journal.data.subject}") - end - end - end - end - - shared_examples "activity tab" do - before do - work_package_page.visit_tab! "activity" - work_package_page.ensure_page_loaded - expect(page).to have_css(".user-comment > .message", - text: initial_comment) - end - - context "with permission" do - let(:role) do - create(:project_role, permissions: %i[view_work_packages add_work_package_notes]) - end - let(:user) do - create(:user, - member_with_roles: { project => role }) - end - - context "with ascending comments" do - let(:comments_in_reverse) { false } - - it_behaves_like "shows activities in order" - end - - context "with reversed comments" do - let(:comments_in_reverse) { true } - - it_behaves_like "shows activities in order" - end - - it "can deep link to an activity" do - visit "/work_packages/#{work_package.id}/activity#activity-#{comment_journal.id}" - - work_package_page.ensure_page_loaded - expect(page).to have_css(".user-comment > .message", - text: initial_comment) - - expect(page.current_url).to match /\/work_packages\/#{work_package.id}\/activity#activity-#{comment_journal.id}/ - end - - it "can toggle between activities and comments-only" do - expect(page).to have_css(".work-package-details-activities-activity-contents", count: 3) - expect(page).to have_css(".user-comment > .message", text: comment_journal.notes) - - # Show only comments - find(".activity-comments--toggler").click - - # It should remove the middle - expect(page).to have_css(".work-package-details-activities-activity-contents", count: 2) - expect(page).to have_css(".user-comment > .message", text: initial_comment) - expect(page).to have_css(".user-comment > .message", text: comment_journal.notes) - - # Show all again - find(".activity-comments--toggler").click - expect(page).to have_css(".work-package-details-activities-activity-contents", count: 3) - end - - it "can quote a previous comment" do - activity_tab.hover_action("1", :quote) - - field = TextEditorField.new work_package_page, - "comment", - selector: ".work-packages--activity--add-comment" - - field.expect_active! - - # Add our comment - quote = field.input_element.text - expect(quote).to include(initial_comment) - field.input_element.base.send_keys "\nthis is some remark under a quote" - field.submit_by_click - - expect(page).to have_css(".user-comment > .message", count: 3) - expect(page).to have_css(".user-comment > .message blockquote") - end - end - - context "with no permission" do - let(:role) do - create(:project_role, permissions: [:view_work_packages]) - end - let(:user) do - create(:user, - member_with_roles: { project => role }) - end - - it "shows the activities, but does not allow commenting" do - expect(page).to have_no_css(".work-packages--activity--add-comment", visible: :visible) - end - end - end - - context "if on split screen" do - let(:work_package_page) { Pages::SplitWorkPackage.new(work_package, project) } - - it_behaves_like "activity tab" - end - - context "if on full screen" do - let(:work_package_page) { Pages::FullWorkPackage.new(work_package) } - - it_behaves_like "activity tab" - end -end From 3fc9045ff9e837c9cc5f676e94fd60af8e16d094 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 17:55:23 +0100 Subject: [PATCH 05/12] fixed gitlab activity spec --- .../spec/features/work_package_gitlab_issue_activity_spec.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/gitlab_integration/spec/features/work_package_gitlab_issue_activity_spec.rb b/modules/gitlab_integration/spec/features/work_package_gitlab_issue_activity_spec.rb index 858596c24570..a07d87b004f4 100644 --- a/modules/gitlab_integration/spec/features/work_package_gitlab_issue_activity_spec.rb +++ b/modules/gitlab_integration/spec/features/work_package_gitlab_issue_activity_spec.rb @@ -71,6 +71,7 @@ def trigger_issue_action end let(:work_package_page) { Pages::SplitWorkPackage.new(work_package, project) } + let(:activity_tab) { Components::WorkPackages::Activities.new(work_package) } context "when there is an issue event" do before do @@ -90,7 +91,7 @@ def trigger_issue_action end it "renders a comment referencing the issue" do - expect(page).to have_css(".user-comment > .message", text: expected_comment) + activity_tab.expect_journal_notes(text: expected_comment) end end end From b029b812e27778000dc9c3bdaa0262d80a71c984 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 18:13:48 +0100 Subject: [PATCH 06/12] fixed notification_center_spec via adjusted expect_wp_has_been_created_activity method --- spec/support/components/work_packages/activities.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/spec/support/components/work_packages/activities.rb b/spec/support/components/work_packages/activities.rb index c0c8d0781c39..9d03e985460e 100644 --- a/spec/support/components/work_packages/activities.rb +++ b/spec/support/components/work_packages/activities.rb @@ -38,12 +38,13 @@ class Activities def initialize(work_package) @work_package = work_package - @container = ".work-package-details-activities-list" end def expect_wp_has_been_created_activity(work_package) - within @container do - expect(page).to have_content("created on #{work_package.created_at.strftime('%m/%d/%Y')}") + within "#work-package-activites-container" do + created_date = work_package.created_at.strftime("%m/%d/%Y") + expect(page).to have_text("created this on", wait: 10) + expect(page).to have_text(created_date, wait: 10) end end From a4bda31c46ce080a1641cfe1a594586292321cce Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 18:26:12 +0100 Subject: [PATCH 07/12] fixed working_days_spec --- spec/features/admin/working_days_spec.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/features/admin/working_days_spec.rb b/spec/features/admin/working_days_spec.rb index cfddfaad9ed1..532365c85264 100644 --- a/spec/features/admin/working_days_spec.rb +++ b/spec/features/admin/working_days_spec.rb @@ -121,10 +121,11 @@ def working_days_setting # The updated work packages will have a journal entry informing about the change wp_page = Pages::FullWorkPackage.new(earliest_work_package) + activity_tab = Components::WorkPackages::Activities.new(earliest_work_package) wp_page.visit! - wp_page.expect_activity_message( - "Dates changed by changes to working days (Monday is now non-working, Friday is now non-working)" + activity_tab.expect_journal_changed_attribute( + text: "Dates changed by changes to working days (Monday is now non-working, Friday is now non-working)" ) end From c4796ed321ba1b6130a32e3ab5f83e2a174b37c1 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 18:30:58 +0100 Subject: [PATCH 08/12] fixed edit_work_package_spec --- .../work_packages/edit_work_package_spec.rb | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/spec/features/work_packages/edit_work_package_spec.rb b/spec/features/work_packages/edit_work_package_spec.rb index d71c5775d885..4144397a99ff 100644 --- a/spec/features/work_packages/edit_work_package_spec.rb +++ b/spec/features/work_packages/edit_work_package_spec.rb @@ -60,6 +60,7 @@ let(:new_subject) { "Some other subject" } let(:wp_page) { Pages::FullWorkPackage.new(work_package) } + let(:activity_tab) { Components::WorkPackages::Activities.new(work_package) } let(:priority2) { create(:priority) } let(:status2) { create(:status) } let(:workflow) do @@ -108,7 +109,9 @@ def visit! wp_page.update_attributes status: status2.name wp_page.expect_attributes status: status2.name - wp_page.expect_activity_message("Status changed from #{status.name} to #{status2.name}") + activity_tab.expect_journal_changed_attribute( + text: "Status changed from #{status.name} to #{status2.name}" + ) end end @@ -140,13 +143,17 @@ def visit! version: version.name, category: category.name - wp_page.expect_activity_message("Status changed from #{status.name} to #{status2.name}") + activity_tab.expect_journal_changed_attribute( + text: "Status changed from #{status.name} to #{status2.name}" + ) end it "correctly assigns and un-assigns users" do wp_page.update_attributes assignee: manager.name wp_page.expect_attributes assignee: manager.name - wp_page.expect_activity_message("Assignee set to #{manager.name}") + activity_tab.expect_journal_changed_attribute( + text: "Assignee set to #{manager.name}" + ) field = wp_page.edit_field :assignee field.unset_value @@ -174,8 +181,12 @@ def visit! wp_page.expect_attributes assignee: placeholder_user.name, responsible: placeholder_user.name - wp_page.expect_activity_message("Assignee set to #{placeholder_user.name}") - wp_page.expect_activity_message("Accountable set to #{placeholder_user.name}") + activity_tab.expect_journal_changed_attribute( + text: "Assignee set to #{placeholder_user.name}" + ) + activity_tab.expect_journal_changed_attribute( + text: "Accountable set to #{placeholder_user.name}" + ) end context "switching to custom field with required CF" do From c0919b2d02569f82fc4f0803f7c0b0a0586295e9 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 18:31:39 +0100 Subject: [PATCH 09/12] fixed work_package_workflow_form_spec --- .../work_packages/work_package_workflow_form_spec.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/spec/features/work_packages/work_package_workflow_form_spec.rb b/spec/features/work_packages/work_package_workflow_form_spec.rb index a77e07a59f39..af7d429baec9 100644 --- a/spec/features/work_packages/work_package_workflow_form_spec.rb +++ b/spec/features/work_packages/work_package_workflow_form_spec.rb @@ -58,6 +58,7 @@ work_package end let(:wp_page) { Pages::FullWorkPackage.new(work_package) } + let(:activity_tab) { Components::WorkPackages::Activities.new(work_package) } let(:status_from) { work_package.status } let(:status_intermediate) { create(:status) } @@ -93,14 +94,16 @@ wp_page.update_attributes status: status_intermediate.name wp_page.expect_attributes status: status_intermediate.name - wp_page.expect_activity_message "Status changed from #{status_from.name} " \ - "to #{status_intermediate.name}" + activity_tab.expect_journal_changed_attribute( + text: "Status changed from #{status_from.name} to #{status_intermediate.name}" + ) wp_page.update_attributes status: status_to.name wp_page.expect_attributes status: status_to.name - wp_page.expect_activity_message "Status changed from #{status_from.name} " \ - "to #{status_to.name}" + activity_tab.expect_journal_changed_attribute( + text: "Status changed from #{status_from.name} to #{status_to.name}" + ) work_package.reload expect(work_package.status).to eq(status_to) From e81e9845f4a7e21384a9754bc6fc536ed7e327e4 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 18:32:19 +0100 Subject: [PATCH 10/12] fixed scheduling_mode_spec --- .../work_packages/scheduling/scheduling_mode_spec.rb | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/features/work_packages/scheduling/scheduling_mode_spec.rb b/spec/features/work_packages/scheduling/scheduling_mode_spec.rb index 816cfc664f8a..b622cb7f2b5a 100644 --- a/spec/features/work_packages/scheduling/scheduling_mode_spec.rb +++ b/spec/features/work_packages/scheduling/scheduling_mode_spec.rb @@ -100,7 +100,7 @@ parent: wp_suc) end let(:work_packages_page) { Pages::SplitWorkPackage.new(wp, project) } - + let(:activity_tab) { Components::WorkPackages::Activities.new(wp) } let(:combined_field) { work_packages_page.edit_field(:combinedDate) } def expect_dates(work_package, start_date, due_date) @@ -131,7 +131,9 @@ def expect_dates(work_package, start_date, due_date) work_packages_page.expect_and_dismiss_toaster message: "Successful update." # Changing the scheduling mode is journalized - work_packages_page.expect_activity_message("Manual scheduling activated") + activity_tab.expect_journal_changed_attribute( + text: "Manual scheduling activated" + ) expect_dates(wp, "2016-01-05", "2016-01-10") expect(wp.schedule_manually).to be_truthy From 1a386ef2ce793479621feafee1636b61ddf816b0 Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 18:43:46 +0100 Subject: [PATCH 11/12] fixed copy_spec --- spec/features/work_packages/copy_spec.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spec/features/work_packages/copy_spec.rb b/spec/features/work_packages/copy_spec.rb index 9479edfb8c21..6d2130662c66 100644 --- a/spec/features/work_packages/copy_spec.rb +++ b/spec/features/work_packages/copy_spec.rb @@ -111,7 +111,7 @@ expect(copied_work_package).not_to eql original_work_package work_package_page = Pages::FullWorkPackage.new(copied_work_package, project) - + activity_tab = Components::WorkPackages::Activities.new(copied_work_package) work_package_page.ensure_page_loaded work_package_page.expect_attributes Subject: original_work_package.subject, Description: "Copied WP Description", @@ -120,7 +120,7 @@ Assignee: original_work_package.assigned_to.name, Responsible: original_work_package.responsible.name - work_package_page.expect_activity user, number: 1 + activity_tab.expect_journal_details_header(text: user.name) work_package_page.expect_current_path work_package_page.visit_tab! :relations @@ -152,6 +152,7 @@ it "on split screen page" do original_work_package_page = Pages::SplitWorkPackage.new(original_work_package, project) + activity_tab = Components::WorkPackages::Activities.new(original_work_package) to_copy_work_package_page = original_work_package_page.visit_copy! to_copy_work_package_page.expect_current_path @@ -169,6 +170,7 @@ work_package_page = Pages::SplitWorkPackage.new(copied_work_package, project) work_package_page.ensure_page_loaded + work_package_page.expect_attributes Subject: original_work_package.subject, Description: "Copied WP Description", Version: original_work_package.version, @@ -176,7 +178,11 @@ Assignee: original_work_package.assigned_to, Responsible: original_work_package.responsible - work_package_page.expect_activity user, number: 1 + work_package_page.switch_to_tab(tab: :activity) + activity_tab.expect_journal_details_header(text: user.name) + + work_package_page.switch_to_tab(tab: :overview) + work_package_page.expect_current_path work_package_page.visit_tab!("relations") From f73eb6df5ae64e47b2ed17e8b30b69b7c40d52ad Mon Sep 17 00:00:00 2001 From: Jonas Jabari Date: Tue, 26 Nov 2024 18:44:01 +0100 Subject: [PATCH 12/12] cleanup --- .../pages/work_packages/abstract_work_package.rb | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/spec/support/pages/work_packages/abstract_work_package.rb b/spec/support/pages/work_packages/abstract_work_package.rb index e9983e406981..deb038dfeb68 100644 --- a/spec/support/pages/work_packages/abstract_work_package.rb +++ b/spec/support/pages/work_packages/abstract_work_package.rb @@ -151,18 +151,6 @@ def expect_no_attribute(label) alias :expect_attribute_hidden :expect_no_attribute - def expect_activity(user, number: nil) - container = "#work-package-activites-container" - container += " #activity-#{number}" if number - - expect(page).to have_css("#{container} .op-user-activity--user-line", text: user.name) - end - - def expect_activity_message(message) - expect(page).to have_css(".work-package-details-activities-messages .message", - text: message) - end - def expect_no_parent visit_tab!("relations")