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.

Sign up for GitHub

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

Jump to bottom

Reports with "report not found" error cause Inbox tab RBR #51675

Merged
merged 6 commits into from
Nov 6, 2024
Merged

Reports with "report not found" error cause Inbox tab RBR #51675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
81b6b6e
fix: RBR showing in inbox tab because of a report not included in LHN
pac-guerreiro Oct 29, 2024
6c5b958
Merge branch 'main' into pac-guerreiro/fix/51488-reports-with-report-…
pac-guerreiro Oct 29, 2024
f56b1f3
chore: revert changes
pac-guerreiro Oct 31, 2024
32e0ffa
Merge branch 'main' into pac-guerreiro/fix/51488-reports-with-report-…
pac-guerreiro Oct 31, 2024
7a069b3
chore: resolve eslint issues
pac-guerreiro Oct 31, 2024
acba675
Merge branch 'main' into pac-guerreiro/fix/51488-reports-with-report-…
pac-guerreiro Oct 31, 2024
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
44 changes: 2 additions & 42 deletions src/hooks/useReportIDs.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ import React, {createContext, useCallback, useContext, useMemo} from 'react';
import type {OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import {getPolicyEmployeeListByIdWithoutCurrentUser} from '@libs/PolicyUtils';
import * as ReportActionsUtils from '@libs/ReportActionsUtils';
import SidebarUtils from '@libs/SidebarUtils';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type * as OnyxTypes from '@src/types/onyx';
import type {Message} from '@src/types/onyx/ReportAction';
import mapOnyxCollectionItems from '@src/utils/mapOnyxCollectionItems';
import useActiveWorkspace from './useActiveWorkspace';
import useCurrentReportID from './useCurrentReportID';
Expand All @@ -34,33 +32,6 @@ const ReportIDsContext = createContext<ReportIDsContextValue>({
policyMemberAccountIDs: [],
});

/**
* This function (and the few below it), narrow down the data from Onyx to just the properties that we want to trigger a re-render of the component. This helps minimize re-rendering
* and makes the entire component more performant because it's not re-rendering when a bunch of properties change which aren't ever used in the UI.
*/
const reportActionsSelector = (reportActions: OnyxEntry<OnyxTypes.ReportActions>): ReportActionsSelector =>
(reportActions &&
Object.values(reportActions)
.filter(Boolean)
.map((reportAction) => {
const {reportActionID, actionName, errors = []} = reportAction;
const originalMessage = ReportActionsUtils.getOriginalMessage(reportAction);
const message = ReportActionsUtils.getReportActionMessage(reportAction);
const decision = message?.moderationDecision?.decision;

return {
reportActionID,
actionName,
errors,
message: [
{
moderationDecision: {decision},
},
] as Message[],
originalMessage,
};
})) as ReportActionsSelector;

const policySelector = (policy: OnyxEntry<OnyxTypes.Policy>): PolicySelector =>
(policy && {
type: policy.type,
Expand All @@ -84,7 +55,6 @@ function ReportIDsContextProvider({
const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE, {initialValue: CONST.PRIORITY_MODE.DEFAULT});
const [chatReports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {selector: (c) => mapOnyxCollectionItems(c, policySelector)});
const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {selector: (c) => mapOnyxCollectionItems(c, reportActionsSelector)});
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const [reportsDrafts] = useOnyx(ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT);
const [betas] = useOnyx(ONYXKEYS.BETAS);
Expand All @@ -99,20 +69,10 @@ function ReportIDsContextProvider({

const getOrderedReportIDs = useCallback(
(currentReportID?: string) =>
SidebarUtils.getOrderedReportIDs(
currentReportID ?? null,
chatReports,
betas,
policies,
priorityMode,
allReportActions,
transactionViolations,
activeWorkspaceID,
policyMemberAccountIDs,
),
SidebarUtils.getOrderedReportIDs(currentReportID ?? null, chatReports, betas, policies, priorityMode, transactionViolations, activeWorkspaceID, policyMemberAccountIDs),
// we need reports draft in deps array for reloading of list when reportsDrafts will change
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
[chatReports, betas, policies, priorityMode, allReportActions, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, reportsDrafts],
[chatReports, betas, policies, priorityMode, transactionViolations, activeWorkspaceID, policyMemberAccountIDs, reportsDrafts],
);

const orderedReportIDs = useMemo(() => getOrderedReportIDs(), [getOrderedReportIDs]);
Expand Down
21 changes: 18 additions & 3 deletions src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/BottomTabBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {PressableWithFeedback} from '@components/Pressable';
import type {SearchQueryString} from '@components/Search/types';
import Text from '@components/Text';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import useCurrentReportID from '@hooks/useCurrentReportID';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
Expand Down Expand Up @@ -65,15 +66,23 @@ function BottomTabBar({selectedTab}: BottomTabBarProps) {
const styles = useThemeStyles();
const {translate} = useLocalize();
const {activeWorkspaceID} = useActiveWorkspace();
const {currentReportID} = useCurrentReportID() ?? {currentReportID: null};
const [user] = useOnyx(ONYXKEYS.USER);
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE);
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS);
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const [chatTabBrickRoad, setChatTabBrickRoad] = useState<BrickRoad>(getChatTabBrickRoad(activeWorkspaceID));
const [chatTabBrickRoad, setChatTabBrickRoad] = useState<BrickRoad>(
getChatTabBrickRoad(activeWorkspaceID, currentReportID, reports, betas, policies, priorityMode, transactionViolations),
);

useEffect(() => {
setChatTabBrickRoad(getChatTabBrickRoad(activeWorkspaceID));
}, [activeWorkspaceID, transactionViolations, reports, reportActions]);
setChatTabBrickRoad(getChatTabBrickRoad(activeWorkspaceID, currentReportID, reports, betas, policies, priorityMode, transactionViolations));
// 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
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, why is this comment just about reportActions. All of these are needed since any of them can affect the RBR, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@puneetlath I left the comment specific for reportActions because they are not being used inside the effect explicitly. So to prevent someone from removing the prop from the dependencies list I added that comment 😄

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah ok I see. That makes sense.

}, [activeWorkspaceID, transactionViolations, reports, reportActions, betas, policies, priorityMode, currentReportID]);

const navigateToChats = useCallback(() => {
if (selectedTab === SCREENS.HOME) {
Expand Down Expand Up @@ -118,6 +127,12 @@ function BottomTabBar({selectedTab}: BottomTabBarProps) {
selectedTab={selectedTab}
chatTabBrickRoad={chatTabBrickRoad}
activeWorkspaceID={activeWorkspaceID}
reports={reports}
currentReportID={currentReportID}
betas={betas}
policies={policies}
transactionViolations={transactionViolations}
priorityMode={priorityMode}
/>
)}
<View style={styles.bottomTabBarContainer}>
Expand Down
16 changes: 11 additions & 5 deletions src/libs/Navigation/AppNavigator/createCustomBottomTabNavigator/DebugTabView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React, {useCallback, useMemo} from 'react';
import {View} from 'react-native';
import type {OnyxEntry} from 'react-native-onyx';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import {useOnyx} from 'react-native-onyx';
import Button from '@components/Button';
import Icon from '@components/Icon';
Expand All @@ -21,12 +21,18 @@ import ONYXKEYS from '@src/ONYXKEYS';
import type {Route} from '@src/ROUTES';
import ROUTES from '@src/ROUTES';
import SCREENS from '@src/SCREENS';
import type {ReimbursementAccount} from '@src/types/onyx';
import type {Beta, Policy, PriorityMode, ReimbursementAccount, Report, TransactionViolations} from '@src/types/onyx';

type DebugTabViewProps = {
selectedTab?: string;
chatTabBrickRoad: BrickRoad;
activeWorkspaceID?: string;
currentReportID: string | null;
reports: OnyxCollection<Report>;
betas: OnyxEntry<Beta[]>;
policies: OnyxCollection<Policy>;
transactionViolations: OnyxCollection<TransactionViolations>;
priorityMode: OnyxEntry<PriorityMode>;
};

function getSettingsMessage(status: IndicatorStatus | undefined): TranslationPaths | undefined {
Expand Down Expand Up @@ -91,7 +97,7 @@ function getSettingsRoute(status: IndicatorStatus | undefined, reimbursementAcco
}
}

function DebugTabView({selectedTab = '', chatTabBrickRoad, activeWorkspaceID}: DebugTabViewProps) {
function DebugTabView({selectedTab = '', chatTabBrickRoad, activeWorkspaceID, currentReportID, reports, betas, policies, transactionViolations, priorityMode}: DebugTabViewProps) {
const StyleUtils = useStyleUtils();
const theme = useTheme();
const styles = useThemeStyles();
Expand Down Expand Up @@ -131,7 +137,7 @@ function DebugTabView({selectedTab = '', chatTabBrickRoad, activeWorkspaceID}: D

const navigateTo = useCallback(() => {
if (selectedTab === SCREENS.HOME && !!chatTabBrickRoad) {
const report = getChatTabBrickRoadReport(activeWorkspaceID);
const report = getChatTabBrickRoadReport(activeWorkspaceID, currentReportID, reports, betas, policies, priorityMode, transactionViolations);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use chatTabBrickRoad directly instead of getChatTabBrickRoadReport function?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

chatTabBrickRoad doesn't include the report that I need to redirect the user

Copy link
Contributor

Choose a reason for hiding this comment

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

getChatTabBrickRoad return a report, right?

Screenshot 2024-10-31 at 11 19 16

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DylanDylann nope, it returns a BrickRoad


if (report) {
Navigation.navigate(ROUTES.REPORT_WITH_ID.getRoute(report.reportID));
Expand All @@ -144,7 +150,7 @@ function DebugTabView({selectedTab = '', chatTabBrickRoad, activeWorkspaceID}: D
Navigation.navigate(route);
}
}
}, [selectedTab, chatTabBrickRoad, activeWorkspaceID, status, reimbursementAccount, policyIDWithErrors]);
}, [selectedTab, chatTabBrickRoad, activeWorkspaceID, currentReportID, reports, betas, policies, priorityMode, transactionViolations, status, reimbursementAccount, policyIDWithErrors]);

if (!([SCREENS.HOME, SCREENS.SETTINGS.ROOT] as string[]).includes(selectedTab) || !indicator) {
return null;
Expand Down
124 changes: 76 additions & 48 deletions ...libs/Navigation/AppNavigator/createCustomPlatformStackBottomTabNavigator/BottomTabBar.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import {PressableWithFeedback} from '@components/Pressable';
import type {SearchQueryString} from '@components/Search/types';
import Tooltip from '@components/Tooltip';
import useActiveWorkspace from '@hooks/useActiveWorkspace';
import useCurrentReportID from '@hooks/useCurrentReportID';
import useLocalize from '@hooks/useLocalize';
import useTheme from '@hooks/useTheme';
import useThemeStyles from '@hooks/useThemeStyles';
import * as Session from '@libs/actions/Session';
import interceptAnonymousUser from '@libs/interceptAnonymousUser';
import DebugTabView from '@libs/Navigation/AppNavigator/createCustomBottomTabNavigator/DebugTabView';
import Navigation from '@libs/Navigation/Navigation';
import type {AuthScreensParamList} from '@libs/Navigation/types';
import {isCentralPaneName} from '@libs/NavigationUtils';
Expand Down Expand Up @@ -72,12 +74,23 @@ function BottomTabBar({selectedTab}: BottomTabBarProps) {
const navigation = useNavigation();
const {activeWorkspaceID} = useActiveWorkspace();
const [isLoadingApp] = useOnyx(ONYXKEYS.IS_LOADING_APP);
const transactionViolations = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const [chatTabBrickRoad, setChatTabBrickRoad] = useState<BrickRoad>(getChatTabBrickRoad(activeWorkspaceID));
const {currentReportID} = useCurrentReportID() ?? {currentReportID: null};
const [user] = useOnyx(ONYXKEYS.USER);
const [betas] = useOnyx(ONYXKEYS.BETAS);
const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE);
const [reports] = useOnyx(ONYXKEYS.COLLECTION.REPORT);
const [policies] = useOnyx(ONYXKEYS.COLLECTION.POLICY);
const [reportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS);
const [transactionViolations] = useOnyx(ONYXKEYS.COLLECTION.TRANSACTION_VIOLATIONS);
const [chatTabBrickRoad, setChatTabBrickRoad] = useState<BrickRoad>(
getChatTabBrickRoad(activeWorkspaceID, currentReportID, reports, betas, policies, priorityMode, transactionViolations),
);

useEffect(() => {
setChatTabBrickRoad(getChatTabBrickRoad(activeWorkspaceID));
}, [activeWorkspaceID, transactionViolations]);
setChatTabBrickRoad(getChatTabBrickRoad(activeWorkspaceID, currentReportID, reports, betas, policies, priorityMode, transactionViolations));
// 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
puneetlath marked this conversation as resolved.
Show resolved Hide resolved
}, [activeWorkspaceID, transactionViolations, reports, reportActions, betas, policies, priorityMode, currentReportID]);

useEffect(() => {
const navigationState = navigation.getState();
Expand Down Expand Up @@ -138,51 +151,66 @@ function BottomTabBar({selectedTab}: BottomTabBarProps) {
}, [activeWorkspaceID, selectedTab]);

return (
<View style={styles.bottomTabBarContainer}>
<Tooltip text={translate('common.inbox')}>
<PressableWithFeedback
onPress={navigateToChats}
role={CONST.ROLE.BUTTON}
accessibilityLabel={translate('common.inbox')}
wrapperStyle={styles.flex1}
style={styles.bottomTabBarItem}
>
<View>
<Icon
src={Expensicons.Inbox}
fill={selectedTab === SCREENS.HOME ? theme.iconMenu : theme.icon}
width={variables.iconBottomBar}
height={variables.iconBottomBar}
/>
{!!chatTabBrickRoad && (
<View style={styles.bottomTabStatusIndicator(chatTabBrickRoad === CONST.BRICK_ROAD_INDICATOR_STATUS.INFO ? theme.iconSuccessFill : theme.danger)} />
)}
</View>
</PressableWithFeedback>
</Tooltip>
<Tooltip text={translate('common.search')}>
<PressableWithFeedback
onPress={navigateToSearch}
role={CONST.ROLE.BUTTON}
accessibilityLabel={translate('common.search')}
wrapperStyle={styles.flex1}
style={styles.bottomTabBarItem}
>
<View>
<Icon
src={Expensicons.MoneySearch}
fill={selectedTab === SCREENS.SEARCH.BOTTOM_TAB ? theme.iconMenu : theme.icon}
width={variables.iconBottomBar}
height={variables.iconBottomBar}
/>
</View>
</PressableWithFeedback>
</Tooltip>
<BottomTabAvatar isSelected={selectedTab === SCREENS.SETTINGS.ROOT} />
<View style={[styles.flex1, styles.bottomTabBarItem]}>
<BottomTabBarFloatingActionButton />
<>
{!!user?.isDebugModeEnabled && (
<DebugTabView
selectedTab={selectedTab}
chatTabBrickRoad={chatTabBrickRoad}
activeWorkspaceID={activeWorkspaceID}
reports={reports}
currentReportID={currentReportID}
betas={betas}
policies={policies}
transactionViolations={transactionViolations}
priorityMode={priorityMode}
/>
)}
<View style={styles.bottomTabBarContainer}>
<Tooltip text={translate('common.inbox')}>
<PressableWithFeedback
onPress={navigateToChats}
role={CONST.ROLE.BUTTON}
accessibilityLabel={translate('common.inbox')}
wrapperStyle={styles.flex1}
style={styles.bottomTabBarItem}
>
<View>
<Icon
src={Expensicons.Inbox}
fill={selectedTab === SCREENS.HOME ? theme.iconMenu : theme.icon}
width={variables.iconBottomBar}
height={variables.iconBottomBar}
/>
{!!chatTabBrickRoad && (
<View style={styles.bottomTabStatusIndicator(chatTabBrickRoad === CONST.BRICK_ROAD_INDICATOR_STATUS.INFO ? theme.iconSuccessFill : theme.danger)} />
)}
</View>
</PressableWithFeedback>
</Tooltip>
<Tooltip text={translate('common.search')}>
<PressableWithFeedback
onPress={navigateToSearch}
role={CONST.ROLE.BUTTON}
accessibilityLabel={translate('common.search')}
wrapperStyle={styles.flex1}
style={styles.bottomTabBarItem}
>
<View>
<Icon
src={Expensicons.MoneySearch}
fill={selectedTab === SCREENS.SEARCH.BOTTOM_TAB ? theme.iconMenu : theme.icon}
width={variables.iconBottomBar}
height={variables.iconBottomBar}
/>
</View>
</PressableWithFeedback>
</Tooltip>
<BottomTabAvatar isSelected={selectedTab === SCREENS.SETTINGS.ROOT} />
<View style={[styles.flex1, styles.bottomTabBarItem]}>
<BottomTabBarFloatingActionButton />
</View>
</View>
</View>
</>
);
}

Expand Down
3 changes: 1 addition & 2 deletions src/libs/SidebarUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {Str} from 'expensify-common';
import type {OnyxCollection, OnyxEntry} from 'react-native-onyx';
import Onyx from 'react-native-onyx';
import type {ValueOf} from 'type-fest';
import type {PolicySelector, ReportActionsSelector} from '@hooks/useReportIDs';
import type {PolicySelector} from '@hooks/useReportIDs';
import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import type {PersonalDetails, PersonalDetailsList, ReportActions, TransactionViolation} from '@src/types/onyx';
Expand Down Expand Up @@ -88,7 +88,6 @@ function getOrderedReportIDs(
betas: OnyxEntry<Beta[]>,
policies: OnyxCollection<PolicySelector>,
priorityMode: OnyxEntry<PriorityMode>,
allReportActions: OnyxCollection<ReportActionsSelector>,
transactionViolations: OnyxCollection<TransactionViolation[]>,
currentPolicyID = '',
policyMemberAccountIDs: number[] = [],
Expand Down
Loading
Loading