Merge pull request Expensify#11450 from Expensify/marcaaron-removeCur…

…rentlyViewedReportID

Kill `currentlyViewedReportID`

src/ONYXKEYS.js

@@ -24,9 +24,6 @@ export default {
     // Stores current date
     CURRENT_DATE: 'currentDate',
 
-    // Currently viewed reportID
-    CURRENTLY_VIEWED_REPORTID: 'currentlyViewedReportID',
-
     // Credentials to authenticate the user
     CREDENTIALS: 'credentials',
 

src/components/ReportActionItem/IOUAction.js

@@ -14,7 +14,7 @@ const propTypes = {
     action: PropTypes.shape(reportActionPropTypes).isRequired,
 
     /** The associated chatReport */
-    chatReportID: PropTypes.number.isRequired,
+    chatReportID: PropTypes.string.isRequired,
 
     /** Is this IOUACTION the most recent? */
     isMostRecentIOUReportAction: PropTypes.bool.isRequired,
@@ -47,7 +47,7 @@ const IOUAction = (props) => {
             {((props.isMostRecentIOUReportAction && Boolean(props.action.originalMessage.IOUReportID))
                 || (props.action.originalMessage.type === 'pay')) && (
                     <IOUPreview
-                        iouReportID={props.action.originalMessage.IOUReportID}
+                        iouReportID={props.action.originalMessage.IOUReportID.toString()}
                         chatReportID={props.chatReportID}
                         onPayButtonPressed={launchDetailsModal}
                         onPreviewPressed={launchDetailsModal}

src/components/ReportActionItem/IOUPreview.js

@@ -33,10 +33,10 @@ const propTypes = {
     onPayButtonPressed: PropTypes.func,
 
     /** The active IOUReport, used for Onyx subscription */
-    iouReportID: PropTypes.number.isRequired,
+    iouReportID: PropTypes.string.isRequired,
 
     /** The associated chatReport */
-    chatReportID: PropTypes.number.isRequired,
+    chatReportID: PropTypes.string.isRequired,
 
     /** Callback for the preview pressed */
     onPreviewPressed: PropTypes.func,

src/libs/Navigation/AppNavigator/MainDrawerNavigator.js

@@ -67,7 +67,16 @@ const MainDrawerNavigator = (props) => {
     // This is usually needed after login/create account and re-launches
     return (
         <BaseDrawerNavigator
-            drawerContent={() => <SidebarScreen />}
+            drawerContent={({navigation, state}) => {
+                // This state belongs to the drawer so it should always have the ReportScreen as it's initial (and only) route
+                const reportIDFromRoute = lodashGet(state, ['routes', 0, 'params', 'reportID']);
+                return (
+                    <SidebarScreen
+                        navigation={navigation}
+                        reportIDFromRoute={reportIDFromRoute}
+                    />
+                );
+            }}
             screens={[
                 {
                     name: SCREENS.REPORT,

src/libs/Navigation/Navigation.js

@@ -1,4 +1,5 @@
 import _ from 'underscore';
+import lodashGet from 'lodash/get';
 import {Keyboard} from 'react-native';
 import {DrawerActions, getPathFromState, StackActions} from '@react-navigation/native';
 import Onyx from 'react-native-onyx';
@@ -182,6 +183,19 @@ function getActiveRoute() {
         : '';
 }
 
+/**
+ * @returns {String}
+ */
+function getReportIDFromRoute() {
+    if (!navigationRef.current) {
+        return '';
+    }
+
+    const drawerState = lodashGet(navigationRef.current.getState(), ['routes', 0, 'state']);
+    const reportRoute = lodashGet(drawerState, ['routes', 0]);
+    return lodashGet(reportRoute, ['params', 'reportID'], '');
+}
+
 /**
  * Check whether the passed route is currently Active or not.
  *
@@ -230,6 +244,7 @@ export default {
     setDidTapNotification,
     isNavigationReady,
     setIsNavigationReady,
+    getReportIDFromRoute,
     isDrawerReady,
     setIsDrawerReady,
     isDrawerRoute,

src/libs/OptionsListUtils.js

@@ -9,6 +9,7 @@ import * as ReportUtils from './ReportUtils';
 import * as Localize from './Localize';
 import Permissions from './Permissions';
 import * as CollectionUtils from './CollectionUtils';
+import Navigation from './Navigation/Navigation';
 
 /**
  * OptionsListUtils is used to build a list options passed to the OptionsList component. Several different UI views can
@@ -22,12 +23,6 @@ Onyx.connect({
     callback: val => currentUserLogin = val && val.email,
 });
 
-let currentlyViewedReportID;
-Onyx.connect({
-    key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
-    callback: val => currentlyViewedReportID = val,
-});
-
 let priorityMode;
 Onyx.connect({
     key: ONYXKEYS.NVP_PRIORITY_MODE,
@@ -428,12 +423,11 @@ function isCurrentUser(userDetails) {
  *
  * @param {Object} reports
  * @param {Object} personalDetails
- * @param {String} activeReportID
  * @param {Object} options
  * @returns {Object}
  * @private
  */
-function getOptions(reports, personalDetails, activeReportID, {
+function getOptions(reports, personalDetails, {
     reportActions = {},
     betas = [],
     selectedOptions = [],
@@ -457,7 +451,15 @@ function getOptions(reports, personalDetails, activeReportID, {
     const reportMapForLogins = {};
 
     // Filter out all the reports that shouldn't be displayed
-    const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(report, currentlyViewedReportID, isInGSDMode, currentUserLogin, iouReports, betas, policies));
+    const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(
+        report,
+        Navigation.getReportIDFromRoute(),
+        isInGSDMode,
+        currentUserLogin,
+        iouReports,
+        betas,
+        policies,
+    ));
 
     // Sorting the reports works like this:
     // - Order everything by the last message timestamp (descending)
@@ -639,7 +641,7 @@ function getSearchOptions(
     searchValue = '',
     betas,
 ) {
-    return getOptions(reports, personalDetails, 0, {
+    return getOptions(reports, personalDetails, {
         betas,
         searchValue: searchValue.trim(),
         includeRecentReports: true,
@@ -706,7 +708,7 @@ function getNewChatOptions(
     selectedOptions = [],
     excludeLogins = [],
 ) {
-    return getOptions(reports, personalDetails, 0, {
+    return getOptions(reports, personalDetails, {
         betas,
         searchValue: searchValue.trim(),
         selectedOptions,
@@ -733,7 +735,7 @@ function getMemberInviteOptions(
     searchValue = '',
     excludeLogins = [],
 ) {
-    return getOptions([], personalDetails, 0, {
+    return getOptions([], personalDetails, {
         betas,
         searchValue: searchValue.trim(),
         excludeDefaultRooms: true,

src/libs/ReportUtils.js

@@ -894,15 +894,15 @@ function hasOutstandingIOU(report, currentUserLogin, iouReports) {
  * filter out the majority of reports before filtering out very specific minority of reports.
  *
  * @param {Object} report
- * @param {String} currentlyViewedReportID
+ * @param {String} reportIDFromRoute
  * @param {Boolean} isInGSDMode
  * @param {String} currentUserLogin
  * @param {Object} iouReports
  * @param {String[]} betas
  * @param {Object} policies
  * @returns {boolean}
  */
-function shouldReportBeInOptionList(report, currentlyViewedReportID, isInGSDMode, currentUserLogin, iouReports, betas, policies) {
+function shouldReportBeInOptionList(report, reportIDFromRoute, isInGSDMode, currentUserLogin, iouReports, betas, policies) {
     // Exclude reports that have no data because there wouldn't be anything to show in the option item.
     // This can happen if data is currently loading from the server or a report is in various stages of being created.
     if (!report || !report.reportID || !report.participants || _.isEmpty(report.participants)) {
@@ -912,7 +912,7 @@ function shouldReportBeInOptionList(report, currentlyViewedReportID, isInGSDMode
     // Include the currently viewed report. If we excluded the currently viewed report, then there
     // would be no way to highlight it in the options list and it would be confusing to users because they lose
     // a sense of context.
-    if (report.reportID === currentlyViewedReportID) {
+    if (report.reportID === reportIDFromRoute) {
         return true;
     }
 

src/libs/SidebarUtils.js

@@ -12,7 +12,7 @@ import * as CollectionUtils from './CollectionUtils';
 // keys that are connected to SidebarLinks withOnyx(). If there was a key missing from SidebarLinks and it's data was updated
 // for that key, then there would be no re-render and the options wouldn't reflect the new data because SidebarUtils.getOrderedReportIDs() wouldn't be triggered.
 // There are a couple of keys here which are OK to have stale data. iouReports for example, doesn't need to exist in withOnyx() because
-// when IOUs change, it also triggers a change on the reports collection. Having redudant subscriptions causes more re-renders which should be avoided.
+// when IOUs change, it also triggers a change on the reports collection. Having redundant subscriptions causes more re-renders which should be avoided.
 // Session also can remain stale because the only way for the current user to change is to sign out and sign in, which would clear out all the Onyx
 // data anyway and cause SidebarLinks to rerender.
 
@@ -29,12 +29,6 @@ Onyx.connect({
     callback: val => personalDetails = val,
 });
 
-let currentlyViewedReportID;
-Onyx.connect({
-    key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
-    callback: val => currentlyViewedReportID = val,
-});
-
 let priorityMode;
 Onyx.connect({
     key: ONYXKEYS.NVP_PRIORITY_MODE,
@@ -88,9 +82,10 @@ Onyx.connect({
 });
 
 /**
+ * @param {String} reportIDFromRoute
  * @returns {String[]} An array of reportIDs sorted in the proper order
  */
-function getOrderedReportIDs() {
+function getOrderedReportIDs(reportIDFromRoute) {
     let recentReportOptions = [];
     const pinnedReportOptions = [];
     const iouDebtReportOptions = [];
@@ -100,7 +95,7 @@ function getOrderedReportIDs() {
     const isInDefaultMode = !isInGSDMode;
 
     // Filter out all the reports that shouldn't be displayed
-    const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(report, currentlyViewedReportID, isInGSDMode, currentUserLogin, iouReports, betas, policies));
+    const filteredReports = _.filter(reports, report => ReportUtils.shouldReportBeInOptionList(report, reportIDFromRoute, isInGSDMode, currentUserLogin, iouReports, betas, policies));
 
     // Get all the display names for our reports in an easy to access property so we don't have to keep
     // re-running the logic
@@ -142,7 +137,7 @@ function getOrderedReportIDs() {
 
         // If the active report has a draft, we do not put it in the group of draft reports because we want it to maintain it's current position. Otherwise the report's position
         // jumps around in the LHN and it's kind of confusing to the user to see the LHN reorder when they start typing a comment on a report.
-        } else if (report.hasDraft && report.reportID !== currentlyViewedReportID) {
+        } else if (report.hasDraft && report.reportID !== reportIDFromRoute) {
             draftReportOptions.push(report);
         } else {
             recentReportOptions.push(report);

src/libs/actions/Report.js

@@ -41,12 +41,6 @@ Onyx.connect({
     },
 });
 
-let lastViewedReportID;
-Onyx.connect({
-    key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
-    callback: val => lastViewedReportID = val ? Number(val) : null,
-});
-
 const allReports = {};
 let conciergeChatReportID;
 const typingWatchTimers = {};
@@ -946,13 +940,6 @@ function handleReportChanged(report) {
     }
 }
 
-/**
- * @param {String} reportID
- */
-function updateCurrentlyViewedReportID(reportID) {
-    Onyx.merge(ONYXKEYS.CURRENTLY_VIEWED_REPORTID, String(reportID));
-}
-
 Onyx.connect({
     key: ONYXKEYS.COLLECTION.REPORT,
     callback: handleReportChanged,
@@ -1261,7 +1248,7 @@ function addPolicyReport(policy, reportName, visibility) {
     );
 
     // Onyx.set is used on the optimistic data so that it is present before navigating to the workspace room. With Onyx.merge the workspace room reportID is not present when
-    // storeCurrentlyViewedReport is called on the ReportScreen, so fetchChatReportsByIDs is called which is unnecessary since the optimistic data will be stored in Onyx.
+    // fetchReportIfNeeded is called on the ReportScreen, so fetchChatReportsByIDs is called which is unnecessary since the optimistic data will be stored in Onyx.
     // If there was an error creating the room, then fetchChatReportsByIDs throws an error and the user is navigated away from the report instead of showing the RBR error message.
     // Therefore, Onyx.set is used instead of Onyx.merge.
     const optimisticData = [
@@ -1433,7 +1420,7 @@ function viewNewReportAction(reportID, action) {
     }
 
     // If we are currently viewing this report do not show a notification.
-    if (reportID === lastViewedReportID && Visibility.isVisible()) {
+    if (reportID === Navigation.getReportIDFromRoute() && Visibility.isVisible()) {
         Log.info('[LOCAL_NOTIFICATION] No notification because it was a comment for the current report');
         return;
     }
@@ -1469,7 +1456,7 @@ Onyx.connect({
     initWithStoredValues: false,
     callback: (actions, key) => {
         // reportID can be derived from the Onyx key
-        const reportID = parseInt(key.split('_')[1], 10);
+        const reportID = key.split('_')[1];
         if (!reportID) {
             return;
         }
@@ -1522,7 +1509,6 @@ export {
     saveReportComment,
     broadcastUserIsTyping,
     togglePinnedState,
-    updateCurrentlyViewedReportID,
     editReportComment,
     saveReportActionDraft,
     deleteReportComment,

src/libs/actions/User.js

@@ -30,12 +30,6 @@ Onyx.connect({
     },
 });
 
-let currentlyViewedReportID = '';
-Onyx.connect({
-    key: ONYXKEYS.CURRENTLY_VIEWED_REPORTID,
-    callback: val => currentlyViewedReportID = val || '',
-});
-
 /**
  * Changes a password for a given account
  *
@@ -215,7 +209,6 @@ function setSecondaryLoginAndNavigate(login, password) {
  */
 function validateLogin(accountID, validateCode) {
     const isLoggedIn = !_.isEmpty(sessionAuthToken);
-    const redirectRoute = isLoggedIn ? ROUTES.getReportRoute(currentlyViewedReportID) : ROUTES.HOME;
     Onyx.merge(ONYXKEYS.ACCOUNT, {...CONST.DEFAULT_ACCOUNT_DATA, isLoading: true});
 
     DeprecatedAPI.ValidateEmail({
@@ -237,7 +230,7 @@ function validateLogin(accountID, validateCode) {
         }
     }).finally(() => {
         Onyx.merge(ONYXKEYS.ACCOUNT, {isLoading: false});
-        Navigation.navigate(redirectRoute);
+        Navigation.navigate(ROUTES.HOME);
     });
 }
 

src/pages/home/ReportScreen.js

@@ -115,15 +115,18 @@ class ReportScreen extends React.Component {
     }
 
     componentDidMount() {
-        this.storeCurrentlyViewedReport();
+        this.fetchReportIfNeeded();
+        toggleReportActionComposeView(true);
         this.removeViewportResizeListener = addViewportResizeListener(this.updateViewportOffsetTop);
     }
 
     componentDidUpdate(prevProps) {
         if (this.props.route.params.reportID === prevProps.route.params.reportID) {
             return;
         }
-        this.storeCurrentlyViewedReport();
+
+        this.fetchReportIfNeeded();
+        toggleReportActionComposeView(true);
     }
 
     componentWillUnmount() {
@@ -149,16 +152,9 @@ class ReportScreen extends React.Component {
         return !getReportID(this.props.route) || isLoadingInitialReportActions || !this.props.report.reportID;
     }
 
-    /**
-     * Persists the currently viewed report id
-     */
-    storeCurrentlyViewedReport() {
+    fetchReportIfNeeded() {
         const reportIDFromPath = getReportID(this.props.route);
 
-        // Always reset the state of the composer view when the current reportID changes
-        toggleReportActionComposeView(true);
-        Report.updateCurrentlyViewedReportID(reportIDFromPath);
-
         // It possible that we may not have the report object yet in Onyx yet e.g. we navigated to a URL for an accessible report that
         // is not stored locally yet. If props.report.reportID exists, then the report has been stored locally and nothing more needs to be done.
         // If it doesn't exist, then we fetch the report from the API.

src/pages/home/report/ReportActionItemCreated.js

@@ -15,7 +15,7 @@ import reportPropTypes from '../../reportPropTypes';
 
 const propTypes = {
     /** The id of the report */
-    reportID: PropTypes.number.isRequired,
+    reportID: PropTypes.string.isRequired,
 
     /** The report currently being looked at */
     report: reportPropTypes,