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.

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

[Payment 2025-02-06] [$250] Search- New participant/email is shown as recent chat when found via search #53876

Closed
3 of 8 tasks
lanitochka17 opened this issue Dec 10, 2024 · 64 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@lanitochka17
Copy link

lanitochka17 commented Dec 10, 2024

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


Version Number: 9.0.73-6
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5318978
Email or phone of affected tester (no customers): gatantm@gmail.com
Issue reported by: Applause - Internal Team

Action Performed:

  1. Launch the app logged in and tap on search
  2. Type an email never that you never chatted with

Expected Result:

The new email is shown on top without the recent chats header

Actual Result:

The new email is shown as part of the recent chats section although never talked to it before

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android: Standalone
  • Android: HybridApp
  • Android: mWeb Chrome
  • iOS: Standalone
  • iOS: HybridApp
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
Bug6690166_1733848545538.ScreenRecording_12-10-2024_18-31-12_1.1.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021870567347662440875
  • Upwork Job ID: 1870567347662440875
  • Last Price Increase: 2024-12-28
  • Automatic offers:
    • FitseTLT | Contributor | 105547307
Issue OwnerCurrent Issue Owner: @strepanier03
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Dec 10, 2024
Copy link

melvin-bot bot commented Dec 10, 2024

Triggered auto assignment to @strepanier03 (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@FitseTLT
Copy link
Contributor

FitseTLT commented Dec 10, 2024

Edited by proposal-police: This proposal was edited at 2024-12-10 17:49:25 UTC.

Proposal

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

Search- New participant/email is shown as recent chat when found via search

What is the root cause of that problem?

We are pushing userToInvite to report options list here

if (filteredOptions.userToInvite) {
reportOptions.push(filteredOptions.userToInvite);
}

and displaying it as part of recent chat section here
const styledRecentReports = recentReportsOptions.map((item) => ({...item, pressableStyle: styles.br2, wrapperStyle: [styles.pr3, styles.pl3]}));
sections.push({title: translate('search.recentChats'), data: styledRecentReports});

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

We should display userToInvite on its own section so remove

if (filteredOptions.userToInvite) {
reportOptions.push(filteredOptions.userToInvite);
}

and push a new section with filteredOptions.userToInvite above here
const styledRecentReports = recentReportsOptions.map((item) => ({...item, pressableStyle: styles.br2, wrapperStyle: [styles.pr3, styles.pl3]}));
sections.push({title: translate('search.recentChats'), data: styledRecentReports});

  const [recentReportsOptions, userToInvite] = useMemo(() => {
        if (autocompleteQueryValue.trim() === '') {
            return [searchOptions.recentReports.slice(0, 20)];
        }

        Timing.start(CONST.TIMING.SEARCH_FILTER_OPTIONS);
        const filteredOptions = OptionsListUtils.filterOptions(searchOptions, autocompleteQueryValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true});
        Timing.end(CONST.TIMING.SEARCH_FILTER_OPTIONS);
        const userToInvite = [];
        const reportOptions: OptionData[] = [...filteredOptions.recentReports, ...filteredOptions.personalDetails];
        if (filteredOptions.userToInvite) {
            userToInvite.push(filteredOptions.userToInvite);
        }
        return [reportOptions.slice(0, 20), userToInvite];
    }, [autocompleteQueryValue, searchOptions]);
if (userToInvite?.length) {
        const styledUserToInvite = userToInvite?.map((item) => ({...item, pressableStyle: styles.br2, wrapperStyle: [styles.pr3, styles.pl3]}));

        sections.push({title: undefined, data: styledUserToInvite});
    }

If needed we can use another copy for the title section but it would be aligned with how we are treating userToInvite in WorkspaceInvitePage if we set the title undefined.

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

Alternatively we can also remove

if (filteredOptions.userToInvite) {
reportOptions.push(filteredOptions.userToInvite);
}

and then add this line of codes here

if (filteredOptions.userToInvite) {
        const styledUserToInvite = [filteredOptions.userToInvite]?.map((item) => ({...item, pressableStyle: styles.br2, wrapperStyle: [styles.pr3, styles.pl3]}));

        sections.push({title: undefined, data: styledUserToInvite});
    }

@Tony-MK
Copy link
Contributor

Tony-MK commented Dec 10, 2024

Proposal

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

Search- New participant/email is shown as a recent chat when found via search

What is the root cause of that problem?

The filteredOptions array includes users who are not invited and are unknown because we didn't specify the canInviteUser and excludeUnknownUsers props when calling the OptionsListUtils.filterOptions function to get the recentReportsOptions.

const recentReportsOptions = useMemo(() => {
if (autocompleteQueryValue.trim() === '') {
return searchOptions.recentReports.slice(0, 20);
}
Timing.start(CONST.TIMING.SEARCH_FILTER_OPTIONS);
const filteredOptions = OptionsListUtils.filterOptions(searchOptions, autocompleteQueryValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true});

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

We should filter out some of the unknown users and users who have not been invited.

We set the canInviteUser and excludeUnknownUsers props to false and true respectively as below.

const filteredOptions = OptionsListUtils.filterOptions(searchOptions, autocompleteQueryValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true});

const filteredOptions = OptionsListUtils.filterOptions(searchOptions, autocompleteQueryValue, {sortByReportTypeInSearch: true, preferChatroomsOverThreads: true, canInviteUser: false, excludeUnknownUsers : true});

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We can make tests to make sure that the recentReportsOptions have reports where a participant has started interacting with the current user.

So, we will assert if new participant's email is in recentReportsOptions .

@melvin-bot melvin-bot bot added the Overdue label Dec 13, 2024
Copy link

melvin-bot bot commented Dec 16, 2024

@strepanier03 Eep! 4 days overdue now. Issues have feelings too...

Copy link

melvin-bot bot commented Dec 18, 2024

@strepanier03 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

Copy link

melvin-bot bot commented Dec 20, 2024

@strepanier03 Now this issue is 8 days overdue. Are you sure this should be a Daily? Feel free to change it!

@strepanier03 strepanier03 added the External Added to denote the issue can be worked on by a contributor label Dec 21, 2024
Copy link

melvin-bot bot commented Dec 21, 2024

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

@melvin-bot melvin-bot bot changed the title Search- New participant/email is shown as recent chat when found via search [$250] Search- New participant/email is shown as recent chat when found via search Dec 21, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Dec 21, 2024
Copy link

melvin-bot bot commented Dec 21, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Dec 21, 2024
@strepanier03
Copy link
Contributor

Able to repro this quite easily, moving forward.

@nkdengineer
Copy link
Contributor

Proposal

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

The new email is shown as part of the recent chats section although never talked to it before

What is the root cause of that problem?

We push userToInvite option to the recent chat section here

if (filteredOptions.userToInvite) {
reportOptions.push(filteredOptions.userToInvite);
}

const styledRecentReports = recentReportsOptions.map((item) => ({...item, pressableStyle: styles.br2, wrapperStyle: [styles.pr3, styles.pl3]}));
sections.push({title: translate('search.recentChats'), data: styledRecentReports});

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

We should split the filteredOptions into three sections as we did in NewChatPage here because the user can have the personal detail of other users even never chatted before. Then it doesn't make sense to display the personal detail option in recent chats

const reportOptions: OptionData[] = [...filteredOptions.recentReports, ...filteredOptions.personalDetails];

if (filteredOptions.userToInvite) {
reportOptions.push(filteredOptions.userToInvite);
}

Another problem is when the search is empty, we only splice the recentReports that can be wrong in the case the recentReports < 20 and we have personal detail options

if (autocompleteQueryValue.trim() === '') {
return searchOptions.recentReports.slice(0, 20);
}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

What alternative solutions did you explore? (Optional)

Copy link

melvin-bot bot commented Dec 24, 2024

@strepanier03 @Ollyws this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@melvin-bot melvin-bot bot added the Overdue label Dec 24, 2024
Copy link

melvin-bot bot commented Dec 25, 2024

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

Copy link

melvin-bot bot commented Dec 27, 2024

@strepanier03, @Ollyws Huh... This is 4 days overdue. Who can take care of this?

Copy link

melvin-bot bot commented Dec 28, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

Copy link

melvin-bot bot commented Dec 31, 2024

@strepanier03, @Ollyws 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

@maddylewis
Copy link
Contributor

maddylewis commented Dec 31, 2024

@Ollyws - can you take a look at the contributor proposals when you have a chance? tysm!

@rlinoz
Copy link
Contributor

rlinoz commented Jan 27, 2025

New PR is being worked on.

@FitseTLT
Copy link
Contributor

PR raised

@melvin-bot melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jan 30, 2025
@melvin-bot melvin-bot bot changed the title [HOLD for payment 2025-01-24] [$250] Search- New participant/email is shown as recent chat when found via search [HOLD for payment 2025-02-06] [HOLD for payment 2025-01-24] [$250] Search- New participant/email is shown as recent chat when found via search Jan 30, 2025
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Jan 30, 2025
Copy link

melvin-bot bot commented Jan 30, 2025

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

Copy link

melvin-bot bot commented Jan 30, 2025

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

If no regressions arise, payment will be issued on 2025-02-06. 🎊

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

Copy link

melvin-bot bot commented Jan 30, 2025

@Ollyws @strepanier03 @Ollyws The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@strepanier03 strepanier03 changed the title [HOLD for payment 2025-02-06] [HOLD for payment 2025-01-24] [$250] Search- New participant/email is shown as recent chat when found via search [Payment 2025-02-06] [$250] Search- New participant/email is shown as recent chat when found via search Jan 31, 2025
@strepanier03 strepanier03 added Daily KSv2 and removed Weekly KSv2 labels Feb 6, 2025
@strepanier03
Copy link
Contributor

@Ollyws - I'll check tomorrow for the checklist and reg test steps and then post the payment summary.

@FitseTLT - I paid your contract in Upwork and closed the contract.

@Ollyws
Copy link
Contributor

Ollyws commented Feb 7, 2025

BugZero Checklist:

  • [Contributor] Classify the bug:
Bug classification

Source of bug:

  • 1a. Result of the original design (eg. a case wasn't considered)
  • 1b. Mistake during implementation
  • 1c. Backend bug
  • 1z. Other:

Where bug was reported:

  • 2a. Reported on production
  • 2b. Reported on staging (deploy blocker)
  • 2c. Reported on both staging and production
  • 2d. Reported on a PR
  • 2z. Other:

Who reported the bug:

  • 3a. Expensify user
  • 3b. Expensify employee
  • 3c. Contributor
  • 3d. QA
  • 3z. Other:
  • [Contributor] 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:

  • [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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:

  • [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template
  • [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

    Link to issue:

Regression Test Proposal

Precondition:

Test:

1. Launch the app and tap on search
2. type anything
3. Verify the Recent chats header disappears once you type something

Do we agree 👍 or 👎

Copy link

melvin-bot bot commented Feb 10, 2025

@rlinoz, @strepanier03, @Ollyws, @FitseTLT Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@melvin-bot melvin-bot bot added the Overdue label Feb 10, 2025
@FitseTLT
Copy link
Contributor

@strepanier03 Payment overdue

@strepanier03
Copy link
Contributor

@FitseTLT - I paid your contract and closed it last week.

@melvin-bot melvin-bot bot removed the Overdue label Feb 11, 2025
@strepanier03
Copy link
Contributor

@Ollyws payment summary below, feel free to request payment now.

@strepanier03
Copy link
Contributor

strepanier03 commented Feb 11, 2025

Payment Summary

@github-project-automation github-project-automation bot moved this from MEDIUM to DONE in [#whatsnext] #retain Feb 11, 2025
@Ollyws
Copy link
Contributor

Ollyws commented Feb 11, 2025

Requested in ND.

@garrettmknight
Copy link
Contributor

$250 approved for @Ollyws

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
Status: DONE
Development

No branches or pull requests