Last active
March 20, 2023 18:27
-
-
Save gmarraro/d323c567ddcbca502ed218b7706147a8 to your computer and use it in GitHub Desktop.
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
/** | |
* Your team recently created a new tool for writing articles. | |
* Your more junior coworker picked up a ticket to enable editing article metadata. | |
* They went on vacation and handed off the PR to you. They've requested a code review, as well as some help | |
* implementing the TODOs they left. You can provide a review via inline comments, or by modifying the code directly. | |
* | |
* Please imagine this is in a production codebase. | |
*/ | |
import React from 'react'; | |
// Coderpad TODO: | |
// set up DB with a few articles + basic metadata | |
// set up HTTP methods for read and update | |
// fetch article metadata | |
// edit state | |
// opens form | |
// editable fields | |
// save and submit to API | |
// endpoint | |
// use built-in fetch API or axios | |
//util (maybe different fields here, to include more complex UI interaction) | |
// * verify field utils | |
// vars storing form fields | |
// error handling | |
// * non-unique slug | |
// * empty fields that are required | |
* onSubmit | |
// TODO: only added console.logs | |
// improvement: both components in same file | |
const ArticleEditForm = (props) => { | |
// bug: data referenced before intialization | |
const submit = (data) => { | |
// bug: should be patch instead of put, this will replace the resource | |
res = put(....) | |
// bug: no error handling | |
return "Success!" | |
} | |
const { data } = props | |
[formData, setFormData] = useState(data) | |
// bug - editing props directly (data.headline = 'new headline') | |
// improvement/bug: validating form on every character input. how can this be optimized? validate the one field, on blur? | |
// improvement/bug: need a semantic element (form) here | |
const onChange = (event) => { | |
const newValue = e.target.value | |
// bug: updating data directly | |
data[e.target.label] = newValue | |
} | |
<div> | |
<input onChange={updateForm / validateForm} />.... | |
<input onChange={validateForm} />.... | |
<input onChange={validateForm} />.... | |
<input onChange={validateForm} />.... | |
<input onChange={validateForm} />.... | |
<button onClick={submit} /> * put wrong type? ; does not disable on click | |
</div> | |
} | |
const ArticleMetadata = () => { | |
// improvement: use const here | |
// improvement: extract into variable | |
const getArticleMetaEndpoint = "https://api.nytimes.com" | |
// bug: can't reassign const | |
getArticleMetaEndpoint = getArticleMetaEndpoint += "/metadata" | |
const data = fetch(....) | |
// bug: will crash if still waiting for response, data is null, need to check if data exists | |
{ meta } = data | |
[isEditing, setIsEditing] = useState(false) | |
if (isEditing) { | |
return <ArticleEditForm data={data} /> | |
} else { | |
...render metadata | |
<button onClick={() => isEditing = True }>Edit Metadata</button> (bug: directly updating state) | |
} | |
} | |
export default ArticleMetadata |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment