Skip to content

Commit

Permalink
teachers: edited imports, crud_teacher: added comments, models, class…
Browse files Browse the repository at this point in the history
… Student: splitted f-string into two parts for better readability
  • Loading branch information
olesya-karpets committed Jun 9, 2024
1 parent 84f21d6 commit b92424e
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 39 deletions.
58 changes: 27 additions & 31 deletions src/app/api/api_v1/routes/teachers.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,7 @@
from fastapi import APIRouter, HTTPException, Body, status
from db.models import Course, Student
from crud import crud_user
from crud import crud_teacher, crud_student
from crud.crud_course import get_course_by_id
from crud.crud_course import course_exists, get_course_common_info, hide_course
from crud.crud_section import create_sections, get_section_by_id, update_section_info, delete_section, validate_section
from crud.crud_tag import create_tags, delete_tag_from_course, course_has_tag, check_tag_associations, delete_tag
from db.models import Course, Student
from crud import crud_user, crud_teacher, crud_student
from crud import crud_course, crud_section, crud_tag
from schemas.teacher import TeacherEdit, TeacherCreate, TeacherSchema, TeacherApproveRequest
from schemas.course import CourseCreate, CourseUpdate, CourseSectionsTags, CourseBase, CoursePendingRequests
from schemas.section import SectionBase, SectionUpdate
Expand Down Expand Up @@ -139,7 +135,7 @@ async def create_course(
- `HTTPException 401`: If the teacher is not authenticated.
"""
if await course_exists(db, course.title):
if await crud_course.course_exists(db, course.title):
raise HTTPException(
status_code=status.HTTP_409_CONFLICT,
detail="Course with such title already exists",
Expand Down Expand Up @@ -171,7 +167,7 @@ async def update_course_home_page_picture(
- `HTTPException 403`: If the teacher does not have access to the course.
- `HTTPException 400`: If the file is corrupted or the media type is not supported.
"""
course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
Expand Down Expand Up @@ -259,7 +255,7 @@ async def view_course_by_id(
detail=f"Invalid sort_by parameter"
)

course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
Expand Down Expand Up @@ -295,7 +291,7 @@ async def approve_enrollment(db: dbDep,
- `HTTPException 403`, if the teacher is not the owner of the course.
"""

course: Course = await get_course_by_id(db, course_id)
course: Course = await crud_course.get_course_by_id(db, course_id)
if not course:
raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail='No such course')

Expand Down Expand Up @@ -331,7 +327,7 @@ async def update_course_info(
- `HTTPException 401`, if the teacher is not authenticated.
- `HTTPException 403`: If the authenticated teacher does not have permission to update the course.
"""
course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
Expand Down Expand Up @@ -367,23 +363,23 @@ async def update_section(
- `HTTPException 403`: If the authenticated teacher does not have permission to update the section.
- `HTTPException 404`: If the section with the given ID does not exist or is not part of the specified course.
"""
course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=msg
)

section = await get_section_by_id(db, section_id)
valid_section, msg = validate_section(section, course_id)
section = await crud_section.get_section_by_id(db, section_id)
valid_section, msg = crud_section.validate_section(section, course_id)
if not valid_section:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=msg
)

return await update_section_info(db, section, updates)
return await crud_section.update_section_info(db, section, updates)


@router.post("/courses/{course_id}/sections", status_code=status.HTTP_201_CREATED, response_model=List[SectionBase])
Expand All @@ -408,15 +404,15 @@ async def add_sections(
- `HTTPException 401`, if the teacher is not authenticated.
- `HTTPException 403`: If the teacher does not have permission to add sections to the course.
"""
course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=msg
)

created_sections = await create_sections(db, sections, course_id)
created_sections = await crud_section.create_sections(db, sections, course_id)
return created_sections


Expand All @@ -443,23 +439,23 @@ async def remove_section(
- `HTTPException 403`: If the teacher does not have access to the course.
- `HTTPException 404`: If the section is not found or does not belong to the specified course.
"""
course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=msg
)

section = await get_section_by_id(db, section_id)
valid_section, msg = validate_section(section, course_id)
section = await crud_section.get_section_by_id(db, section_id)
valid_section, msg = crud_section.validate_section(section, course_id)
if not valid_section:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=msg
)

await delete_section(db, section)
await crud_section.delete_section(db, section)
return


Expand Down Expand Up @@ -488,15 +484,15 @@ async def add_tags(
- `HTTPException 401`: If the teacher is not authenticated.
- `HTTPException 403`: If the teacher does not have permission to add tags to the course.
"""
course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=msg
)

created_tags = await create_tags(db, tags, course_id)
created_tags = await crud_tag.create_tags(db, tags, course_id)
return created_tags


Expand All @@ -523,26 +519,26 @@ async def remove_tag(
- `HTTPException 403`: If the teacher does not have access to the course.
- `HTTPException 404`: If the tag is not associated with the course.
"""
course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
status_code=status.HTTP_403_FORBIDDEN,
detail=msg
)

course_tag = await course_has_tag(db, course_id, tag_id)
course_tag = await crud_tag.course_has_tag(db, course_id, tag_id)
if not course_tag:
raise HTTPException(
status_code=status.HTTP_404_NOT_FOUND,
detail=f"Tag with ID:{tag_id} not associated with course ID:{course_id}"
)

await delete_tag_from_course(db, course_tag)
await crud_tag.delete_tag_from_course(db, course_tag)

tag_associations = await check_tag_associations(db, tag_id)
tag_associations = await crud_tag.check_tag_associations(db, tag_id)
if tag_associations == 0:
await delete_tag(db, tag_id)
await crud_tag.delete_tag(db, tag_id)

return

Expand All @@ -564,7 +560,7 @@ async def deactivate_course(db: dbDep, course_id: int, teacher: TeacherAuthDep):
- `HTTPException 403`: If the teacher does not have access to the course.
- `HTTPException 400`: If there are students enrolled in the course.
"""
course = await get_course_common_info(db, course_id)
course = await crud_course.get_course_common_info(db, course_id)
user_has_access, msg = crud_teacher.validate_course_access(course, teacher)
if not user_has_access:
raise HTTPException(
Expand All @@ -577,7 +573,7 @@ async def deactivate_course(db: dbDep, course_id: int, teacher: TeacherAuthDep):
status_code=status.HTTP_400_BAD_REQUEST,
detail="Cannot deactivate a course with enrolled students"
)
await hide_course(db, course)
await crud_course.hide_course(db, course)
return


Expand Down
10 changes: 5 additions & 5 deletions src/app/crud/crud_teacher.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ async def get_entire_course(db: Session, course: Course, teacher: Teacher, sort:
if sort_by:
sections_query = sections_query.order_by(
getattr(Section, sort_by).desc() if sort and sort == 'desc' else getattr(Section, sort_by).asc()
)
) # when sort_by is provided without sort, the sections are sorted in ascending order by default

sections = sections_query.all()
for section in sections:
Expand Down Expand Up @@ -182,8 +182,8 @@ async def calculate_student_progresses(db: Session, courses_with_students: List[

async def get_courses_reports(db: Session, teacher: Teacher, min_progress: float, sort: str = None):
courses_query = (
select(Course)
.options(joinedload(Course.students_enrolled))
select(Course) #preventing the "N+1 problem"
.options(joinedload(Course.students_enrolled)) #all related students are fetched in the same query as the courses
.where(Course.owner_id == teacher.teacher_id)
)

Expand All @@ -192,8 +192,8 @@ async def get_courses_reports(db: Session, teacher: Teacher, min_progress: float
elif sort == 'desc':
courses_query = courses_query.order_by(Course.course_id.desc())

result = db.execute(courses_query)
courses_with_students = result.scalars().unique().all()
result = db.execute(courses_query) #result contains raw SQL rows
courses_with_students = result.scalars().all() # scalars() converts raw SQL rows into ORM objects(Course instances)

student_progress_dict = await calculate_student_progresses(db, courses_with_students)
courses_reports = generate_reports(courses_with_students, student_progress_dict, min_progress)
Expand Down
4 changes: 2 additions & 2 deletions src/app/db/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ class Student(Base):
courses_enrolled: Mapped[List['Course']] = relationship(
'Course',
secondary='students_courses',
primaryjoin=f"and_(Student.student_id == foreign(StudentCourse.student_id), StudentCourse.status == {
Status.active.value})",
primaryjoin=f"and_(Student.student_id == foreign(StudentCourse.student_id), "
f"StudentCourse.status == {Status.active.value})",
secondaryjoin="Course.course_id == foreign(StudentCourse.course_id)",
back_populates="students_enrolled"
)
Expand Down
3 changes: 2 additions & 1 deletion src/app/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from tests.api.api_v1.endpoints.student_test import dummy_student
from tests.api.api_v1.endpoints.teacher_test import dummy_teacher
from tests import dummies
from typing import Generator


@pytest.fixture
Expand All @@ -17,7 +18,7 @@ def client():


@pytest.fixture(scope='function')
def db() -> Session:
def db() -> Generator:
"""
This fixture returns a session connected to an in-memory SQLite db.
It's intended for use in tests that require db access but should not
Expand Down

0 comments on commit b92424e

Please sign in to comment.