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

Fix bank account page doesnt load when coming from desktop #56131

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+4 −2
Open

Fix bank account page doesnt load when coming from desktop #56131

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d11f7df
encode exitTo url
bernhardoj Jan 31, 2025
b4c292b
only set the loading immediately if policyID param exists
bernhardoj Jan 31, 2025
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
2 changes: 1 addition & 1 deletion src/libs/actions/Link.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import ONYXKEYS from '@src/ONYXKEYS';
import type {Route} from '@src/ROUTES';
import ROUTES from '@src/ROUTES';
import * as Session from './Session';

Check failure on line 15 in src/libs/actions/Link.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check 

Namespace imports are not allowed. Use named imports instead. Example: import { method } from "./libs/module"

let isNetworkOffline = false;
Onyx.connect({
Expand All @@ -26,7 +26,7 @@
key: ONYXKEYS.SESSION,
callback: (value) => {
currentUserEmail = value?.email ?? '';
currentUserAccountID = value?.accountID ?? -1;

Check failure on line 29 in src/libs/actions/Link.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

},
});

Expand Down Expand Up @@ -168,7 +168,7 @@
// the reportID is extracted from the URL and then opened as an internal link, taking the user straight to the chat in the same tab.
if (hasExpensifyOrigin && href.indexOf('newdotreport?reportID=') > -1) {
const reportID = href.split('newdotreport?reportID=').pop();
const reportRoute = ROUTES.REPORT_WITH_ID.getRoute(reportID ?? '-1');

Check failure on line 171 in src/libs/actions/Link.ts

View workflow job for this annotation

GitHub Actions / Changed files ESLint check

Navigation.navigate(reportRoute);
return;
}
Expand Down Expand Up @@ -197,7 +197,7 @@
function buildURLWithAuthToken(url: string, shortLivedAuthToken?: string) {
const authTokenParam = shortLivedAuthToken ? `shortLivedAuthToken=${shortLivedAuthToken}` : '';
const emailParam = `email=${encodeURIComponent(currentUserEmail)}`;
const exitTo = `exitTo=${url}`;
const exitTo = `exitTo=${encodeURIComponent(url)}`;
const accountID = `accountID=${currentUserAccountID}`;
const referrer = 'referrer=desktop';
const paramsArray = [accountID, emailParam, authTokenParam, exitTo, referrer];
Expand Down
4 changes: 3 additions & 1 deletion src/pages/ReimbursementAccount/ReimbursementAccountPage.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,9 @@ function ReimbursementAccountPage({route, policy, isLoadingPolicy}: Reimbursemen
return;
}

setReimbursementAccountLoading(true);
if (policyIDParam) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just return early here if there is no policyIDParam?
We don't need to call clearReimbursementAccountDraft and fetchData won't be called if there is no policyID. Or do we still need isStepToOpenEmpty check in this case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't follow why we don't need to call clearReimbursementAccountDraft. isStepToOpenEmpty is not related to the policyID param, so I would leave it as it is. We can update the code to this if needed:

...
const isStepToOpenEmpty = getStepToOpenFromRouteParams(route) === '';
if (isStepToOpenEmpty) {
  setBankAccountSubStep(null);
  setPlaidEvent(null);
}

if (!policyIDParam) {
  return;
}
setReimbursementAccountLoading(true);
fetchData();

setReimbursementAccountLoading(true);
}
clearReimbursementAccountDraft();

// If the step to open is empty, we want to clear the sub step, so the connect option view is shown to the user
Expand Down
Loading