Skip to content

Add tests for profile models#161

Merged
omgdory merged 3 commits intomainfrom
profile_models_extended_tests
Apr 23, 2025
Merged

Add tests for profile models#161
omgdory merged 3 commits intomainfrom
profile_models_extended_tests

Conversation

@omgdory
Copy link
Contributor

@omgdory omgdory commented Apr 20, 2025

Tests for Profile Models

Adds tests for ParentProfile and StudentProfile models.


Overview

What does this PR do?

Adds tests for ParentProfile and StudentProfile models. Associated views are updated to pass the tests. Tests are complete, sensical, and now passing.


Key Features & Changes

What has been added or changed?

UserRegistrationTests tests for the updated registration endpoint handling different types of profiles.
UserProfileViewTests tests for the added GET and PATCH endpoints.
Registration view reworked to accept a "role" field from the request data


Why This Is Needed

What problem does this solve?

To adhere to TDD (test-driven development) practices, ensuring that our implementation works as expected and that the expectations for frontend requests are consistent.


Related Issue

🔗 This PR addresses Issue #104


Implementation Details

How was this feature developed?

Django


How to Test This PR

Step-by-step testing guide

  1. Navigate to the backend folder.
  2. Enable your virtual environment. For Mac/Linux (bash/zsh) users, this is env/Scripts/activate. For Windows (PowerShell) users, this is .\env\Scripts\Activate.ps1.
  3. Run pip install -r requirements.txt
  4. Run python manage.py migrate
  5. Run python manage.py test apps.users
  6. Ensure that all tests pass.
  7. Navigate to apps/users/tests.py and ensure that the tests make sense, and reflect appropriate usage.

Files Added/Modified

📂 List of important files changed in this PR

File Name Description
backend/apps/users/tests.py Passing test implementations (UserRegistrationTests and UserProfileViewTests)
backend/apps/users/views.py Update register view to appropriately handle the role

AI Code Usage

🤖 Was AI-generated code used in this PR?

  • Yes (For quick reference and a general outline for how to write the tests)
  • No

Checklist for Reviewers

Things to check before approving this PR:

  • Does the feature work as expected?
  • Is the code clean, modular, and well-documented?
  • Do the tests pass?
  • Do the tests make sense?

Next Steps & Future Enhancements

What improvements can be made after merging?

  • Frontend connection should be priority

@codecov
Copy link

codecov bot commented Apr 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.18%. Comparing base (1f88f39) to head (181b0f1).
Report is 81 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff             @@
##             main     #161       +/-   ##
===========================================
+ Coverage   57.05%   71.18%   +14.13%     
===========================================
  Files          14       14               
  Lines         475      531       +56     
  Branches       37       37               
===========================================
+ Hits          271      378      +107     
+ Misses        196      137       -59     
- Partials        8       16        +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ChrisCRL ChrisCRL added the Backend Issue is primarily backend related label Apr 20, 2025
@ChrisCRL ChrisCRL linked an issue Apr 20, 2025 that may be closed by this pull request
Copy link
Contributor

@ChrisCRL ChrisCRL left a comment

Choose a reason for hiding this comment

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

🔍 Peer Review: User Profile Tests Implementation

Hey Dory👋 — Here's my feedback:

✅ What Looks Great

  • The test organization is clear with separate test classes for registration, profile views, and general user API functions.
  • Your authentication setup is well-implemented with proper JWT token handling in the setUp methods.
  • Test coverage includes all major user types (students, tutors, parents) with appropriate profile creation verification.
  • The assertion strategy is thorough, checking both status codes and actual database state after operations.
  • Good testing of error cases like profile not found and invalid login credentials.

💡 Suggestions for Improvement

  • There are several commented-out test methods that should be removed.
  • Implement tearDown methods consistently across all test classes (currently only in UserAPITestCase).

🧪 Tested This Feature

I ran the following test steps:

  • ✅ Reviewed test coverage for user registration across all user types
  • ✅ Verified authentication flow testing (login, token acquisition, authorized requests)
  • ✅ Checked profile update functionality with PATCH requests
  • ✅ Confirmed error handling for missing profiles and invalid credentials
  • ✅ Validated user deletion and logout functionality tests

🚀 Final Thoughts

The test suite provides good coverage of the basic user profile functionality. The authentication flow is well-tested, and you've covered the major user types in your system.

TLDR

lgtm :))))
istockphoto-157030584-612x612

Copy link
Contributor

@aviendha-andrus aviendha-andrus left a comment

Choose a reason for hiding this comment

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

🔍 Peer Review: Add tests for profile models

Hey Dorian 👋 — just finished reviewing your PR titled Add tests for profile models. Here’s my feedback:


✅ What Looks Great

  • The tests are well organized into three separate sections which allows for easy navigation and readability.
  • Test coverage for ParentProfile and StudentProfile models are implemented cleanly and align with issue #104.

💡 Suggestions for Improvement

  • I would suggest running coverage run manage.py test apps.users and then coverage report to view your test coverage. I think we are reaching for 90% overall coverage so anything we can do to get that number up would be great!
  • For example, this is the test coverage for managers.py.
    apps/users/models.py 69 12 83% 43, 64, 82-83, 89, 94-95, 110-111, 116, 121-122
    • Some are as easy as testing the string methods (line 43), and it looks like a couple of your clean methods don't have tests.
  • I think it might be worth creating a separate github issue for testing, just so that things get clearer organizationally.

🧪 Tested This Feature

I ran the following test steps:

  • ✅ Ran python manage.py test apps.users — all tests passed.
  • ✅ Ran coverage run manage.py test apps.users.
  • ✅ Reviewed tests.py file to confirm structure, assertions, and coverage.
  • ✅ Verified PATCH + GET profile flows, along with registration logic for all roles.

Everything behaved as expected 🎉


🔄 Next Steps

  • Run the coverage report and add in tests that raise your coverage scores.

🚀 Final Thoughts

These tests are instrumental to clean working code, thank you for doing them!
Looking forward to seeing this merged and live soon! 💯

@omgdory omgdory merged commit 7dd01a6 into main Apr 23, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Backend Issue is primarily backend related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Task: Profile Models & User Registration

3 participants