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
Fix/34611: Remove old merchant page #35641
Fix/34611: Remove old merchant page #35641
Changes from all commits
c4f0a67
731235e
4a4e854
690554b
eb26f8b
0e20d75
8b94671
992a3ca
4504a50
227e0dd
12c9f48
60a5de6
c496c44
6ddb061
16686ca
f508fc6
d8e915d
c2857d1
6a6bee4
f73f9f7
22f9231
5a2e86b
206ef8a
3172e24
f026983
403356e
13d5d31
1ac3823
b7e5570
ce89de5
594d1cb
c19f26c
6e24b84
70fc1c8
d55757f
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.
Should this be required? And should the property isPolicyExpenseChat also be required?
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.
@cead22 in the new TS type
App/src/types/onyx/Transaction.ts
Line 136 in cc0e82d
App/src/types/onyx/IOU.ts
Line 9 in cc0e82d
These fields are not required.
participants
exists after the user selects participant so why do you think these fields should be required?What do you think about adding a fallback value for
transaction.participant
hereThere 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.
What you said makes sense, I only thought maybe they could be required since we're using them.
We can, and it would make it more obvious that
isPolicyExpenseChat
isn't a required prop, but at the same time, the result is the same becauseparticipant.isPolicyExpenseChat
will be cast to bool even without the explicit cast, so I don't feel strongly either way