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
[GL] Tax - Add tax code #43156
[GL] Tax - Add tax code #43156
Changes from 36 commits
5d711c4
f66b22b
9b1b630
8703337
235158b
6e9009b
02e0428
093711b
d3f86b7
1eca1e1
da57f9e
d046fe0
ed414d7
451548c
a69ff83
1131d5d
f782d3f
bee9d44
6dedec2
69ddb27
2c17a1f
78a4aeb
5d00b9f
7336fa1
f49b915
9f95f89
2f8b223
5bd6329
b167d5c
702e1d2
36e8d9c
08e7fbd
9ca398b
828d62f
62f5256
07067a6
04f31e0
81bc5b0
a78d2b5
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.
Why can't we use
import INPUT_IDS_TAX_CODE from '@src/types/form/WorkspaceTaxCodeForm';
here?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.
because it'll conflict with
import INPUT_IDS from '@src/types/form/WorkspaceNewTaxForm
.The default import name is
INPUT_IDS
for formsThere 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.
NAB - are we using this anywhere? not sure we need it
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.
yes, if the action fails then we need to reset the taxCode. The previousTaxCode will be used for it
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.
Same here - do we need this?
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.
im using it for pending fields and showing errors. we need a key inside the object for which an error or pending action can be associated
https://github.com/Expensify/App/pull/43156/files/81bc5b0bc03d1cbf731a4ab2de1e1968e0aef781#diff-008bfc3488a59780cc0f301b91072fc1e18513a7bca89cb81b883772a76275a5R128
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.
ah right, thanks!
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.
You'll need to pass the taxID (the name) here. We only really use it if the newTaxCode is clear, but you might s well pass it always, for simplicity
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.
taxCode is same as taxID right.
do you mean that the api expects ?
taxID: newTaxCode
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.e. just a different variable name
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.
Ah, not quite. It expects the
name
, which is often similar (as we get the default code based on the name, which is why we need it when empty), but not the same as the taxCode.This one: