[HOLD for payment 2023-07-03] [$1000] Profile page loads infinitely with accountID is undefined #20854

kavimuru · 2023-06-15T20:18:43Z

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Action Performed:

Go to https://staging.new.expensify.com/
Login with any account
Open with URL https://staging.new.expensify.com/a/undefined

Expected Result:

Profile page should show ''Hmm... it's not here"

Actual Result:

Profile page loads infinitely

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?

Version Number: 1.3.28-3
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
Screencast from 09-06-2023 13_35_24.webm

Recording.980.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dukenv0307
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1686292516315989

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01b5268811dd99e507
Upwork Job ID: 1670909262930079744
Last Price Increase: 2023-06-19

melvin-bot · 2023-06-15T20:18:48Z

Triggered auto assignment to @anmurali (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-06-15T20:18:50Z

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

kavimuru · 2023-06-15T20:18:58Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

With accountID being undefined string, openPublicProfilePage isn't called in useEffect.

App/src/pages/ProfilePage.js

Lines 96 to 100 in 82ce35f

    
           useEffect(() => { 
        
               if (accountID > 0) { 
        
                   PersonalDetails.openPublicProfilePage(accountID); 
        
               } 
        
           }, [accountID]);

So details always an empty object, hasMinimumDetails is false, isLoading is true

App/src/pages/ProfilePage.js

Lines 124 to 125 in 82ce35f

    
           const hasMinimumDetails = !_.isEmpty(details.avatar); 
        
           const isLoading = lodashGet(details, 'isLoading', false) || _.isEmpty(details);

And loading component always displays

App/src/pages/ProfilePage.js

Line 212 in 82ce35f

    
           {!hasMinimumDetails && isLoading && <FullScreenLoadingIndicator style={styles.flex1} />}

What changes do you think we should make in order to solve the problem?

We should check accountID is valid or not in here to change isLoading to false if accountID is invalid
(lodashGet(details, 'isLoading', false) || _.isEmpty(details)) && (accountID > 0)

App/src/pages/ProfilePage.js

Line 125 in 82ce35f

const isLoading = lodashGet(details, 'isLoading', false) || _.isEmpty(details);

What alternative solutions did you explore? (Optional)

Pujan92 · 2023-06-15T22:07:40Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Profile page loads infinitely with accountID is undefined

What is the root cause of that problem?

Seems the variable isLoading is not derived and assigned correctly to the Views. isLoading we are updating optimistically with the OpenPublicProfilePage api.

What changes do you think we should make in order to solve the problem?

We should depend only on optimistic isLoading value
const isLoading = lodashGet(details, 'isLoading', false);

App/src/pages/ProfilePage.js

Line 125 in 9fda6a5

const isLoading = lodashGet(details, 'isLoading', false) || _.isEmpty(details);
For showing the profile details we also need to check isLoading
{hasMinimumDetails && !isLoading && (

App/src/pages/ProfilePage.js

Line 140 in 9fda6a5

{hasMinimumDetails && (
To show loading indicator we can only use isLoading value
{isLoading && <FullScreenLoadingIndicator style={styles.flex1} />}

App/src/pages/ProfilePage.js

Line 212 in 9fda6a5

{!hasMinimumDetails && isLoading && <FullScreenLoadingIndicator style={styles.flex1} />}

Result

Screen.Recording.2023-06-16.at.3.33.40.AM.mp4

hungvu193 · 2023-06-16T11:31:07Z

Proposal

Please re-state the problem that we are trying to solve in this issue.

Infinite loading when user open invalid profile page.

What is the root cause of that problem?

We are checking if the accountID > 0 here to load the user profile, but for some cases this validation is not enough (ie: for route: 0x100000000) which made PersonalDetails.openPublicProfilePage(accountID); never called and isLoading is always true which caused the issue

App/src/pages/ProfilePage.js

Lines 97 to 99 in f6008bb

    
           if (accountID > 0) { 
        
               PersonalDetails.openPublicProfilePage(accountID); 
        
           }

What changes do you think we should make in order to solve the problem?

Instead of checking accountID > 0 we should have a validation for accountID route. Let say that accountId should be a number. (After personalDetails refactor they're now all number 😄 ). But we can improve this validation based on the requirement.

Add a account route validation

// we can change this validation base on requirement of accountId route
function isValidAccountRoute(accountId) {
    return CONST.REGEX.NUMBER.test(accountId) && accountId > 0;
}

Update useEffect condition

    useEffect(() => {
        if (isValidAccountRoute(routeName))) {
            PersonalDetails.openPublicProfilePage(accountID);
        }
    }, [accountID]);

Update condition to prevent infinite loading

    // if the accountId is invalid then we should show the blocking view instead of infinite loading
    const isLoading = (lodashGet(details, 'isLoading', false) || _.isEmpty(details)) && ValidationUtil.isValidAccountRoute(accountID);
    const shouldShowBlockingView = (!hasMinimumDetails && !isLoading) || !ValidationUtil.isValidAccountRoute(accountID);

What alternative solutions did you explore? (Optional)

N/A

melvin-bot · 2023-06-19T20:11:00Z

@anmurali Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

anmurali · 2023-06-19T21:39:38Z

Reproduced, added an external label for contributors to pick up

melvin-bot · 2023-06-19T21:39:49Z

Job added to Upwork: https://www.upwork.com/jobs/~01b5268811dd99e507

melvin-bot · 2023-06-19T21:39:52Z

Current assignee @anmurali is eligible for the External assigner, not assigning anyone new.

melvin-bot · 2023-06-19T21:39:58Z

Triggered auto assignment to Contributor-plus team member for initial proposal review - @sobitneupane (External)

sobitneupane · 2023-06-21T11:33:38Z

Proposal from @dukenv0307 looks good to me

🎀 👀 🎀 C+ reviewed

melvin-bot · 2023-06-21T11:33:44Z

Triggered auto assignment to @arosiclair, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

Pujan92 · 2023-06-21T11:51:24Z

@sobitneupane isn't this makes more sense where for isLoading state we don't rely on the empty details object?

sobitneupane · 2023-06-21T12:21:31Z

@Pujan92 These few lines have some context.

App/src/pages/ProfilePage.js

Lines 124 to 128 in 9fda6a5

    
           const hasMinimumDetails = !_.isEmpty(details.avatar); 
        
           const isLoading = lodashGet(details, 'isLoading', false) || _.isEmpty(details); 
        
           // If the API returns an error for some reason there won't be any details and isLoading will get set to false, so we want to show a blocking screen 
        
           const shouldShowBlockingView = !hasMinimumDetails && !isLoading;

It was added in this PR. It should have been added to prevent showing blocking view if the details are yet to be loaded.

Pujan92 · 2023-06-21T12:34:47Z

It should have been added to prevent showing blocking view if the details are yet to be loaded.

I think that won't be the case as isLoading we are only turning to false on api success/failure. So the priority should be loader -> blocking view -> details content. My thinking is to as we already maintaining isLoading then only use that instead of combining other fields.

arosiclair · 2023-06-21T15:01:50Z

@Pujan92 The client may already have personal details loaded even while we are loading updates so we shouldn't block the UI on isLoading in this case. That's what we have hasMinimumDetails for.

arosiclair · 2023-06-21T15:36:22Z

I actually think @hungvu193 is closer to the mark here. accountID > 0 isn't good enough to validate any string. A couple improvements for your proposal:

We probably don't need to add a new ValidationUtils function. Instead we can just use ValidationUtils.isNumeric()
Instead of step 3 in your solution, I think we should set the default value of isLoading on details so the rest of the logic just works correctly. eg:

const details = lodashGet(props.personalDetails, accountID, {isLoading: ValidationUtils.isNumeric(accountID)});

What do you think?

hungvu193 · 2023-06-21T15:41:07Z

I actually think @hungvu193 is closer to the mark here. accountID > 0 isn't good enough to validate any string. A couple improvements for your proposal:

We probably don't need to add a new ValidationUtils function. Instead we can just use ValidationUtils.isNumeric()

Instead of step 3 in your solution, I think we should set the default value of isLoading on details so the rest of the logic just works correctly. eg:
const details = lodashGet(props.personalDetails, accountID, {isLoading: ValidationUtils.isNumeric(accountID)});
What do you think?

That's cool.
~~But I believe we still need to keep accountId > 0 beside isNumberic validation, because 0 is an invalid accountID~~
Nevermind, with invalid number accountID, we can let backend handle it.

dukenv0307 · 2023-06-21T15:58:40Z

I think update the check of accountID just is an improvement of this issue to prevent calling API if accountID is invalid.

melvin-bot · 2023-06-26T20:27:48Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2023-06-26T20:27:49Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.3.32-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix: infinite loading with invalid account route #21286

If no regressions arise, payment will be issued on 2023-07-03. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

As a reminder, here are the bonuses/penalties that should be applied for any External issue:

Merged PR within 3 business days of assignment - 50% bonus
Merged PR more than 9 business days after assignment - 50% penalty

melvin-bot · 2023-06-26T20:27:51Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:
[@sobitneupane] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@sobitneupane] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@sobitneupane] Determine if we should create a regression test for this bug.
[@sobitneupane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@anmurali] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

anmurali · 2023-07-06T02:54:00Z

Sent @hungvu193 an offer - https://www.upwork.com/nx/wm/pre-hire/c/8577561/offer/25386217
@sobitneupane can you please complete the checklist and create a manual request?

hungvu193 · 2023-07-06T02:54:50Z

Sent @hungvu193 an offer - https://www.upwork.com/nx/wm/pre-hire/c/8577561/offer/25386217

@sobitneupane can you please complete the checklist and create a manual request?

Thank you. Accepted

sobitneupane · 2023-07-10T08:12:56Z

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@sobitneupane] The PR that introduced the bug has been identified. Link to the PR:

#20144

[@sobitneupane] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:

#20144 (comment)

[@sobitneupane] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:

I don't think any change in PR review checklist is required. It was overlooked while adding a new component.

[@sobitneupane] Determine if we should create a regression test for this bug.

Yes.

[@sobitneupane] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

sobitneupane · 2023-07-10T08:15:18Z

Regression Test Proposal

Login with any account
Open any profile route that has invalid route number(eg: /a/undefined)
Verify that Not Found Page is displayed.

Do we agree 👍 or 👎

sobitneupane · 2023-07-10T09:05:44Z

Requested money from Expensify.

melvin-bot · 2023-07-13T20:10:46Z

@arosiclair, @anmurali, @hungvu193, @sobitneupane Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

anmurali · 2023-07-17T17:10:00Z

Everyone's paid!

dukenv0307 · 2023-08-03T07:50:17Z

@anmurali hi I haven't been hired as bug reporter for this yet. Would you mind taking a look?
Thank you!

dukenv0307 · 2023-10-03T05:54:43Z

@anmurali hi I haven't been hired as bug reporter for this yet. Would you mind taking a look?

@arosiclair hi would you be able to help here? I've Slacked @anmurali as well but it doesn't seem like she's available at the moment to handle this.

Appreciate it!

arosiclair · 2023-10-03T09:41:08Z

I reached out to @anmurali

anmurali · 2023-10-03T14:37:15Z

Sorry about that. I now paid him. And am closing this issue.

kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jun 15, 2023

melvin-bot bot assigned anmurali Jun 15, 2023

melvin-bot bot added the Overdue label Jun 19, 2023

melvin-bot bot removed the Overdue label Jun 19, 2023

anmurali added the External Added to denote the issue can be worked on by a contributor label Jun 19, 2023

melvin-bot bot changed the title ~~Profile page loads infinitely with accountID is undefined~~ [$1000] Profile page loads infinitely with accountID is undefined Jun 19, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 19, 2023

melvin-bot bot assigned sobitneupane Jun 19, 2023

Pujan92 mentioned this issue Jun 20, 2023

Infinite loading when user open invalid profile page. #21145

Closed

6 tasks

melvin-bot bot assigned arosiclair Jun 21, 2023

melvin-bot bot changed the title ~~[$1000] Profile page loads infinitely with accountID is undefined~~ [HOLD for payment 2023-07-03] [$1000] Profile page loads infinitely with accountID is undefined Jun 26, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Jun 26, 2023

melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Jul 3, 2023

melvin-bot bot removed the Overdue label Jul 6, 2023

melvin-bot bot added the Overdue label Jul 10, 2023

sobitneupane mentioned this issue Jul 10, 2023

Add new Profile page #20144

Merged

57 tasks

melvin-bot bot removed the Overdue label Jul 10, 2023

melvin-bot bot added the Overdue label Jul 10, 2023

melvin-bot bot added Overdue and removed Overdue labels Jul 10, 2023

anmurali closed this as completed Jul 17, 2023

melvin-bot bot removed the Overdue label Jul 17, 2023

arosiclair reopened this Oct 3, 2023

anmurali closed this as completed Oct 3, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2023-07-03] [$1000] Profile page loads infinitely with accountID is undefined #20854

[HOLD for payment 2023-07-03] [$1000] Profile page loads infinitely with accountID is undefined #20854

kavimuru commented Jun 15, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Jun 15, 2023

melvin-bot bot commented Jun 15, 2023

kavimuru commented Jun 15, 2023

Pujan92 commented Jun 15, 2023 •

edited

Loading

hungvu193 commented Jun 16, 2023 •

edited

Loading

melvin-bot bot commented Jun 19, 2023

anmurali commented Jun 19, 2023

melvin-bot bot commented Jun 19, 2023

melvin-bot bot commented Jun 19, 2023

melvin-bot bot commented Jun 19, 2023

sobitneupane commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

Pujan92 commented Jun 21, 2023

sobitneupane commented Jun 21, 2023

Pujan92 commented Jun 21, 2023 •

edited

Loading

arosiclair commented Jun 21, 2023

arosiclair commented Jun 21, 2023

hungvu193 commented Jun 21, 2023 •

edited

Loading

dukenv0307 commented Jun 21, 2023

melvin-bot bot commented Jun 26, 2023

melvin-bot bot commented Jun 26, 2023

melvin-bot bot commented Jun 26, 2023

anmurali commented Jul 6, 2023

hungvu193 commented Jul 6, 2023

sobitneupane commented Jul 10, 2023

sobitneupane commented Jul 10, 2023

sobitneupane commented Jul 10, 2023

melvin-bot bot commented Jul 13, 2023

anmurali commented Jul 17, 2023

dukenv0307 commented Aug 3, 2023

dukenv0307 commented Oct 3, 2023

arosiclair commented Oct 3, 2023

anmurali commented Oct 3, 2023

[HOLD for payment 2023-07-03] [$1000] Profile page loads infinitely with accountID is undefined #20854

[HOLD for payment 2023-07-03] [$1000] Profile page loads infinitely with accountID is undefined #20854

Comments

kavimuru commented Jun 15, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

melvin-bot bot commented Jun 15, 2023

melvin-bot bot commented Jun 15, 2023

Bug0 Triage Checklist (Main S/O)

kavimuru commented Jun 15, 2023

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

Pujan92 commented Jun 15, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

hungvu193 commented Jun 16, 2023 • edited Loading

Proposal

Please re-state the problem that we are trying to solve in this issue.

What is the root cause of that problem?

What changes do you think we should make in order to solve the problem?

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Jun 19, 2023

anmurali commented Jun 19, 2023

melvin-bot bot commented Jun 19, 2023

melvin-bot bot commented Jun 19, 2023

melvin-bot bot commented Jun 19, 2023

sobitneupane commented Jun 21, 2023

melvin-bot bot commented Jun 21, 2023

Pujan92 commented Jun 21, 2023

sobitneupane commented Jun 21, 2023

Pujan92 commented Jun 21, 2023 • edited Loading

arosiclair commented Jun 21, 2023

arosiclair commented Jun 21, 2023

hungvu193 commented Jun 21, 2023 • edited Loading

dukenv0307 commented Jun 21, 2023

melvin-bot bot commented Jun 26, 2023

melvin-bot bot commented Jun 26, 2023

melvin-bot bot commented Jun 26, 2023

anmurali commented Jul 6, 2023

hungvu193 commented Jul 6, 2023

sobitneupane commented Jul 10, 2023

sobitneupane commented Jul 10, 2023

sobitneupane commented Jul 10, 2023

melvin-bot bot commented Jul 13, 2023

anmurali commented Jul 17, 2023

dukenv0307 commented Aug 3, 2023

dukenv0307 commented Oct 3, 2023

arosiclair commented Oct 3, 2023

anmurali commented Oct 3, 2023

kavimuru commented Jun 15, 2023 •

edited by melvin-bot bot

Loading

Pujan92 commented Jun 15, 2023 •

edited

Loading

hungvu193 commented Jun 16, 2023 •

edited

Loading

Pujan92 commented Jun 21, 2023 •

edited

Loading

hungvu193 commented Jun 21, 2023 •

edited

Loading