Admin page refresh, admin manager modal #443
Conversation
…ending/reported - length feature implemented too
…apping to class names
|
[diff-counting] Significant lines: 455. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| .catch((err) => { | ||
| console.log('error retrieving totalReviews ', err) | ||
| }) | ||
| getReviewsPerClassCSV() { |
There was a problem hiding this comment.
Also I think for the CSV, since className shows the database id of the class, maybe you can make another endpoint after calling /fetchAllReviews that will get the class name from the classes database using the class id stored in each review and then map that to the review count instead.
There was a problem hiding this comment.
The admin page looks a lot better now and the changes you made definitely make everything easier to read and use. I think everything looks really good, however maybe a small visual change you could make would be to wrap the "admin interface", "pending reviews", and "reported reviews" labels in a css class as well and add some more margins or padding to align it with everything else.
There was a problem hiding this comment.
Great work Andrew! This is a very heavy task - a lot of sifting through old code and trying to figure how it all works. I also don't think I was super helpful either giving you pretty ambiguous directions and goals. After looking through your code and the admin page a bit more, I'll try to consolidate what I would like to seen accomplished (hopefully by the end of the semester). Do as much as you can for now - and don't worry too much if you can't get it all done.
- Fix the loading time by separating endpoints (discussed)
- Update old class components to functional components
- Remove any outdated and unused code (unused states, functions, endpoints) such as raffle winner and stat charts (let me know if there is anything else you are considering)
- CSV generation done in back end
- Admin & user privilege management tool/interface (will try to sketch this one out) - feel free to make a sketch of your initial idea and design too! and we'll give feedback or iterate upon it
- actual updating roster buttons & functionality - [ a hard one, first, if you can summarize and breakdown how they work, that would be a helpful first step ( because this is kind of the back bone of our database and app functionality :o ) ]
- should add a wrapper for the admin page to set a max-wdith (ie. 1440px is standard)
- add some left-right and top-bottom padding too (ie
padding: 64px 32px), we can play around with this too and see what looks best.
I left some other comments, a lot of which we already discussed but I wanted to put it in the PR. You probably updated a lot since then but haven't pushed yet - so some comments might already be addressed. Keep up the great work! This tasks is definitely very complex, requires quite a bit of thought, lots of moving pieces, and low-key pretty impactful.
server/src/admin/admin.router.ts
Outdated
There was a problem hiding this comment.
I think we should keep an endpoint for getting pending reviews and reported reviews instead of one endpoint to request a large number of review items (causing a heavy initial load time).
IE. endpoints
- /api/admin/get/reviews/pending
- /api/admin/get/reviews/reported
Now, we do want "all" reviews to "count" how many each class has for our CSV.
This should be handled in the backend and our frontend shouldn't handle any of this or need to load the review objects themselves to massage/do calculations with.
For example, we can instead have an endpoint like:
/api/admin/get/reviewcountbycourse or /api/admin/get/csv/reviewcount
- this endpoint can either return a JSON of courseID/courseName (key) -> count (value) dictionary or just a
csvstring - it should NOT return all review objects to our frontend => our frontend SHOULD NOT loop through all review objects and then generate on the frontend.
There was a problem hiding this comment.
Also change this to functional component (not class)
| ) | ||
| } else { | ||
| console.log('Error at fetchPendingReviews') | ||
| console.log('Error at fetchAllReviews') |
There was a problem hiding this comment.
Looks like there are console logs initially - (from before you worked on it) - fine for testing right now but should delete all console logs when you are done
There was a problem hiding this comment.
We discussed a lot of this already - but yep - separate the reviews endpoint into reported & pending and do not return all reviews to the frontend!
A quick solution I can think of right now is:
useEffect(
axios.hit('api/admin/get/reviews/pending' and '.../reported`)
=> setLoading(false) => setPendingReviews( res.data.pending ... ) , setReported( res.data.reported ... )
)return <div> ...
<PendingReviews props = { ... } />
<ReportedReviews /> // might want to create a new component to make the Admin page less messy and chaotic
</div>
qiandrewj
changed the title
Summary
This PR works on the Admin page feature, fixing old bugs and adding the ability to manage admin users by net-id.
PR Type
Mobile + Desktop Screenshots & Recordings
QA - Test Plan
Breaking Changes & Notes
Added to documentation?