Solution by erm0lenk0 · Pull Request #262 · mate-academy/py-fastapi-homework-4-task

erm0lenk0 · 2026-02-15T17:56:57Z

No description provided.

mateacademy-ai-mentor

Review Summary

I am requesting changes because there are still several critical issues that break required functionality and violate explicit checklist requirements. The /users/{user_id}/profile/ endpoint has incorrect authorization (it never decodes the JWT to get the current user), uses an invalid existing-profile lookup (db.get(..., {"user_id": user_id})), lacks the required response_model=ProfileResponseSchema, and returns a slightly different 500 error message than specified; all of these will cause tests and core requirements for profile creation and permissions to fail. The profile schemas also don’t yet match the spec: the field must be named date_of_birth (not birth_date), info needs a validator to reject empty/whitespace strings, and avatar should be an UploadFile with validation aligned to the validate_image helper, otherwise file upload behavior and validations will not work as required. In routes/accounts.py, several endpoints still depend on get_postgresql_db instead of the mandated get_db, and the activate_account endpoint does not inject BackgroundTasks and EmailSenderInterface to send the activation-complete email, which is a core requirement for the account flow. On the infrastructure side, the testing compose file currently points ALEMBIC_CONFIG to a different path than where alembic.ini is mounted, so alembic upgrade head will fail before tests run, and the Dockerfile omits dev dependencies by using poetry install --only main, which prevents running the development server as described.

To move this forward, please: (1) fix the ALEMBIC_CONFIG vs volume path mismatch and ensure pytest.ini is available at the path you pass to pytest, (2) switch all route DB dependencies to db: AsyncSession = Depends(get_db), (3) update activate_account to use BackgroundTasks and EmailSenderInterface to send the activation-complete email, (4) correct the profiles schema (field names/types and validators) and the profiles route (token decoding, permission check against the authenticated user, existing-profile query, exact error messages, and response_model=ProfileResponseSchema), and (5) adjust the Dockerfile to install dev dependencies or otherwise support the expected dev server command. Once these are addressed, your implementation should be much closer to fully satisfying the functional and testing requirements, and the remaining minor issues (unused imports, small Dockerfile cleanups, fixture optimizations) can be polished afterward.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-02-15T18:00:54Z

docker-compose-tests.yml

    environment:
      - PYTHONPATH=/usr/src/fastapi
      - ENVIRONMENT=testing
+      - ALEMBIC_CONFIG=/usr/src/alembic/alembic.ini


You set ALEMBIC_CONFIG to /usr/src/alembic/alembic.ini here, but the service mounts the file to a different path (see volume mapping). This mismatch will cause alembic to fail to find its config. Either set ALEMBIC_CONFIG to /usr/src/config/alembic.ini or mount the file to /usr/src/alembic/alembic.ini.

mateacademy-ai-mentor · 2026-02-15T18:00:54Z

docker-compose-tests.yml

        condition: service_healthy
    volumes:
      - ./src:/usr/src/fastapi
+      - ./alembic.ini:/usr/src/config/alembic.ini


You mount ./alembic.ini to /usr/src/config/alembic.ini here — that does not match the ALEMBIC_CONFIG env value on line 15. Align the mounted destination and ALEMBIC_CONFIG value so Alembic can locate the config file.

mateacademy-ai-mentor · 2026-02-15T18:00:54Z

docker-compose-tests.yml

+    command: >
+      sh -c "
+        alembic upgrade head &&
+        pytest -c /usr/src/config/pytest.ini -m e2e --maxfail=5 --disable-warnings -v --tb=short


The pytest command uses -c /usr/src/config/pytest.ini. Ensure pytest.ini actually exists at that path inside the container (either include it in the image or mount it). Otherwise pytest will run with default settings or fail to load the expected configuration.

mateacademy-ai-mentor · 2026-02-15T18:00:54Z

docker-compose-tests.yml

-               "-m", "e2e", "--maxfail=5", "--disable-warnings", "-v", "--tb=short"]
+    command: >
+      sh -c "
+        alembic upgrade head &&


You run alembic upgrade head before running pytest. This is correct in principle, but because ALEMBIC_CONFIG path is incorrect (see comment on line 15) this step will likely fail. Also ensure the database is reachable/ready before running Alembic (consider a short wait or a retry loop).

mateacademy-ai-mentor · 2026-02-15T18:00:54Z

docker-compose-tests.yml

    container_name: backend_theater_test
-    command: [ "pytest", "-c", "/usr/src/config/pytest.ini",
-               "-m", "e2e", "--maxfail=5", "--disable-warnings", "-v", "--tb=short"]
+    command: >


The command is executed via sh -c " ... ". This is fine, but confirm the working directory and mounted files inside the test image so both Alembic and pytest find their configs and the application code as expected.

mateacademy-ai-mentor · 2026-02-15T18:00:55Z

Dockerfile

 # Copy the source code
 COPY ./src .

+COPY ./alembic.ini /usr/src/fastapi/alembic.ini


This line duplicates an earlier COPY ./alembic.ini (you previously copied it to /usr/src/alembic/alembic.ini). If you need alembic.ini in both locations keep it, otherwise remove this redundant copy to avoid confusion.

mateacademy-ai-mentor · 2026-02-15T18:00:55Z

Dockerfile

 COPY ./src .

+COPY ./alembic.ini /usr/src/fastapi/alembic.ini
+


General note: the Dockerfile currently installs only main dependencies (poetry install --no-root --only main), which will omit dev dependencies like uvicorn/watchfiles needed for the development server. For the development image you should install dev dependencies (for example by running poetry install --no-root --with dev or not using --only main) so the /commands/run_web_server_dev.sh script can run. Also consider adding a default CMD/ENTRYPOINT (or document that docker-compose supplies it).

mateacademy-ai-mentor · 2026-02-15T18:00:55Z

src/routes/accounts.py

 )
 from security.interfaces import JWTAuthManagerInterface

+from database.session_postgresql import get_postgresql_db


The module imports get_postgresql_db and the routes use it as the DB dependency. The task requirements mandate routes should accept db: AsyncSession = Depends(get_db) (so DI and session lifecycle behave as required). Replace this import/usage with get_db to satisfy the checklist.

mateacademy-ai-mentor · 2026-02-15T18:00:55Z

src/routes/accounts.py

 from security.interfaces import JWTAuthManagerInterface

+from database.session_postgresql import get_postgresql_db
+from notifications.emails import EmailSender


EmailSender is imported here but never used. Per the requirements emails must be sent using the injected get_accounts_email_notificator (EmailSenderInterface). Remove this unused import to avoid confusion.

mateacademy-ai-mentor · 2026-02-15T18:00:55Z

src/routes/accounts.py

        user_data: UserRegistrationRequestSchema,
-        db: AsyncSession = Depends(get_db),
+        background_tasks: BackgroundTasks,
+        db: AsyncSession = Depends(get_postgresql_db),


register_user depends on get_postgresql_db. Per the requirements routes must accept db: AsyncSession = Depends(get_db). Change the dependency to get_db (and ensure get_db is imported).

mateacademy-ai-mentor · 2026-02-15T18:00:58Z

src/routes/accounts.py

 async def activate_account(
        activation_data: UserActivationRequestSchema,
-        db: AsyncSession = Depends(get_db),
+        db: AsyncSession = Depends(get_postgresql_db),


The activate_account endpoint currently only takes db. The requirements explicitly require that account controllers send email notifications for activation completions using BackgroundTasks and the injected email sender. Add parameters background_tasks: BackgroundTasks and email_sender: EmailSenderInterface = Depends(get_accounts_email_notificator) and schedule a background task to send the activation-complete email after successful activation.

mateacademy-ai-mentor · 2026-02-15T18:00:58Z

src/routes/accounts.py

    reset_token = PasswordResetTokenModel(user_id=cast(int, user.id))
    db.add(reset_token)
    await db.commit()
+    reset_link = f"http://127.0.0.1/accounts/reset-password/complete/?token={reset_token.token}&email={user.email}"


request_password_reset_token uses BackgroundTasks and the injected email_sender correctly — keep this pattern. However note earlier comment: replace get_postgresql_db with get_db for the db dependency to meet the DI requirement.

mateacademy-ai-mentor · 2026-02-15T18:00:58Z

src/routes/accounts.py

 async def reset_password(
        data: PasswordResetCompleteRequestSchema,
-        db: AsyncSession = Depends(get_db),
+        background_tasks: BackgroundTasks,


The reset_password endpoint properly schedules the password-reset-complete email via BackgroundTasks and email_sender. Ensure consistency by also switching the db dependency here to get_db (instead of get_postgresql_db).

mateacademy-ai-mentor · 2026-02-15T18:00:58Z

src/routes/accounts.py

 async def login_user(
        login_data: UserLoginRequestSchema,
-        db: AsyncSession = Depends(get_db),
+        db: AsyncSession = Depends(get_postgresql_db),


login_user depends on get_postgresql_db. Change this to get_db: AsyncSession = Depends(get_db) per the requirements so the route uses the expected dependency provider and session lifecycle.

mateacademy-ai-mentor · 2026-02-15T18:00:58Z

src/routes/accounts.py

 async def refresh_access_token(
        token_data: TokenRefreshRequestSchema,
-        db: AsyncSession = Depends(get_db),
+        db: AsyncSession = Depends(get_postgresql_db),


refresh_access_token depends on get_postgresql_db. Replace it with get_db as required by the specification so the DI and session closing behave as expected.

mateacademy-ai-mentor · 2026-02-15T18:00:58Z

src/config/settings.py

+# class TestingSettings(BaseAppSettings):
+#     SECRET_KEY_ACCESS: str = "SECRET_KEY_ACCESS"
+#     SECRET_KEY_REFRESH: str = "SECRET_KEY_REFRESH"
+#     JWT_SIGNING_ALGORITHM: str = "HS256"
+#
+#     def model_post_init(self, __context: dict[str, Any] | None = None) -> None:
+#         object.__setattr__(self, 'PATH_TO_DB', ":memory:")
+#         object.__setattr__(
+#             self,
+#             'PATH_TO_MOVIES_CSV',
+#             str(self.BASE_DIR / "database" / "seed_data" / "test_data.csv")
+#         )


There is a commented-out alternate TestingSettings block — remove or clearly document why it’s kept. Leaving duplicate implementations commented in the code is confusing and error-prone.

mateacademy-ai-mentor · 2026-02-15T18:00:58Z

src/config/settings.py

+    POSTGRES_USER: str = "admin"
+    POSTGRES_PASSWORD: str = "some_password"
+    POSTGRES_HOST: str = "postgres_theater"
+    POSTGRES_DB_PORT: int = 5432
+    POSTGRES_DB: str = "movies_db"


TestingSettings still defines Postgres connection values (POSTGRES_USER, POSTGRES_PASSWORD, POSTGRES_HOST, POSTGRES_DB_PORT, POSTGRES_DB). Tests should use SQLite (and model_post_init correctly sets PATH_TO_DB to ':memory:'). Consider removing or documenting these Postgres fields in TestingSettings so they don't mislead readers about the test DB being SQLite.

mateacademy-ai-mentor · 2026-02-15T18:00:59Z

src/schemas/profiles.py

+    first_name: str
+    last_name: str
+    gender: str
+    birth_date: date


The field is declared as birth_date but the task requires the field be named date_of_birth. Rename the field to date_of_birth and update its validator decorator accordingly so the schema exposes the expected field name to clients and tests.

mateacademy-ai-mentor · 2026-02-15T18:00:59Z

src/schemas/profiles.py

+    last_name: str
+    gender: str
+    birth_date: date
+    info: str


info has no validation. The requirements state info cannot be empty or contain only spaces. Add a @field_validator('info') that strips the value and raises an HTTPException (400) if it's empty or only whitespace.

mateacademy-ai-mentor · 2026-02-15T18:00:59Z

src/schemas/profiles.py

+    gender: str
+    birth_date: date
+    info: str
+    avatar: bytes


avatar is typed as bytes. For FastAPI file uploads you should use UploadFile (and accept it via File(...) at the route level). Change avatar to UploadFile (or another approach consistent with how endpoints receive files) and ensure the validator works with that type.

mateacademy-ai-mentor · 2026-02-15T18:01:02Z

src/schemas/profiles.py

+        return v
+
+
+    @field_validator("birth_date")


The birth/date validator targets birth_date. After renaming the field to date_of_birth update this decorator to @field_validator('date_of_birth'). Also ensure validate_birth_date enforces the 18+ age rule as required by the specification.

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/schemas/profiles.py

+        validate_birth_date(v)
+        return v
+
+    @field_validator("avatar")


The avatar validator calls validate_image(v). If you change avatar to UploadFile, confirm validate_image accepts an UploadFile (or convert the file to bytes and pass appropriate metadata). Also ensure the validator enforces allowed formats (JPG/JPEG/PNG) and max size (1MB) per requirements and raises a clear HTTPException on failure.

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/tests/conftest.py



-@pytest_asyncio.fixture(scope="function")
+@pytest_asyncio.fixture(scope="function", autouse=True)


This autouse fixture always inserts user groups before every test. Because reset_db skips resetting the DB for tests marked 'e2e', this can attempt to insert duplicate groups during E2E runs and lead to IntegrityError. Consider checking if the groups already exist before inserting, or skip this autouse behavior for tests with the 'e2e' marker (e.g., accept request and check if 'e2e' in request.keywords).

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/tests/conftest.py



-@pytest_asyncio.fixture(scope="function")
+@pytest_asyncio.fixture(scope="function", autouse=True)


This autouse fixture seeds the CSV dataset if the DB is empty; that check is good. For clarity and performance you might consider making this session-scoped for test suites where the DB is reset once, or ensure it cooperates with the user-groups seeding guard to avoid conflicts in E2E mode.

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/tests/conftest.py

        await seeder.seed()

    yield db_session
+


File end marker. As a general note, ensure tests run with ENVIRONMENT=testing so get_settings() returns the testing settings (SQLite, test MinIO/MailHog endpoints). Also consider centralizing which fixtures are function- vs session-scoped to avoid unnecessary repeated seeding and potential state leakage between E2E tests.

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/routes/profiles.py

@@ -1,5 +1,63 @@
-from fastapi import APIRouter
+from fastapi import APIRouter, Depends, HTTPException, status
+from poetry.core.packages.dependency import Dependency


This import (Dependency from poetry) is unrelated to FastAPI dependencies and is unused in this file—remove it to avoid confusion.

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/routes/profiles.py

+from fastapi import APIRouter, Depends, HTTPException, status
+from poetry.core.packages.dependency import Dependency
+from sqlalchemy.ext.asyncio import AsyncSession
+from sqlalchemy.testing.pickleable import User


sqlalchemy.testing.pickleable.User is a test utility and is not used here; remove this import.

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/routes/profiles.py

+from sqlalchemy.testing.pickleable import User
+
+from config.dependencies import get_s3_storage_client
+from database.models.accounts import UserProfileModel


You import UserProfileModel here but re-import it again on line 13. Keep a single import from the correct module to avoid confusion and possible shadowing.

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/routes/profiles.py

+from schemas.profiles import ProfileCreate
+from security.http import get_token
+from storages.interfaces import S3StorageInterface
+from storages.s3 import S3StorageClient


S3StorageClient is imported but never used in this module. The route should rely on the interface injected by get_s3_storage_client (which you already type-hint). Remove this unused import.

mateacademy-ai-mentor · 2026-02-15T18:01:03Z

src/routes/profiles.py

+from security.http import get_token
+from storages.interfaces import S3StorageInterface
+from storages.s3 import S3StorageClient
+from database.session_postgresql import get_postgresql_db


This file depends on get_postgresql_db. The task requires routes use db: AsyncSession = Depends(get_db) so the get_db dependency provider is used and sessions are closed automatically. Replace get_postgresql_db with get_db (and import get_db).

mateacademy-ai-mentor · 2026-02-15T18:01:07Z

src/routes/profiles.py

 router = APIRouter()

-# Write your code here
+@router.post("/users/{user_id}/profile/", status_code=status.HTTP_201_CREATED)


The route decorator lacks the required response model. According to the spec the endpoint must return ProfileResponseSchema on success; add response_model=ProfileResponseSchema to the decorator so FastAPI validates/serializes output.

mateacademy-ai-mentor · 2026-02-15T18:01:07Z

src/routes/profiles.py

+        user_id: int,
+        profile: ProfileCreate,
+        db: AsyncSession = Depends(get_postgresql_db),
+        token: str = Depends(get_token),


The handler depends only on get_token which returns the raw token string. You must decode/validate the token (e.g. via get_jwt_auth_manager / JWTAuthManagerInterface) to obtain the current user id and detect expired tokens. Without decoding you cannot enforce "user can only create their own profile" and cannot map expired-token behavior to return "Token has expired." as required.

mateacademy-ai-mentor · 2026-02-15T18:01:07Z

src/routes/profiles.py

+    if not user or not user.is_active:
+        raise HTTPException(status_code=status.HTTP_401_UNAUTHORIZED, detail="User not found or not active.")
+
+    if user.id != user_id and user.group.name == "USER":


Permission check is incorrect: user is the target user fetched by user_id, so user.id != user_id will always be False. You need to compare the target user_id to the current authenticated user id (obtained by decoding the token) and check the current user's group. Update logic to decode token -> get current_user -> then enforce: if current_user.id != user_id and current_user.group.name == 'USER' then 403.

mateacademy-ai-mentor · 2026-02-15T18:01:07Z

src/routes/profiles.py

+    if user.id != user_id and user.group.name == "USER":
+        raise HTTPException(status_code=status.HTTP_403_FORBIDDEN, detail="You don't have permission to edit this profile.")
+
+    existing_profile = await  db.get(UserProfileModel, {"user_id": user_id})


db.get(UserProfileModel, {"user_id": user_id}) is invalid: Session.get() expects a primary key value, not a filter dict. Query for existing profile by attribute, e.g. select(UserProfileModel).filter_by(user_id=user_id) (or use appropriate ORM query) to check whether a profile exists.

mateacademy-ai-mentor · 2026-02-15T18:01:07Z

src/routes/profiles.py

+    try:
+        avatar_url = await s3_client.upload_file(profile.avatar, f"{user_id}_avatar.jpg")
+    except Exception:
+        raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, detail="Failed to upload avatar. Please try again.")


The avatar upload failure message differs from the required text. The checklist requires the 500 response body to be {"detail": "Failed to upload avatar. Please try again later."} — update the message to exactly match this and ensure status 500 is returned.

mateacademy-ai-mentor · 2026-02-15T18:01:07Z

src/routes/profiles.py

+        first_name=profile.first_name,
+        last_name=profile.last_name,
+        gender=profile.gender,
+        date_of_birth=profile.birth_date,


You use profile.birth_date here but the required schema field name is date_of_birth. Use profile.date_of_birth (or align with the exact schema field names in schemas/profiles.py) to meet the spec and avoid attribute errors.

mateacademy-ai-mentor · 2026-02-15T18:01:07Z

src/routes/profiles.py

+    await db.commit()
+    await db.refresh(new_profile)
+
+    return {


The endpoint returns a raw dict. The spec requires returning a ProfileResponseSchema and the route must declare response_model=ProfileResponseSchema. After adding that, return an instance or let FastAPI validate the dict — ensure field names and types match the response schema (including generating the final avatar URL).

mateacademy-ai-mentor

Review Summary

I am requesting changes to your solution. While you've made good progress, several critical issues prevent full functionality and compliance with the task requirements. The E2E test environment's database configuration in src/config/settings.py still uses PostgreSQL, contradicting the explicit requirement for SQLite, which causes test failures. Additionally, the activate_account endpoint in src/routes/accounts.py is missing the required activation_complete email notification. In src/routes/profiles.py, you have duplicate API endpoints that need to be removed, and several error messages for token validation, authorization, and S3 upload failures do not precisely match the specified text and HTTP status codes. Finally, the main Dockerfile needs to install development dependencies to allow the local server to run. Please address these points to ensure all core requirements are met.

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button² in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag³.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

mateacademy-ai-mentor · 2026-02-19T19:10:56Z

src/config/settings.py

+    POSTGRES_USER: str = "admin"
+    POSTGRES_PASSWORD: str = "some_password"
+    POSTGRES_HOST: str = "postgres_theater"
+    POSTGRES_DB_PORT: int = 5432
+    POSTGRES_DB: str = "movies_db"


The TestingSettings class is currently configured for PostgreSQL. The task explicitly requires a lightweight SQLite database for testing, particularly for E2E tests. Please remove these PostgreSQL-specific environment variables and ensure TestingSettings aligns with the SQLite requirement.

mateacademy-ai-mentor · 2026-02-19T19:10:56Z

Dockerfile

 # Copy the source code
 COPY ./src .

+COPY ./alembic.ini /usr/src/fastapi/alembic.ini


This COPY command is redundant. alembic.ini is already copied to /usr/src/alembic/alembic.ini earlier (line 25), which is where the ALEMBIC_CONFIG environment variable points (line 7). Removing this line would simplify the Dockerfile without impacting functionality.

mateacademy-ai-mentor · 2026-02-19T19:10:56Z

src/routes/profiles.py

+from fastapi import APIRouter, Depends, HTTPException, status, UploadFile, BackgroundTasks
+from sqlalchemy.ext.asyncio import AsyncSession
+from sqlalchemy import select
+from config.settings import Settings


The Settings import is unused in this file. Please remove it to keep the code clean and prevent unnecessary imports.

mateacademy-ai-mentor · 2026-02-19T19:10:56Z

src/routes/profiles.py

+    except Exception:
+        raise HTTPException(
+            status_code=status.HTTP_401_UNAUTHORIZED,
+            detail="Invalid or expired token"


The get_current_user function catches a generic Exception. The task requires specific error messages for 'Token has expired.' (HTTP 401). Catching BaseSecurityError (or more specific exceptions from token_manager.py) would allow for more precise error handling and messaging, matching the requirement.

mateacademy-ai-mentor · 2026-02-19T19:10:56Z

src/routes/profiles.py

+@router.post("/profiles/", response_model=ProfileResponseSchema)
+async def create_profile(
+    profile: ProfileCreateSchema,
+    db: AsyncSession = Depends(get_db),
+    current_user: UserModel = Depends(get_current_user),
+):
+    stmt = select(UserProfileModel).where(UserProfileModel.user_id == current_user.id)
+    result = await db.execute(stmt)
+    existing_profile = result.scalars().first()
+    if existing_profile:
+        raise HTTPException(
+            status_code=status.HTTP_400_BAD_REQUEST,
+            detail="Profile already exists."
+        )
+
+    new_profile = UserProfileModel(
+        user_id=current_user.id,
+        first_name=profile.first_name,
+        last_name=profile.last_name,
+        gender=profile.gender,
+        date_of_birth=profile.date_of_birth,
+        info=profile.info,
+    )
+    db.add(new_profile)
+    await db.commit()
+    await db.refresh(new_profile)
+
+    return new_profile


The /profiles/ endpoint duplicates functionality with /users/{user_id}/profile/ and is not explicitly required by the task. The task focuses on the /users/{user_id}/profile/ endpoint for user profile creation. Please remove this duplicated endpoint to streamline the API and avoid redundancy.

mateacademy-ai-mentor · 2026-02-19T19:10:56Z

src/routes/profiles.py

+    if current_user.id != user_id:
+        raise HTTPException(
+            status_code=status.HTTP_403_FORBIDDEN,
+            detail="Not authenticated to create profile for another user."


The task specifies that if a regular user tries to create a profile for another user, the error message should be: "You don't have permission to edit this profile.". Your current message "Not authenticated to create profile for another user." is slightly different. Please ensure the message matches the requirement exactly.

mateacademy-ai-mentor · 2026-02-19T19:10:57Z

src/routes/profiles.py

+        raise HTTPException(
+            status_code=status.HTTP_403_FORBIDDEN,
+            detail="Not authenticated to create profile for another user."
+        )


The task requires checking if the user_id from the path exists and is active. While current_user confirms the authenticated user is active, this check doesn't verify the status of the user_id from the path if it happens to be the same as current_user.id but the user's is_active status somehow changed or wasn't verified here. An explicit check if not user.is_active: (where user is fetched by user_id) should be added here to fulfill the requirement "User not found or not active." more precisely.

mateacademy-ai-mentor · 2026-02-19T19:10:57Z

src/routes/profiles.py

+    avatar_url = None
+    if avatar:
+        validate_image(avatar)
+        avatar_key = f"avatars/{user_id}_avatar.jpg"
+        await s3_client.upload_fileobj(avatar.file, avatar_key)
+        avatar_url = await s3_client.get_file_url(avatar_key)


The task explicitly states: If the avatar upload fails, the endpoint must return: {"detail": "Failed to upload avatar. Please try again later."} with Error Code: 500 Internal Server Error. The current implementation lacks a try-except block around the s3_client operations to handle potential upload failures and return the specified error message.

mateacademy-ai-mentor · 2026-02-19T19:10:57Z

src/routes/profiles.py

+@router.post(
+    "/accounts/activate/",
+    response_model=MessageResponseSchema,
+    status_code=status.HTTP_200_OK,
+)
+async def activate_account(
+        data: UserActivationRequestSchema,
+        background_tasks: BackgroundTasks,
+        db: AsyncSession = Depends(get_db),
+        email_sender: EmailSenderInterface = Depends(get_accounts_email_notificator),
+):
+    stmt = (
+        select(ActivationTokenModel)
+        .join(UserModel)
+        .where(UserModel.email == data.email, ActivationTokenModel.token == data.token)
+    )
+    result = await db.execute(stmt)
+    activation_token = result.scalars().first()
+    if not activation_token:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,
+            detail="Invalid or expired activation token."
+        )
+
+    user = activation_token.user
+    user.is_active = True
+    await db.commit()
+
+    background_tasks.add_task(
+        email_sender.send_activation_complete_email,
+        email=user.email,
+        login_link="https://example.com/login",
+    )
+
+    return {"message": "User account activated successfully."}


This activate_account endpoint is a duplicate of the one found in src/routes/accounts.py. The task description indicates that routes/profiles.py should be for profile management, and account activation should be handled in routes/accounts.py. Please remove this duplicate endpoint from routes/profiles.py.

mateacademy-ai-mentor · 2026-02-19T19:10:57Z

src/routes/accounts.py

+from database.session_postgresql import get_postgresql_db
+from notifications.emails import EmailSender


These imports (get_postgresql_db and EmailSender) appear to be unused. Since you are using dependency injection for the email sender and get_db for the database session, these can likely be removed to keep your imports clean.

mateacademy-ai-mentor

Review Summary

I'm approving your submission as an educational override so you can continue making progress. You've done a fantastic job implementing the test environment configuration and integrating email notifications!

However, there are a few critical issues to address. Your migration script in src/database/migrations/env.py will fail in development due to a missing DATABASE_URL attribute in your settings. Your E2E tests will also fail on subsequent runs because the seed_user_groups fixture in src/tests/conftest.py attempts to re-insert data, causing a database error. Additionally, please ensure the profile creation endpoint in src/routes/profiles.py returns a 401 Unauthorized status for inactive users, not 404 Not Found.

Also, double-check your Pydantic validators for missing return statements, as seen in src/schemas/profiles.py. Keep up the great momentum!

✨ Please rate this review in the Platform's notification center¹. I will be grateful for your feedback! ✨

Rate AI review example ↩

mateacademy-ai-mentor · 2026-02-22T13:20:23Z

src/config/dependencies.py

 from security.interfaces import JWTAuthManagerInterface
 from security.token_manager import JWTAuthManager
 from storages import S3StorageInterface, S3StorageClient
+from config.settings import settings


This settings object is imported but never used in this file. It can be safely removed to keep the code clean.

mateacademy-ai-mentor · 2026-02-22T13:20:23Z

src/database/migrations/env.py

 # my_important_option = config.get_main_option("my_important_option")
 # ... etc.
+settings = get_settings()
+DATABASE_URL = settings.DATABASE_URL.replace("+aiosqlite", "")


This line will raise an AttributeError when running migrations in a development environment. The Settings class in src/config/settings.py (used for development) does not define a DATABASE_URL attribute. You'll need to add this attribute to the Settings class, likely as a property that constructs the PostgreSQL connection string from the other POSTGRES_* variables.

mateacademy-ai-mentor · 2026-02-22T13:20:24Z

src/routes/accounts.py

    TokenRefreshResponseSchema
 )
 from security.interfaces import JWTAuthManagerInterface
+background = "#282a36"


This background variable is defined but never used anywhere in the file. It should be removed to keep the code clean.

mateacademy-ai-mentor · 2026-02-22T13:20:24Z

src/routes/accounts.py

+    background_tasks.add_task(
+        email_sender.send_activation_email,
+        email=user.email,
+        login_link="https://example.com/login"
+    )


This background task sends the send_activation_email, which is intended for the initial registration process. According to the requirements, upon successful activation, an activation_complete email should be sent, which you are correctly doing in the next code block. This call is redundant and incorrect for this context.

mateacademy-ai-mentor · 2026-02-22T13:20:24Z

src/schemas/profiles.py

@@ -1,13 +1,63 @@
 from datetime import date
-
 from fastapi import UploadFile, Form, File, HTTPException


The imports Form, File, and HTTPException are not used in this file. It's good practice to remove unused imports to keep the code clean.

mateacademy-ai-mentor · 2026-02-22T13:20:24Z

src/schemas/profiles.py

+    def validate_info_field(cls, v):
+        if not v.strip():
+            raise ValueError("Info must not be empty or whitespace")


This validator function is missing a return v statement. Pydantic field validators must always return the validated value. Without it, the info field will be set to None after validation, which is likely not the intended behavior.

mateacademy-ai-mentor · 2026-02-22T13:20:24Z

src/routes/profiles.py

+from fastapi import APIRouter, Depends, HTTPException, status, UploadFile, BackgroundTasks
+from sqlalchemy.ext.asyncio import AsyncSession
+from sqlalchemy import select
+
+from database import get_db, ActivationTokenModel, UserGroupEnum
+from database.models.accounts import UserModel, UserProfileModel
+from exceptions import TokenExpiredError, InvalidTokenError
+from schemas.profiles import ProfileCreateSchema, ProfileResponseSchema
+from schemas.accounts import UserActivationRequestSchema, MessageResponseSchema
+from security.token_manager import JWTAuthManager
+from storages.interfaces import S3StorageInterface
+from config.dependencies import get_s3_storage_client, get_accounts_email_notificator
+from validation.profile import validate_image
+from notifications.interfaces import EmailSenderInterface
+from fastapi.security import OAuth2PasswordBearer


There are several unused imports in this file (e.g., BackgroundTasks, UserActivationRequestSchema, get_accounts_email_notificator). Please remove any imports that are not being used to keep the code clean and maintainable.

mateacademy-ai-mentor · 2026-02-22T13:20:24Z

src/routes/profiles.py

+token_manager = JWTAuthManager(
+    secret_key_access=os.getenv("SECRET_KEY_ACCESS", "default_access_secret"),
+    secret_key_refresh=os.getenv("SECRET_KEY_REFRESH", "default_refresh_secret"),
+    algorithm=os.getenv("JWT_SIGNING_ALGORITHM", "HS256"),
+)


Instantiating JWTAuthManager directly in the route file bypasses the project's dependency injection system and uses potentially insecure default secrets. For consistency, testability, and security, you should inject this using the provided dependency function, get_jwt_auth_manager, within your get_current_user function.

mateacademy-ai-mentor · 2026-02-22T13:20:24Z

src/routes/profiles.py

+
+    if not user or not user.is_active:
+        raise HTTPException(
+            status_code=status.HTTP_404_NOT_FOUND,


The task requirements specify that if a user is not found or not active, the endpoint should return a 401 Unauthorized status code. You are currently returning a 404 NOT_FOUND.

mateacademy-ai-mentor · 2026-02-22T13:20:24Z

src/tests/conftest.py

+@pytest_asyncio.fixture(scope="function", autouse=True)
 async def seed_user_groups(db_session: AsyncSession):


This fixture is configured to run before every test. While this works for integration tests where the database is reset each time, it will cause an IntegrityError during E2E tests. After the first E2E test completes, the user groups will remain in the database, and this fixture will attempt to re-insert them on the next test, violating the unique constraint on the group name.

To fix this, you should make this fixture idempotent by first checking if the user groups already exist before attempting to insert them, similar to the pattern used in the seed_database fixture.

Solution

5ec8133

mateacademy-ai-mentor suggested changes Feb 15, 2026

View reviewed changes

mateacademy-ai-mentor reviewed Feb 15, 2026

View reviewed changes

Solution2

5aab862

erm0lenk0 requested a review from mateacademy-ai-mentor February 19, 2026 18:56

mateacademy-ai-mentor suggested changes Feb 19, 2026

View reviewed changes

Solution3

d105ad3

erm0lenk0 requested a review from mateacademy-ai-mentor February 22, 2026 13:17

mateacademy-ai-mentor approved these changes Feb 22, 2026

View reviewed changes

Solution4

cb2b583

erm0lenk0 requested a review from mateacademy-ai-mentor February 27, 2026 13:53



		@pytest_asyncio.fixture(scope="function")
		@pytest_asyncio.fixture(scope="function", autouse=True)

		from database.session_postgresql import get_postgresql_db
		from notifications.emails import EmailSender

		@@ -1,13 +1,63 @@
		from datetime import date

		from fastapi import UploadFile, Form, File, HTTPException

		@pytest_asyncio.fixture(scope="function", autouse=True)
		async def seed_user_groups(db_session: AsyncSession):

Conversation

erm0lenk0 commented Feb 15, 2026

Uh oh!

mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

Review Summary

Footnotes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!