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

bug/60449 Reminders generate two or more lines for a work package in notification center, clicking one selects all #17611

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions config/locales/js-en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -681,11 +681,11 @@ en:
date_alerts:
milestone_date: "Milestone date"
overdue: "Overdue"
overdue_since: "since %{difference_in_days}"
property_today: "is today"
property_is: "is in %{difference_in_days}"
property_was: "was %{difference_in_days} ago"
property_is_deleted: "is deleted"
overdue_since: "since %{difference_in_days}."
property_today: "is today."
property_is: "is in %{difference_in_days}."
property_was: "was %{difference_in_days} ago."
property_is_deleted: "is deleted."
upsale:
title: "Date alerts"
description: "With date alerts, you will be notified of upcoming start or finish dates so that you never miss or forget an important deadline."
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,28 +111,7 @@ export class IanCenterService extends UntilDestroyedMixin {
notifications$ = this
.aggregatedCenterNotifications$
.pipe(
map((items) => {
return Object.values(items).reduce((acc, workPackageNotificationGroup) => {
const { reminders, others } = workPackageNotificationGroup.reduce((result, notification) => {
if (notification.reason === 'reminder') {
result.reminders.push(notification);
} else {
result.others.push(notification);
}
return result;
}, { reminders: [] as INotification[], others: [] as INotification[] });

// Extract reminders into standalone groups so they can be displayed individually
if (reminders.length > 0) {
reminders.forEach((reminder) => acc.push([reminder]));
}
if (others.length > 0) {
acc.push(others);
}

return acc;
}, [] as INotification[][]);
}),
map((items) => Object.values(items)),
distinctUntilChanged(),
);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
<op-in-app-notification-date-alert
*ngIf="hasDateAlert"
[aggregatedNotifications]="dateAlerts">
</op-in-app-notification-date-alert>
<op-in-app-notification-relative-time
*ngIf="!hasDateAlert"
[notification]="reminderAlert"
[hasActorByLine]="false"
/>
[hasActorByLine]="false">
</op-in-app-notification-relative-time>

<span
class="op-ian-reminder-alert--note"
[textContent]="reminderNote"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ export class InAppNotificationReminderAlertComponent implements OnInit {

reminderNote:string;
reminderAlert:INotification;
hasDateAlert = false;
dateAlerts:INotification[] = [];

constructor(
private I18n:I18nService,
Expand All @@ -31,6 +33,8 @@ export class InAppNotificationReminderAlertComponent implements OnInit {
ngOnInit():void {
this.reminderAlert = this.deriveMostRecentReminder(this.aggregatedNotifications);
this.reminderNote = this.extractReminderNoteValue(this.reminderAlert._embedded.details);
this.dateAlerts = this.aggregatedNotifications.filter((notification) => notification.reason === 'dateAlert');
this.hasDateAlert = this.dateAlerts.length > 0;
}

private deriveMostRecentReminder(aggregatedNotifications:INotification[]):INotification {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ def run_create_date_alerts_notifications_job
it "shows the upsale page" do
side_menu.click_item "Date alert"

expect(page).to have_current_path /notifications\/date_alerts/
expect(page).to have_current_path(/notifications\/date_alerts/)
expect(page).to have_text "Date alerts is an Enterprise"
expect(page).to have_text "Please upgrade to a paid plan "

Expand All @@ -207,18 +207,18 @@ def run_create_date_alerts_notifications_job

context "with date alerts ee", with_ee: %i[date_alerts] do
it "shows the date alerts according to specification" do
center.expect_item(notification_wp_start_past, "Start date was 1 day ago")
center.expect_item(notification_wp_start_future, "Start date is in 7 days")
center.expect_item(notification_wp_start_past, "Start date was 1 day ago.")
center.expect_item(notification_wp_start_future, "Start date is in 7 days.")

center.expect_item(notification_wp_due_past, "Overdue since 3 days")
center.expect_item(notification_wp_due_future, "Finish date is in 3 days")
center.expect_item(notification_wp_due_past, "Overdue since 3 days.")
center.expect_item(notification_wp_due_future, "Finish date is in 3 days.")

center.expect_item(notification_milestone_past, "Overdue since 2 days")
center.expect_item(notification_milestone_future, "Milestone date is in 1 day")
center.expect_item(notification_milestone_past, "Overdue since 2 days.")
center.expect_item(notification_milestone_future, "Milestone date is in 1 day.")

center.expect_item(notification_wp_unset_date, "Finish date is deleted")
center.expect_item(notification_wp_unset_date, "Finish date is deleted.")

center.expect_item(notification_wp_due_today, "Finish date is today")
center.expect_item(notification_wp_due_today, "Finish date is today.")

# Doesn't show the date alert for the mention, not the alert
center.expect_item(notification_wp_double_mention, /(seconds|minutes) ago by Anonymous/)
Expand All @@ -227,7 +227,7 @@ def run_create_date_alerts_notifications_job
# When switch to date alerts, it shows the alert, no longer the mention
side_menu.click_item "Date alert"
wait_for_network_idle
center.expect_item(notification_wp_double_date_alert, "Finish date is in 1 day")
center.expect_item(notification_wp_double_date_alert, "Finish date is in 1 day.")
center.expect_no_item(notification_wp_double_mention)

# Ensure that start is created later than due for implicit ID sorting
Expand All @@ -236,7 +236,7 @@ def run_create_date_alerts_notifications_job

# We see that start is actually the newest ID, hence shown as the primary notification
# but the date alert still shows the finish date
center.expect_item(double_alert_start, "Finish date is in 1 day")
center.expect_item(double_alert_start, "Finish date is in 1 day.")
center.expect_no_item(double_alert_due)

# Opening a date alert opens in overview
Expand All @@ -262,7 +262,7 @@ def run_create_date_alerts_notifications_job
page.driver.refresh
wait_for_reload

center.expect_item(notification_wp_double_date_alert, "Finish date is in 5 days")
center.expect_item(notification_wp_double_date_alert, "Finish date is in 5 days.")
center.expect_no_item(notification_wp_double_mention)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,19 @@
member_with_permissions: { project => %w[view_work_packages] })
end
shared_let(:work_package) { create(:work_package, project:, due_date: 1.day.ago) }
shared_let(:work_package2) { create(:work_package, project:) }

shared_let(:notification_mention) do
create(:notification,
reason: :mentioned,
recipient: user,
resource: work_package,
actor:)
shared_let(:notification_mentions) do
[create(:notification,
reason: :mentioned,
recipient: user,
resource: work_package,
actor:),
create(:notification,
reason: :mentioned,
recipient: user,
resource: work_package2,
actor: actor)]
end

shared_let(:notification_date_alert) do
Expand All @@ -30,13 +36,11 @@
end

shared_let(:notification_reminder) do
reminder = create(:reminder, remindable: work_package, creator: user, note: "This is an important reminder")
notification = create(:notification,
reason: :reminder,
recipient: user,
resource: work_package)
create(:reminder_notification, reminder:, notification:)
notification
create_reminder_notification_for(work_package: work_package, user: user)
end

shared_let(:notification_reminder2) do
create_reminder_notification_for(work_package: work_package2, user: user)
end

let(:center) { Pages::Notifications::Center.new }
Expand All @@ -47,18 +51,31 @@
wait_for_reload
end

it "shows the reminder alert in own entry" do
center.within_item(notification_reminder) do
expect(page).to have_text("##{work_package.id}\n- #{project.name} -\nReminder")
expect(page).to have_no_text("Actor user")
expect(page).to have_text("a few seconds ago.\nNote: “This is an important reminder”")
context "with a reminder, mention and date alert" do
it "shows the reminder alert within aggregation with date alert + reminder note" do
center.within_item(notification_reminder) do
expect(page).to have_text("##{work_package.id}\n- #{project.name} -\nDate alert, Mentioned, Reminder")
expect(page).to have_text("Overdue since 1 day.\nNote: “This is an important reminder”")
end
end
end

it "shows other notification reasons aggregated" do
center.within_item(notification_date_alert) do
expect(page).to have_text("##{work_package.id}\n- #{project.name} -\nDate alert, Mentioned")
expect(page).to have_no_text("Actor user")
context "with a reminder and mention, no date alert" do
it "shows the reminder alert within aggregation with mention" do
center.within_item(notification_reminder2) do
expect(page).to have_text("##{work_package2.id}\n- #{project.name} -\nMentioned, Reminder")
expect(page).to have_text("a few seconds ago.\nNote: “This is an important reminder”")
end
end
end

def create_reminder_notification_for(work_package:, user:)
reminder = create(:reminder, remindable: work_package, creator: user, note: "This is an important reminder")
notification = create(:notification,
reason: :reminder,
recipient: user,
resource: work_package)
create(:reminder_notification, reminder:, notification:)
notification
end
end
Loading