Add delete button to allow coordinators to drop sections#443
Add delete button to allow coordinators to drop sections#443KartavyaSharma wants to merge 13 commits intomasterfrom
Conversation
Passing run #235 ↗︎
Details:
Review all test suite changes for PR #443 ↗︎ |
|||||||||||||||
KartavyaSharma
added
enhancement
KartavyaSharma
changed the title
|
Adding another test for student count blocking section deletion would be good! After that, rebase and squash this all down into one commit. |
smartspot2
left a comment
smartspot2
left a comment
There was a problem hiding this comment.
Several small things here; most important ones are additional tests, and adding failure cases in the frontend.
There are also some linting warnings in the JS; mostly about unused variables, which should be removed in the code. There is also a lot of commented out code that should be deleted as well.
Lastly, there are still 11 commits here, which should be rebased and squashed down to one commit (or two) before merging (feel free to wait until these reviews are resolved prior to squashing).
| import React, { useState } from "react"; | ||
| import { useSectionStudents } from "../../utils/queries/sections"; | ||
| import { Mentor, Spacetime, Student } from "../../utils/types"; | ||
| import { Navigate, Route, Routes } from "react-router-dom"; |
There was a problem hiding this comment.
There are a lot of ESLint warnings here about unused variables; you should delete the unused variables here.
| // background-color: #fc4a14; | ||
| background-color: #ff7272; | ||
| cursor: pointer; | ||
| border-radius: 10px; | ||
| border: none; | ||
| outline: none; | ||
| transition: background-color 0.4s; | ||
| //box-shadow: 0px 8px 24px rgba(149, 157, 165, 0.5); | ||
| box-shadow: 0px 4px 4px rgba(198, 198, 198, 0.25); | ||
| } | ||
|
|
||
| .coordinator-delete-link { | ||
| display: flex; | ||
| text-decoration: none; | ||
| color: white; | ||
| justify-content: space-between; | ||
| align-items: center; | ||
| // background: none; | ||
|
|
||
| // border: none; | ||
| // padding: 0; | ||
| // font: inherit; | ||
| // cursor: pointer; | ||
| // outline: inherit; | ||
|
|
||
| // :hover { | ||
| // background-color: #f15120; | ||
| // } |
There was a problem hiding this comment.
If these styles are unused, they should be deleted, rather than commented out.
| const performDrop = () => { | ||
| sectionDropMutation.mutate(undefined, { | ||
| onSuccess: () => { | ||
| // console.log(sectionId) | ||
| setStage(DropSectionStage.DROPPED); | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
There should be a failure case handled here (ex. if there are students in the section). The query hook will also need to be modified to raise this error as well.
4 participants
Currently, there is no way for a coordinator or the tech team to remove sections after they are no longer valid. Eric has to manually drop individual entries from our database for stale/inactive sections. This PR adds a button and an API endpoint that deletes a section from the database when an HTTP request to
DELETE /api/sections/<id>/.