-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Align default IDs in IOU.ts file #54534
Open
VickyStash
wants to merge
40
commits into
Expensify:main
Choose a base branch
from
callstack-internal:chore/eslint-consistent-id-iou
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+309
−282
Open
Changes from all commits
Commits
Show all changes
40 commits
Select commit
Hold shift + click to select a range
2cd62ce
chore: test commit for the linter
adhorodyski 2f53a1a
chore: fix lint errors on missing CONST.DEFAULT_NUMBER_ID usages for …
adhorodyski 9f0352f
chore: rm a dummy console log
adhorodyski dd0de46
chore: remove an eslint override for the IOU.ts file exclusion
adhorodyski 661790f
chore: add an early return for transactionThreadReportIDs of undefined
adhorodyski 1ddfa90
chore: add an early return on invalid input to the payInvoice function
adhorodyski 529b1f6
chore: filter out undefiend values
adhorodyski 17829b7
chore: allow undefined output from getIOURequestPolicyID
adhorodyski cf617bd
chore: remove a fallback index access on the failure data
adhorodyski 1b42796
chore: early return on missing input to the cancelPayment func
adhorodyski 2042569
chore: remove more onyx key fallbacks
adhorodyski c98da87
Merge branch 'Expensify:main' into chore/eslint-consistent-id-iou
adhorodyski d268002
chore: optionally push to the optimistic data
adhorodyski 93d2cbd
chore: wrap a success data push in an if statement for undefined input
adhorodyski 2c83daa
chore: wrap a failure data push in an if statement for undefined input
adhorodyski 135baa7
chore: wrap L1831 failure data push
adhorodyski 0ab1de1
chore: early return on missing splitData.createdReportActionID for th…
adhorodyski 46e8c8e
chore: pass in undefined with a missing childReportID
adhorodyski 390df26
chore: extract an optimisticData push to ensure a param
adhorodyski cdccb13
chore: use string or undefined as a value
adhorodyski de2c69b
chore: call 2 new optimistic updates only on conditions met
adhorodyski 2ece26c
Merge branch 'refs/heads/main' into chore/eslint-consistent-id-iou
VickyStash 2332beb
Update getLastVisibleAction and getLastVisibleMessage functions. Remo…
VickyStash 871a257
Remove '-1' id from object keys
VickyStash 80901b5
Fix test
VickyStash 617f8b0
Merge branch 'refs/heads/main' into chore/eslint-consistent-id-iou
VickyStash 0caab12
Fix params related lint errors
VickyStash ac221d1
Merge branch 'refs/heads/main' into chore/eslint-consistent-id-iou
VickyStash bf73430
Lint fix
VickyStash a0905d6
Code polish
VickyStash 4a7725c
Merge branch 'refs/heads/main' into chore/eslint-consistent-id-iou
VickyStash 3980ff6
Minor updates after merging main
VickyStash 6c6026c
Remove null value merging
VickyStash 435df71
Minor improvement
VickyStash 64a3bb9
Fix reportPreviewAction type
VickyStash a870404
Merge branch 'refs/heads/main' into chore/eslint-consistent-id-iou
VickyStash f6e9122
Update Transaction type
VickyStash eebf2ed
Lint fix
VickyStash de9d29a
Apply reviewer feedback
VickyStash 5c94b53
Add earlier return
VickyStash File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Where did we eventually land in terms of the route configurations?
In a parallel PR, we talked that it doesn't make sense to allow nullable params in routes like this, and we'd rather handle the
policyID
absence where we call the route.This looks like the same case to me unless you think this one's different.
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.
There was one more similar discussion in the slack lately: link.
The approach with
undefined
doesn't change the app logic, considering the invalid-1
id value was used before. In most of the cases the app behavior is left the same as before in this case -Not Found
page is showing if you navigate to the page with invalid id.Another approach is to add a condition above and prevent navigation to the invalid page, + add a
Log.warn
so it can be catched and investigated.I think we need to stop on one of the approaches and add it to the guide.
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.
@iwiznia @neil-marcellini what do you think?
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.
I think I rather navigate to the error page AND log a warning so we can investigate