[CRITICAL][HOLD for payment 2024-05-03][Track Expense] [$500] Allow the self-DM to show up within the 'Start Chat' participant selector

[thienlnam]

Right now, you can only find the self-DM via the global chat search. This is not entirely intuitive that you have to search for your email / you to find the self-DM this way.

Let's allow you to select the self-DM in the chat search and have it open up the self-DM
From the global create,

1. Click 'Start Chat'
2. Have the self-DM show up in the participant selector
3. When clicking on it, it opens up the self-DM

Note: You should not be able to add the self-DM to group

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0112a1eb2ca973eaaa
• Upwork Job ID: 1780001898833866752
• Last Price Increase: 2024-05-07
• Automatic offers:
• s77rt | Reviewer | 0
• nkdengineer | Contributor | 0

[melvin-bot]

Triggered auto assignment to @mallenexpensify (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details. Please add this Feature request to a GH project, as outlined in the SO.

[melvin-bot]

⚠️ It looks like this issue is labelled as a New Feature but not tied to any GitHub Project. Keep in mind that all new features should be tied to GitHub Projects in order to properly track external CAP software time ⚠️

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~0112a1eb2ca973eaaa

[melvin-bot]

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

[nkdengineer]

Proposal

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

What is the root cause of that problem?

• In here, We do not call getFilteredOptions with includeSelfDM: true

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

1. We should use includeSelfDM: true when calling getFilteredOptions in start chat page
2. Then, we need to update this to:

    const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}];
    if (!includeSelfDM) {
        optionsToExclude.push({login: currentUserLogin});
    }

if not, the selfDM is excluded from the results.
3. When clicking on selfDM option, it will call createChat, that will call navigateToAndOpenReport, that will call getChatByParticipants. Because the selfDM will have participantAccountIDs: [] initially, so getChatByParticipants will return empty object, lead to the new chat is created in here though we already had a selfDM report, that leads to an error when calling OpenReport API. We can update the createChat to:

const createChat = useCallback(
        (option?: OptionsListUtils.Option) => {
+          if (option?.isSelfDM) {
+              const report = getReport(option.reportID);
+              Navigation.dismissModalWithReport(report);
+              return;
            }
            ...
        },
        [selectedOptions],
    );

4. We should hide the Add to group button in case of selfDM in here

5. As mentioned in comment, we need to update this to:

    const isSelfDM = ReportUtils.isSelfDM(report);
    const accountIDs = isSelfDM ? [currentUserAccountID ?? 0] : report.participantAccountIDs ?? [];

What alternative solutions did you explore? (Optional)

• NA

Result

Screen.Recording.2024-04-17.at.03.14.14.mov

[abzokhattab]

Proposal

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

Allow the self-DM to show up within the 'Start Chat' participant selector

What is the root cause of that problem?

new feature

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

1. we need to allow the self dm in the getFilteredOptions inside the new chat page

       const filteredOptions = OptionsListUtils.getFilteredOptions(
            listOptions.reports ?? [],
            listOptions.personalDetails ?? [],
            betas ?? [],
            debouncedSearchTerm,
            selectedOptions,
            isGroupChat ? excludedGroupEmails : [],
            false,
            true,
            false,
            {},
            [],
            false,
            {},
            [],
            true,
            false,
            false,
            {} as TaxRatesWithDefault,
            true,
        );

2. that's not enough to make the self dm return in the options list because currently we have a bug in the getOptions function, as here we exclude the self dm even if the include self dm is passed. to fix that we need to change it so that it checks if the includeSelfDm flag is passed it will filter the current account login as follows:

    const optionsToExclude: Option[] = includeSelfDM ? [{login: CONST.EMAIL.NOTIFICATIONS}] : [{login: currentUserLogin}, {login: CONST.EMAIL.NOTIFICATIONS}];

• We should disable the current user login in the personalDetails as the selfDM is already included in the recentReports list. This will prevent duplicate records. To do this, modify this to:

            if (optionsToExclude.some((optionToExclude) => optionToExclude.login === personalDetailOption.login || personalDetailOption.login === currentUserLogin)) {

3. now in order to make the user correctly navigate to the self dm we need to mimic the navigation behaviour in the search page here inside the createChat function as follows:

  if (option?.reportID) {
               Navigation.dismissModal(option.reportID);
           } else {
               Report.navigateToAndOpenReport([login]);
           }

OR we a cleaner way would be to consider the selfDM inside the Report.navigateToAndOpenReport function:

   const isSelfDM = userLogins.length === 1 && userLogins?.[0] === currentUserEmail;
    if (isSelfDM) {
        const selfDMReportID = ReportUtils.findSelfDMReportID();
        const selfDMReport = ReportUtils.getReport(selfDMReportID) ?? {};
        Navigation.dismissModalWithReport(selfDMReport);
        return;
    }

4. Finally, to disable the "add to group" action for the self dm option we should add isDisabled={item.isSelfDM} to the button inside the itemRightSideComponent. or instead we can return null if the current option is a selfDM

[ahmedGaber93]

Proposal

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

[Track Expense] Allow the self-DM to show up within the 'Start Chat' participant selector

What is the root cause of that problem?

New feature.

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

We can follow task self assign SelectionList design which create a new section at top to self assign (see attached picture below).

see task self assign design

the cases for showing section selfDM will be similar to self assign

• section selfDM will display by default when search text is empty.
• when user type search text it will displayed based on existing in search result, and under selfDM section.

For that we need the following change:

1. we need to get selfDM independent in getFilteredOptions result, for that we need to pass includeSelfDM true in NewChatPage.tsx here, and edit OptionsListUtils.ts return value to return selfDM independent here

// NewChatPage.tsx pass `includeSelfDM` true
OptionsListUtils.getFilteredOptions(
  ... cureent params,
  false,
  false,
  {} as TaxRatesWithDefault,
  true,
);

// OptionsListUtils.ts return selfDM independent
let selfDM = allReportOptions.find(option => !!option.isSelfDM && !option.isThread);
if (searchValue && selfDM && !isSearchStringMatch(searchValue, selfDM.searchText)) {
    selfDM = undefined;
}

return {..., selfDM}

// of course we will update typesrcipt in PR

2. in NewChatPage.tsx here we need to add new section at the top (me)

// recieve selfDM in useOptions
const {..., selfDM} = useOptions(...);

// add new section me at the top
if (selfDM) {
    sectionsList.push({
        title: translate('newTaskPage.me'),
        data: [selfDM],
        shouldShow: true,
    });
}

4. to hide add group for selfDM, we will update itemRightSideComponent here to return early null if option is selfDM

const itemRightSideComponent = useCallback(...) => {
  if(item.isSelfDM) {
      return null;
  }
  ...

5. we also need to edit no result found to handle selfDM and hide this message when selfDM exist by adding || filteredOptions.selfDM condition to OptionsListUtils.getHeaderMessage > hasMatchedParticipant here

// selectedOptions.some((participant) => participant?.searchText?.toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())),
selectedOptions.some((participant) => participant?.searchText?.toLowerCase?.().includes(debouncedSearchTerm.trim().toLowerCase())) || filteredOptions.selfDM,

result

Screen.Recording.2024-04-16.at.5.54.21.AM.mov

What alternative solutions did you explore? (Optional)

[mallenexpensify]

@s77rt , plz review the proposals above.
I thought the details in the OP might be a bit too simplified but then I tested

Only other thought I had was that self DM shouldn't be prioritized any differently that whatever we're currently doing for the chats that show there. @thienlnam , comment if you disagree.

[s77rt]

@nkdengineer Thanks for the proposal. The suggested solution is over-engineered:

• In step 2, updating the optionsToExclude is the correct approach and not adding more conditions to the if statement
• In step 3, we shouldn't call findSelfDMReportID because the id is already within the option item, we should do whatever we do with the Search case

Also searching for you should return the self dm

[s77rt]

@abzokhattab Thanks for the proposal. Your solution is not complete:

• Clicking on the self dm does not open the self dm
• Searching for you does not return the self dm

[s77rt]

@ahmedGaber93 Thanks for the proposal. I don't see why we need to move the self dm to a separate section. Let's avoid that.

[nkdengineer]

@s77rt Thanks for your feedback.

Also searching for you should return the self dm

• Can you help check again. I re-tested my proposal and here is result:
  Now, I can reproduce the bug. The RCA is that, in here, we call createOptionList to get the initial options. It works properly as it has the logic to get the correct accountIDs:

App/src/libs/OptionsListUtils.ts

Lines 1475 to 1476 in 0eb9ef2

	// Currently, currentUser is not included in visibleChatMemberAccountIDs, so for selfDM we need to add the currentUser as participants.
	const accountIDs = isSelfDM ? [currentUserAccountID ?? 0] : report.visibleChatMemberAccountIDs ?? [];

but when the reports are updated, we have logic to update the options in here. It leads to the error you mentioned as it has the incorrect logic to get the accountIDs:

App/src/libs/OptionsListUtils.ts

Line 1508 in 0eb9ef2

const accountIDs = report.participantAccountIDs ?? [];

• Calling the createOption with accountIDs: [ ] leads to the report's login field is set to undefined in here.
• Finally, because the report option has login field is undefined, leads to this logic is called and the selfDM is not added to results.
• I updated the proposal to fix it

In step 2, updating the optionsToExclude is the correct approach and not adding more conditions to the if statement

• Thanks for your suggestion. Based on that, another approach is updating this to:

    const optionsToExclude: Option[] = [{login: CONST.EMAIL.NOTIFICATIONS}];
    if (!includeSelfDM) {
        optionsToExclude.push({login: currentUserLogin});
    }

In step 3, we shouldn't call findSelfDMReportID because the id is already within the option item, we should do whatever we do with the Search case

• In step 3, we can update the createChat to:

const createChat = useCallback(
        (option?: OptionsListUtils.Option) => {
+          if (option?.isSelfDM) {
+              const report = getReport(option.reportID);
+              Navigation.dismissModalWithReport(report);
+              return;
            }
            ...
        },
        [selectedOptions],
    );

[ahmedGaber93]

I don't see why we need to move the self dm to a separate section.

@s77rt The main goal of this is ensuring display selfDM to the user Without having to search for it, and also this what we do in self assign task.

task self assign design

[nkdengineer]

@s77rt I updated comment. I have a question about your feedback:

In step 2, updating the optionsToExclude is the correct approach and not adding more conditions to the if statement

Why is my initial solution from step 2 considered incorrect? I've tested both your suggestion and mine, and they both function correctly.

[abzokhattab]

@s77rt updated my proposal to fix the self dm navigation .. please try it out with a new account

[nkdengineer]

@s77rt I updated comment to address the bug you mentioned:

Searching for you should return the self dm

[s77rt]

@nkdengineer Thanks for looking into this. That makes sense! Can you please update your proposal to cover that? (all details should be in one comment)

Why is my initial solution from step 2 considered incorrect?

Your solution was excluding the current user login and add a condition to "ignore" that exclusion. This could cause problems if we use optionsToExclude in other places (unless we use same condition) which kinda breaks the one source of truth logic. The correct solution is to just not exclude the current user login in the first place.

[s77rt]

@ahmedGaber93 The goal is to be able to open self dm from chats.

[melvin-bot]

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

[melvin-bot]

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

• fix allow to search selfDM in new chat page #40672

If no regressions arise, payment will be issued on 2024-05-03. 🎊

For reference, here are some details about the assignees on this issue:

• @s77rt requires payment automatic offer (Reviewer)
• @nkdengineer requires payment automatic offer (Contributor)

[melvin-bot]

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

• [@s77rt] Please propose regression test steps to ensure the new feature will work correctly on production in further releases.
• [@mallenexpensify] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[s77rt]

Regression Test Proposal

1. Press FAB then Start chat
2. In the search field, type your email, your name or the word "you" ("tù" if language is set to Spanish)
3. Verify you see the self dm report (a report with your name and avatar)
4. Press the self dm report
5. Verify you are navigated to the self dm report

[mallenexpensify]

Thanks @s77rt , testrail GH created

• https://github.com/Expensify/Expensify/issues/392149

[mallenexpensify]

Contributor: @nkdengineer paid $500 via Upwork
Contributor+: @s77rt paid $500 via Upwork

@nkdengineer can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~0112a1eb2ca973eaaa

[nkdengineer]

@mallenexpensify Based on this comment, the issue`s price should be updated to $500, right? Please correct me if I miss anything

[mallenexpensify]

Yes @nkdengineer , I missed that (and automation doesn't appear to be working as I'd expect).
can you please accept the job and reply here once you have? I updated the amounts in the payment comment above.

[melvin-bot]

Current assignee @mallenexpensify is eligible for the Bug assigner, not assigning anyone new.

[melvin-bot]

Upwork job price has been updated to $500

[nkdengineer]

@nkdengineer can you please accept the job and reply here once you have?

Thanks @mallenexpensify 🙇 I've applied to the job

[mallenexpensify]

@nkdengineer can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~0112a1eb2ca973eaaa

Before submitting new offers, can you check next time to see if there are existing ones that we sent where we're just waiting for you to accept? There were two for this job awaiting acceptance. Thx

[nkdengineer]

@mallenexpensify Thanks 🙇 I accepted the offer!

can you check next time to see if there are existing ones that we sent where we're just waiting for you to accept?

@mallenexpensify Got it, but those were for the wrong amount so I didn't accept

[mallenexpensify]

Paid @nkdengineer , thx. Payment comment updated above.