Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implementation/59251 fully remove feature flag for primerized activity tab #17282

Draft
wants to merge 13 commits into
base: dev
Choose a base branch
from
Draft
1 change: 0 additions & 1 deletion .github/workflows/pullpreview.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/')
Expand Down
1 change: 0 additions & 1 deletion config/initializers/feature_decisions.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
# OpenProject::FeatureDecisions.add :some_flag
# end

OpenProject::FeatureDecisions.add :primerized_work_package_activities
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to revert #17192 as well 🧹 ✨

OpenProject::FeatureDecisions.add :built_in_oauth_applications,
description: "Allows the display and use of built-in OAuth applications."

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand All @@ -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
*/
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div *ngIf="primerizedActivitiesEnabled" id="work-package-activites-container">
<div id="work-package-activites-container">
<turbo-frame [src]="turboFrameSrc" id="work-package-activities-tab-content" loading="lazy">
<op-content-loader viewBox="0 0 100 100">
<svg:rect x="0" y="0" width="70" height="5" rx="1" />
Expand All @@ -11,51 +11,3 @@
</op-content-loader>
</turbo-frame>
</div>

<div *ngIf="!primerizedActivitiesEnabled" id="work-package-activites-container">
<div id="work-package-comment-container">

<ng-container *ngIf="!showAbove" [ngTemplateOutlet]="template"></ng-container>

<div
class="work-package--details--long-field work-packages--activity--add-comment hide-when-print"
[ngClass]="{'work-packages--activity--add-comment_top': showAbove} "
#commentContainer
*ngIf="canAddComment"
>
<div class="inline-edit--container inplace-edit">
<edit-form-portal
*ngIf="active"
[schemaInput]="schema"
[changeInput]="change"
[editFieldHandler]="handler"
>
</edit-form-portal>
<div
*ngIf="!active"
(dragover)="startDragOverActivation($event)"
(dragenter)="startDragOverActivation($event)"
>
<button
class="work-package-comment inline-edit--display-field"
[title]="text.editTitle"
(click)="activate()"
>
<span
class="work-package-comment--text"
[textContent]="text.placeholder"
>
</span>

<op-icon
class="work-package-comment--icon"
icon-classes="icon-edit" [icon-title]="text.editTitle"
></op-icon>
</button>
</div>
</div>
</div>

<ng-container *ngIf="showAbove" [ngTemplateOutlet]="template"></ng-container>
</div>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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$
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -45,22 +44,17 @@ 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();
}

ngOnInit() {
this.workPackageId = this.workPackage?.id || this.$state.params.workPackageId as string;

this.primerizedActivitiesEnabled = this.configurationService.activeFeatureFlags.includes('primerizedWorkPackageActivities');

this
.apiV3Service
.work_packages
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,2 @@
<wp-single-view [workPackage]="workPackage"
*ngIf="workPackage"></wp-single-view>

<div class="attributes-group" *ngIf="workPackage && !primerizedActivitiesEnabled">
<div class="attributes-group--header">
<div class="attributes-group--header-container">
<h3 class="attributes-group--header-text" [textContent]="tabName"></h3>
</div>
</div>

<newest-activity-on-overview [workPackage]="workPackage"></newest-activity-on-overview>
</div>
*ngIf="workPackage"></wp-single-view>
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion spec/features/activities/work_package/activities_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
require "spec_helper"
require "support/flash/expectations"

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
include Flash::Expectations

let(:project) { create(:project) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
5 changes: 3 additions & 2 deletions spec/features/admin/working_days_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 9 additions & 3 deletions spec/features/work_packages/copy_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -169,14 +170,19 @@
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,
Priority: original_work_package.priority,
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")
Expand Down
Loading
Loading