-
Notifications
You must be signed in to change notification settings - Fork 0
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
239 requests page tests #241
Changes from 3 commits
ddc7819
2da872e
ae1c3cb
da9e34c
52f33ab
dc06850
79f8f0c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"ware_refs": [ | ||
{ | ||
"id": 123 | ||
} | ||
] | ||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -69,7 +69,7 @@ export const configureRequests = ({ data, path }) => { | |||||
export const configureErrors = (errors) => { | ||||||
const env = process.env.NODE_ENV | ||||||
const remainingErrors = errors | ||||||
.filter(error => Object.keys(error).length) | ||||||
.filter(error => error && Object.keys(error).length) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't have to explicitly check for the truthiness of an error here because
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but what if the array ends up with an item that is undefined? like That is what I was running into in the test that prompted me to change this. idk if its possible for this array of errors to contain one that is undefined, but it was happening in the test. so I figured adding a check there to make sure the error was not undefined was basically some TDD. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. when an error was undefined, Object.keys(error) was v angry |
||||||
.map(error => ({ | ||||||
...error, | ||||||
message: `${error.message} (${error.response?.data?.message})`, | ||||||
|
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 does loading equal undefined instead of false?
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.
all these variables are doing are switching between the different intercept cases, so it could be either undefined or false, it wouldn't really matter as long as its not true. But, it does make it easier to read to be false so i just changed 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.
ok.
yes, the test passes either way but I was thinking from the POV of someone who is unfamiliar with this code. just like we check the api specs to see how things work, someone may check these specs. so it's better, imo, to keep the tests in line with what the actual pages are demonstrating.