Skip to content
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

Add delete button to allow coordinators to drop sections #443

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 73 additions & 2 deletions csm_web/frontend/src/components/section/MentorSectionInfo.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,37 @@
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";

Check warning on line 2 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L2

'Route' is defined but never used (@typescript-eslint/no-unused-vars)

Check warning on line 2 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L2

'Routes' is defined but never used (@typescript-eslint/no-unused-vars)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a lot of ESLint warnings here about unused variables; you should delete the unused variables here.

import { useSectionStudents, useDropSectionMutation } from "../../utils/queries/sections";
import { Mentor, Spacetime, Student, Course } from "../../utils/types";

Check warning on line 4 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L4

'Course' is defined but never used (@typescript-eslint/no-unused-vars)
import LoadingSpinner from "../LoadingSpinner";
import { CoordinatorAddStudentModal } from "./CoordinatorAddStudentModal";
import MetaEditModal from "./MetaEditModal";
import { InfoCard, SectionSpacetime } from "./Section";
import SpacetimeEditModal from "./SpacetimeEditModal";
import StudentDropper from "./StudentDropper";
import SpacetimeDeleteModal from "./SpacetimeDeleteModal";
import { fetchWithMethod, HTTP_METHODS } from "../../utils/api";

Check warning on line 12 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L12

'fetchWithMethod' is defined but never used (@typescript-eslint/no-unused-vars)

Check warning on line 12 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L12

'HTTP_METHODS' is defined but never used (@typescript-eslint/no-unused-vars)

import { useProfiles, useUserInfo } from "../../utils/queries/base";

Check warning on line 14 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L14

'useProfiles' is defined but never used (@typescript-eslint/no-unused-vars)

Check warning on line 14 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L14

'useUserInfo' is defined but never used (@typescript-eslint/no-unused-vars)
import { useCourses } from "../../utils/queries/courses";

import Modal from "../Modal";

import { useMutation, UseMutationResult, useQuery, useQueryClient, UseQueryResult } from "@tanstack/react-query";

Check warning on line 19 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L19

'useMutation' is defined but never used (@typescript-eslint/no-unused-vars)

Check warning on line 19 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L19

'UseMutationResult' is defined but never used (@typescript-eslint/no-unused-vars)

Check warning on line 19 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L19

'useQuery' is defined but never used (@typescript-eslint/no-unused-vars)

Check warning on line 19 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L19

'useQueryClient' is defined but never used (@typescript-eslint/no-unused-vars)

Check warning on line 19 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L19

'UseQueryResult' is defined but never used (@typescript-eslint/no-unused-vars)
import {
handleError,

Check warning on line 21 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L21

'handleError' is defined but never used (@typescript-eslint/no-unused-vars)
handlePermissionsError,

Check warning on line 22 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L22

'handlePermissionsError' is defined but never used (@typescript-eslint/no-unused-vars)
handleRetry,

Check warning on line 23 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L23

'handleRetry' is defined but never used (@typescript-eslint/no-unused-vars)
PermissionError,

Check warning on line 24 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L24

'PermissionError' is defined but never used (@typescript-eslint/no-unused-vars)
ServerError

Check warning on line 25 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L25

'ServerError' is defined but never used (@typescript-eslint/no-unused-vars)
} from "./../../utils/queries/helpers";

// Images
import XIcon from "../../../static/frontend/img/x.svg";
import PencilIcon from "../../../static/frontend/img/pencil.svg";

// Styles
import "../../css/coordinator-add-student.scss";
import { NavLink } from "react-router-dom";

Check warning on line 34 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L34

'NavLink' is defined but never used (@typescript-eslint/no-unused-vars)

enum ModalStates {
NONE = "NONE",
Expand Down Expand Up @@ -44,6 +61,8 @@
}: MentorSectionInfoProps) {
const { data: students, isSuccess: studentsLoaded, isError: studentsLoadError } = useSectionStudents(sectionId);

const { data: courses, isSuccess: coursesLoaded, isError: coursesLoadError } = useCourses();

Check warning on line 64 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L64

'courses' is assigned a value but never used (@typescript-eslint/no-unused-vars)

Check warning on line 64 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L64

'coursesLoaded' is assigned a value but never used (@typescript-eslint/no-unused-vars)

Check warning on line 64 in csm_web/frontend/src/components/section/MentorSectionInfo.tsx

View workflow job for this annotation

GitHub Actions / ESLint

csm_web/frontend/src/components/section/MentorSectionInfo.tsx#L64

'coursesLoadError' is assigned a value but never used (@typescript-eslint/no-unused-vars)

const [showModal, setShowModal] = useState(ModalStates.NONE);
const [focusedSpacetimeID, setFocusedSpacetimeID] = useState<number>(-1);
const [isAddingStudent, setIsAddingStudent] = useState<boolean>(false);
Expand Down Expand Up @@ -232,6 +251,58 @@
</InfoCard>
</div>
</div>
<DropSection sectionId={sectionId} />
</React.Fragment>
);
}

interface DropSectionProps {
sectionId: number;
}

enum DropSectionStage {
INITIAL = "INITIAL",
CONFIRM = "CONFIRM",
DROPPED = "DROPPED"
}

function DropSection({ sectionId }: DropSectionProps) {
const sectionDropMutation = useDropSectionMutation(sectionId);
const [stage, setStage] = useState<DropSectionStage>(DropSectionStage.INITIAL);

const performDrop = () => {
sectionDropMutation.mutate(undefined, {
onSuccess: () => {
// console.log(sectionId)
setStage(DropSectionStage.DROPPED);
}
});
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


switch (stage) {
case DropSectionStage.INITIAL:
return (
<InfoCard title="Drop Section" showTitle={false}>
<h5>Delete Section</h5>
<button className="danger-btn" onClick={() => setStage(DropSectionStage.CONFIRM)}>
<XIcon className="icon" />
Delete
</button>
</InfoCard>
);
case DropSectionStage.CONFIRM:
return (
<Modal closeModal={() => setStage(DropSectionStage.INITIAL)}>
<div className="drop-confirmation">
<h5>Are you sure you want to delete?</h5>

<button className="danger-btn" onClick={performDrop}>
Confirm
</button>
</div>
</Modal>
);
case DropSectionStage.DROPPED:
return <Navigate to="/" />;
}
}
42 changes: 42 additions & 0 deletions csm_web/frontend/src/css/coordinator-add-student.scss
Original file line number Diff line number Diff line change
Expand Up @@ -308,3 +308,45 @@ $modal-effective-height: calc($modal-height - $modal-padding-y);
.coordinator-email-response-item-right > * {
flex: 1;
}

.coordinator-delete-position {
display: flex;
justify-content: flex-end;
}

.coordinator-delete-button {
width: 250px;
height: 50px;
display: flex;
justify-content: center;
align-items: center;

// 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;
// }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these styles are unused, they should be deleted, rather than commented out.

}
34 changes: 34 additions & 0 deletions csm_web/frontend/src/utils/queries/sections.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { useMutation, UseMutationResult, useQuery, useQueryClient, UseQueryResul
import { fetchNormalized, fetchWithMethod, HTTP_METHODS } from "../api";
import { Attendance, RawAttendance, Section, Spacetime, Student } from "../types";
import { handleError, handlePermissionsError, handleRetry, PermissionError, ServerError } from "./helpers";
import { useProfiles } from "./base";

/* ===== Queries ===== */

Expand Down Expand Up @@ -501,3 +502,36 @@ export const useSectionUpdateMutation = (
handleError(mutationResult);
return mutationResult;
};

/**
* Hook to drop the current section
*
* Invalidates the current user profile query.
*/
export const useDropSectionMutation = (sectionId: number): UseMutationResult<void, ServerError, void> => {
const queryClient = useQueryClient();
const mutationResult = useMutation<void, Error, void>(
async () => {
if (isNaN(sectionId) || isNaN(sectionId)) {
throw new PermissionError("Invalid section id");
}
const response = await fetchWithMethod(`sections/${sectionId}`, HTTP_METHODS.DELETE);
if (response.ok) {
return;
} else {
handlePermissionsError(response.status);
throw new ServerError(`Failed to drop section ${sectionId}`);
}
},
{
onSuccess: () => {
queryClient.invalidateQueries(["sections", sectionId]);
queryClient.invalidateQueries(["courses"]);
},
retry: handleRetry
}
);

handleError(mutationResult);
return mutationResult;
};
77 changes: 77 additions & 0 deletions csm_web/scheduler/tests/models/test_section.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import pytest
from django.urls import reverse
from scheduler.factories import (
CoordinatorFactory,
CourseFactory,
MentorFactory,
SectionFactory,
StudentFactory,
UserFactory,
)
from scheduler.models import Section


@pytest.fixture(name="setup")
def setup_test():
"""
Create a course, coordinator, mentor, and a student for testing.
"""
# Setup course
course = CourseFactory.create()
# Setup sections
section_one = create_section(course)
section_two = create_section(course)
# Setup students
create_students(course, section_one, 3)
create_students(course, section_two, 3)
# Setup coordinator for course
coordinator_user = UserFactory.create()
# Create coordinator for course
CoordinatorFactory.create(user=coordinator_user, course=course)

return (
section_one,
coordinator_user,
)


@pytest.mark.django_db
def test_section_delete(client, setup):
KartavyaSharma marked this conversation as resolved.
Show resolved Hide resolved
"""
Test that a section can be deleted.
"""
(
section_one,
coordinator_user,
) = setup
# Login as coordinator
client.force_login(coordinator_user)
# Delete section
response = client.delete(reverse("section-detail", kwargs={"pk": section_one.id}))
# Check that section was deleted
assert response.status_code == 204
assert Section.objects.count() == 1


def create_students(course, section, quantity):
"""
Creates a given number of students for a given section.
"""
student_users = UserFactory.create_batch(quantity)
students = []
for student_user in student_users:
student = StudentFactory.create(
user=student_user, course=course, section=section
)
students.append(student)
return students


def create_section(course):
"""
Creates a section for a given course.
"""
mentor_user = UserFactory.create()
mentor = MentorFactory.create(user=mentor_user, course=course)
section = SectionFactory.create(mentor=mentor)
return section
37 changes: 37 additions & 0 deletions csm_web/scheduler/views/section.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,43 @@ def create(self, request):
serializer = self.serializer_class(section)
return Response(serializer.data, status=status.HTTP_201_CREATED)

def destroy(self, request, pk=None):
"""
Handle request to delete section through the UI;
deletes mentor and spacetimes along with it
"""
section = get_object_or_error(self.get_queryset(), pk=pk)
KartavyaSharma marked this conversation as resolved.
Show resolved Hide resolved
course = section.mentor.course
# If the course has students, we cannot delete the section
if section.students.count() > 0:
logger.error(
(
"<Section Deletion:Failure> Could not delete section %s, it has"
" students. Remove all students manually first."
),
log_str(section),
)
KartavyaSharma marked this conversation as resolved.
Show resolved Hide resolved
is_coordinator = course.coordinator_set.filter(user=request.user).exists()
if not is_coordinator:
logger.error(
(
"<Spacetime Deletion:Failure> Could not delete spacetime, user %s"
" does not have proper permissions"
),
log_str(request.user),
)
raise PermissionDenied(
"You must be a coordinator to delete this spacetime!"
)
KartavyaSharma marked this conversation as resolved.
Show resolved Hide resolved
# Delete all spacetimes in the section
for spacetime in section.spacetimes.all():
spacetime.delete()
# Delete the mentor
section.mentor.delete()

section.delete()
KartavyaSharma marked this conversation as resolved.
Show resolved Hide resolved
return Response(status=status.HTTP_204_NO_CONTENT)

def partial_update(self, request, pk=None):
"""Update section metadata (capacity and description)"""
section = get_object_or_error(self.get_queryset(), pk=pk)
Expand Down
Loading