Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Workspaces in money request modal #17132
Workspaces in money request modal #17132
Changes from all commits
1d602e4
bddf4b0
80efa65
22f6234
964c5ce
b31fb44
1d9f7be
bb95eae
dd7da6a
ca7d9c7
13ff8e3
b1654c5
54e5be9
c59279a
ba9741a
33969d7
fcda6a8
3c9ebb2
e813f18
a42f122
f5aa96e
37e7740
b3e2660
3278622
c200d93
41b9d74
ffd15bf
4f995a9
9754a32
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Not exactly a regression but we shouldn't have used
expenseReport.displayName
here because the display name is not necessary the policy name. The display name is the policy name only if you are the policy expense chat owner.This worked fine with money request because you are the only one who can see the participants list (and that's only at the creation phase). But that's not the case with split action.
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.
We should have used the already existing
createOption
to generate these options. Using the icons object like this caused #28731There 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.
Just wondering: Any reason we didn't add
isMoneyRequestReport: true
here? Since I believe this is only used in money request reportsThere 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.
Maybe because of the split case, since those don't create an IOU/Expense report for the group chat?
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.
innnnnnnnnnnnnnnnnnnnnnnteresting that would make sense :D