Authentication + more #39
Conversation
…ion was creating a new fucntion definition with the same functionality
…authentication
Donaldblanc
added
enhancement
There was a problem hiding this comment.
Looks good overall, I’m assuming some of this will change with UI updates over the next day or two. (Only real change request would be for #39 (comment))
|
|
||
| if (isDev) { | ||
| BrowserWindow.addDevToolsExtension( | ||
| path.join(os.homedir(), './Library/Application Support/Google/Chrome/Default/Extensions/fmkadmapgofadopljbjfkapdkoienihi/3.6.0_0'), |
There was a problem hiding this comment.
As we discussed, it’d probably be best to move this elsewhere for production, but looks fine for now 👌
src/main/components/HeaderBar.jsx
Outdated
| <div> | ||
| <p>Headers</p> | ||
| <form> | ||
| <select name='header' id='headerTypeInput' multiple={false} value={header} |
There was a problem hiding this comment.
I think it makes more sense to just label this as "Authentication", and move the “none” option to the authType select (since the header we send auth info on should never change)
There was a problem hiding this comment.
@conorsexton I made the label to reflect Authentication. I am not following the second request in regards to "none" and authType
| // header info | ||
| const [headerType, setHeaderType] = useState('Authorization'); | ||
| const [authType, setType] = useState('Bearer Token'); | ||
| const [headerKey, setHeaderKey] = useState(''); | ||
|
|
||
| const handleChange = (e) => { | ||
| const { name, value } = e.target; | ||
| if (name === 'method') setSelected(value); |
There was a problem hiding this comment.
As you mentioned, @Donaldblanc let’s plan to refactor this to a switch statement on a future iteration. Created an issue so we don’t forget (#40)
| }; | ||
|
|
||
| const runTest = (link, sendingObj, testsClone, i) => { | ||
| const test = testsClone; |
There was a problem hiding this comment.
I don't think this duplicates testsClone-- now tests is just a reference to testsClone. Not a prob bc presumably it isn't modifying state considering it's called "clone", just worth noting
There was a problem hiding this comment.
I agree its point to a reference in memory, but for cleanliness we I did not want to mutate directly from a parameter passed in.
|
|
||
| fetch(uri, sendingObj) | ||
| .then(res => res.json()) | ||
| .then(res => setData(res)); | ||
| } else if (SourceOrDest === 'dest') { | ||
| const testsClone = [...tests]; | ||
| const sendingObj = { method: selected, mode: 'cors' }; | ||
| let counter = 0; | ||
| if (headerType !== 'NONE') sendingObj.headers = { [headerType]: headerKey }; |
There was a problem hiding this comment.
we should pull both the
const sendingObj = { method: selected, mode: 'cors' };
and the
if (headerType !== 'NONE') sendingObj.headers = { [headerType]: headerKey };
outside of the if-statement. I wrote the first one, no problems on getting approved bc it works just fine, just noticed it now.
| @@ -10,7 +10,7 @@ const ResponseComponent = (props) => { | |||
|
|
|||
| return ( | |||
| <div> | |||
| <p>{`${checkmark} ' - ' ${status}`}</p> | |||
| <p>{`${checkmark} - ${status}`}</p> | |||
Sorry guys I went a little off scrum with this commit but there are several issues in this commit.
[] this disables CORS for electron so we can make HTTP methods without issues
[] I have enabled React Dev tools with in electron, will need to show the team how to get that successfully running.
[] Created the header field
[] User can select to have authentication or No Authentication and will update state respectively
[] User can make a request without auth
[] User can make request with Auth, testing using yelp API, will show the team during stand up.
[] Remove the function declaration with in the loop of the fetch and created a helper function instead. It is not best practice to define functions within loops. Each iteration will create a new function in memory doing the exact same thing. For space complexity, made is a helper function and called that instead.
[] Remove the unnecessary quotes in the template literals in response component.