Skip to content

Conversation

AbdurrahmanIdr
Copy link

Description

This pull request enhances the /users/ API endpoint by adding search, filtering, and pagination capabilities, while resolving multiple issues encountered during development and testing. The changes improve user management efficiency, fix runtime errors, ensure proper routing and authentication, update Pydantic configurations, and address a missing database table to enable superadmin creation.

Key changes include:

  • Added search (case-insensitive) on first_name, last_name, and email.
  • Implemented filtering by is_active status.
  • Enhanced pagination with configurable page and limit (default: 1, 20; max: 50).
  • Fixed NameError in routes/user.py due to missing schema import.
  • Corrected routing to use /api/v1/users/ instead of /users/.
  • Addressed 401 Not Authenticated by adding a superadmin creation script.

Motivation and Context

The /users/ endpoint initially lacked efficient data retrieval options, making it challenging to manage large user datasets. Additionally, several runtime errors and configuration issues prevented the application from functioning correctly:

  • Missing search/filtering hampered usability.

How Has This Been Tested?

  • Environment: Python 3.12, FastAPI, SQLAlchemy, PostgreSQL, Uvicorn (port 7001), local dev setup.
  • Steps:
    1. Schema Updates:
      • Added Pagination and updated AllUsersResponse in schema/user.py.
      • Fixed Pydantic config (from_attributes=True).
    2. Service Enhancements:
      • Updated UserService.fetch_all with search, filtering, and pagination logic.
      • Tested with mock data for efficiency.
    3. Route Fixes:
      • Corrected imports in routes/user.py to fix NameError.
      • Verified /api/v1/users/ routing with superadmin token.
  1. Endpoint Tests:
    • curl -H "Authorization: Bearer <token>" "http://127.0.0.1:7001/api/v1/users?search=john"
    • curl -H "Authorization: Bearer <token>" "http://127.0.0.1:7001/api/v1/users?is_active=true"
    • curl -H "Authorization: Bearer <token>" "http://127.0.0.1:7001/api/v1/users?page=2&limit=20"
    • Confirmed 200 OK responses with correct data structure.
  2. Unit Tests:
    • Added tests for UserService.fetch_all (search, filter, pagination).
  • Results: All endpoints return expected responses; errors resolved; tests pass.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • Fixed NameError, 404, 401, UndefinedTable, and Pydantic warnings.
  • New feature (non-breaking change which adds functionality)
  • Added search, filtering, pagination, and superadmin script.

…ponse

Introduce a structured pagination model and update the AllUsersResponse schema to improve response consistency for the /users/ endpoint. This change adds a Pagination model with page, limit, total_pages, and total_users fields, and restructures the response data as a dictionary containing 'users' and 'pagination' keys, aligning with API design standards.
…a import

Resolve a NameError caused by an undefined 'user' reference in the /users/ endpoint by explicitly importing required schemas from api.v1.schemas.user. This ensures proper recognition of AllUsersResponse and other schema classes, preventing runtime errors during application initialization.
…ervice

Enhance the fetch_all method to support case-insensitive search across first_name, last_name, and email, filtering by is_active status, and robust pagination with configurable page and limit parameters (default: page=1, limit=20, max: 50). Optimize database queries using SQLAlchemy's or_ and func.lower for efficient execution, and update response formatting to include detailed pagination metadata.
Add comprehensive unit tests to validate the fetch_all method's search, filtering, and pagination functionality in UserService. Include test cases for case-insensitive search accuracy, is_active filtering correctness, and pagination behavior under varying conditions, using a mocked SQLAlchemy session to ensure robust and reliable implementation.
@Takinnuoye5
Copy link
Contributor

ensure all checks are complete , resolve the conflicts too

AbdurrahmanIdr and others added 6 commits March 1, 2025 11:06
Resolved Git merge conflict between filter and dev branches by adopting the enhanced AllUsersResponse with Pagination from filter branch. Removed outdated version from dev branch and preserved confirm_password addition to UserCreate schema.
Signed-off-by: Abdurrahman Idris <[email protected]>
Signed-off-by: Abdurrahman Idris <[email protected]>
- Updated test_fetch_all_* tests to include all required UserData fields in mock User objects, fixing ValidationErrors.
- Modified test_get_all_users to use updated AllUsersResponse structure with data dict (users and pagination), replacing outdated page/per_page/total fields. Adjusted assertions and JSON comparison accordingly.

	modified:   tests/v1/user/test_get_all_users.py
	modified:   tests/v1/user/test_user_service.py
@AbdurrahmanIdr
Copy link
Author

all checks passed

Copy link
Contributor

@samuelogboye samuelogboye left a comment

Choose a reason for hiding this comment

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

Do we need .idea in prod?

Please remove it.

AbdurrahmanIdr and others added 3 commits March 1, 2025 22:46
Added a default value 'HNG Boilerplate' to APP_NAME in Settings class to prevent UndefinedValueError when the environment variable is not set, improving local development reliability.
@AbdurrahmanIdr
Copy link
Author

removed .idea and added to .gitignore

Copy link
Contributor

@joboy-dev joboy-dev left a comment

Choose a reason for hiding this comment

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

Attend to my comment

AbdurrahmanIdr and others added 3 commits March 2, 2025 17:05
Signed-off-by: Abdurrahman Idris <[email protected]>
Revised comments in the fetch_all method of UserService for improved clarity and precision:
- Updated class docstring to reflect broader service purpose.
- Rewrote method docstring with detailed Args, Returns, and Raises sections.
- Streamlined inline comments to be concise, action-oriented, and descriptive of functionality.
No functional changes made to the code logic.
@AbdurrahmanIdr
Copy link
Author

conflict resolved.

@AbdurrahmanIdr AbdurrahmanIdr requested a review from joboy-dev March 2, 2025 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants