-
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
I74 edit evaluation page #88
Conversation
Seems we are now in the MVP stage, I would prefer that we no longer copy paste code... but rather try to address it in face hand... because the code base is becoming big now... and it will be hard to move further if we keep copy pasting code... so please have a very good reason as to why we copy paste code from now on |
I understand that there are some places where copy pasting code is best because sometimes DRY... can make it less DRY ... for example the EOCModal for the reviewer is different from the EOCModal for the coordinator ... especially if you take a look at the base logic of it |
like for example for this one, it's actually better to reuse code and it's also easier to do so |
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 removed the copy pasted code... All good
useEffect(async () => { | ||
const response = await services['course-evaluation'].get({ | ||
_id: evaluationId, | ||
}); | ||
const evalData = response?.value; | ||
setCode(evalData?.courseId); | ||
setDescription(evalData?.reviewDescription); | ||
setDueDate(new Date(evalData?.dueDate)); | ||
}, []); |
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 don't have to query again for the data... you already have the data in the store
closes #74
Simply adds a dialog to edit evaluation metadata (unit code, description, due date)
Kinda gross code copy paste, but it works I suppose.