Change Invite whisper actions#61289
Conversation
|
One small change: could we use singular or plural consistently? Either expense or expenses:
|
|
Singular is fine. |
Sounds good, let's change it to INVITE_TO_SUBMIT_EXPENSE then in both FE and BE. |
|
@carlosmiceli I have incorporated changes as you suggested. I noticed when I add the member to workspace on |
|
Are you asking about the new resolution that invites the mentioned user to the workspace? If so, that's the expected behaviour, we want Invite to Chat to invite the user to the chat where it's being tagged, and Invite to Submit Expense to invite the user to the workspace and create a policy expense chat for that user. If we're calling AddMembersToWorkspace with Invite to Submit Expense, then we should see a new member in the workspace panel and a new expense chat created for the invited user. |
|
Ok, thanks for the explanation. It's ready. You can test this whenever the backend changes are done. |
|
@rushatgabhane 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] |
|
@rushatgabhane We are waiting on backend changes to test this. |
|
okayy thank you for letting me know |
|
Tested it locally, seems to work great, @rushatgabhane feel free to review :) |
|
@carlosmiceli We can't test this until your changes are deployed to staging server. |
|
Oh, I meant to review, but will let you know once it's ready. I'm troubleshooting still because I realized that the text in the whispers is not coming through yet, so will report back. |
|
@carlosmiceli Any update on backend PR? |
|
let's retest @parasharrajat please merge main |
…rkspace-chat-invite-whisper
|
Thanks for waiting. ON it. |
|
For this bug #61289 (comment) Should I just hide the invite to submit expenses action if the user is not a admin if the policy of that workspace chat? @carlosmiceli If you think that it should be admin only whisper then backend needs to stop sending this whisper for non-admin users. |
|
I just noticed that a non-admin user can invite a user to the workspace via this prompt when we select
But I need confirmation. @carlosmiceli |
Yes, this should be only for admins. Would this solve the issue completely or do we also need the BE change? |
|
@parasharrajat to clarify, we should keep the existing behavior as it is right now for non-admin users. Admins should get both options of inviting to workspace and also just to chat. |
It should as we won't show the button at all that invites to workspace. Only admins will see that. Yes |
…rkspace-chat-invite-whisper
|
Made that change. |
This reverts commit 2ab2e4d.
|
@rushatgabhane Ready for testing. |
|
@rushatgabhane think we can get a final review and test today/tomorrow so we can have this live before EOW? |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppWhatsApp.Video.2025-07-09.at.01.53.08.mp4Android: mWeb ChromeiOS: HybridAppScreen.Recording.2025-07-09.at.01.55.28.moviOS: mWeb SafariScreen.Recording.2025-07-09.at.01.59.27.movMacOS: Chrome / SafariScreen.Recording.2025-07-09.at.02.01.42.movMacOS: Desktop |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/carlosmiceli in version: 9.1.79-0 🚀
|
|
@parasharrajat can we get a new PR up to fix the reason we reverted this one? |
|
Yes. I will do that asap |
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.1.79-11 🚀
|
|
I have created a new PR here #66049 |
Explanation of Change
Fixed Issues
#58811
$ #60690
$ #60689
PROPOSAL: #60690 (comment)
Tests
If they choose [Invite to submit expenses]:
If they choose [Invite to chat only]
If no selection is made, the mentioned user isn’t invited to the workspace chat and the whisper remains.
If
Do Nothingis selected, whisper hides and nothing happens.Offline tests
You can't take actions on whispers while offline.
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop