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
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
31 changes: 17 additions & 14 deletions spec/features/work_packages/custom_actions/custom_actions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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!

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading