From 01761cc190f07b72053233685c86fb47d8162b05 Mon Sep 17 00:00:00 2001 From: Sibtain Ali <allroundexperts@gmail.com> Date: Fri, 5 Jan 2024 17:59:28 +0500 Subject: [PATCH 1/7] fix: re-add notification preference condition to unread reports calculation --- src/libs/UnreadIndicatorUpdater/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libs/UnreadIndicatorUpdater/index.ts b/src/libs/UnreadIndicatorUpdater/index.ts index 4126f2af4e83..832a00fb8f37 100644 --- a/src/libs/UnreadIndicatorUpdater/index.ts +++ b/src/libs/UnreadIndicatorUpdater/index.ts @@ -2,6 +2,7 @@ import type {OnyxCollection} from 'react-native-onyx'; import Onyx from 'react-native-onyx'; import * as ReportUtils from '@libs/ReportUtils'; import Navigation, {navigationRef} from '@navigation/Navigation'; +import CONST from '@src/CONST'; import ONYXKEYS from '@src/ONYXKEYS'; import type {Report} from '@src/types/onyx'; import updateUnread from './updateUnread'; @@ -13,7 +14,10 @@ const triggerUnreadUpdate = () => { // We want to keep notification count consistent with what can be accessed from the LHN list const unreadReports = Object.values(allReports ?? {}).filter( - (report) => ReportUtils.isUnread(report) && ReportUtils.shouldReportBeInOptionList(report, currentReportID ?? '', false, [], {}), + (report) => + ReportUtils.isUnread(report) && + ReportUtils.shouldReportBeInOptionList(report, currentReportID ?? '', false, [], {}) && + report?.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, ); updateUnread(unreadReports.length); }; From 8c3c4e24e55f52409a74c1b6bc9a51e60f0c67e9 Mon Sep 17 00:00:00 2001 From: Sibtain Ali <allroundexperts@gmail.com> Date: Sun, 14 Jan 2024 19:30:57 +0500 Subject: [PATCH 2/7] fix: encapsulate unread report logic into getUnreadReportsForUnreadIndicator function --- src/libs/UnreadIndicatorUpdater/index.ts | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/libs/UnreadIndicatorUpdater/index.ts b/src/libs/UnreadIndicatorUpdater/index.ts index 832a00fb8f37..7ada51c7f381 100644 --- a/src/libs/UnreadIndicatorUpdater/index.ts +++ b/src/libs/UnreadIndicatorUpdater/index.ts @@ -9,16 +9,20 @@ import updateUnread from './updateUnread'; let allReports: OnyxCollection<Report> = {}; -const triggerUnreadUpdate = () => { - const currentReportID = navigationRef.isReady() ? Navigation.getTopmostReportId() : ''; - - // We want to keep notification count consistent with what can be accessed from the LHN list - const unreadReports = Object.values(allReports ?? {}).filter( +export default function getUnreadReportsForUnreadIndicator(currentReportID: string) { + return Object.values(allReports ?? {}).filter( (report) => ReportUtils.isUnread(report) && ReportUtils.shouldReportBeInOptionList(report, currentReportID ?? '', false, [], {}) && report?.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, ); +} + +const triggerUnreadUpdate = () => { + const currentReportID = navigationRef.isReady() ? Navigation.getTopmostReportId() ?? '' : ''; + + // We want to keep notification count consistent with what can be accessed from the LHN list + const unreadReports = getUnreadReportsForUnreadIndicator(currentReportID); updateUnread(unreadReports.length); }; From d85cc5f2965a54154c4826627d48806e4e1638cb Mon Sep 17 00:00:00 2001 From: Sibtain Ali <allroundexperts@gmail.com> Date: Wed, 17 Jan 2024 00:41:20 +0500 Subject: [PATCH 3/7] feat: add unit test for unread indicator --- src/CONST.ts | 1 + src/libs/UnreadIndicatorUpdater/index.ts | 6 ++-- tests/unit/UnreadIndicatorUpdaterTest.ts | 41 ++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) create mode 100644 tests/unit/UnreadIndicatorUpdaterTest.ts diff --git a/src/CONST.ts b/src/CONST.ts index b1a6b6895de7..2c7e067bcc4e 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -652,6 +652,7 @@ const CONST = { MUTE: 'mute', DAILY: 'daily', ALWAYS: 'always', + // 'hidden' is a special report level preference that appears when you look at a thread without commenting on it HIDDEN: 'hidden', }, // Options for which room members can post diff --git a/src/libs/UnreadIndicatorUpdater/index.ts b/src/libs/UnreadIndicatorUpdater/index.ts index 7ada51c7f381..2efead35f503 100644 --- a/src/libs/UnreadIndicatorUpdater/index.ts +++ b/src/libs/UnreadIndicatorUpdater/index.ts @@ -9,8 +9,8 @@ import updateUnread from './updateUnread'; let allReports: OnyxCollection<Report> = {}; -export default function getUnreadReportsForUnreadIndicator(currentReportID: string) { - return Object.values(allReports ?? {}).filter( +export default function getUnreadReportsForUnreadIndicator(reports: OnyxCollection<Report>, currentReportID: string) { + return Object.values(reports ?? {}).filter( (report) => ReportUtils.isUnread(report) && ReportUtils.shouldReportBeInOptionList(report, currentReportID ?? '', false, [], {}) && @@ -22,7 +22,7 @@ const triggerUnreadUpdate = () => { const currentReportID = navigationRef.isReady() ? Navigation.getTopmostReportId() ?? '' : ''; // We want to keep notification count consistent with what can be accessed from the LHN list - const unreadReports = getUnreadReportsForUnreadIndicator(currentReportID); + const unreadReports = getUnreadReportsForUnreadIndicator(allReports, currentReportID); updateUnread(unreadReports.length); }; diff --git a/tests/unit/UnreadIndicatorUpdaterTest.ts b/tests/unit/UnreadIndicatorUpdaterTest.ts new file mode 100644 index 000000000000..48aed6ba53b5 --- /dev/null +++ b/tests/unit/UnreadIndicatorUpdaterTest.ts @@ -0,0 +1,41 @@ +/* eslint-disable @typescript-eslint/naming-convention */ +import CONST from '../../src/CONST'; +import getUnreadReportsForUnreadIndicator from '../../src/libs/UnreadIndicatorUpdater'; + +describe('UnreadIndicatorUpdaterTest', () => { + describe('should return correct number of unread reports', () => { + it('given last read time < last visible action created', () => { + const reportsToBeUsed = { + 1: {reportID: '1', reportName: 'test', type: CONST.REPORT.TYPE.EXPENSE, lastReadTime: '2023-07-08 07:15:44.030', lastVisibleActionCreated: '2023-08-08 07:15:44.030'}, + 2: {reportID: '2', reportName: 'test', type: CONST.REPORT.TYPE.TASK, lastReadTime: '2023-02-05 09:12:05.000', lastVisibleActionCreated: '2023-02-06 07:15:44.030'}, + 3: {reportID: '3', reportName: 'test', type: CONST.REPORT.TYPE.TASK}, + }; + expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '4').length).toBe(2); + }); + + it('given some reports are incomplete', () => { + const reportsToBeUsed = { + 1: {reportID: '1', type: CONST.REPORT.TYPE.EXPENSE, lastReadTime: '2023-07-08 07:15:44.030', lastVisibleActionCreated: '2023-08-08 07:15:44.030'}, + 2: {reportID: '2', type: CONST.REPORT.TYPE.TASK, lastReadTime: '2023-02-05 09:12:05.000', lastVisibleActionCreated: '2023-02-06 07:15:44.030'}, + 3: {reportID: '3', type: CONST.REPORT.TYPE.TASK}, + }; + expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '4').length).toBe(0); + }); + + it('given notification preference of some reports is hidden', () => { + const reportsToBeUsed = { + 1: { + reportID: '1', + reportName: 'test', + type: CONST.REPORT.TYPE.EXPENSE, + notificationPreference: CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, + lastReadTime: '2023-07-08 07:15:44.030', + lastVisibleActionCreated: '2023-08-08 07:15:44.030', + }, + 2: {reportID: '2', reportName: 'test', type: CONST.REPORT.TYPE.TASK, lastReadTime: '2023-02-05 09:12:05.000', lastVisibleActionCreated: '2023-02-06 07:15:44.030'}, + 3: {reportID: '3', reportName: 'test', type: CONST.REPORT.TYPE.TASK}, + }; + expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '4').length).toBe(1); + }); + }); +}); From 305178951527aa99896a61f61cb2ca5c2f801dda Mon Sep 17 00:00:00 2001 From: Sibtain Ali <allroundexperts@gmail.com> Date: Thu, 18 Jan 2024 04:16:15 +0500 Subject: [PATCH 4/7] handle more pr comments --- src/CONST.ts | 1 - tests/unit/UnreadIndicatorUpdaterTest.ts | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/CONST.ts b/src/CONST.ts index 60acf9360ec2..d6f3d3cdcef6 100755 --- a/src/CONST.ts +++ b/src/CONST.ts @@ -656,7 +656,6 @@ const CONST = { MUTE: 'mute', DAILY: 'daily', ALWAYS: 'always', - // 'hidden' is a special report level preference that appears when you look at a thread without commenting on it HIDDEN: 'hidden', }, // Options for which room members can post diff --git a/tests/unit/UnreadIndicatorUpdaterTest.ts b/tests/unit/UnreadIndicatorUpdaterTest.ts index 48aed6ba53b5..a5f58b57793a 100644 --- a/tests/unit/UnreadIndicatorUpdaterTest.ts +++ b/tests/unit/UnreadIndicatorUpdaterTest.ts @@ -10,7 +10,7 @@ describe('UnreadIndicatorUpdaterTest', () => { 2: {reportID: '2', reportName: 'test', type: CONST.REPORT.TYPE.TASK, lastReadTime: '2023-02-05 09:12:05.000', lastVisibleActionCreated: '2023-02-06 07:15:44.030'}, 3: {reportID: '3', reportName: 'test', type: CONST.REPORT.TYPE.TASK}, }; - expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '4').length).toBe(2); + expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '3').length).toBe(2); }); it('given some reports are incomplete', () => { @@ -19,7 +19,7 @@ describe('UnreadIndicatorUpdaterTest', () => { 2: {reportID: '2', type: CONST.REPORT.TYPE.TASK, lastReadTime: '2023-02-05 09:12:05.000', lastVisibleActionCreated: '2023-02-06 07:15:44.030'}, 3: {reportID: '3', type: CONST.REPORT.TYPE.TASK}, }; - expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '4').length).toBe(0); + expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '3').length).toBe(0); }); it('given notification preference of some reports is hidden', () => { @@ -35,7 +35,7 @@ describe('UnreadIndicatorUpdaterTest', () => { 2: {reportID: '2', reportName: 'test', type: CONST.REPORT.TYPE.TASK, lastReadTime: '2023-02-05 09:12:05.000', lastVisibleActionCreated: '2023-02-06 07:15:44.030'}, 3: {reportID: '3', reportName: 'test', type: CONST.REPORT.TYPE.TASK}, }; - expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '4').length).toBe(1); + expect(getUnreadReportsForUnreadIndicator(reportsToBeUsed, '3').length).toBe(1); }); }); }); From ce64efa8915dab99369c76940829f2d9ddaafa93 Mon Sep 17 00:00:00 2001 From: Sibtain Ali <allroundexperts@gmail.com> Date: Fri, 19 Jan 2024 18:01:54 +0500 Subject: [PATCH 5/7] fix: lint errors --- src/libs/UnreadIndicatorUpdater/index.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libs/UnreadIndicatorUpdater/index.ts b/src/libs/UnreadIndicatorUpdater/index.ts index 62055dc7e611..044b643f5322 100644 --- a/src/libs/UnreadIndicatorUpdater/index.ts +++ b/src/libs/UnreadIndicatorUpdater/index.ts @@ -14,13 +14,13 @@ export default function getUnreadReportsForUnreadIndicator(reports: OnyxCollecti (report) => ReportUtils.isUnread(report) && ReportUtils.shouldReportBeInOptionList({ - report, - currentReportId: currentReportID ?? '', - betas: [], - policies: {}, - doesReportHaveViolations: false, - isInGSDMode: false, - excludeEmptyChats: false, + report, + currentReportId: currentReportID ?? '', + betas: [], + policies: {}, + doesReportHaveViolations: false, + isInGSDMode: false, + excludeEmptyChats: false, }) && report?.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, ); From 2d4a8a1005f3e3f4a483ce7ec89a196bfe049822 Mon Sep 17 00:00:00 2001 From: Sibtain Ali <allroundexperts@gmail.com> Date: Sat, 20 Jan 2024 01:39:25 +0500 Subject: [PATCH 6/7] re-add comment to better explain hidden notification condition --- src/libs/UnreadIndicatorUpdater/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libs/UnreadIndicatorUpdater/index.ts b/src/libs/UnreadIndicatorUpdater/index.ts index 044b643f5322..5ba15f519ce9 100644 --- a/src/libs/UnreadIndicatorUpdater/index.ts +++ b/src/libs/UnreadIndicatorUpdater/index.ts @@ -22,6 +22,9 @@ export default function getUnreadReportsForUnreadIndicator(reports: OnyxCollecti isInGSDMode: false, excludeEmptyChats: false, }) && + // Chats with hidden preference remain invisible in the LHN and are not considered "unread." + // They are excluded from the LHN rendering, but not filtered from the "option list." + // This ensures they appear in Search, but not in the LHN or unread count. report?.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, ); } From 7673831850f1f0b40c2c00f3f0ee5e25affe8409 Mon Sep 17 00:00:00 2001 From: Sibtain Ali <allroundexperts@gmail.com> Date: Sat, 20 Jan 2024 01:41:29 +0500 Subject: [PATCH 7/7] re-add comment to better explain hidden notification condition --- src/libs/UnreadIndicatorUpdater/index.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/libs/UnreadIndicatorUpdater/index.ts b/src/libs/UnreadIndicatorUpdater/index.ts index 5ba15f519ce9..2a7019686308 100644 --- a/src/libs/UnreadIndicatorUpdater/index.ts +++ b/src/libs/UnreadIndicatorUpdater/index.ts @@ -22,9 +22,11 @@ export default function getUnreadReportsForUnreadIndicator(reports: OnyxCollecti isInGSDMode: false, excludeEmptyChats: false, }) && - // Chats with hidden preference remain invisible in the LHN and are not considered "unread." - // They are excluded from the LHN rendering, but not filtered from the "option list." - // This ensures they appear in Search, but not in the LHN or unread count. + /** + * Chats with hidden preference remain invisible in the LHN and are not considered "unread." + * They are excluded from the LHN rendering, but not filtered from the "option list." + * This ensures they appear in Search, but not in the LHN or unread count. + */ report?.notificationPreference !== CONST.REPORT.NOTIFICATION_PREFERENCE.HIDDEN, ); }