[HOLD for payment 2024-07-22] [HOLD for payment 2024-07-17] [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [HOLD for payment 2024-06-13] Group Chats Front end clean up & refactors

[marcaaron]

@s77rt I would say this list is not exclusive. If you have any ideas on how to improve the code further we can take an approach where you propose what we should change, I'll review that, and we can go from there and add things to the list below.

Otherwise, this is the minimum expectation:

• Refactor navigateToMostRecentReport(). There is some overlap with findLastAccessedReport() and navigateToMostRecentReport() and we can probably centralize this logic. Typescript and logic could also be cleaned up for this in some places (e.g. https://github.com/Expensify/App/pull/39757/files#r1560501974, https://github.com/Expensify/App/pull/39757/files#r1560497281 and https://github.com/Expensify/App/pull/39757/files#r1560510114)

• Combine the r/:reportID/settings/group-name route and room-name route. Let’s just have one top level page for editing both (separate components can be rendered depending on chat type - just one idea). And call this settings/name. Reduce potential duplication if possible.

• NewChatConfirmPage - move or useCallback for these functions https://github.com/Expensify/App/pull/39757/files#r1560025967

• ReportDetailsPage - Capture all avatar options in the renderAvatar as per this suggestion here https://github.com/Expensify/App/pull/39757/files#r1560030786 optionally, this could also be a new component as well.

• getReportName() refactor suggestion from here [Group Chats] Add remaining features #39757 (comment)

• newChatNameForm Onyx key - seems this is not working as intended [Group Chats] Add remaining features #39757 (comment)

• Ensure that we can not access an invalid member page as found here - [Group Chats] Add remaining features #39757 (comment)

Issue Owner

Current Issue Owner: @adelekennedy

[s77rt]

No update yet

[marcaaron]

This can come off HOLD now.

[s77rt]

I didn't get a chance to look into this yet

[s77rt]

Raised a PR for the first list item (navigateToMostRecentReport simplification)

[s77rt]

Raised a PR for the second list item (combining report name routes)

[s77rt]

Raised a PR for the remaining tasks:

• NewChatConfirmPage - move or useCallback for these functions: Done
• ReportDetailsPage - Capture all avatar options in the renderAvatar: This was handled here
• getReportName() refactor: getReportName already supports group chat, nothing to do here
• newChatNameForm Onyx key - seems this is not working as intended: False positive. This is working as expected, we are not using draft here and the empty newChatNameForm object shown in the video is for the form state i.e. isLoading, errors. Since it's not loading and we don't have errors it's empty.
• Ensure that we can not access an invalid member page: Done

[adelekennedy]

Final PR hasn't merged yet

[melvin-bot]

Payment Summary

Upwork Job

• ROLE: @s77rt paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@adelekennedy)

• I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
• I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants//hired)
• I have paid out the Upwork contracts or cancelled the ones that are incorrect
• I have verified the payment summary above is correct

[adelekennedy]

I think we're just waiting for the final PR to pay out the reviewer fee

[marcaaron]

Is this done yet?

[s77rt]

@marcaaron Almost, it requires your review 👀 #42365

[adelekennedy]

It looks like that last PR was just approved and merged!

Final payout is due to @fedirjh

[melvin-bot]

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

[adelekennedy]

not deployed to prod yet

[melvin-bot]

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

• Cleanup: Simplify navigateToMostRecentReport function #42365

If no regressions arise, payment will be issued on 2024-07-17. 🎊

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

• @s77rt requires payment through NewDot Manual Requests

[melvin-bot]

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

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] 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:
• [@s77rt] A discussion in #expensify-bugs 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:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@adelekennedy] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[s77rt]

Checklist ^ does not apply here. This was just a cleanup task.

Issue can be closed after paying out @fedirjh

[adelekennedy]

2 days away from payment!

[melvin-bot]

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

• Cleanup: Simplify navigateToMostRecentReport function #42365

If no regressions arise, payment will be issued on 2024-07-22. 🎊

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

• @s77rt requires payment through NewDot Manual Requests

[melvin-bot]

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

• [@s77rt] The PR that introduced the bug has been identified. Link to the PR:
• [@s77rt] 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:
• [@s77rt] A discussion in #expensify-bugs 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:
• [@s77rt] Determine if we should create a regression test for this bug.
• [@s77rt] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@adelekennedy] Link the GH issue for creating/updating the regression test once above steps have been agreed upon: