-
Notifications
You must be signed in to change notification settings - Fork 2.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make sure when admin request from its workspace the participants are correctly listed #18717
Conversation
@s77rt @aldo-expensify One of you needs to 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] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@s77rt when you get back, this one would be great to test/add the checklist and then we can get it merged. No crazy urgency, just thinking that this is a fix we can get in ahead of the feature launch late next week. |
@JmillsExpensify I think we are discussing on slack if we can make some changes here. Either way will work on another PR first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🚀
We still need the reviewer checklist. @s77rt can you help with that? |
Reviewer Checklist
Screenshots/VideosMobile Web - ChromeMobile Web - SafariDesktopiOSAndroid |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
@luacmartins Thanks a lot for taking this to the finish line 🚀! |
🚀 Deployed to staging by https://github.com/luacmartins in version: 1.3.14-0 🚀
|
🚀 Deployed to production by https://github.com/yuwenmemon in version: 1.3.14-14 🚀
|
In case admin has been requesting money from its own workspace we have listed all the members of the policy instead of just the workspace because we have been returning also the policy workspace chats in which the admin is not an employee
Details
Fixed Issues
$ #18708
Tests
Create a workspace as userA (the admin).
Invite userB
As userA request money in your own workspace chat
Add amount
Verify the To: section only shows the workspace and no other participants
Click request money from the global create
Add amount
Choose workspace you are admin at
verify there is only that workspace listed in the
To:
sectionOffline tests
N/A
QA Steps
Smae as tests
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
This is platform agnostic code for urgency only adding web screenshots
Member:
![image](https://private-user-images.githubusercontent.com/36083550/237432790-b252e26e-8348-4430-8d8f-21ea10d6deec.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzQ0NzE4MTgsIm5iZiI6MTczNDQ3MTUxOCwicGF0aCI6Ii8zNjA4MzU1MC8yMzc0MzI3OTAtYjI1MmUyNmUtODM0OC00NDMwLThkOGYtMjFlYTEwZDZkZWVjLnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEyMTclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMjE3VDIxMzgzOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPWIxMjliNjMyOGFiOWIyMDYwZGUyZmIyNDFlY2M1OGY2N2Q2ZWQ1NzZiMDNkZDdkMDRkYzc1NjUxZDZhMDY3ZjMmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.jIVZMiHa4x9ugMmK2W5GzkLsmHrL63ccuHQ_FdSnon8)
Admin:
![image](https://private-user-images.githubusercontent.com/36083550/237432943-5950c110-4c0e-4744-b32f-c9a27b540797.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzQ0NzE4MTgsIm5iZiI6MTczNDQ3MTUxOCwicGF0aCI6Ii8zNjA4MzU1MC8yMzc0MzI5NDMtNTk1MGMxMTAtNGMwZS00NzQ0LWIzMmYtYzlhMjdiNTQwNzk3LnBuZz9YLUFtei1BbGdvcml0aG09QVdTNC1ITUFDLVNIQTI1NiZYLUFtei1DcmVkZW50aWFsPUFLSUFWQ09EWUxTQTUzUFFLNFpBJTJGMjAyNDEyMTclMkZ1cy1lYXN0LTElMkZzMyUyRmF3czRfcmVxdWVzdCZYLUFtei1EYXRlPTIwMjQxMjE3VDIxMzgzOFomWC1BbXotRXhwaXJlcz0zMDAmWC1BbXotU2lnbmF0dXJlPTRmYzZjOTc5ZjY0MDQ4NTVjZmI5OTU1MjFkYjQyMWRiY2QzODA5MDI4NGI1M2FjMjk0NjhhNGUwOGEyZjRlYTkmWC1BbXotU2lnbmVkSGVhZGVycz1ob3N0In0.C88Z3OM3m6bGgWq48Fsxbzs1P8KnI3-4JhWlNmglX5o)
Web
Mobile Web - Chrome
Mobile Web - Safari
Desktop
iOS
Android