Add automated tests for `getGroupChatName()`

[marcaaron]

cc @roryabraham coming from https://github.com/Expensify/App/pull/39757

Let's perform the following checks and add automated testing in this manner:

• [ ] Verify that the routes for Group participants invites, participant details, and role selection works as expected. Test corner cases like deep link, etc.
• [ ] Add UI test to confirm that getGroupChatName() appears correctly in the appropriate places (minimally the LHN and/or Report header)
• [ ] Add unit test to confirm that getGroupChatName() works correctly
• [ ] Verify that the performance of the options list in InviteReportParticipantsPage is adequate. Apply debounce suggestion from here if necessary: https://github.com/Expensify/App/pull/39757/files#r1561332638

[roryabraham]

love it - once https://github.com/Expensify/App/pull/39757 is merged will you make this external then?

[marcaaron]

Gonna give first licks to @s77rt since they have a ton of context at this point. But if they don't want to work on it we can make it External.

[s77rt]

To not block on this let's make it external for now (I will be C+ if C+ is needed here). I will pick this up if no one took it (once I get more work done)

[ShridharGoel]

Verify that the routes for Group participants invites, participant details, and role selection works as expected. Test corner cases like deep link, etc.

Does this mean manual verification or via tests only?

Verify that the performance of the options list in InviteReportParticipantsPage is adequate.

Same question for this.

[ShridharGoel]

Proposal

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

Add tests for getGroupChatName() to verify that it is working correctly.

What is the root cause of that problem?

New tests.

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

• Test that the routes for Group participants invites, participant details, and role selection works as expected. Test corner cases like deep link, etc.

Add UI and unit tests.

Some examples of UI tests:

• Check the title in LHN. -- If reportName is present, then use the reportName. -- participantAccountIDs can be undefined/null or it can have an array, both cases need to be tested. -- If number of participants is 2, it should show 2 names. -- If number of participants is 8, it should show 8 names if shouldApplyLimit is false. -- If 2, then it should show 2 names if shouldApplyLimit is true (limit will be applied). -- If 8, then it should show 5 names if shouldApplyLimit is true (limit will be applied).

We can also have a test to check the alternateText.

• Check the title in report header -- If reportName is present, then use the reportName. -- participantAccountIDs can be undefined/null or it can have an array, both cases need to be tested. -- If number of participants is 2, it should show 2 names. -- If number of participants is 8, it should show 8 names if shouldApplyLimit is false. -- If 2, then it should show 2 names if shouldApplyLimit is true (limit will be applied). -- If 8, then it should show 5 names if shouldApplyLimit is true (limit will be applied).

Unit tests:

• Call getGroupChatName and pass a list of participant IDs (again check with different combination of number of IDs to test the limit as well)
• shouldApplyLimit can be true or false, test cases should check for both combinations.
• reportName can be present, then it will be used.
• If reportName is not there, then participantAccountIDs will be used.
• participantAccountIDs can be passed directly to getGroupChatName is some test cases.
• If participantAccountIDs is not passed, then it should be fetched using reportID.

In these tests, we can also test for cases when participants's list has only one item. In this, long form of display name should show.

We can also test for case when formattedLogin is used.

Example code for UI test:

describe('Group name', () => {
    afterEach(() => {
        jest.clearAllMocks();
        Onyx.clear();

        // Unsubscribe to pusher channels
        PusherHelper.teardown();
    });

    it('Check if group name is showing correctly in LHN', () =>
        signInAndGetApp("A, B, C, D")
            .then(() => {
                // Verify the sidebar links are rendered
                const sidebarLinksHintText = Localize.translateLocal('sidebarScreen.listOfChats');
                const sidebarLinks = screen.queryAllByLabelText(sidebarLinksHintText);
                expect(sidebarLinks).toHaveLength(1);

                // Verify there is only one option in the sidebar (Optional)
                const optionRowsHintText = Localize.translateLocal('accessibilityHints.navigatesToChat');
                const optionRows = screen.queryAllByAccessibilityHint(optionRowsHintText);
                expect(optionRows).toHaveLength(1);

                // Get the display name text
                const displayNameHintText = Localize.translateLocal('accessibilityHints.chatUserDisplayNames');
                const displayNameText = screen.queryByLabelText(displayNameHintText);

                // A, B, C, D are the display names of the participants in the group chat
                return waitFor(() => expect(displayNameText?.props?.children?.[0]).toBe("A, B, C, D"));
            }));
});

Note that the names taken will be diff combinations, so that we can test the sorting logic as well.

signInAndGetApp code

function signInAndGetApp(reportName: string | null = null): Promise<void> {
    // Render the App and sign in as a test user.
    render(<App/>);
    return waitForBatchedUpdatesWithAct()
        .then(async () => {
            await waitForBatchedUpdatesWithAct();
            const hintText = Localize.translateLocal('loginForm.loginForm');
            const loginForm = screen.queryAllByLabelText(hintText);
            expect(loginForm).toHaveLength(1);

            await act(async () => {
                await TestHelper.signInWithTestUser(USER_A_ACCOUNT_ID, USER_A_EMAIL, undefined, undefined, 'A');
            });
            return waitForBatchedUpdatesWithAct();
        })
        .then(() => {
            User.subscribeToUserEvents();
            return waitForBatchedUpdates();
        })
        .then(async () => {
            const TEN_MINUTES_AGO = subMinutes(new Date(), 10);
            reportActionCreatedDate = format(addSeconds(TEN_MINUTES_AGO, 30), CONST.DATE.FNS_DB_FORMAT_STRING);
            reportAction2CreatedDate = format(addSeconds(TEN_MINUTES_AGO, 60), CONST.DATE.FNS_DB_FORMAT_STRING);

            // Simulate setting a group chat and personal details
            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT}${REPORT_ID}`, {
                reportID: REPORT_ID,
                reportName: reportName,
                lastReadTime: reportActionCreatedDate,
                lastVisibleActionCreated: reportAction2CreatedDate,
                lastMessageText: 'Test',
                participantAccountIDs: [USER_A_ACCOUNT_ID, USER_B_ACCOUNT_ID, USER_C_ACCOUNT_ID, USER_D_ACCOUNT_ID],
                groupChatAdminLogins: USER_A_EMAIL,
                lastActorAccountID: USER_B_ACCOUNT_ID,
                type: CONST.REPORT.TYPE.CHAT,
                chatType: CONST.REPORT.CHAT_TYPE.GROUP,
            });
            const createdReportActionID = NumberUtils.rand64().toString();
            await Onyx.merge(`${ONYXKEYS.COLLECTION.REPORT_ACTIONS}${REPORT_ID}`, {
                [createdReportActionID]: {
                    actionName: CONST.REPORT.ACTIONS.TYPE.CREATED,
                    automatic: false,
                    created: format(TEN_MINUTES_AGO, CONST.DATE.FNS_DB_FORMAT_STRING),
                    reportActionID: createdReportActionID,
                    message: [
                        {
                            style: 'strong',
                            text: '__FAKE__',
                            type: 'TEXT',
                        },
                        {
                            style: 'normal',
                            text: 'created this report',
                            type: 'TEXT',
                        },
                    ],
                },
            });
            await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, {
                [USER_A_ACCOUNT_ID]: TestHelper.buildPersonalDetails(USER_A_EMAIL, USER_A_ACCOUNT_ID, 'A'),
                [USER_B_ACCOUNT_ID]: TestHelper.buildPersonalDetails(USER_B_EMAIL, USER_B_ACCOUNT_ID, 'B'),
                [USER_C_ACCOUNT_ID]: TestHelper.buildPersonalDetails(USER_C_EMAIL, USER_C_ACCOUNT_ID, 'C'),
                [USER_D_ACCOUNT_ID]: TestHelper.buildPersonalDetails(USER_D_EMAIL, USER_D_ACCOUNT_ID, 'D'),
            });

            // We manually setting the sidebar as loaded since the onLayout event does not fire in tests
            AppActions.setSidebarLoaded();
            return waitForBatchedUpdatesWithAct();
        });
}

Example code of unit tests:

describe('getGroupChatName tests', () => {
    it('Should show all participants name if count <= 5 and shouldApplyLimit is false', async function () {
        const report = {
            ...LHNTestUtils.getFakeReport([1, 2, 3, 4]),
            chatType: CONST.REPORT.CHAT_TYPE.GROUP,
        };

        await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, participantsPersonalDetails);
        expect(ReportUtils.getGroupChatName(report?.participantAccountIDs ?? [])).toEqual("(833) 240-3627, [email protected], Lagertha, Ragnar");
    });

    it('Should show 5 participants name if count > 5 and shouldApplyLimit is true', async function () {
        const report = {
            ...LHNTestUtils.getFakeReport([1, 2, 3, 4, 5, 6, 7, 8]),
            chatType: CONST.REPORT.CHAT_TYPE.GROUP,
        };

        await Onyx.merge(ONYXKEYS.PERSONAL_DETAILS_LIST, participantsPersonalDetails);
        expect(ReportUtils.getGroupChatName(report?.participantAccountIDs ?? [], true)).toEqual("(833) 240-3627, [email protected], Lagertha, Lagertha, Ragnar");
    });
});

• Check the performance of the options list in InviteReportParticipantsPage.
• Apply debounce suggestion from here and test it.

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Triggered auto assignment to @miljakljajic (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[bot]]

:warning: 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 :warning:

[s77rt]

@ShridharGoel Thanks for the proposal. Overall this looks good to me 👍

🎀 👀 🎀 C+ reviewed Link to proposal

[melvin-bot[bot]]

Current assignee @marcaaron is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

[melvin-bot[bot]]

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job

[melvin-bot[bot]]

📣 @ShridharGoel 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer 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 📖

[marcaaron]

Thanks very much for your proposal @ShridharGoel 🙇

Does this mean manual verification or via tests only?

Manual verification is sufficient. As far as I can tell we don't have automated tests for complex navigation flows.

Rest of the proposal looks 💎

[ShridharGoel]

Thanks, link to the PR: https://github.com/Expensify/App/pull/40658

[ShridharGoel]

While testing, I found these bugs:

• User can leave the group even if they are the only admin. This shouldn’t be allowed, either leave button shouldn’t show in this case, or we can ask the user about whom to make the admin when leave button is clicked.

• Clicking on the leave button doesn’t show any confirmation dialog.

Thoughts on these bugs?

Should we create issues for these? I’ll like to work on these if possible.

[marcaaron]

Thanks @ShridharGoel, but they're not bugs.

This shouldn’t be allowed, either leave button shouldn’t show in this case, or we can ask the user about whom to make the admin when leave button is clicked.

This is allowed. An admin gets promoted automatically.

Clicking on the leave button doesn’t show any confirmation dialog.

Because there's nothing to confirm.

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @s77rt, @miljakljajic, @marcaaron, @ShridharGoel eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[s77rt]

Bumped the PR

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

• https://github.com/Expensify/App/pull/40658

If no regressions arise, payment will be issued on 2024-06-28. :confetti_ball:

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

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

[melvin-bot[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.
• [ ] [@miljakljajic] Link the GH issue for creating/updating the regression test once above steps have been agreed upon.

[s77rt]

No regression test needed. We only adds tests.

[melvin-bot[bot]]

@s77rt, @miljakljajic, @marcaaron, @ShridharGoel Whoops! This issue is 2 days overdue. Let's get this updated quick!

[miljakljajic]

@s77rt paid - @ShridharGoel please accept the new offer, it seems there was an error with the original one.

[marcaaron]

Not overdue. Seems like we're almost done with this one.

[miljakljajic]

@ShridharGoel bump

[ShridharGoel]

This was accepted few days back, thanks.

[miljakljajic]

Paid and ready to close