[$2000] Chat - App crashed tapping profile icon

[kavimuru]

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:

1. Go to https://staging.new.expensify.com/[](javascript:)
2. Enter email address for a new account
3. Check inbox and click link and change it to staging url
4. Set Password
5. From the chat click on Concierge icon

Expected Result:

App is not crashed after click on Concierge icon and it should display the details

Actual Result:

App is crashed after click on Concierge icon

Workaround:

Unknown

Platform:

Where is this issue occurring?

• Web
• iOS
• Android
• Desktop App
• Mobile Web

Version Number: 1.1.52.0

Reproducible in staging?: y

Reproducible in production?: y

Email or phone of affected tester (no customers):

Logs: https://stackoverflow.com/c/expensify/questions/4856

Notes/Photos/Videos:

Bug5523829_Recording__179.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause

Slack conversation:

View all open jobs on GitHub

[melvin-bot]

Triggered auto assignment to @thienlnam (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[thienlnam]

Interesting, looks like it is only broken in that flow and not when you log into an existing account. Looks like it can be an external fix though

[melvin-bot]

Triggered auto assignment to @kadiealexander (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[kadiealexander]

Posted to Upwork:
Internal: https://www.upwork.com/ab/applicants/1512290602886025216/job-details
External: https://www.upwork.com/jobs/~014661f822c5c475a7

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (Exported)

[melvin-bot]

Current assignee @thienlnam is eligible for the Exported assigner, not assigning anyone new.

[Santhosh-Sellavel]

@kadiealexander Let's double this one.

cc: @thienlnam

[kadiealexander]

Good call Santhosh, done! Get those proposals in team!

[Santhosh-Sellavel]

Still waiting for proposals here!

[Santhosh-Sellavel]

Let's double this one, do you agree? @thienlnam

[metehanozyurt]

Proposal

Cause

When user first time login chat list not yet created. personalDetailsList request returned empty array. Avatar does not appear due to lack of user information. Clicking on the empty avatar also gives an error because there is no user (Concierge) information.

Solution

We can wait for the Concierge report to be created and then get the PersonalDetails for first logins.
We need to change like this in this file report.js:

App/src/libs/actions/Report.js

Lines 1000 to 1029 in f174c32

	return API.Get({
	returnValueList: 'chatList',
	})
	.then((response) => {
	if (response.jsonCode !== 200) {
	return;
	}
	// The cast here is necessary as Get rvl='chatList' may return an int or Array
	const reportIDs = _.filter(String(response.chatList).split(','), _.identity);
	// Get all the chat reports if they have any, otherwise create one with concierge
	if (reportIDs.length > 0) {
	return fetchChatReportsByIDs(reportIDs);
	}
	return fetchOrCreateChatReport([currentUserEmail, CONST.EMAIL.CONCIERGE], false);
	})
	.then((returnedReports) => {
	Onyx.set(ONYXKEYS.INITIAL_REPORT_DATA_LOADED, true);
	Onyx.set(ONYXKEYS.IS_LOADING_REPORT_DATA, false);
	// If at this point the user still doesn't have a Concierge report, create it for them.
	// This means they were a participant in reports before their account was created (e.g. default rooms)
	const hasConciergeChat = _.some(returnedReports, report => ReportUtils.isConciergeChatReport(report));
	if (!hasConciergeChat) {
	fetchOrCreateChatReport([currentUserEmail, CONST.EMAIL.CONCIERGE], false);
	}
	if (shouldRecordHomePageTiming) {

should change like this

    Onyx.set(ONYXKEYS.IS_LOADING_REPORT_DATA, true);
    let firstLogin = false;
    return API.Get({
        returnValueList: 'chatList',
    })
        .then((response) => {
            if (response.jsonCode !== 200) {
                return;
            }

            // The cast here is necessary as Get rvl='chatList' may return an int or Array
            const reportIDs = _.filter(String(response.chatList).split(','), _.identity);

            // Get all the chat reports if they have any, otherwise create one with concierge
            if (reportIDs.length > 0) {
                return fetchChatReportsByIDs(reportIDs);
            }
            firstLogin = true;
            return fetchOrCreateChatReport([currentUserEmail, CONST.EMAIL.CONCIERGE], false);
        .then((returnedReports) => {
            Onyx.set(ONYXKEYS.INITIAL_REPORT_DATA_LOADED, true);
            Onyx.set(ONYXKEYS.IS_LOADING_REPORT_DATA, false);

            // If at this point the user still doesn't have a Concierge report, create it for them.
            // This means they were a participant in reports before their account was created (e.g. default rooms)
            const hasConciergeChat = _.some(returnedReports, report => ReportUtils.isConciergeChatReport(report));
            if (!hasConciergeChat) {
                fetchOrCreateChatReport([currentUserEmail, CONST.EMAIL.CONCIERGE], false)
                    .then((returnedReportsSecond) => {
                        const hasConciergeChatSecond = _.some(returnedReportsSecond, report => ReportUtils.isConciergeChatReport(report));
                        if (hasConciergeChatSecond) {
                            PersonalDetails.fetchPersonalDetails();
                        }
                    });
            } else if (firstLogin) {
                PersonalDetails.fetchPersonalDetails();
            }

video:

web-first-login-2022-05-01.mp4

[Santhosh-Sellavel]

@metehanozyurt Thanks for the proposal! But I am not 100% on board with it. I think the root cause is different.

Reason:
I am able to reproduce the same issue while trying to create a New Chat.

Click -> FAB -> New Chat.
Initiate a chat with a user never contacted and click the Chat Header immediately.
the app will crash.
But If you didn't tap immediately details will get fetched eventually the issue will not be able to reproduce.

Personal details should be fetched automatically, we should look for why it's not fetched automatically. So I think we are missing something here.

[metehanozyurt]

You are right, I have observed this situation now. Thank you for the reply 🙏 .

[eVoloshchak]

Updated proposal:
To avoid fetching all personal details every time, we can make use of the API.PersonalDetails_GetForEmails

1. In PersonalDetails.js implement getPersonalDetailsForEmail method

function getPersonalDetailsForEmail(email) {
    API.PersonalDetails_GetForEmails({emailList: email})
        .then((data) => {
            const details = _.pick(data, email);
            Onyx.merge(ONYXKEYS.PERSONAL_DETAILS, formatPersonalDetails(details));
        });
}

2. To resolve the case with tapping Concierge icon after the first login, add the following after this line

const hasConciergeDetails = _.some(allPersonalDetails, person => person.login === CONST.EMAIL.CONCIERGE);
if (!hasConciergeDetails) {
    getPersonalDetailsForEmail(CONST.EMAIL.CONCIERGE);
}

3. To resolve the case with tapping on profile icon after the chat has been created, add this in DetailsPage.js

const DetailsPage = (props) => {
    const details = props.personalDetails[props.route.params.login];
    if (!details) {
        PersonalDetails.getPersonalDetailsForEmail(props.route.params.login);
        return <FullScreenLoadingIndicator />;
    }

Result:

cinnamon-20220518-4.mp4

[Santhosh-Sellavel]

@thienlnam
I like the @eVoloshchak proposal!
🎀👀🎀 C+ reviewed!

Just one concern if needed we should refactor DetailsPage component or any other changes to ensure this getPersonalDetailsForEmail called only once!

    if (!details) {
        PersonalDetails.getPersonalDetailsForEmail(props.route.params.login);
        return <FullScreenLoadingIndicator />;
    }

[eVoloshchak]

Just one concern if needed we should refactor DetailsPage component or any other changes to ensure this getPersonalDetailsForEmail called only once!

Ok, I'll test this and apply changes if needed

[kadiealexander]

@thienlnam any thoughts on the above?

[thienlnam]

Just came back from OOO,

I understand that refetching the personal details will fix it from crashing, but why do those personal details not exist in the first place when we're fetching those person details of a report? That seems like the root cause we should try to resolve

[Santhosh-Sellavel]

@thienlnam

For Concierge chat when we fetch Personal Details the concierge report is not yet created. So the personal details are missing for the concierge.

For another case after creating a new chat when immediately pressing the header, personalDetails for that report is being fetched.

[thienlnam]

For Concierge chat when we fetch Personal Details the concierge report is not yet created. So the personal details are missing for the concierge.

This makes sense and is okay

For another case after creating a new chat when immediately pressing the header, personalDetails for that report is being fetched.

Can we just wait until personalDetails is finished loaded? We're just grabbing it again here right?

[eVoloshchak]

@thienlnam

Can we just wait until personalDetails is finished loaded? We're just grabbing it again here right?

In this case we're just waiting for the getFromReportParticipants to finish, so yes, we actually don't need to make a network reqest on the third step of my prorosal. It would just need to show loading indicator until network request is finished.

const DetailsPage = (props) => {
    const details = props.personalDetails[props.route.params.login];
    if (!details) {
        return <FullScreenLoadingIndicator />;
    }

[thienlnam]

@eVoloshchak Thank you, can you show another video with that updated proposal?

[eVoloshchak]

@eVoloshchak Thank you, can you show another video with that updated proposal?

@thienlnam, sure

cinnamon-20220526-2.mp4

[thienlnam]

Awesome thanks - 👍 to the updated proposal

[melvin-bot]

📣 @eVoloshchak You have been assigned to this job by @thienlnam!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[kadiealexander]

Offers sent on upwork to @Santhosh-Sellavel and @eVoloshchak :)

[Santhosh-Sellavel]

@eVoloshchak Any update here?

[Santhosh-Sellavel]

@eVoloshchak any update?

[eVoloshchak]

@eVoloshchak any update?

Sorry for the delay, expect a PR in 28 hours

[Santhosh-Sellavel]

@thienlnam
The proposal is approved and waiting for PR from @eVoloshchak.

I'm going OOO, I request to add a hold on this issue until I am back or un-assign me and add a new C+ here.

Thanks!

cc: @kadiealexander

[kadiealexander]

Thanks Santhosh! @eVoloshchak could you please let us know about the PR?

[eVoloshchak]

@eVoloshchak could you please let us know about the PR?

Encountered an issue on native platforms, now all resolved, PR is up

[marcaaron]

Left a note on the linked PR, but I think this should be made Internal and rolled into the current platform-wide API refactor? We're adding a change we'll have to undo very soon that also breaks our API design philosophy. We will, of course, still pay everyone involved - but don't think we can proceed as planned.

[kadiealexander]

@marcaaron could you please lay out the steps we need to proceed with that? Should we just close the current PR and pay @eVoloshchak and @Santhosh-Sellavel now?

[thienlnam]

@kadiealexander

Let's pay out the amounts to @eVoloshchak and @Santhosh-Sellavel. We will be taking this internal and will be re-worked as we work on our API refactors

[kadiealexander]

Awesome, thanks @thienlnam! I've paid everyone.