From e09a33c84b7f52ac4d2faab31e94828b8d0792e7 Mon Sep 17 00:00:00 2001 From: jbkyei1 Date: Sun, 5 Apr 2026 19:33:09 +0000 Subject: [PATCH 1/2] Refactor user permissions and enhance stack, tag, and techie of the month routes --- api/api_models/projects.py | 13 ++++--- api/api_models/skills.py | 4 +- api/routes/announcements.py | 32 ++++++++++++++-- api/routes/coding_challenges.py | 13 +++---- api/routes/email_templates.py | 5 ++- api/routes/feeds.py | 13 ++++--- api/routes/profile_page.py | 26 +++++++++---- api/routes/project.py | 8 ++-- api/routes/skills.py | 22 +++++------ api/routes/stacks.py | 8 ++-- api/routes/tags.py | 8 ++-- api/routes/techieotm.py | 8 ++-- api/routes/technical_task.py | 18 ++++----- api/routes/users.py | 16 ++++---- api/routes/weekly_meetings.py | 13 +++---- db/repository/skills.py | 18 +++++++-- db/repository/users.py | 10 ++++- services/project_service.py | 59 ++++++++++++++++++++++++----- services/skill_service.py | 30 +++++++++------ services/user_service.py | 5 ++- test/conftest.py | 8 +++- test/test_announcements.py | 18 ++++++--- test/test_coding_challenges.py | 16 +++++++- test/test_email_templates.py | 2 +- test/test_feeds.py | 24 +++++++----- test/test_org_chart.py | 15 ++++++-- test/test_profile_page.py | 4 +- test/test_projects.py | 16 ++++---- test/test_skill_service_response.py | 14 +++---- test/test_skills.py | 18 +++++---- test/test_stacks.py | 8 ++-- test/test_techieotm.py | 15 ++++++-- test/test_weekly_meetings.py | 16 +++++++- utils/permissions.py | 28 +++++++------- 34 files changed, 344 insertions(+), 187 deletions(-) diff --git a/api/api_models/projects.py b/api/api_models/projects.py index 52485e9..572a22e 100644 --- a/api/api_models/projects.py +++ b/api/api_models/projects.py @@ -51,12 +51,13 @@ class ProjectResponse(ProjectBase): class UpdateProject(BaseModel): - name: Optional[str] - description: Optional[str] - project_type: Optional[ProjectType] - project_priority: Optional[ProjectPriority] - project_tools: Optional[List[int]] = Field(None) - manager_id: Optional[int] + name: Optional[str] = None + description: Optional[str] = None + project_type: Optional[ProjectType] = None + project_priority: Optional[ProjectPriority] = None + project_tools: Optional[List[int]] = None + stacks: Optional[List[int]] = None + manager_id: Optional[int] = None class ProjectMember(BaseModel): diff --git a/api/api_models/skills.py b/api/api_models/skills.py index fc5118f..05752ac 100644 --- a/api/api_models/skills.py +++ b/api/api_models/skills.py @@ -8,8 +8,8 @@ class SkillBase(BaseModel): model_config = ConfigDict(from_attributes=True) @field_validator('name') - def convert_to_lower_case(cls, v): - return v.lower() + def normalize_name(cls, v): + return v.title() class SkillCreate(SkillBase): diff --git a/api/routes/announcements.py b/api/routes/announcements.py index a830c6c..ee5345d 100644 --- a/api/routes/announcements.py +++ b/api/routes/announcements.py @@ -1,6 +1,6 @@ from typing import Annotated, Any -from fastapi import APIRouter, Depends, status +from fastapi import APIRouter, Depends, File, HTTPException, UploadFile, status from fastapi_pagination.ext.sqlalchemy import paginate from fastapi_pagination.links import Page from sqlalchemy.orm import Session @@ -9,18 +9,42 @@ from db.database import get_db from db.repository.announcements import AnnouncementRepository from services.announcement_service import AnnouncementService -from utils.permissions import is_admin +from utils.cloudinary import upload_file +from utils.permissions import is_admin, user_accepted +from utils.utils import is_image_file announcement_route = APIRouter(tags=["Announcements"], prefix="/announcements") DBSession = Annotated[Session, Depends(get_db)] AdminUser = Annotated[Any, Depends(is_admin)] +AcceptedUser = Annotated[Any, Depends(user_accepted)] def _service(db: Session) -> AnnouncementService: return AnnouncementService(AnnouncementRepository(db)) +@announcement_route.post("/image", status_code=status.HTTP_200_OK) +async def upload_announcement_image( + file: UploadFile = File(...), + _admin: Any = Depends(is_admin), +): + if not is_image_file(file.filename): + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid file format. Please upload an image.", + ) + resource_type = "announcement" + upload_folder = "announcements" + url = await upload_file(file, resource_type, upload_folder) + if not url: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail="Failed to upload image", + ) + return {"url": url} + + @announcement_route.post("/", status_code=status.HTTP_201_CREATED, response_model=AnnouncementResponse) def create_announcement( announcement: AnnouncementCreate, @@ -31,13 +55,13 @@ def create_announcement( @announcement_route.get("/", status_code=status.HTTP_200_OK, response_model=Page[AnnouncementResponse]) -def get_announcements(db: DBSession): +def get_announcements(current_user: AcceptedUser, db: DBSession): return paginate(db, _service(db).get_all_query()) @announcement_route.get("/{announcement_id}", status_code=status.HTTP_200_OK, response_model=AnnouncementResponse) -def get_announcement_by_id(announcement_id: int, db: DBSession): +def get_announcement_by_id(announcement_id: int, current_user: AcceptedUser, db: DBSession): return _service(db).get_by_id(announcement_id) diff --git a/api/routes/coding_challenges.py b/api/routes/coding_challenges.py index fc1d8a2..7622192 100644 --- a/api/routes/coding_challenges.py +++ b/api/routes/coding_challenges.py @@ -11,8 +11,7 @@ from db.database import get_db from db.repository.coding_challenges import CodingChallengeRepository from services.coding_challenge_service import CodingChallengeService -from utils.permissions import is_admin -from utils.oauth2 import get_current_user +from utils.permissions import is_admin, user_accepted coding_challenge_route = APIRouter(tags=["Coding Challenges"], prefix="/coding-challenges") @@ -32,14 +31,14 @@ def create_challenge( @coding_challenge_route.get("/latest", status_code=status.HTTP_200_OK, response_model=CodingChallengeResponse | None) -def get_latest_challenge(db: Session = Depends(get_db), current_user=Depends(get_current_user)): - """Get the most recent coding challenge - All users""" +def get_latest_challenge(db: Session = Depends(get_db), current_user=Depends(user_accepted)): + """Get the most recent coding challenge - Accepted users""" return _service(db).get_latest() @coding_challenge_route.get("/", status_code=status.HTTP_200_OK, response_model=Page[CodingChallengeResponse]) -def get_all_challenges(db: Session = Depends(get_db), current_user=Depends(get_current_user)): - """Get all coding challenges - All users""" +def get_all_challenges(db: Session = Depends(get_db), current_user=Depends(user_accepted)): + """Get all coding challenges - Accepted users""" return paginate(db, _service(db).get_all_query()) @@ -47,7 +46,7 @@ def get_all_challenges(db: Session = Depends(get_db), current_user=Depends(get_c def get_challenge_by_id( challenge_id: int, db: Session = Depends(get_db), - current_user=Depends(get_current_user) + current_user=Depends(user_accepted) ): """Get a specific challenge by ID""" return _service(db).get_by_id(challenge_id) diff --git a/api/routes/email_templates.py b/api/routes/email_templates.py index 9a001cb..0607298 100644 --- a/api/routes/email_templates.py +++ b/api/routes/email_templates.py @@ -24,12 +24,13 @@ def create_email_template(template: EmailTemplateCreate, db: Session = Depends(g @email_templates_route.get("/{template_id}", response_model=EmailTemplateResponse) -def read_email_template(template_id: int, db: Session = Depends(get_db)): +def read_email_template(template_id: int, db: Session = Depends(get_db), + current_user=Depends(is_admin)): return _service(db).get_by_id(template_id) @email_templates_route.get("/", response_model=Page[EmailTemplateResponse]) -def read_all_email_templates(db: Session = Depends(get_db)): +def read_all_email_templates(db: Session = Depends(get_db), current_user=Depends(is_admin)): return paginate(db, _service(db).get_all_query()) diff --git a/api/routes/feeds.py b/api/routes/feeds.py index b05ebd1..6ecf183 100644 --- a/api/routes/feeds.py +++ b/api/routes/feeds.py @@ -7,7 +7,7 @@ from db.database import get_db from db.repository.feeds import FeedRepository from services.feed_service import FeedService -from utils.oauth2 import get_current_user +from utils.permissions import user_accepted feed_route = APIRouter(tags=["Feed"], prefix="/feed") @@ -18,27 +18,28 @@ def _service(db: Session) -> FeedService: @feed_route.post("/", status_code=status.HTTP_201_CREATED, response_model=Feeds) async def create_feed(content: str = Form(...), feed_pic_url: UploadFile = File(None), - current_user=Depends(get_current_user), db: Session = Depends(get_db)): + current_user=Depends(user_accepted), db: Session = Depends(get_db)): return await _service(db).create_feed(current_user.id, current_user.username, content, feed_pic_url) @feed_route.put("/{feed_id}", status_code=status.HTTP_200_OK, response_model=Feeds) def update_feed_by_id(feed_id: int, updated_feed: FeedUpdate, - db: Session = Depends(get_db), current_user=Depends(get_current_user)): + db: Session = Depends(get_db), current_user=Depends(user_accepted)): return _service(db).update_feed(feed_id, current_user.id, updated_feed.model_dump()) @feed_route.delete("/{feed_id}", status_code=status.HTTP_204_NO_CONTENT) def delete_feed_by_id(feed_id: int, db: Session = Depends(get_db), - current_user=Depends(get_current_user)): + current_user=Depends(user_accepted)): _service(db).delete_feed(feed_id, current_user.id) @feed_route.get("/{feed_id}", status_code=status.HTTP_200_OK, response_model=Feeds) -def get_feed_by_id(feed_id: int, db: Session = Depends(get_db)): +def get_feed_by_id(feed_id: int, db: Session = Depends(get_db), + current_user=Depends(user_accepted)): return _service(db).get_feed(feed_id) @feed_route.get("/", response_model=Page[Feeds]) -def get_all_feeds(db: Session = Depends(get_db)): +def get_all_feeds(db: Session = Depends(get_db), current_user=Depends(user_accepted)): return paginate(db, _service(db).get_all_query()) diff --git a/api/routes/profile_page.py b/api/routes/profile_page.py index 7ae4db3..95ea95a 100644 --- a/api/routes/profile_page.py +++ b/api/routes/profile_page.py @@ -26,8 +26,7 @@ class BatchStatusUpdateResponse(BaseModel): from db.repository.technical_tasks import TechnicalTaskSubmissionRepository from db.repository.users import UserRepository from services.user_service import UserService -from utils.oauth2 import get_current_user -from utils.permissions import is_admin +from utils.permissions import is_admin, user_accepted profile_route = APIRouter(tags=["User"], prefix="/users") logger = logging.getLogger(__name__) @@ -43,12 +42,12 @@ def _service(db: Session) -> UserService: @profile_route.get("/profile/{user_id}", response_model=ApplicantProfileResponse) async def get_profile(user_id: int, db: Session = Depends(get_db), - current_user: User = Depends(get_current_user)): + current_user: User = Depends(user_accepted)): return _service(db).get_profile(user_id) @profile_route.put("/profile", response_model=ProfileResponse) -async def update_profile(userDetails: ProfileUpdate, current_user: User = Depends(get_current_user), +async def update_profile(userDetails: ProfileUpdate, current_user: User = Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).update_profile(current_user.id, userDetails.model_dump(exclude_unset=True)) @@ -56,8 +55,18 @@ async def update_profile(userDetails: ProfileUpdate, current_user: User = Depend @profile_route.get("/", response_model=Page[ProfileResponse]) def get_all_profile(skill: str = Query(None), stack: str = Query(None), active: Optional[bool] = None, p: Optional[str] = None, - db: Session = Depends(get_db), current_user: User = Depends(get_current_user)): - query = _service(db).build_search_query(skill, stack, active, p) + status: Optional[UserStatus] = Query( + None, + description="Filter users by status. Accepted values are defined by the UserStatus enum." + ), + db: Session = Depends(get_db), current_user: User = Depends(user_accepted)): + query = _service(db).build_search_query( + skill, + stack, + active, + p, + status.value if status is not None else None + ) return paginate(db, query) @@ -69,7 +78,8 @@ def update_profile_status(user_id: int, db: Session = Depends(get_db), @profile_route.get("/user_info", response_model=dict) -def get_user_info(email: str, db: Session = Depends(get_db)): +def get_user_info(email: str, db: Session = Depends(get_db), + current_user: User = Depends(user_accepted)): return _service(db).get_user_info(email) @@ -82,7 +92,7 @@ async def update_user_status(user_id: int, new_status: UserStatus, db: Session = @profile_route.patch("/profile/avatar", response_model=ProfileResponse, status_code=status.HTTP_200_OK) -async def update_avi(current_user: User = Depends(get_current_user), +async def update_avi(current_user: User = Depends(user_accepted), db: Session = Depends(get_db), file: UploadFile = File(...)): return await _service(db).update_avatar(current_user, file) diff --git a/api/routes/project.py b/api/routes/project.py index 8668199..11d6910 100644 --- a/api/routes/project.py +++ b/api/routes/project.py @@ -14,7 +14,7 @@ from db.repository.users import UserRepository from services.project_service import ProjectService from utils.enums import ProjectTeam -from utils.permissions import is_admin, is_project_manager +from utils.permissions import is_admin, is_project_manager, user_accepted project_router = APIRouter(tags=["Project"], prefix="/projects") @@ -45,12 +45,12 @@ def delete(project_id: int, db: Session = Depends(get_db), user: User = Depends( @project_router.get("/{project_id}", status_code=status.HTTP_200_OK, response_model=ProjectResponse) -def get(project_id: int, db: Session = Depends(get_db)): +def get(project_id: int, db: Session = Depends(get_db), current_user=Depends(user_accepted)): return _service(db).get_project(project_id) @project_router.get("/", status_code=status.HTTP_200_OK, response_model=Page[ProjectResponse]) -def get_all(db: Session = Depends(get_db)): +def get_all(db: Session = Depends(get_db), current_user=Depends(user_accepted)): service = _service(db) page = paginate(db, service.get_all_query()) # Enrich members with team data @@ -77,5 +77,5 @@ def remove_user_from_project( @project_router.get("/{project_id}/members", status_code=status.HTTP_200_OK, response_model=List[MembersResponse]) def get_project_members(project_id: int, team: Optional[ProjectTeam] = None, - db: Session = Depends(get_db)): + db: Session = Depends(get_db), current_user=Depends(user_accepted)): return _service(db).get_project_members(project_id, team) diff --git a/api/routes/skills.py b/api/routes/skills.py index ddedf90..55fa258 100644 --- a/api/routes/skills.py +++ b/api/routes/skills.py @@ -2,8 +2,6 @@ from fastapi import APIRouter, Depends, Query, status from fastapi.params import Body -from fastapi_pagination.ext.sqlalchemy import paginate -from fastapi_pagination.links import Page from pydantic import BaseModel from sqlalchemy.orm import Session @@ -11,14 +9,14 @@ from db.database import get_db from db.repository.skills import SkillRepository from services.skill_service import SkillService -from utils.oauth2 import get_current_user -from utils.permissions import is_admin +from utils.permissions import is_admin, user_accepted class SkillCreate(BaseModel): name: str image_url: Optional[str] = None + skill_route = APIRouter(tags=["Skills"], prefix="/skills") @@ -27,35 +25,35 @@ def _service(db: Session) -> SkillService: @skill_route.get("/", response_model=List[Skills], status_code=status.HTTP_200_OK) -def get_skills(user=Depends(get_current_user), db: Session = Depends(get_db)): +def get_skills(user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).get_user_skills(user.id) @skill_route.post("/", response_model=List[Skills], status_code=status.HTTP_201_CREATED) def add_skills(skill_ids: list[int] = Body(...), db: Session = Depends(get_db), - current_user=Depends(get_current_user)): + current_user=Depends(user_accepted)): return _service(db).add_skills(current_user, skill_ids) @skill_route.delete("/{skill_id}", status_code=status.HTTP_204_NO_CONTENT) -def delete_skill_by_id(skill_id: int, user=Depends(get_current_user), +def delete_skill_by_id(skill_id: int, user=Depends(user_accepted), db: Session = Depends(get_db)): _service(db).delete_skill(skill_id) -@skill_route.get("/all", status_code=status.HTTP_200_OK, response_model=Page[Skills]) -def get_all(db: Session = Depends(get_db)): - return paginate(db, _service(db).get_all_query()) +@skill_route.get("/all", status_code=status.HTTP_200_OK, response_model=List[Skills]) +def get_all(db: Session = Depends(get_db), current_user=Depends(user_accepted)): + return _service(db).get_all_flat() @skill_route.post("/data") -def populate_skills(db: Session = Depends(get_db)): +def populate_skills(db: Session = Depends(get_db), _admin=Depends(is_admin)): return _service(db).populate_skills() @skill_route.get("/search", response_model=List[dict], status_code=status.HTTP_200_OK) def search_skills(name: str = Query(..., min_length=1, max_length=50), - db: Session = Depends(get_db)): + db: Session = Depends(get_db), current_user=Depends(user_accepted)): return _service(db).search_skills(name) diff --git a/api/routes/stacks.py b/api/routes/stacks.py index 261850a..2f51c23 100644 --- a/api/routes/stacks.py +++ b/api/routes/stacks.py @@ -6,7 +6,7 @@ from db.models.users import User from db.repository.stacks import StackRepository from services.stack_service import StackService -from utils.permissions import is_admin +from utils.permissions import is_admin, user_accepted stack_router = APIRouter(tags=["Stacks"], prefix="/stacks") @@ -16,7 +16,8 @@ def _service(db: Session) -> StackService: @stack_router.get("/", response_model=list[stack_schemas.Stacks]) -async def list_stacks(db: Session = Depends(get_db), page: int = 1, limit: int = 100): +async def list_stacks(db: Session = Depends(get_db), page: int = 1, limit: int = 100, + current_user=Depends(user_accepted)): return _service(db).list_stacks(page=page, limit=limit) @@ -27,7 +28,8 @@ async def create_stack(stack: stack_schemas.StackCreate, user: User = Depends(is @stack_router.get("/{stack_id}", response_model=stack_schemas.Stacks) -async def read_stack(stack_id: int, db: Session = Depends(get_db)): +async def read_stack(stack_id: int, db: Session = Depends(get_db), + current_user=Depends(user_accepted)): return _service(db).get_by_id(stack_id) diff --git a/api/routes/tags.py b/api/routes/tags.py index 6ad7615..025a704 100644 --- a/api/routes/tags.py +++ b/api/routes/tags.py @@ -6,7 +6,7 @@ from db.database import get_db from db.repository.tags import TagRepository from services.tag_service import TagService -from utils.oauth2 import get_current_user +from utils.permissions import user_accepted tag_route = APIRouter(tags=["User"], prefix="/users") @@ -16,17 +16,17 @@ def _service(db: Session) -> TagService: @tag_route.get("/tags") -def get_current_user_tags(user=Depends(get_current_user), db: Session = Depends(get_db)): +def get_current_user_tags(user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).get_user_tags(user) @tag_route.post("/tags", response_model=Tags, status_code=status.HTTP_201_CREATED) -def create_tag(tag: TagCreate, current_user=Depends(get_current_user), +def create_tag(tag: TagCreate, current_user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).create_tag(current_user, tag.name) @tag_route.delete("/tags/{tag_id}", status_code=status.HTTP_204_NO_CONTENT) -def delete_tag_by_id(tag_id: int, user=Depends(get_current_user), +def delete_tag_by_id(tag_id: int, user=Depends(user_accepted), db: Session = Depends(get_db)): _service(db).delete_tag(tag_id) diff --git a/api/routes/techieotm.py b/api/routes/techieotm.py index 338a81a..569f6f5 100644 --- a/api/routes/techieotm.py +++ b/api/routes/techieotm.py @@ -8,7 +8,7 @@ from db.repository.techieotm import TechieOTMRepository from db.repository.users import UserRepository from services.techieotm_service import TechieOTMService -from utils.permissions import is_admin +from utils.permissions import is_admin, user_accepted techieotm_router = APIRouter(tags=["User"], prefix="/users/techieotm") @@ -26,10 +26,12 @@ def create_techie_of_the_month( @techieotm_router.get("/latest", response_model=TechieOTMResponse) -def get_latest_techie_of_the_month(db: Session = Depends(get_db)): +def get_latest_techie_of_the_month(db: Session = Depends(get_db), + current_user=Depends(user_accepted)): return _service(db).get_latest() @techieotm_router.get("/", response_model=Page[TechieOTMResponse]) -def get_all_techies_of_the_months(db: Session = Depends(get_db)): +def get_all_techies_of_the_months(db: Session = Depends(get_db), + current_user=Depends(user_accepted)): return paginate(db, _service(db).get_all_query()) diff --git a/api/routes/technical_task.py b/api/routes/technical_task.py index 68e08e0..75efe90 100644 --- a/api/routes/technical_task.py +++ b/api/routes/technical_task.py @@ -9,7 +9,7 @@ from db.database import get_db from db.repository.technical_tasks import TechnicalTaskRepository, TechnicalTaskSubmissionRepository from services.technical_task_service import TechnicalTaskService -from utils.permissions import is_admin, user_accepted, get_current_user +from utils.permissions import is_admin, user_accepted tech_task_router = APIRouter(tags=["Applicant Task"], prefix="/applicant/task") sub_tech_task_router = APIRouter(tags=["Applicant Task Submission"], prefix="/applicant/submission") @@ -26,8 +26,7 @@ def create_task(tech_task: TechnicalTaskBase, current_user=Depends(is_admin), @tech_task_router.get("/", status_code=status.HTTP_200_OK, response_model=list[TechnicalTaskResponse]) -def get_tasks(current_user=Depends(get_current_user), db: Session = Depends(get_db), - user_status=Depends(user_accepted)): +def get_tasks(current_user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).get_all_tasks() @@ -40,7 +39,7 @@ def update_task(task_id: int, payload: TechnicalTaskBase, current_user=Depends(i @tech_task_router.get("/{task_id}", status_code=status.HTTP_200_OK, response_model=TechnicalTaskResponse) -def get_task(task_id: int, current_user=Depends(get_current_user), db: Session = Depends(get_db)): +def get_task(task_id: int, current_user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).get_task(task_id) @@ -54,33 +53,32 @@ def delete_task(task_id: int, current_user=Depends(is_admin), db: Session = Depe @sub_tech_task_router.post("/", status_code=status.HTTP_201_CREATED, response_model=TechnicalTaskSubmissionResponse) def create_task_submission(tech_task: TechnicalTaskSubmissionBase, - current_user=Depends(get_current_user), db: Session = Depends(get_db)): + current_user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).create_submission(current_user, dict(tech_task)) @sub_tech_task_router.get("/", status_code=status.HTTP_200_OK, response_model=list[TechnicalTaskSubmissionResponse]) -def get_task_submissions(current_user=Depends(get_current_user), db: Session = Depends(get_db), - user_status=Depends(user_accepted)): +def get_task_submissions(current_user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).get_all_submissions() @sub_tech_task_router.patch("/{submission_id}", status_code=status.HTTP_200_OK, response_model=TechnicalTaskSubmissionResponse) def update_submission(submission_id: int, payload: TechnicalTaskSubmissionBase, - current_user=Depends(get_current_user), db: Session = Depends(get_db)): + current_user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).update_submission(submission_id, payload.model_dump(exclude_unset=True)) @sub_tech_task_router.get("/{submission_id}", status_code=status.HTTP_200_OK, response_model=TechnicalTaskSubmissionResponse) -def get_submission(submission_id: int, current_user=Depends(get_current_user), +def get_submission(submission_id: int, current_user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).get_submission(submission_id) @sub_tech_task_router.get("/{user_id}/users", status_code=status.HTTP_200_OK, response_model=TechnicalTaskSubmissionResponse) -def get_submission_by_user(user_id: int, current_user=Depends(get_current_user), +def get_submission_by_user(user_id: int, current_user=Depends(user_accepted), db: Session = Depends(get_db)): return _service(db).get_submission_by_user(user_id) diff --git a/api/routes/users.py b/api/routes/users.py index 124f2d8..d177962 100644 --- a/api/routes/users.py +++ b/api/routes/users.py @@ -1,14 +1,14 @@ """ Org-chart endpoints. -**Admin-only** (full visibility): - GET /users/org-chart – complete organisational tree - GET /users/{user_id}/manager – manager of any user - GET /users/{user_id}/subordinates – direct reports of any user - GET /users/{user_id}/org-chart – full subtree rooted at any user - PATCH /users/{user_id}/manager – assign / remove a user's manager +**Admin-only** (write / destructive operations): + PATCH /users/{user_id}/manager – assign / remove a user's manager + GET /users/{user_id}/manager – manager of any user (admin) + GET /users/{user_id}/subordinates – direct reports of any user (admin) + GET /users/{user_id}/org-chart – full subtree rooted at any user (admin) -**Authenticated accepted users**: +**Authenticated accepted users** (read-only): + GET /users/org-chart – complete organisational tree GET /users/me/manager – my manager GET /users/me/subordinates – my direct reports GET /users/view/{user_id}/manager – manager of any user @@ -83,7 +83,7 @@ def get_my_subordinates( ) def get_full_org_chart( max_depth: int = Query(default=5, ge=1, le=20), - current_user=Depends(is_admin), + current_user=Depends(user_accepted), service: OrgChartService = Depends(_get_service), ): return service.get_full_org_chart(max_depth) diff --git a/api/routes/weekly_meetings.py b/api/routes/weekly_meetings.py index fcc0d68..94e60a7 100644 --- a/api/routes/weekly_meetings.py +++ b/api/routes/weekly_meetings.py @@ -11,8 +11,7 @@ from db.database import get_db from db.repository.weekly_meetings import WeeklyMeetingRepository from services.weekly_meeting_service import WeeklyMeetingService -from utils.permissions import is_admin -from utils.oauth2 import get_current_user +from utils.permissions import is_admin, user_accepted weekly_meeting_route = APIRouter(tags=["Weekly Meetings"], prefix="/weekly-meetings") @@ -32,14 +31,14 @@ def create_meeting( @weekly_meeting_route.get("/active", status_code=status.HTTP_200_OK, response_model=WeeklyMeetingResponse | None) -def get_active_meeting(db: Session = Depends(get_db), current_user=Depends(get_current_user)): - """Get the current active meeting - All users""" +def get_active_meeting(db: Session = Depends(get_db), current_user=Depends(user_accepted)): + """Get the current active meeting - Accepted users""" return _service(db).get_active() @weekly_meeting_route.get("/", status_code=status.HTTP_200_OK, response_model=Page[WeeklyMeetingResponse]) -def get_all_meetings(db: Session = Depends(get_db), current_user=Depends(get_current_user)): - """Get all meetings - All users""" +def get_all_meetings(db: Session = Depends(get_db), current_user=Depends(user_accepted)): + """Get all meetings - Accepted users""" return paginate(db, _service(db).get_all_query()) @@ -47,7 +46,7 @@ def get_all_meetings(db: Session = Depends(get_db), current_user=Depends(get_cur def get_meeting_by_id( meeting_id: int, db: Session = Depends(get_db), - current_user=Depends(get_current_user) + current_user=Depends(user_accepted) ): """Get a specific meeting by ID""" return _service(db).get_by_id(meeting_id) diff --git a/db/repository/skills.py b/db/repository/skills.py index 783a18a..8d5db11 100644 --- a/db/repository/skills.py +++ b/db/repository/skills.py @@ -9,6 +9,10 @@ from db.repository.base import BaseRepository +def _normalize_skill_name(name: str) -> str: + return name.strip().title() + + class SkillRepository(BaseRepository): model = Skill @@ -49,20 +53,26 @@ def get_all_paginated_query(self) -> Any: return select(Skill).order_by(desc(Skill.created_at)) def get_by_name(self, name: str) -> Optional[Skill]: - return self.db.query(Skill).filter(Skill.name == name).first() + # All skill names are stored title-cased (see upsert/create). Using exact + # match on the normalised form hits the unique index on `name` directly. + normalized = _normalize_skill_name(name) + return self.db.query(Skill).filter(Skill.name == normalized).first() def upsert(self, name: str, image_url: Optional[str]) -> Skill: - skill = self.get_by_name(name) + normalized = _normalize_skill_name(name) + skill = self.get_by_name(normalized) if skill: if not skill.image_url and image_url: skill.image_url = image_url + else: - skill = Skill(name=name, image_url=image_url) + skill = Skill(name=normalized, image_url=image_url) self.db.add(skill) return skill def create(self, name: str, image_url: Optional[str]) -> Skill: - skill = Skill(name=name, image_url=image_url) + normalized = _normalize_skill_name(name) + skill = Skill(name=normalized, image_url=image_url) self.db.add(skill) self.db.commit() self.db.refresh(skill) diff --git a/db/repository/users.py b/db/repository/users.py index bdd7dd1..445fa84 100644 --- a/db/repository/users.py +++ b/db/repository/users.py @@ -65,8 +65,9 @@ def update_avatar(self, user: User, url: str) -> User: return user def build_search_query(self, skill: Optional[str], stack: Optional[str], - active: Optional[bool], p: Optional[str]): - from sqlalchemy import or_ + active: Optional[bool], p: Optional[str], + status: Optional[str] = None): + from sqlalchemy import or_, func query = select(User).order_by(desc(User.created_at)) if skill: @@ -87,12 +88,17 @@ def build_search_query(self, skill: Optional[str], stack: Optional[str], User.is_active.is_(True) & (User.status != "ACCEPTED") ) ) + if status: + query = query.filter(User.status == status) if p: p_escaped = p.replace("%", r"\%").replace("_", r"\_") + full_name = func.concat(User.first_name, " ", User.last_name) query = query.filter( User.username.ilike(f"%{p_escaped}%", escape="\\") | User.first_name.ilike(f"%{p_escaped}%", escape="\\") | User.last_name.ilike(f"%{p_escaped}%", escape="\\") + | User.email.ilike(f"%{p_escaped}%", escape="\\") + | full_name.ilike(f"%{p_escaped}%", escape="\\") ) return query diff --git a/services/project_service.py b/services/project_service.py index f86d9d4..d69706a 100644 --- a/services/project_service.py +++ b/services/project_service.py @@ -55,30 +55,28 @@ def create_project(self, project_data: CreateProject) -> Project: new_project = Project(**data) if project_data.stacks: - seen = [] + # A set has fewer elements than the list only when duplicates exist + if len(set(project_data.stacks)) != len(project_data.stacks): + raise HTTPException(status_code=status.HTTP_406_NOT_ACCEPTABLE, + detail="Duplicate stack ids provided") for stack_id in project_data.stacks: - if stack_id in seen: - raise HTTPException(status_code=status.HTTP_406_NOT_ACCEPTABLE, - detail=f"Duplicate stack id: {stack_id}") stack = self.stack_repo.get_by_id(stack_id) if not stack: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=f"Stack {stack_id} not found") new_project.stacks.append(stack) - seen.append(stack_id) if project_data.project_tools: - seen = [] + # A set has fewer elements than the list only when duplicates exist + if len(set(project_data.project_tools)) != len(project_data.project_tools): + raise HTTPException(status_code=status.HTTP_406_NOT_ACCEPTABLE, + detail="Duplicate skill ids provided") for skill_id in project_data.project_tools: - if skill_id in seen: - raise HTTPException(status_code=status.HTTP_406_NOT_ACCEPTABLE, - detail=f"Duplicate skill id: {skill_id}") skill = self.skill_repo.get_by_id(skill_id) if not skill: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail=f"Skill {skill_id} not found") new_project.project_tools.append(skill) - seen.append(skill_id) saved = self.project_repo.save(new_project) return self._enrich_project_members_with_team(saved) @@ -91,9 +89,50 @@ def update_project(self, project_id: int, update_data: UpdateProject) -> Project if update_data.manager_id != project.manager_id: if not self.user_repo.get_by_id(update_data.manager_id): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Manager not found") + updated = self.project_repo.update( project, update_data.model_dump(exclude=["members", "stacks", "project_tools"], exclude_none=True) ) + + try: + validated_project_tools = None + validated_stacks = None + + if update_data.project_tools is not None: + validated_project_tools = [] + for skill_id in update_data.project_tools: + skill = self.skill_repo.get_by_id(skill_id) + if not skill: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, + detail=f"Skill {skill_id} not found") + validated_project_tools.append(skill) + + if update_data.stacks is not None: + validated_stacks = [] + for stack_id in update_data.stacks: + stack = self.stack_repo.get_by_id(stack_id) + if not stack: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, + detail=f"Stack {stack_id} not found") + validated_stacks.append(stack) + + if validated_project_tools is not None: + updated.project_tools = validated_project_tools + + if validated_stacks is not None: + updated.stacks = validated_stacks + + if validated_project_tools is not None or validated_stacks is not None: + self.project_repo.db.commit() + self.project_repo.db.refresh(updated) + except HTTPException: + self.project_repo.db.rollback() + raise + except Exception as e: + self.project_repo.db.rollback() + raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail=f"Failed to update project relationships: {e}") + return self._enrich_project_members_with_team(updated) def delete_project(self, project_id: int) -> None: diff --git a/services/skill_service.py b/services/skill_service.py index b08ea1e..a69ee42 100644 --- a/services/skill_service.py +++ b/services/skill_service.py @@ -21,13 +21,15 @@ def add_skills(self, current_user: User, skill_ids: list[int]) -> list[Skill]: detail="No skills found with the given ids" ) existing = self.skill_repo.get_user_skill_entries(current_user.id, skill_ids) - if existing: - existing_ids = [e.skill_id for e in existing] + existing_ids = {e.skill_id for e in existing} + new_skills = [skill for skill in db_skills if skill.id not in existing_ids] + if not new_skills: + already = ", ".join(skill.name for skill in db_skills if skill.id in existing_ids) raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"User already has skills with IDs: {existing_ids}" + detail=f"Already in your profile: {already}" ) - return self.skill_repo.add_skills_to_user(current_user.id, db_skills, current_user) + return self.skill_repo.add_skills_to_user(current_user.id, new_skills, current_user) def delete_skill(self, skill_id: int) -> None: entry = self.skill_repo.get_user_skill_entry(skill_id) @@ -41,6 +43,9 @@ def delete_skill(self, skill_id: int) -> None: def get_all_query(self) -> list[Skill]: return self.skill_repo.get_all_paginated_query() + def get_all_flat(self) -> list[Skill]: + return self.skill_repo.get_all_flat() + def populate_skills(self) -> dict: from db.database import create_roles from utils.endpoints_status import create_signup_endpoint @@ -79,26 +84,27 @@ def populate_skills(self) -> dict: def search_skills(self, name: str) -> list[dict]: skills = self.skill_repo.get_all_flat() - threshold = 78 + query = name.strip().lower() + threshold = 70 return [ { - "skill_id": skill.id, - "skill_name": skill.name, - + "id": skill.id, + "name": skill.name, "image_url": skill.image_url or "" } for skill in skills - if fuzz.partial_ratio(name.lower(), skill.name.lower()) >= threshold + if fuzz.partial_ratio(query, skill.name.lower()) >= threshold ] def create_pool_skill(self, name: str, image_url=None): - existing = self.skill_repo.get_by_name(name) + normalized = name.strip().title() + existing = self.skill_repo.get_by_name(normalized) if existing: raise HTTPException( status_code=400, - detail=f"Skill '{name}' already exists" + detail=f"Skill '{existing.name}' already exists in the pool" ) - return self.skill_repo.create(name, image_url) + return self.skill_repo.create(normalized, image_url) def delete_pool_skill(self, skill_id: int) -> None: skill = self.skill_repo.get_by_id(skill_id) diff --git a/services/user_service.py b/services/user_service.py index 2d792de..d128243 100644 --- a/services/user_service.py +++ b/services/user_service.py @@ -44,8 +44,9 @@ def update_profile(self, user_id: int, update_data: dict) -> User: return updated def build_search_query(self, skill: Optional[str], stack: Optional[str], - active: Optional[bool], p: Optional[str]): - return self.user_repo.build_search_query(skill, stack, active, p) + active: Optional[bool], p: Optional[str], + status: Optional[str] = None): + return self.user_repo.build_search_query(skill, stack, active, p, status) def activate_user(self, user_id: int) -> User: user = self.user_repo.get_by_id(user_id) diff --git a/test/conftest.py b/test/conftest.py index e0170f4..1bf9382 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -18,7 +18,7 @@ ForgotPasswordRequest, # UserSignUp ) -from utils.enums import EmailTemplateName +from utils.enums import EmailTemplateName, UserStatus from utils.tools import tools as skills_data from db.models.skills import Skill from db.models.projects import Project @@ -114,7 +114,7 @@ def create_signup_endpoint(session): @pytest.fixture -def test_user(client): +def test_user(client, session): user = { "username": "slightlytechie1", "first_name": "Slightly", @@ -134,6 +134,10 @@ def test_user(client): assert res.status_code == 201 new_user = res.json() new_user["password"] = user.get("password") + # Admin user must be ACCEPTED for the is_admin → user_accepted guard to pass + db_user = session.query(User).filter(User.id == new_user["id"]).first() + db_user.status = UserStatus.ACCEPTED + session.commit() return new_user diff --git a/test/test_announcements.py b/test/test_announcements.py index ea52dd5..31cc2c3 100644 --- a/test/test_announcements.py +++ b/test/test_announcements.py @@ -2,8 +2,10 @@ from api.api_models.announcements import AnnouncementResponse -def test_get_all_announcements(client, test_announcements): - response = client.get("/api/v1/announcements?page=1&size=50") +def test_get_all_announcements(client, test_user, test_announcements): + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + response = client.get("/api/v1/announcements?page=1&size=50", headers={"Authorization": f"Bearer {token}"}) assert response.status_code == 200 announcements = response.json() assert len(announcements["items"]) == 4 @@ -12,8 +14,10 @@ def test_get_all_announcements(client, test_announcements): assert announcements["items"][1]["content"] == test_announcements[1].content -def test_get_one_announcement(client, test_announcements): - res = client.get(f"/api/v1/announcements/{test_announcements[0].id}") +def test_get_one_announcement(client, test_user, test_announcements): + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + res = client.get(f"/api/v1/announcements/{test_announcements[0].id}", headers={"Authorization": f"Bearer {token}"}) announcement = AnnouncementResponse(**res.json()) assert announcement.id == test_announcements[0].id assert announcement.title == test_announcements[0].title @@ -21,8 +25,10 @@ def test_get_one_announcement(client, test_announcements): assert res.status_code == 200 -def test_get_one_announcement_does_not_exist(client): - res = client.get("api/v1/announcements/100") +def test_get_one_announcement_does_not_exist(client, test_user): + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + res = client.get("api/v1/announcements/100", headers={"Authorization": f"Bearer {token}"}) assert res.status_code == 404 diff --git a/test/test_coding_challenges.py b/test/test_coding_challenges.py index 73f5cd6..6197be2 100644 --- a/test/test_coding_challenges.py +++ b/test/test_coding_challenges.py @@ -28,7 +28,13 @@ def test_non_admin_cannot_create_challenge(client, test_user1): assert res.status_code == 403 -def test_authenticated_user_can_get_challenges(client, test_user, test_user1): +def test_authenticated_user_can_get_challenges(client, test_user, test_user1, session): + from db.models.users import User + from utils.enums import UserStatus + db_user1 = session.query(User).filter(User.id == test_user1["id"]).first() + db_user1.status = UserStatus.ACCEPTED + session.commit() + admin_headers = _auth_header(client, test_user["email"], test_user["password"]) user_headers = _auth_header(client, test_user1["email"], test_user1["password"]) @@ -44,7 +50,13 @@ def test_authenticated_user_can_get_challenges(client, test_user, test_user1): assert len(list_res.json()["items"]) >= 1 -def test_get_latest_challenge(client, test_user, test_user1): +def test_get_latest_challenge(client, test_user, test_user1, session): + from db.models.users import User + from utils.enums import UserStatus + db_user1 = session.query(User).filter(User.id == test_user1["id"]).first() + db_user1.status = UserStatus.ACCEPTED + session.commit() + admin_headers = _auth_header(client, test_user["email"], test_user["password"]) user_headers = _auth_header(client, test_user1["email"], test_user1["password"]) diff --git a/test/test_email_templates.py b/test/test_email_templates.py index a704ca3..35b11e9 100644 --- a/test/test_email_templates.py +++ b/test/test_email_templates.py @@ -69,5 +69,5 @@ def test_delete_email_template(client, test_email_templates, test_user): response = client.delete(f"/api/v1/email-templates/{template_id}",headers={'Authorization': f'Bearer {token}'}) assert response.status_code == 204 - response = client.get(f"/api/v1/email-templates/{template_id}") + response = client.get(f"/api/v1/email-templates/{template_id}", headers={'Authorization': f'Bearer {token}'}) assert response.status_code == 404 diff --git a/test/test_feeds.py b/test/test_feeds.py index eb26e53..f6c5793 100644 --- a/test/test_feeds.py +++ b/test/test_feeds.py @@ -2,27 +2,33 @@ from api.api_models.user import Feeds, FeedUpdate -def test_get_all_feeds(client, test_feeds): - response = client.get("/api/v1/feed/?page=1&size=50") +def test_get_all_feeds(client, test_user, test_feeds): + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + response = client.get("/api/v1/feed/?page=1&size=50", headers={"Authorization": f"Bearer {token}"}) feeds = response.json() - + assert len(feeds["items"]) == 4 assert response.status_code == 200 assert feeds["items"][1]["content"] == test_feeds[1].content - assert feeds["items"][1]["user"]["id"] == test_feeds[1].user.id + assert feeds["items"][1]["user"]["id"] == test_feeds[1].user_id -def test_get_one_feed(client, test_feeds): - res = client.get(f"/api/v1/feed/{test_feeds[0].id}") +def test_get_one_feed(client, test_user, test_feeds): + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + res = client.get(f"/api/v1/feed/{test_feeds[0].id}", headers={"Authorization": f"Bearer {token}"}) feed = Feeds(**res.json()) assert feed.id == test_feeds[0].id assert feed.content == test_feeds[0].content - assert feed.user.id == test_feeds[0].user.id + assert feed.user.id == test_feeds[0].user_id assert res.status_code == 200 -def test_get_one_feed_does_not_exist(client, test_feeds): - res = client.get(f"api/v1/feed/10000") +def test_get_one_feed_does_not_exist(client, test_user, test_feeds): + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + res = client.get(f"api/v1/feed/10000", headers={"Authorization": f"Bearer {token}"}) assert res.status_code == 404 diff --git a/test/test_org_chart.py b/test/test_org_chart.py index bc3741e..e1b05ab 100644 --- a/test/test_org_chart.py +++ b/test/test_org_chart.py @@ -56,6 +56,7 @@ def accepted_admin_headers(client, test_user, session): @pytest.fixture def org_tree(client, test_user, test_user1, session, admin_headers): """Build a small org tree: test_user -> test_user1 (test_user is manager of test_user1).""" + _make_admin_accepted(session, test_user) res = client.patch( f"/api/v1/users/{test_user1['id']}/manager", json={"manager_id": test_user["id"]}, @@ -70,8 +71,8 @@ def org_tree(client, test_user, test_user1, session, admin_headers): # =========================================================================== -class TestAdminGetFullOrgChart: - """GET /api/v1/users/org-chart (admin only)""" +class TestGetFullOrgChart: + """GET /api/v1/users/org-chart (accepted users only)""" def test_returns_roots(self, client, test_user, test_user1, admin_headers, session): # Make users ACCEPTED so they appear in org chart @@ -108,7 +109,15 @@ def test_max_depth_param(self, client, org_tree, admin_headers): res = client.get("/api/v1/users/org-chart?max_depth=1", headers=admin_headers) assert res.status_code == 200 - def test_forbidden_for_non_admin(self, client, user_headers): + def test_accepted_non_admin_can_access(self, client, test_user1, session): + # Endpoint is open to all accepted users, not just admins + _make_admin_accepted(session, test_user1) + headers = _auth_header(client, test_user1["email"], test_user1["password"]) + res = client.get("/api/v1/users/org-chart", headers=headers) + assert res.status_code == 200 + + def test_forbidden_for_non_accepted_user(self, client, user_headers): + # Users whose status is not ACCEPTED are denied res = client.get("/api/v1/users/org-chart", headers=user_headers) assert res.status_code == 403 diff --git a/test/test_profile_page.py b/test/test_profile_page.py index 325b77a..f5d631b 100644 --- a/test/test_profile_page.py +++ b/test/test_profile_page.py @@ -141,10 +141,12 @@ def test_activate_invalid_user_profile(client, test_user): def test_get_user_info(client, test_user): # Given a test_user email = test_user["email"] + login_res = client.post("/api/v1/users/login", data={"username": email, "password": test_user["password"]}) + token = login_res.json()["token"] # When the test_user requests user information based on email response = client.get( - f"/api/v1/users/user_info?email={email}" + f"/api/v1/users/user_info?email={email}", headers={"Authorization": f"Bearer {token}"} ) # Then the response should be successful with a 200 OK status code diff --git a/test/test_projects.py b/test/test_projects.py index 89a562c..2b32bfb 100644 --- a/test/test_projects.py +++ b/test/test_projects.py @@ -57,29 +57,29 @@ def test_create_project_not_authorized(client, test_projects): assert res.status_code == 401 -def test_get_all_projects(client, test_projects): - response = client.get(f"{project_url}?page=1&size=2") +def test_get_all_projects(client, user_cred, test_projects): + response = client.get(f"{project_url}?page=1&size=2", headers={"Authorization": f"{user_cred.token_type} {user_cred.token}"}) assert response.status_code == 200 projects = response.json() assert len(projects["items"]) == 2 - assert projects["items"][0]["name"] == test_projects[0].name + assert projects["items"][0]["name"] == test_projects[0].name -def test_get_project(session, client, test_projects): +def test_get_project(session, client, user_cred, test_projects): url = project_url + str(test_projects[0].id) - res = client.get(url) + res = client.get(url, headers={"Authorization": f"{user_cred.token_type} {user_cred.token}"}) res_data = res.json() assert res.status_code == status.HTTP_200_OK assert res_data['name'] == test_projects[0].name -def test_get_project_not_found(session, client, test_projects): +def test_get_project_not_found(session, client, user_cred, test_projects): url = project_url + str(100) - res = client.get(url) + res = client.get(url, headers={"Authorization": f"{user_cred.token_type} {user_cred.token}"}) assert res.status_code == status.HTTP_404_NOT_FOUND @@ -241,7 +241,7 @@ def test_remove_user_project_unauthorized(client, test_projects, test_user, test def test_get_all_team_members(client, user_cred, test_user1, test_projects): test_add_user_project(client, test_projects, user_cred, test_user1) url = project_url + str(1) + "/members?team=FRONTEND" - res = client.get(url) + res = client.get(url, headers={"Authorization": f"{user_cred.token_type} {user_cred.token}"}) members = res.json() assert res.status_code == 200 diff --git a/test/test_skill_service_response.py b/test/test_skill_service_response.py index 4c0db39..61ffc59 100644 --- a/test/test_skill_service_response.py +++ b/test/test_skill_service_response.py @@ -9,7 +9,7 @@ class TestSkillServiceResponseFormat: """Test that search_skills returns consistent response format""" def test_search_skills_returns_correct_field_names(self): - """search_skills should return skill_id, skill_name, image_url""" + """search_skills should return id, name, image_url""" skill_repo = Mock(spec=SkillRepository) service = SkillService(skill_repo) @@ -29,12 +29,12 @@ def test_search_skills_returns_correct_field_names(self): assert isinstance(result[0], dict) # Verify field names - assert "skill_id" in result[0] - assert "skill_name" in result[0] + assert "id" in result[0] + assert "name" in result[0] assert "image_url" in result[0] def test_search_skills_maps_model_fields_correctly(self): - """search_skills should map skill model id→skill_id, name→skill_name""" + """search_skills should map skill model id→id, name→name""" skill_repo = Mock(spec=SkillRepository) service = SkillService(skill_repo) @@ -47,8 +47,8 @@ def test_search_skills_maps_model_fields_correctly(self): result = service.search_skills("react") - assert result[0]["skill_id"] == 5 - assert result[0]["skill_name"] == "React" + assert result[0]["id"] == 5 + assert result[0]["name"] == "React" assert result[0]["image_url"] == "https://example.com/react.png" def test_search_skills_handles_null_image_url(self): @@ -101,5 +101,5 @@ def test_search_skills_fuzzy_matching(self): # Should find JavaScript at minimum assert len(result) > 0 # Should contain JavaScript - found_js = any(s["skill_name"] == "JavaScript" for s in result) + found_js = any(s["name"] == "JavaScript" for s in result) assert found_js diff --git a/test/test_skills.py b/test/test_skills.py index b1f3c6b..653897b 100644 --- a/test/test_skills.py +++ b/test/test_skills.py @@ -4,16 +4,20 @@ client = TestClient(app) -def test_populate_skills(client): - res = client.post("/api/v1/skills/data") +def test_populate_skills(client, test_user): + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + res = client.post("/api/v1/skills/data", headers={"Authorization": f"Bearer {token}"}) assert res.status_code == 200 assert res.json()["message"] == "Skills table populated successfully!" -def test_get_all_skills(client, populate_skills): - res = client.get("/api/v1/skills/all") +def test_get_all_skills(client, test_user, populate_skills): + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + res = client.get("/api/v1/skills/all", headers={"Authorization": f"Bearer {token}"}) assert res.status_code == 200 - assert len(res.json()["items"]) == 50 + assert len(res.json()) == len(populate_skills) # this is the same as test_add_skill @@ -91,12 +95,12 @@ def test_admin_can_create_skill_in_pool(client, test_user): res = client.post( "/api/v1/skills/pool", - json={"name": "SuperNewSkill", "image_url": "https://example.com/s.png"}, + json={"name": "React Native", "image_url": "https://example.com/s.png"}, headers={"Authorization": f"Bearer {token}"}, ) assert res.status_code == 201 - assert res.json()["name"] == "SuperNewSkill" + assert res.json()["name"] == "React Native" def test_non_admin_cannot_create_skill_in_pool(client, test_user1): diff --git a/test/test_stacks.py b/test/test_stacks.py index 4d8c1bc..a0ff1f5 100644 --- a/test/test_stacks.py +++ b/test/test_stacks.py @@ -21,7 +21,7 @@ def create_stack(name): -def test_list_stacks(session, client, stack_factory): +def test_list_stacks(session, client, user_cred, stack_factory): url = app.url_path_for("list_stacks") # list of stack names @@ -29,7 +29,7 @@ def test_list_stacks(session, client, stack_factory): for stack in stack_names: stack_factory(stack) - res = client.get(url) + res = client.get(url, headers={"Authorization": f"{user_cred.token_type} {user_cred.token}"}) res_data = res.json() assert res.status_code == status.HTTP_200_OK @@ -58,12 +58,12 @@ def test_create_stack_no_user(session, client): -def test_read_stack(session, client, stack_factory): +def test_read_stack(session, client, user_cred, stack_factory): # create stack stack = stack_factory("backend") url = app.url_path_for("read_stack", stack_id=stack.id) - res = client.get(url) + res = client.get(url, headers={"Authorization": f"{user_cred.token_type} {user_cred.token}"}) res_data = res.json() assert res.status_code == status.HTTP_200_OK diff --git a/test/test_techieotm.py b/test/test_techieotm.py index 522d4d7..4023b5f 100644 --- a/test/test_techieotm.py +++ b/test/test_techieotm.py @@ -76,14 +76,19 @@ def test_get_latest_techie_of_the_month(client, test_user, session): latest_techieotm = TechieOTM(user_id=1, points=200) db.add(latest_techieotm) db.commit() + db.refresh(latest_techieotm) + expected_id = latest_techieotm.id + expected_user_id = latest_techieotm.user_id - response = client.get("/api/v1/users/techieotm/latest") + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + response = client.get("/api/v1/users/techieotm/latest", headers={"Authorization": f"Bearer {token}"}) assert response.status_code == status.HTTP_200_OK techieotm_response = response.json() - assert techieotm_response["id"] == latest_techieotm.id - assert techieotm_response["user"]["id"] == latest_techieotm.user_id + assert techieotm_response["id"] == expected_id + assert techieotm_response["user"]["id"] == expected_user_id def test_get_all_techies_of_the_months(client, test_user, session): @@ -94,7 +99,9 @@ def test_get_all_techies_of_the_months(client, test_user, session): db.add_all(techieotms) db.commit() - response = client.get("/api/v1/users/techieotm/?page=1&size=2") + login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) + token = login_res.json()["token"] + response = client.get("/api/v1/users/techieotm/?page=1&size=2", headers={"Authorization": f"Bearer {token}"}) assert response.status_code == status.HTTP_200_OK techieotm_response = response.json() diff --git a/test/test_weekly_meetings.py b/test/test_weekly_meetings.py index de5a1cc..952fdf8 100644 --- a/test/test_weekly_meetings.py +++ b/test/test_weekly_meetings.py @@ -27,7 +27,13 @@ def test_non_admin_cannot_create_meeting(client, test_user1): assert res.status_code == 403 -def test_authenticated_user_can_get_meetings(client, test_user, test_user1): +def test_authenticated_user_can_get_meetings(client, test_user, test_user1, session): + from db.models.users import User + from utils.enums import UserStatus + db_user1 = session.query(User).filter(User.id == test_user1["id"]).first() + db_user1.status = UserStatus.ACCEPTED + session.commit() + admin_headers = _auth_header(client, test_user["email"], test_user["password"]) user_headers = _auth_header(client, test_user1["email"], test_user1["password"]) @@ -43,7 +49,13 @@ def test_authenticated_user_can_get_meetings(client, test_user, test_user1): assert len(list_res.json()["items"]) >= 1 -def test_get_active_meeting(client, test_user, test_user1): +def test_get_active_meeting(client, test_user, test_user1, session): + from db.models.users import User + from utils.enums import UserStatus + db_user1 = session.query(User).filter(User.id == test_user1["id"]).first() + db_user1.status = UserStatus.ACCEPTED + session.commit() + admin_headers = _auth_header(client, test_user["email"], test_user["password"]) user_headers = _auth_header(client, test_user1["email"], test_user1["password"]) diff --git a/utils/permissions.py b/utils/permissions.py index 4e98aee..1c9bb9d 100644 --- a/utils/permissions.py +++ b/utils/permissions.py @@ -20,8 +20,18 @@ def is_authenticated(user: UserResponse = Depends(get_current_user)): return user +def user_accepted(user: UserResponse = Depends(get_current_user)): + """Only active and accepted users can access protected resources""" + if not user.is_active or user.status != UserStatus.ACCEPTED: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="You do not have access to this resource" + ) + return user + + # Admin permission dependency -def is_admin(user: UserResponse = Depends(is_authenticated)): +def is_admin(user: UserResponse = Depends(user_accepted)): if not user.role or user.role.name != RoleChoices.ADMIN: raise ForbiddenError() @@ -31,7 +41,7 @@ def is_admin(user: UserResponse = Depends(is_authenticated)): def is_project_manager( request: Request, db: Session = Depends(get_db), - user: UserResponse = Depends(get_current_user), + user: UserResponse = Depends(user_accepted), ): # Allow admins if user.role and user.role.name == RoleChoices.ADMIN: @@ -44,9 +54,7 @@ def is_project_manager( Project.id == int(project_id), Project.manager_id == user.id ) if not project.first(): - raise HTTPException( - status_code=403, detail="Only the project manager can perform this action" - ) + raise ForbiddenError() return user @@ -79,13 +87,3 @@ def is_owner(user, obj): return True raise ForbiddenError() - - -def user_accepted(user: UserResponse = Depends(get_current_user)): - """Only active and accepted users can access protected resources""" - if not user.is_active or user.status != UserStatus.ACCEPTED: - raise HTTPException( - status_code=status.HTTP_403_FORBIDDEN, - detail="You do not have access to this resource" - ) - return user From e8c3fcbaf9091a8c4a6bd10b079cbf5deafd9e12 Mon Sep 17 00:00:00 2001 From: jbkyei1 Date: Sun, 5 Apr 2026 19:57:29 +0000 Subject: [PATCH 2/2] fix: Normalize skill name input and improve error handling for announcements and feeds routes --- api/api_models/skills.py | 2 +- api/routes/announcements.py | 10 ++++++++ services/project_service.py | 51 ++++++++++++++++++++----------------- test/test_announcements.py | 2 +- test/test_feeds.py | 2 +- 5 files changed, 40 insertions(+), 27 deletions(-) diff --git a/api/api_models/skills.py b/api/api_models/skills.py index 05752ac..fcace75 100644 --- a/api/api_models/skills.py +++ b/api/api_models/skills.py @@ -9,7 +9,7 @@ class SkillBase(BaseModel): @field_validator('name') def normalize_name(cls, v): - return v.title() + return v.strip().title() class SkillCreate(SkillBase): diff --git a/api/routes/announcements.py b/api/routes/announcements.py index ee5345d..96b5026 100644 --- a/api/routes/announcements.py +++ b/api/routes/announcements.py @@ -24,6 +24,11 @@ def _service(db: Session) -> AnnouncementService: return AnnouncementService(AnnouncementRepository(db)) +_ALLOWED_IMAGE_CONTENT_TYPES = { + "image/jpeg", "image/png", "image/gif", "image/webp", "image/svg+xml", "image/bmp", +} + + @announcement_route.post("/image", status_code=status.HTTP_200_OK) async def upload_announcement_image( file: UploadFile = File(...), @@ -34,6 +39,11 @@ async def upload_announcement_image( status_code=status.HTTP_400_BAD_REQUEST, detail="Invalid file format. Please upload an image.", ) + if file.content_type not in _ALLOWED_IMAGE_CONTENT_TYPES: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Invalid file type. Please upload an image.", + ) resource_type = "announcement" upload_folder = "announcements" url = await upload_file(file, resource_type, upload_folder) diff --git a/services/project_service.py b/services/project_service.py index d69706a..fff3094 100644 --- a/services/project_service.py +++ b/services/project_service.py @@ -90,32 +90,38 @@ def update_project(self, project_id: int, update_data: UpdateProject) -> Project if not self.user_repo.get_by_id(update_data.manager_id): raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Manager not found") + validated_project_tools = None + validated_stacks = None + + if update_data.project_tools is not None: + if len(set(update_data.project_tools)) != len(update_data.project_tools): + raise HTTPException(status_code=status.HTTP_406_NOT_ACCEPTABLE, + detail="Duplicate skill ids provided") + validated_project_tools = [] + for skill_id in update_data.project_tools: + skill = self.skill_repo.get_by_id(skill_id) + if not skill: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, + detail=f"Skill {skill_id} not found") + validated_project_tools.append(skill) + + if update_data.stacks is not None: + if len(set(update_data.stacks)) != len(update_data.stacks): + raise HTTPException(status_code=status.HTTP_406_NOT_ACCEPTABLE, + detail="Duplicate stack ids provided") + validated_stacks = [] + for stack_id in update_data.stacks: + stack = self.stack_repo.get_by_id(stack_id) + if not stack: + raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, + detail=f"Stack {stack_id} not found") + validated_stacks.append(stack) + updated = self.project_repo.update( project, update_data.model_dump(exclude=["members", "stacks", "project_tools"], exclude_none=True) ) try: - validated_project_tools = None - validated_stacks = None - - if update_data.project_tools is not None: - validated_project_tools = [] - for skill_id in update_data.project_tools: - skill = self.skill_repo.get_by_id(skill_id) - if not skill: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, - detail=f"Skill {skill_id} not found") - validated_project_tools.append(skill) - - if update_data.stacks is not None: - validated_stacks = [] - for stack_id in update_data.stacks: - stack = self.stack_repo.get_by_id(stack_id) - if not stack: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, - detail=f"Stack {stack_id} not found") - validated_stacks.append(stack) - if validated_project_tools is not None: updated.project_tools = validated_project_tools @@ -125,9 +131,6 @@ def update_project(self, project_id: int, update_data: UpdateProject) -> Project if validated_project_tools is not None or validated_stacks is not None: self.project_repo.db.commit() self.project_repo.db.refresh(updated) - except HTTPException: - self.project_repo.db.rollback() - raise except Exception as e: self.project_repo.db.rollback() raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, diff --git a/test/test_announcements.py b/test/test_announcements.py index 31cc2c3..c3583f9 100644 --- a/test/test_announcements.py +++ b/test/test_announcements.py @@ -28,7 +28,7 @@ def test_get_one_announcement(client, test_user, test_announcements): def test_get_one_announcement_does_not_exist(client, test_user): login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) token = login_res.json()["token"] - res = client.get("api/v1/announcements/100", headers={"Authorization": f"Bearer {token}"}) + res = client.get("/api/v1/announcements/100", headers={"Authorization": f"Bearer {token}"}) assert res.status_code == 404 diff --git a/test/test_feeds.py b/test/test_feeds.py index f6c5793..0e3c2ff 100644 --- a/test/test_feeds.py +++ b/test/test_feeds.py @@ -28,7 +28,7 @@ def test_get_one_feed(client, test_user, test_feeds): def test_get_one_feed_does_not_exist(client, test_user, test_feeds): login_res = client.post("/api/v1/users/login", data={"username": test_user["email"], "password": test_user["password"]}) token = login_res.json()["token"] - res = client.get(f"api/v1/feed/10000", headers={"Authorization": f"Bearer {token}"}) + res = client.get("/api/v1/feed/10000", headers={"Authorization": f"Bearer {token}"}) assert res.status_code == 404