Skip to content
8 changes: 4 additions & 4 deletions src/components/Navigation/NavigationTabBar/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ function NavigationTabBar({selectedTab, isTooltipAllowed = false, isTopLevelBar
const {activeWorkspaceID} = useActiveWorkspace();
const {orderedReportIDs} = useSidebarOrderedReportIDs();
const [account] = useOnyx(ONYXKEYS.ACCOUNT, {canBeMissing: false});
const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {canBeMissing: true});
const [reportAttributes] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, {canBeMissing: true});
const [reports = []] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {
selector: (values) => orderedReportIDs.map((reportID) => values?.[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`]),
canBeMissing: true,
Expand All @@ -74,9 +74,9 @@ function NavigationTabBar({selectedTab, isTooltipAllowed = false, isTopLevelBar

useEffect(() => {
setChatTabBrickRoad(getChatTabBrickRoad(activeWorkspaceID, reports));
// We need to get a new brick road state when report actions are updated, otherwise we'll be showing an outdated brick road.
// That's why reportActions is added as a dependency here
}, [activeWorkspaceID, reports, reportActions]);
// We need to get a new brick road state when report attributes are updated, otherwise we'll be showing an outdated brick road.
// That's why reportAttributes is added as a dependency here
}, [activeWorkspaceID, reports, reportAttributes]);

const navigateToChats = useCallback(() => {
if (selectedTab === NAVIGATION_TABS.HOME) {
Expand Down
76 changes: 9 additions & 67 deletions src/libs/WorkspacesSettingsUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import type {LocaleContextProps} from '@components/LocaleContextProvider';
import CONST from '@src/CONST';
import type {TranslationPaths} from '@src/languages/types';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Policy, ReimbursementAccount, Report, ReportAction, ReportActions, TransactionViolations} from '@src/types/onyx';
import type {Policy, ReimbursementAccount, Report, ReportActions, ReportAttributesDerivedValue} from '@src/types/onyx';
import type {PolicyConnectionSyncProgress, Unit} from '@src/types/onyx/Policy';
import {isConnectionInProgress} from './actions/connections';
import {shouldShowQBOReimbursableExportDestinationAccountError} from './actions/connections/QuickbooksOnline';
import {convertToDisplayString} from './CurrencyUtils';
import {isPolicyAdmin, shouldShowCustomUnitsError, shouldShowEmployeeListError, shouldShowPolicyError, shouldShowSyncError, shouldShowTaxRateError} from './PolicyUtils';
import {getOneTransactionThreadReportID} from './ReportActionsUtils';
import {getAllReportErrors, hasReportViolations, isReportOwner, isUnread, isUnreadWithMention, requiresAttentionFromCurrentUser, shouldDisplayViolationsRBRInLHN} from './ReportUtils';
import {isUnread} from './ReportUtils';

type CheckingMethod = () => boolean;

Expand All @@ -27,80 +27,22 @@ Onyx.connect({
},
});

let allReportActions: OnyxCollection<ReportActions>;
let reportAttributes: ReportAttributesDerivedValue['reports'];
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT_ACTIONS,
waitForCollectionCallback: true,
callback: (actions) => {
if (!actions) {
return;
}
allReportActions = actions;
},
});

let reportsCollection: OnyxCollection<Report>;
Onyx.connect({
key: ONYXKEYS.COLLECTION.REPORT,
waitForCollectionCallback: true,
callback: (value) => {
reportsCollection = value;
},
});

let allTransactionViolations: NonNullable<OnyxCollection<TransactionViolations>> = {};
Onyx.connect({
key: ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS,
waitForCollectionCallback: true,
key: ONYXKEYS.DERIVED.REPORT_ATTRIBUTES,
callback: (value) => {
if (!value) {
allTransactionViolations = {};
return;
}

allTransactionViolations = value;
reportAttributes = value.reports;
},
});

/**
* @param altReportActions Replaces (local) allReportActions used within (local) function getWorkspacesBrickRoads
* @returns BrickRoad for the policy passed as a param and optionally actionsByReport (if passed)
*/
const getBrickRoadForPolicy = (report: Report, altReportActions?: OnyxCollection<ReportActions>): BrickRoad => {
const reportActions = (altReportActions ?? allReportActions)?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.reportID}`] ?? {};
const reportErrors = getAllReportErrors(report, reportActions);
const oneTransactionThreadReportID = getOneTransactionThreadReportID(report.reportID, reportActions);
const oneTransactionThreadReport = reportsCollection?.[`${ONYXKEYS.COLLECTION.REPORT}${oneTransactionThreadReportID}`];
let doesReportContainErrors = Object.keys(reportErrors ?? {}).length !== 0 ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : undefined;

if (!doesReportContainErrors) {
const shouldDisplayViolations = shouldDisplayViolationsRBRInLHN(report, allTransactionViolations);
const shouldDisplayReportViolations = isReportOwner(report) && hasReportViolations(report.reportID);
const hasViolations = shouldDisplayViolations || shouldDisplayReportViolations;
if (hasViolations) {
doesReportContainErrors = CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
}
}

if (oneTransactionThreadReportID && !doesReportContainErrors) {
if (shouldDisplayViolationsRBRInLHN(oneTransactionThreadReport, allTransactionViolations)) {
doesReportContainErrors = CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
}
}

if (doesReportContainErrors) {
return CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR;
}

// To determine if the report requires attention from the current user, we need to load the parent report action
let itemParentReportAction: OnyxEntry<ReportAction>;
if (report.parentReportID) {
const itemParentReportActions = allReportActions?.[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${report.parentReportID}`] ?? {};
itemParentReportAction = report.parentReportActionID ? itemParentReportActions[report.parentReportActionID] : undefined;
}
const reportOption = {...report, isUnread: isUnread(report, oneTransactionThreadReport), isUnreadWithMention: isUnreadWithMention(report)};
const shouldShowGreenDotIndicator = requiresAttentionFromCurrentUser(reportOption, itemParentReportAction);
return shouldShowGreenDotIndicator ? CONST.BRICK_ROAD_INDICATOR_STATUS.INFO : undefined;
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This function seems to share the same logic that is used to determine GBR/RBR status for a report in a derived value, so it is safe to simply check the brick road status directly instead of computing it

const getBrickRoadForPolicy = (report: Report): BrickRoad => {
return reportAttributes?.[report.reportID]?.brickRoadStatus;
};

function hasGlobalWorkspaceSettingsRBR(policies: OnyxCollection<Policy>, allConnectionProgresses: OnyxCollection<PolicyConnectionSyncProgress>) {
Expand Down Expand Up @@ -175,7 +117,7 @@ function getChatTabBrickRoad(policyID: string | undefined, orderedReports: Array
/**
* @returns a map where the keys are policyIDs and the values are BrickRoads for each policy
*/
function getWorkspacesBrickRoads(reports: OnyxCollection<Report>, policies: OnyxCollection<Policy>, reportActions: OnyxCollection<ReportActions>): Record<string, BrickRoad> {
function getWorkspacesBrickRoads(reports: OnyxCollection<Report>, policies: OnyxCollection<Policy>): Record<string, BrickRoad> {
if (!reports) {
return {};
}
Expand All @@ -198,7 +140,7 @@ function getWorkspacesBrickRoads(reports: OnyxCollection<Report>, policies: Onyx
if (!report || workspacesBrickRoadsMap[policyID] === CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR) {
return;
}
const workspaceBrickRoad = getBrickRoadForPolicy(report, reportActions);
const workspaceBrickRoad = getBrickRoadForPolicy(report);

if (!workspaceBrickRoad && !!workspacesBrickRoadsMap[policyID]) {
return;
Expand Down
6 changes: 5 additions & 1 deletion src/libs/actions/OnyxDerived/configs/reportAttributes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,13 @@ export default createOnyxDerivedValueConfig({
dataToIterate = prepareReportKeys(updates);
recentlyUpdated = updates;
} else if (!!transactionsUpdates || !!transactionViolationsUpdates) {
let transactionReportIds: string[] = [];
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
let transactionReportIds: string[] = [];
let transactionReportIDs: string[] = [];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'll update it with one of the following PRs, thanks

if (transactionsUpdates) {
transactionReportIds = Object.values(transactionsUpdates).map((transaction) => `${ONYXKEYS.COLLECTION.REPORT}${transaction?.reportID}`);
}
// if transactions are updated, they might not be directly related to the reports yet (e.g. transaction is optimistically created)
// so we use report keys that were updated before to recompute the reports
const recentReportKeys = prepareReportKeys(recentlyUpdated);
const recentReportKeys = prepareReportKeys([...recentlyUpdated, ...transactionReportIds]);
dataToIterate = recentReportKeys;
}
}
Expand Down
12 changes: 6 additions & 6 deletions src/pages/WorkspaceSwitcherPage/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ function WorkspaceSwitcherPage() {
const {translate} = useLocalize();
const {activeWorkspaceID} = useActiveWorkspace();

const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS);
const [policies, fetchStatus] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email});
const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);
const brickRoadsForPolicies = useMemo(() => getWorkspacesBrickRoads(reports, policies, reportActions), [reports, policies, reportActions]);
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT, {canBeMissing: true});
const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {canBeMissing: true});
const [policies, fetchStatus] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {canBeMissing: true});
const [currentUserLogin] = useOnyx(ONYXKEYS.SESSION, {selector: (session) => session?.email, canBeMissing: true});
const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP, {canBeMissing: true});
Comment on lines +35 to +36
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can these be missing really?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This property is here for a while now but I still find it super confusing. Out of curiosity I just checked the codebase and this key is used 4 times with false and 11 times with true and I can't really say what's the difference. The JSDoc description of this param is not clear to me and based on these numbers I think it might be problematic for more people. Can we do something to improve it? cc @fabioh8010

const brickRoadsForPolicies = useMemo(() => getWorkspacesBrickRoads(reports, policies), [reports, policies]);
const unreadStatusesForPolicies = useMemo(() => getWorkspacesUnreadStatuses(reports, reportActions), [reports, reportActions]);
const shouldShowLoadingIndicator = isLoadingApp && !isOffline;

Expand Down
2 changes: 2 additions & 0 deletions tests/ui/BottomTabBarTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import NavigationTabBar from '@components/Navigation/NavigationTabBar';
import NAVIGATION_TABS from '@components/Navigation/NavigationTabBar/NAVIGATION_TABS';
import OnyxProvider from '@components/OnyxProvider';
import {SidebarOrderedReportIDsContextProvider} from '@hooks/useSidebarOrderedReportIDs';
import initOnyxDerivedValues from '@userActions/OnyxDerived';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';

Expand All @@ -16,6 +17,7 @@ jest.mock('@src/hooks/useRootNavigationState');
describe('NavigationTabBar', () => {
beforeAll(() => {
Onyx.init({keys: ONYXKEYS});
initOnyxDerivedValues();
});
beforeEach(() => {
Onyx.clear();
Expand Down
8 changes: 5 additions & 3 deletions tests/unit/WorkspaceSettingsUtilsTest.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type {OnyxCollection} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import {getBrickRoadForPolicy} from '@libs/WorkspacesSettingsUtils';
import initOnyxDerivedValues from '@userActions/OnyxDerived';
import ONYXKEYS from '@src/ONYXKEYS';
import type {Report, ReportActions, Transaction, TransactionViolations} from '@src/types/onyx';
import type {ReportCollectionDataSet} from '@src/types/onyx/Report';
Expand All @@ -13,6 +14,7 @@ describe('WorkspacesSettingsUtils', () => {
Onyx.init({
keys: ONYXKEYS,
});
initOnyxDerivedValues();
});

beforeEach(() => {
Expand All @@ -39,8 +41,8 @@ describe('WorkspacesSettingsUtils', () => {

await waitForBatchedUpdates();

// When calling getBrickRoadForPolicy with a report and report actions
const result = getBrickRoadForPolicy(report as Report, reportActions as OnyxCollection<ReportActions>);
// When calling getBrickRoadForPolicy with a report
const result = getBrickRoadForPolicy(report as Report);

// The result should be 'error' because there is at least one IOU action associated with a transaction that has a violation.
expect(result).toBe('error');
Expand All @@ -62,7 +64,7 @@ describe('WorkspacesSettingsUtils', () => {
await waitForBatchedUpdates();

// When calling getBrickRoadForPolicy with a report and report actions
const result = getBrickRoadForPolicy(report as Report, reportActions as OnyxCollection<ReportActions>);
const result = getBrickRoadForPolicy(report as Report);

// Then the result should be 'undefined' since no IOU action is linked to a transaction with a violation.
expect(result).toBe(undefined);
Expand Down