App
App copied to clipboard
[HOLD for payment 2024-04-25] Profile - Personal details not open on off line mode
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Action Performed:
- https://staging.new.expensify.com/
- Log in with a HIGH TRAFFIC account.
- Click on the profile icon
- After login in with a High Traffic account - enable Airplane mode to simulate a connection loss
- Click on Personal Details
Expected Result:
Personal details is open and options are visible
Actual Result:
Personal details not open
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Platforms:
Which of our officially supported platforms is this issue occurring on?
- [ ] Android / native
- [ ] Android / Chrome
- [ ] iOS / native
- [ ] iOS / Safari
- [x] MacOS / Chrome / Safari
- [ ] MacOS / Desktop
Version Number: 1.3.47-2 Reproducible in staging?: y Reproducible in production?: y If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation
https://github.com/Expensify/App/assets/43996225/ccab7673-7f42-4dfa-b392-3c1ef488c5f4
Expensify/Expensify Issue URL: Issue reported by: applause internal team Slack conversation:
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~015f3b23a3e3d5db40
- Upwork Job ID: 1686070604511633408
- Last Price Increase: 2023-07-31
Issue Owner
Current Issue Owner: @grgia
Triggered auto assignment to @trjExpensify (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
Bug0 Triage Checklist (Main S/O)
- [ ] This "bug" occurs on a supported platform (ensure
Platforms
in OP are ✅) - [ ] This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
- [ ] This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
- [ ] This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
- [ ] I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync
Proposal
Please re-state the problem that we are trying to solve in this issue.
Profile - Personal details not open on offline mode
What is the root cause of that problem?
After we login successfully, openApp
API is called but this doesn't return privatePersonalDetails
of current user. When we offline, because the openPersonalDetailsPage
API hasn't been called yet, props.privatePersonalDetails
is undefined
, and then the page loading infinitely.
https://github.com/Expensify/App/blob/eb020063789091eb2e3086b5b54af3ead8436645/src/pages/settings/Profile/PersonalDetails/PersonalDetailsInitialPage.js#L86-L89
What changes do you think we should make in order to solve the problem?
We have some suggestions to fix this issue
-
BE should return
privatePersonalDetails
of current user inopenApp
API -
In
componentDidMount
of AuthScreen, we will callPersonalDetails.openPersonalDetailsPage();
to callopenPersonalDetailsPage
API -
If we don't want to do this we should return default
false
here to loading page not should
lodashGet(props.privatePersonalDetails, 'isLoading', false)
https://github.com/Expensify/App/blob/eb020063789091eb2e3086b5b54af3ead8436645/src/pages/settings/Profile/PersonalDetails/PersonalDetailsInitialPage.js#L86-L89
What alternative solutions did you explore? (Optional)
Proposal
Please re-state the problem that we are trying to solve in this issue.
Going offline before fetching the private personal details and clicking on personal details shows a continuous loading indicator
What is the root cause of that problem?
We fetch the private personal details with this hook here
https://github.com/Expensify/App/blob/eb020063789091eb2e3086b5b54af3ead8436645/src/pages/settings/Profile/PersonalDetails/PersonalDetailsInitialPage.js#L59
and
the hook does not call api when it is offline
https://github.com/Expensify/App/blob/eb020063789091eb2e3086b5b54af3ead8436645/src/hooks/usePrivatePersonalDetails.js#L11-L15
but this here still shows the loading indicator
https://github.com/Expensify/App/blob/eb020063789091eb2e3086b5b54af3ead8436645/src/pages/settings/Profile/PersonalDetails/PersonalDetailsInitialPage.js#L86-L88
because the default value is true
in the conditional. So loading indicator is shown in cases where isLoading
does not exist in privatePersonalDetails
as well.
What changes do you think we should make in order to solve the problem?
I think showing loading indicator is fine in this case but we might show an indication that the user is offline especially for smaller screens so that they know the network got disconnected. For larger screens this might not be necessary because the report screen footer shows the offline indicator already. But if we like to cover the rare cases where the report screen shows Hmm it's not there screen, we can show offline indicator in the RHN for large screens also.
This would require changing the return
statement to
return (
<>
<FullscreenLoadingIndicator />
{props.network.isOffline && <OfflineIndicator containerStyles={[styles.chatItemComposeSecondaryRow, styles.ml5, styles.mtAuto]} />}
</>)
We change this in other pages like AddressPage
, DateOfBirth
, LegalName pages also.
What alternative solutions did you explore? (Optional)
Oh hm, interesting! @zanyrenney @twisterdotcom @Beamanator - what was the implementation of this from the account settings doc?
If the personal details
pages require being online, we should be using the full page blocking form (offline pattern D) in that case?
This issue only happens when we don't go to this page before we go offline. If we go to this page at least once in online mode, this will not show loading when we offline.
That's helpful to know. I'm keen to hear from the team on the expected behaviour of accessing the personal details pages when offline.
No these should be available offline. This must be a regression.
Cool, moving on! :)
Job added to Upwork: https://www.upwork.com/jobs/~015f3b23a3e3d5db40
Current assignee @trjExpensify is eligible for the External assigner, not assigning anyone new.
Triggered auto assignment to Contributor-plus team member for initial proposal review - @thesahindia (External
)
Proposal
Please re-state the problem that we are trying to solve in this issue.
if you click at Personal details for the first time, it keeps loading
What is the root cause of that problem?
When you sign in, your personal details are neither retrieved from the backend nor stored in the local storage. Therefore, if you click on 'personal details' for the first time, the app will need to obtain your details and set them in the local storage.
What changes do you think we should make in order to solve the problem?
we should load the private personal details from the back end and set it inside the local storage after you sign in
Check out lines 98-102 of https://github.dev/michaelraoof/Expensify/blob/fix/loadingPersolnalDetailsOfflineMode/src/libs/actions/Welcome.js#L98-L102.
let privatePersonalDetails;
Onyx.connect({
key: ONYXKEYS.PRIVATE_PERSONAL_DETAILS,
callback: (val) => (privatePersonalDetails = val),
});
What alternative solutions did you explore? (Optional)
📣 @michaelraoof! 📣 Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork. Please follow these steps:
- Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
- Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
- Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>
After we login successfully,
openApp
API is called but this doesn't return privatePersonalDetails of current user.
- BE should return privatePersonalDetails of current user in
openApp
API
Just looked at https://github.com/Expensify/App/issues/23873#issuecomment-1657037105, it seem like we need BE changes
cc: @trjExpensify
Ah, so let's take this internal then!
Current assignee @thesahindia is eligible for the Internal assigner, not assigning anyone new.
Triggered auto assignment to @grgia (Engineering
), see https://stackoverflow.com/c/expensify/questions/4319 for more details.
I'm going OOO next week and will not be able to look at this until after I am back. Switching to weekly
Can we pop this back on daily
now?
Going to put this back on daily
, we have a week to go before it breaks WAQ.
@trjExpensify @grgia @thesahindia this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!
Am I correct in understanding:
Problem: When a user logs in and goes offline BEFORE opening the personal details page, their private personal details are not retrieved and the private personal details page loads infinitely?
Solution (internally)
What I'm seeing has been proposed:
return privatePersonalDetails
of current user in openApp
API call.
cc @Beamanator @puneetlath could I get a check on this and your insight on what our methodology is for storing private personal details?
I don't think this has anything to do with the securing logins project that we did. That was all about how we deal with other people's personal details. Which in this case it seems like we just aren't returning the users own private personal details in openApp, which we definitely can start doing. Do we return all other profile/settings stuff in openApp?
Yeah, I think this is more about the offline approach in the account settings doc implementation when these new API commands were introduced, right @Beamanator?
Oof sorry for not responding earlier!!
Yeah in the Account Settings doc we decided it wasn't necessary to pull the user's private personal details in OpenApp - though I can't recall the exact reason why 😬
@zanyrenney @twisterdotcom @cristipaval (team awesome) - do y'all think we should start pulling private personal details in OpenApp as well just so we at least have that data initially, even if it's slightly out of date (assuming the case you signed in at some point and have been offline for some time, when you finally navigate back to your "personal details" page)
Yeah in the Account Settings doc we decided it wasn't necessary to pull the user's private personal details in OpenApp - though I can't recall the exact reason why 😬
Ah, it would be great if a decision like this was documented somewhere in like alt solutions to refer back to. I couldn't find it when taking a quick search through the doc just now.
@zanyrenney @twisterdotcom @cristipaval (team awesome) - do y'all think we should start pulling private personal details in OpenApp as well just so we at least have that data initially, even if it's slightly out of date (assuming the case you signed in at some point and have been offline for some time, when you finally navigate back to your "personal details" page)
@zanyrenney @cristipaval, do you guys agree or recall a reason we didn't do this initially? If there isn't one, let's do it. Of course, let's make sure it doesn't slow down the openApp call considerably or something like that as we proceed with a PR.
@grgia, I think we have enough to prepare a PR to fix this. :)
@trjExpensify hmm I don't remember there being any reason we elected to not load personal details offline. I've searched around in Slack too and not generating heaps there. I think we can change this and do it, I nothing is coming to mind why not to do it.
I don't remember either about a decision made about this. I joined the Account Settings team when the detailed section of the doc was already approved and helped mainly with some implementation of the push to page design in the Account Settings pages.