Skip to content

Commit

Permalink
Merge pull request #41 from software-mansion-labs/ideal-nav-fix-path
Browse files Browse the repository at this point in the history
fix path with policyId inside FullScreenSettings
  • Loading branch information
adamgrzybowski authored Jan 25, 2024
2 parents 3c629da + f3c9605 commit ed57401
Show file tree
Hide file tree
Showing 10 changed files with 58 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/libs/Navigation/dismissModal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function dismissModal(targetReportID: string, navigationRef: NavigationContainer
const policyMemberAccountIDs = getPolicyMemberAccountIDs(policyID);
const targetReport = getReport(targetReportID);
// If targetReport is an empty object, it means that it's a new report, so it can't belong to any workspace
const shouldOpenAllWorkspace = isEmptyObject(targetReport) ? true : !doesReportBelongToWorkspace(targetReport, policyID, policyMemberAccountIDs);
const shouldOpenAllWorkspace = isEmptyObject(targetReport) ? true : !doesReportBelongToWorkspace(targetReport, policyMemberAccountIDs, policyID);
if (shouldOpenAllWorkspace) {
switchPolicyID(navigationRef, {route: ROUTES.HOME});
} else {
Expand Down
8 changes: 6 additions & 2 deletions src/libs/Navigation/getPolicyIdFromState.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import getTopmostBottomTabRoute from './getTopmostBottomTabRoute';
import type {RootStackParamList, State} from './types';

const getPolicyIdFromState = (state: State<RootStackParamList>) => {
const getPolicyIdFromState = (state: State<RootStackParamList>): string | undefined => {
const topmostBottomTabRoute = getTopmostBottomTabRoute(state);

const shouldAddPolicyIdToUrl = !!topmostBottomTabRoute && !!topmostBottomTabRoute.params && 'policyID' in topmostBottomTabRoute.params && !!topmostBottomTabRoute.params?.policyID;

return shouldAddPolicyIdToUrl ? (topmostBottomTabRoute.params?.policyID as string) : '';
if (!shouldAddPolicyIdToUrl) {
return undefined;
}

return topmostBottomTabRoute.params?.policyID as string;
};

export default getPolicyIdFromState;
18 changes: 16 additions & 2 deletions src/libs/Navigation/linkTo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import getTopmostReportId from './getTopmostReportId';
import linkingConfig from './linkingConfig';
import getMatchingBottomTabRouteForState from './linkingConfig/getMatchingBottomTabRouteForState';
import getMatchingCentralPaneRouteForState from './linkingConfig/getMatchingCentralPaneRouteForState';
import replacePathInNestedState from './linkingConfig/replacePathInNestedState';
import type {NavigationRoot, RootStackParamList, StackNavigationAction, State} from './types';

type ActionPayloadParams = {
Expand Down Expand Up @@ -128,6 +129,21 @@ export default function linkTo(navigation: NavigationContainerRef<RootStackParam
const pathWithoutPolicyID = getPathWithoutPolicyID(`/${path}`) as Route;
const rootState = navigation.getRootState() as NavigationState<RootStackParamList>;
const stateFromPath = getStateFromPath(pathWithoutPolicyID) as PartialState<NavigationState<RootStackParamList>>;

// Creating path with /w/ included if necessary.
const extractedPolicyID = extractPolicyIDFromPath(`/${path}`);
const policyIdFromState = getPolicyIdFromState(rootState);
const policyID = extractedPolicyID ?? policyIdFromState;

const isWorkspaceSettingsOpened = getTopmostBottomTabRoute(rootState as State<RootStackParamList>)?.name === SCREENS.WORKSPACE.INITIAL && path.includes('workspace');

if (policyID && !isWorkspaceSettingsOpened) {
// The stateFromPath doesn't include proper path if there is a policy passed with /w/id.
// We need to replace the path in the state with the proper one.
// To avoid this hacky solution we may want to create custom getActionFromState function in the future.
replacePathInNestedState(stateFromPath, `/w/${policyID}${pathWithoutPolicyID}`);
}

const action: StackNavigationAction = getActionFromState(stateFromPath, linkingConfig.config);

// If action type is different than NAVIGATE we can't change it to the PUSH safely
Expand Down Expand Up @@ -171,8 +187,6 @@ export default function linkTo(navigation: NavigationContainerRef<RootStackParam
action.type = CONST.NAVIGATION.ACTION_TYPE.PUSH;
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
} else if (action.payload.name === NAVIGATORS.BOTTOM_TAB_NAVIGATOR) {
const extractedPolicyID = extractPolicyIDFromPath(`/${path}`);
const policyID = extractedPolicyID === 'global' ? undefined : extractedPolicyID ?? getPolicyIdFromState(rootState);
// If path contains a policyID, we should invoke the navigate function
const shouldNavigate = !!extractedPolicyID;
const actionForBottomTabNavigator = getActionForBottomTabNavigator(action, rootState, policyID, shouldNavigate);
Expand Down
14 changes: 8 additions & 6 deletions src/libs/Navigation/linkingConfig/getAdaptedStateFromPath.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import config from './config';
import FULL_SCREEN_TO_RHP_MAPPING from './FULL_SCREEN_TO_RHP_MAPPING';
import getMatchingBottomTabRouteForState from './getMatchingBottomTabRouteForState';
import getMatchingCentralPaneRouteForState from './getMatchingCentralPaneRouteForState';
import replacePathInNestedState from './replacePathInNestedState';

// The function getPathFromState that we are using in some places isn't working correctly without defined index.
const getRoutesWithIndex = (routes: NavigationPartialRoute[]) => ({routes, index: routes.length - 1});
Expand Down Expand Up @@ -144,7 +145,7 @@ function getAdaptedState(state: PartialState<NavigationState<RootStackParamList>
}

routes.push(rhpNavigator);
return {routes};
return getRoutesWithIndex(routes);
}
if (lhpNavigator) {
// Routes
Expand Down Expand Up @@ -190,7 +191,7 @@ function getAdaptedState(state: PartialState<NavigationState<RootStackParamList>
}
routes.push(fullScreenNavigator);

return {routes};
return getRoutesWithIndex(routes);
}
if (centralPaneNavigator) {
// Routes
Expand All @@ -202,7 +203,7 @@ function getAdaptedState(state: PartialState<NavigationState<RootStackParamList>
routes.push(centralPaneNavigator);

// TODO: TEMPORARY FIX - REPLACE WITH getRoutesWithIndex(routes)
return {routes};
return getRoutesWithIndex(routes);
}
if (bottomTabNavigator) {
// Routes
Expand All @@ -219,7 +220,7 @@ function getAdaptedState(state: PartialState<NavigationState<RootStackParamList>
}

// TODO: TEMPORARY FIX - REPLACE WITH getRoutesWithIndex(routes)
return {routes};
return getRoutesWithIndex(routes);
}

return state;
Expand All @@ -231,12 +232,13 @@ const getAdaptedStateFromPath: typeof getStateFromPath = (path, options) => {
// Anonymous users don't have access to workspaces
const policyID = isAnonymous ? undefined : extractPolicyIDFromPath(path);

const state = getStateFromPath(url, options);
const state = getStateFromPath(url, options) as PartialState<NavigationState<RootStackParamList>>;
replacePathInNestedState(state, path);

if (state === undefined) {
throw new Error('Unable to parse path');
}
const adaptedState = getAdaptedState(state as PartialState<NavigationState<RootStackParamList>>, policyID);
const adaptedState = getAdaptedState(state, policyID);
return adaptedState;
};

Expand Down
15 changes: 15 additions & 0 deletions src/libs/Navigation/linkingConfig/replacePathInNestedState.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/* eslint-disable @typescript-eslint/naming-convention */
import {findFocusedRoute} from '@react-navigation/native';
import type {NavigationState, PartialState} from '@react-navigation/native';
import type {RootStackParamList} from '@libs/Navigation/types';

function replacePathInNestedState(state: PartialState<NavigationState<RootStackParamList>>, path: string) {
const found = findFocusedRoute(state);
if (!found) {
return;
}

// @ts-expect-error Updating read only property
found.path = path;
}
export default replacePathInNestedState;
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export default function subscribeToReportCommentPushNotifications() {
const report = getReport(reportID.toString());
const policyMembersAccountIDs = policyID ? getPolicyMemberAccountIDs(policyID) : [];

const reportBelongsToWorkspace = policyID && !isEmptyObject(report) && doesReportBelongToWorkspace(report, policyID, policyMembersAccountIDs);
const reportBelongsToWorkspace = policyID && !isEmptyObject(report) && doesReportBelongToWorkspace(report, policyMembersAccountIDs, policyID);

Log.info('[PushNotification] onSelected() - called', false, {reportID, reportActionID});
Navigation.isNavigationReady()
Expand Down
6 changes: 5 additions & 1 deletion src/libs/PolicyMembersUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ Onyx.connect({
callback: (value) => (policyMembers = value),
});

function getPolicyMemberAccountIDs(policyID: string) {
function getPolicyMemberAccountIDs(policyID?: string) {
if (!policyID) {
return [];
}

const currentUserAccountID = getCurrentUserAccountID();
return policyMembers
? Object.keys(policyMembers[`${ONYXKEYS.COLLECTION.POLICY_MEMBERS}${policyID}`] ?? {})
Expand Down
8 changes: 4 additions & 4 deletions src/libs/ReportUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -896,7 +896,7 @@ function isConciergeChatReport(report: OnyxEntry<Report>): boolean {
* Checks if the supplied report belongs to workspace based on the provided params. If the report's policyID is _FAKE_ or has no value, it means this report is a DM.
* In this case report and workspace members must be compared to determine whether the report belongs to the workspace.
*/
function doesReportBelongToWorkspace(report: Report, policyID: string, policyMemberAccountIDs: number[]) {
function doesReportBelongToWorkspace(report: Report, policyMemberAccountIDs: number[], policyID?: string) {
return (
isConciergeChatReport(report) || (report.policyID === CONST.POLICY.ID_FAKE || !report.policyID ? hasParticipantInArray(report, policyMemberAccountIDs) : report.policyID === policyID)
);
Expand All @@ -905,8 +905,8 @@ function doesReportBelongToWorkspace(report: Report, policyID: string, policyMem
/**
* Given an array of reports, return them filtered by a policyID and policyMemberAccountIDs.
*/
function filterReportsByPolicyIdAndMemberAccountIDs(reports: Report[], policyID = '', policyMemberAccountIDs: number[] = []) {
return reports.filter((report) => !!report && doesReportBelongToWorkspace(report, policyID, policyMemberAccountIDs));
function filterReportsByPolicyIdAndMemberAccountIDs(reports: Report[], policyMemberAccountIDs: number[] = [], policyID?: string) {
return reports.filter((report) => !!report && doesReportBelongToWorkspace(report, policyMemberAccountIDs, policyID));
}

/**
Expand Down Expand Up @@ -1005,7 +1005,7 @@ function findLastAccessedReport(
let reportsValues = Object.values(reports ?? {}) as Report[];

if (!!policyID || policyMemberAccountIDs.length > 0) {
reportsValues = filterReportsByPolicyIdAndMemberAccountIDs(reportsValues, policyID, policyMemberAccountIDs);
reportsValues = filterReportsByPolicyIdAndMemberAccountIDs(reportsValues, policyMemberAccountIDs, policyID);
}

let sortedReports = sortReportsByLastRead(reportsValues, reportMetadata);
Expand Down
2 changes: 1 addition & 1 deletion src/libs/SidebarUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ function getOrderedReportIDs(
const archivedReports: Report[] = [];

if (currentPolicyID || policyMemberAccountIDs.length > 0) {
reportsToDisplay = reportsToDisplay.filter((report) => ReportUtils.doesReportBelongToWorkspace(report, currentPolicyID, policyMemberAccountIDs));
reportsToDisplay = reportsToDisplay.filter((report) => ReportUtils.doesReportBelongToWorkspace(report, policyMemberAccountIDs, currentPolicyID));
}
// There are a few properties that need to be calculated for the report which are used when sorting reports.
reportsToDisplay.forEach((report) => {
Expand Down
4 changes: 1 addition & 3 deletions src/libs/actions/Report.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1895,9 +1895,7 @@ function showReportActionNotification(reportID: string, reportAction: ReportActi
Modal.close(() => {
const policyID = lastVisitedPath && extractPolicyIDFromPath(lastVisitedPath);
const policyMembersAccountIDs = policyID ? getPolicyMemberAccountIDs(policyID) : [];

const reportBelongsToWorkspace = policyID ? doesReportBelongToWorkspace(report, policyID, policyMembersAccountIDs) : true;

const reportBelongsToWorkspace = policyID ? doesReportBelongToWorkspace(report, policyMembersAccountIDs, policyID) : true;
if (!reportBelongsToWorkspace) {
Navigation.navigateWithSwitchPolicyID({policyID: undefined, route: ROUTES.HOME});
}
Expand Down

0 comments on commit ed57401

Please sign in to comment.