[HOLD for payment 2025-01-02] [$250] Invite - Two same QA guides in the member list after onboarding with Manage my team's expenses

[lanitochka17]

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.71-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): applausetester+kh1311020@applause.expensifail.com
Issue reported by: Applause - Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Sign up with a new Gmail account (random email without +)
3. On onboarding purpose, choose Manage my team's expenses
4. Select organization size
5. Select an accounting
6. Complete the onboarding
7. Go to workspace settings > Members
8. Click Invite member

Expected Result:

There will be only one QA guide in the member list

Actual Result:

There are two same QA guides in the member list

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

Bug6684422_1733328208888.bandicam_2024-12-04_23-57-41-348.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021864467241309755517
• Upwork Job ID: 1864467241309755517
• Last Price Increase: 2024-12-05
• Automatic offers:
• ikevin127 | Contributor | 105288097

Issue Owner

Current Issue Owner: @jliexpensify

[melvin-bot]

Triggered auto assignment to @jliexpensify (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.

[jliexpensify]

I can repro on Hybrid App Android and have had a colleague repro on Safari:

[melvin-bot]

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

[melvin-bot]

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

[jliexpensify]

@dukenv0307 - I'm pretty sure this might be Internal, but let me know what you think!

[ikevin127]

Proposal

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

There are two of the same QA guides in the suggested members list.

What is the root cause of that problem?

Note

I was not able to reproduce the issue, on my side it always showed 1 guide from the Expensify team, it never showed qa.guide@team.expensify.com.

I'm not sure of the exact steps required for this to be consistently reproduced, or why the BE (I assume) sends the same value twice in some instances.

I remember similar issues in the past where personalDetailsList was populated with duplicates.

The root cause of why the list can have two of the same values comes from here:

App/src/pages/workspace/WorkspaceInvitePage.tsx

Lines 190 to 196 in 0e28aa4

	const personalDetailsFormatted = personalDetailsWithoutSelected.map((item) => OptionsListUtils.formatMemberForList(item));
	sectionsArr.push({
	title: translate('common.contacts'),
	data: personalDetailsFormatted,
	shouldShow: !isEmptyObject(personalDetailsFormatted),
	});

because personalDetailsFormatted doesn't have any logic to handle duplicates.

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

This is how objects of personalDetailsFormatted (array) look if we log the array:

{
    accountID: 2746397
    alternateText: "zach@expensify.com"
    icons: [Object] (1)
    isDisabled: false
    isSelected: false
    keyForList: "2746397"
    login: "zach@expensify.com"
    pendingAction: undefined
    reportID: ""
    text: "Zach Greenwald (EXFY Product Manager)"
}

Given this and knowing that we use login as unique identifier across the app, we can add the following filtering logic to personalDetailsFormatted to ensure we won't have duplicated entries in the edge case where BE happens to populate personalDetailsList with duplicated values.

const personalDetailsFormatted = personalDetailsFormatted
    .concat(personalDetailsFormatted)
    .filter((item, index, array) => item?.login && index === array.findIndex((i) => i.login === item.login));

Before	After

[dukenv0307]

@jliexpensify I also can't reproduce. Can you share your personalDetailsList?

[dukenv0307]

I think it could be BE bug, otherwise we can use @ikevin127's proposal to filter out the duplicated item

[jliexpensify]

@dukenv0307 @ikevin127 just to confirm: are you creating an entirely new Gmail account for testing?

I also realised I don't know why qa.guide pops up, so asking here.

[jliexpensify]

@lanitochka17 question for you - your instructions say to use Staging, but is this a bug on Production too?

[dukenv0307]

@jliexpensify Yes I used an entirely new Gmail account

[jliexpensify]

Oh, also - what is this?

Can you share your personalDetailsList?

Is this something only on DEV?

[jliexpensify]

@dukenv0307 and @ikevin127 - are you using Staging to test? I've been told that the QA Guide only gets allocated to Staging and Applause emails.

So you'd need to create a new email and be using Staging.

[ikevin127]

I was able to have the qa.guide@team.expensify.com show up, I tested on staging directly - but it doesn't show duplicated on my side as we can see, the personalDetailsList only contains 1 entry:

As mentioned in the proposal:

I remember similar issues in the past where personalDetailsList was populated with duplicates.

In order for somebody with access to BE to check exactly why there are duplicates in the tester's case we would need their policyID (I assume) in order to investigate. But as far as FE goes, the proposed solution would be a good fail-safe in case the BE issue is hard to investigate / fix.

[melvin-bot]

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

[jliexpensify]

Hmm ok, so since there are 2 ways to go about this and we can't consistently reproduce it... @dukenv0307 can you throw the Engineering label on this and we'll see what an Internal Engineer says?

[ikevin127]

@jliexpensify dukenv0307 has the C+ reviewer role, meaning they can't add / remove labels from issues. But you should be able to as far as I know about your role, or at least the Bug label to assign a BZ team member which can then add the Engineering label.

[jliexpensify]

Oh hmm...I can't use the Engineering label as it won't auto-assign anyone. I can throw on the Internal label, but that's just a label and doesn't assign anyone either. I was under the impression if @dukenv0307 approved your proposal, it'd auto-assign an Engineer for review?

[ikevin127]

Oh, you're right - I can do that as well by posting the bowtie since I'm also a C+ and the reviewer mentioned in #53579 (comment):

I think it could be BE bug, otherwise we can use @ikevin127's #53579 (comment) to filter out the duplicated item

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[ikevin127]

PR #54147 is ready for review!

[ikevin127]

♻️ Status update: PR is in review, currently blocked by some requests from reviewer which were not founded (IMO), currently awaiting response on whether we can move forward with the PR and merge.

For context, below are the discussions.

First request from #54147 (comment):

Second request from #54147 (comment):

The second request suggests applying the fix in filterAndOrderOptions which is a function used broadly within the app, hence my reluctance to not complicate things and keep the fix simple within this issue's scope, unless proven that the same issue happens in other mentioned lists.

If proven, I wound't have anything against a broader fix - with a bounty increase to account for the extra work.

[jliexpensify]

@Julesssss @ikevin127 @dukenv0307 just a heads up that I am OOO from the 19th - 29th. I don't think this needs to be reassigned, as it's the Christmas period + there's a review period + the 7 day wait, but if needed, @Julesssss please re-apply the Bug label if payment is needed before I'm back.

Otherwise, I'll catch up and sort out the Payment Summary on the 30th.

[hannojg]

Just as a note:

I implemented a more performant fix for this over at this PR where we replace the search functionality with a suffix search tree implementation:

• Search suffix tree implementation #51954

So my PR effectively will remove the changes introduced here when it gets merged:

• FIX: Two QA guide entries in the member list after onboarding with Manage my team's expenses #54147

---

Edit: on a second thought, I will keep the changes introduced in the PR as well for callsites using filterAndOrderOptions (which we might migrate away from in the future)

[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 9.0.78-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• FIX: Two QA guide entries in the member list after onboarding with Manage my team's expenses #54147

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

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

• @ikevin127 requires payment automatic offer (Contributor)
• @dukenv0307 requires payment through NewDot Manual Requests

[melvin-bot]

@ikevin127 / @dukenv0307 @jliexpensify @ikevin127 / @dukenv0307 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]

[jliexpensify]

Payment Summary

• C: @ikevin127 $250 (paid via Upworks)
• C+: @dukenv0307 $250 (to be paid via ND requests)

Upwork job

[dukenv0307]

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: it can be BE bug, but we need to cover its case on FE side

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. 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: it can be BE bug, but we need to cover its case on FE side

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in https://app.slack.com/client/E047TPA624F/C01GTK53T8Q 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: N/A

• [Contributor] If it was decided to create a regression test for the bug, please propose the https://github.com/Expensify/App/blob/main/contributingGuides/REGRESSION_TEST_BEST_PRACTICES.md steps using the template below to ensure the same bug will not reach production again. Yes

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

Test:

1. Sign up with a new Gmail account (random email without +, because using + will be categorized as test account).
2. On onboarding purpose, choose Manage my team's expenses.
3. Select any organization size.
4. On accounting select None of the above and Continue.
5. Once onboarding is completed, go to workspace settings > Members.
6. Click + Invite member and verify that there's only one qa.guide@team.expensify.com entry in the list instead of two.

Do we agree 👍 or 👎

[jliexpensify]

Paid and job closed.

@dukenv0307 - payment summary here.

[JmillsExpensify]

$250 approved for @dukenv0307