Feat : show Invite button on Members page for money request reports - #88459
Feat : show Invite button on Members page for money request reports#88459M00rish wants to merge 31 commits into
Conversation
|
@abzokhattab Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
M00rish
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const {translate, formatPhoneNumber} = useLocalize(); | ||
| const [countryCode = CONST.DEFAULT_COUNTRY_CODE] = useOnyx(ONYXKEYS.COUNTRY_CODE); | ||
| const [didScreenTransitionEnd, setDidScreenTransitionEnd] = useState(false); | ||
| const reportAttributes = useReportAttributes(); |
There was a problem hiding this comment.
❌ PERF-11 (docs)
useReportAttributes() subscribes to the entire ONYXKEYS.DERIVED.REPORT_ATTRIBUTES record (all reports' attributes). This component only needs reportAttributes?.[report.reportID]?.reportName — a single string. Every time any report's attributes change anywhere in the app, this component will re-render unnecessarily.
Consider a targeted useOnyx call with a selector that returns the primitive reportName string for just this report:
const [reportName] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, {
selector: (data) => data?.reports?.[report.reportID]?.reportName,
});
// Then use: StringUtils.lineBreaksToSpaces(reportName ?? '')A selector returning a primitive (string) is cheap to compare and prevents re-renders from unrelated report attribute changes.
Reviewed at: fbfb102 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
is this ready for review or still waiting for backend work? @M00rish |
|
There's some backend work to be done |
|
PR is ready for review again @abzokhattab thanks! |
|
@codex review |
|
@MelvinBot review |
PR ReviewOverall: Clean, well-scoped PR with good test coverage. A few items to address: 1. Selector re-render risk (agrees with PERF-11 bot flag)
const [reportName = ''] = useOnyx(ONYXKEYS.DERIVED.REPORT_ATTRIBUTES, {
selector: (data) => data?.reports?.[reportID]?.reportName,
});
// Then: StringUtils.lineBreaksToSpaces(reportName)This eliminates the 2. Good catch on optimistic
|
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@abzokhattab can you review it please? |
|
on my list for today |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
@abzokhattab would you review pls ? |
There was a problem hiding this comment.
this file change is not related to the issue ... is it intended?
There was a problem hiding this comment.
yes it is, it solves messaging the invited member error
There was a problem hiding this comment.
sorry i am not following. What messaging error? It would be nice if you could share can video
There was a problem hiding this comment.
This message issue error is not related to the scope of the task, which is to only enable the invite and make sure the user is invited correctly.
Also, I am not able to reproduce the mentioned issue
Screen.Recording.2026-07-23.at.21.45.15.mov
There was a problem hiding this comment.
yes thanks for this, that proves that it was a BE issue perhaps, restored the file to before
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9eba2e9c1f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const isGroupChat = isGroupChatUtils(report); | ||
| const policy = usePolicy(report.policyID); | ||
| const isReportSubmitterOrAdmin = isCurrentUserSubmitter(report) || isPolicyAdmin(policy); | ||
| const shouldShowInviteButton = isReportSubmitterOrAdmin && (isGroupChat || (isMoneyRequestReport(report) && isOpenExpenseReport(report))); |
There was a problem hiding this comment.
When the report is a group chat, this new isReportSubmitterOrAdmin requirement hides the entire action container for group-chat admins who are not also the report owner/workspace admin. The table still enables selection from isCurrentUserGroupChatAdmin, so those admins can select members but no longer see Remove / Make admin actions, and non-owner group chat members also lose the invite button that existed before this change. Gate the group-chat case on the group-chat permissions, or apply the submitter/policy-admin check only to open expense reports.
Useful? React with 👍 / 👎.
| selector: reportAttributesSelector, | ||
| }); | ||
| const backPath = useDynamicBackPath(DYNAMIC_ROUTES.REPORT_PARTICIPANTS_INVITE.path); | ||
| const shouldShowInvitePage = isGroupChat(report) || (isMoneyRequestReport(report) && isOpenExpenseReport(report)); |
There was a problem hiding this comment.
For open expense reports, the participants page hides the invite button unless the current user is the submitter or a policy admin, but this route-level check allows any open expense report. A non-submitter member who opens the invite URL directly can still search users and submit inviteToGroupChat, causing optimistic participant changes/API calls despite the action being hidden in the normal flow. Mirror the same submitter/policy-admin permission here for the money-request case.
Useful? React with 👍 / 👎.
|
could you review again @abzokhattab ? the error checks are from main will merge main once resolved. |
|
Left a comment here #88459 (comment) @M00rish |
|
please check the video |
# Conflicts: # src/pages/DynamicReportParticipantsInvitePage.tsx # src/pages/DynamicReportParticipantsPage.tsx
|
@abzokhattab @M00rish what's the latest here? |
|
@M00rish please let me know when this comment is handled #88459 (comment) .. also can u review the ai genreated reviews and see if they are relvent |
|
i have already responded to your comment pls check |
|
the last comment is still open #88459 (comment) @M00rish |
in 1:1 chat the report.chatype and particpant role are undefined from backend that makes the invite button hidden could you test it from your end @abzokhattab and confirm same? thanks |
9 participants
Explanation of Change
Fixed Issues
$ #80957
PROPOSAL: comment
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android.webm
Android: mWeb Chrome
Android.webm
iOS: Native
ios.webm
iOS: mWeb Safari
iosWEB.webm
MacOS: Chrome / Safari
web.webm