Solution

[borunov65]

No description provided.

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because there are several critical issues that will break functionality and/or fail the task requirements. The profile schemas mix Pydantic v1 and v2 conventions and don’t enable arbitrary_types_allowed for UploadFile, so profile form validation and model creation are likely to fail; you should switch fully to v2-style config using model_config = {"arbitrary_types_allowed": True, "from_attributes": True} and keep validators raising ValueError instead of HTTPException. In src/routes/profiles.py, you access payload['user_id'] directly (risking KeyError), hard‑code the admin group id as 3 instead of using a constant/enum, and hard‑code the avatar extension as .jpg instead of deriving and validating it from the uploaded file. The profile endpoint currently uses UserProfileResponseSchema while the task expects ProfileResponseSchema, and it stores the S3 key in the DB but returns/validates against an HttpUrl—you should align the response model name and ensure the response contains a real URL (and decide whether the DB stores key or URL consistently). Additionally, your use of await on s3_client.upload_file / get_file_url must match the actual interface (async vs sync), and the PostgreSQL volume in docker-compose.yml should be mounted to /var/lib/postgresql/data instead of /var/lib/postgresql to ensure data persistence. Once these blocking issues are fixed, the rest of the implementation (email notifications, main profile flow, and error handling) looks solid and close to meeting all requirements.

---

✨ 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.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

The decorator uses response_model=UserProfileResponseSchema but the assignment in the task requires returning ProfileResponseSchema (or ensure the schema name matches the project specification). Update the import or the response_model to match the required schema name.

[mateacademy-ai-mentor]

You decode the token payload and then access payload['user_id'] directly. If user_id is missing in the token this will raise a KeyError. Add a safe check and raise a controlled 401/400 with an explanatory message if the payload lacks user_id.

[mateacademy-ai-mentor]

Authorization check uses current_user.group_id != 3 with a magic number (3 = адмін). Use the appropriate enum/constant (e.g. UserGroupEnum.ADMIN or a named constant) imported from your models rather than a hard-coded integer to determine elevated permissions.

[mateacademy-ai-mentor]

When generating avatar_key you hard-code the .jpg extension. Use the uploaded file's original filename or content type (after validation) to determine an appropriate extension (and sanitize it). This prevents incorrect extensions when a PNG is uploaded and matches the validation rules for allowed image types.

[mateacademy-ai-mentor]

You call await s3_client.upload_file(...) and await s3_client.get_file_url(...). Ensure the S3StorageInterface implementation actually exposes async methods with these names/signatures; if it's synchronous you must run it in a threadpool or adapt the call. Confirm the storage client contract matches these awaits.

[mateacademy-ai-mentor]

The response schema class is named UserProfileResponseSchema here, but the task specification requires the endpoint to return ProfileResponseSchema. Ensure the schema name matches the required API contract (rename to ProfileResponseSchema or provide an alias) so route definitions and tests expecting ProfileResponseSchema will pass. See task description for the exact expected response schema name and examples .

[mateacademy-ai-mentor]

You use class Config: orm_mode = True (v1-style). If you're on Pydantic v2 (you are using field_validator), replace this with model_config = {"from_attributes": True} to enable ORM-mode behavior in v2. Using v1 Config may have no effect under v2 and cause mismatches when returning ORM objects as responses.

[mateacademy-ai-mentor]

In register_user you schedule the activation email before committing the DB transaction. If the subsequent commit fails the email may reference a token that wasn’t persisted. Consider moving the background_tasks.add_task(...) call until after await db.commit() (or ensure the token is fully persisted before sending).

[mateacademy-ai-mentor]

Similarly, in request_password_reset_token you schedule the password-reset email before calling await db.commit(). Move the background_tasks.add_task(...) after the commit to avoid sending an email if the database transaction fails.

[mateacademy-ai-mentor]

The DB data volume is mounted to /var/lib/postgresql. For the official Postgres image the data directory is typically /var/lib/postgresql/data. Mounting to /var/lib/postgresql may not persist database files as expected. Change this to /var/lib/postgresql/data (or set PGDATA accordingly) to ensure persistence and compatibility with the image.

[mateacademy-ai-mentor]

Review Summary

I am requesting changes because there are several critical issues that will cause runtime errors and prevent the features from working as required. First, the UserProfileSchema currently mixes Form(...)/Pydantic incorrectly and lacks model_config = {"arbitrary_types_allowed": True, "from_attributes": True}, so the as_form dependency for UploadFile will not work reliably and validation may fail before the route is hit. Second, the avatar size limit of 1MB is not actually enforced after reading the file bytes (in src/routes/profiles.py), which violates the explicit requirement; you should either add a direct size check (e.g. if len(file_bytes) > 1_048_576: ...) or ensure validate_image enforces this. Third, there are async/sync mismatches: you await S3 storage methods that may be synchronous, and elsewhere in src/routes/accounts.py you await db.delete(...) even though AsyncSession.delete() is not awaitable; both patterns will raise at runtime and must be replaced with either async storage methods or sync calls, and with db.run_sync(...) or execute(delete(...)) for deletions.

Additional critical correctness issue: the activation link you email includes only the token, while your activation handler looks up records by both email and token; without including the email in the URL (or adjusting the lookup to token-only), users will not be able to activate accounts from the emailed link. Please also adjust your avatar field validator to use if value is None: and raise ValueError instead of HTTPException, so Pydantic can properly collect and return 422 validation errors, and ensure validate_image enforces allowed extensions and size. Once these blocking issues are fixed, your overall structure (background email tasks, profile route flow, S3 usage, and response shaping) will align well with the task; the changes are mostly about making the current design actually work at runtime and comply with the explicit constraints.

---

✨ 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.

Footnotes

1. Rate AI review example ↩

2. Re-request AI review example ↩

3. Add AI review comments example ↩

[mateacademy-ai-mentor]

This route depends on UserProfileSchema.as_form. The provided schema file currently uses an invalid Form(.)/Pydantic mix and likely will fail to produce a working UploadFile / form parsing. Fix the schema to a working as_form implementation and ensure Pydantic config allows UploadFile (see schemas/profiles.py) so this dependency works at runtime .

[mateacademy-ai-mentor]

After reading the uploaded file bytes there is no enforcement of the required 1MB size limit. The task requires avatar files to not exceed 1MB; add a size check (e.g. if len(file_bytes) > 1_048_576: raise HTTPException(status_code=400, detail="Avatar must not exceed 1MB.")) or ensure validate_image enforces this before upload to satisfy the requirement .

[mateacademy-ai-mentor]

You await the S3 client upload_file method and get_file_url. Make sure the S3StorageInterface actually defines these methods as async — otherwise awaiting a sync method will raise at runtime. If the storage client is synchronous, call it without await or provide an async wrapper; confirm the interface in get_s3_storage_client/storages implementation .

[mateacademy-ai-mentor]

You save avatar_db_value = avatar_key (a storage key) into the database but return a full URL in the response. Ensure this is intentional and consistent: either store the public URL in the DB or store the key and always generate the URL when returning it. Tests/clients expect a real URL in the response, so make sure get_file_url reliably returns one and that the DB schema expectations match this approach.

[mateacademy-ai-mentor]

The general exception except Exception: around the upload will produce the required 500 response, but consider catching storage-specific exceptions to avoid hiding unrelated errors. At minimum, keep the required response body ("Failed to upload avatar. Please try again later.") which this code does — good. Still, refining the exception handling will help debugging and tests.

[mateacademy-ai-mentor]

ProfileResponseSchema sets model_config = {"from_attributes": True} while the request schema does not set any model_config. Keep configs consistent — the request schema needs arbitrary_types_allowed (and likely from_attributes) to function correctly. Add the config to UserProfileSchema rather than relying only on the response schema.

[mateacademy-ai-mentor]

The activation link is constructed with only the token parameter. The activate_account handler queries by both UserModel.email and ActivationTokenModel.token (it expects the email too). Either include the email in the link (e.g., ?token=...&email=<email>) or adjust the activation endpoint to accept token-only — otherwise the emailed URL will not activate the account directly (violates the "generate valid URLs for email actions" requirement).

[mateacademy-ai-mentor]

You use await db.delete(token_record) earlier in this function (see the token deletion above). AsyncSession.delete() is not an awaitable in SQLAlchemy — awaiting it will raise. Use a supported pattern: either execute a SQL delete(...) expression with await db.execute(...) or run the ORM delete synchronously inside await db.run_sync(lambda s: s.delete(token_record)). Also standardize deletion approach across this file to avoid intermittent failures.

[mateacademy-ai-mentor]

Good: you add the activation-complete email via background_tasks.add_task(...). However, note that elsewhere in this file token deletions were done with await db.delete(...) which is incorrect (not awaitable). Make sure token deletion used before this point is fixed (use run_sync or execute a delete expression).

[mateacademy-ai-mentor]

In reset_password you use await db.run_sync(lambda s: s.delete(token_record)) to remove token records (this is the correct pattern for ORM delete with AsyncSession). The file mixes this correct approach with await db.delete(...) in other places — please standardize to the run_sync or execute(delete(...)) approach throughout to avoid runtime errors.

[mateacademy-ai-mentor]

Review Summary

I am approving your solution as an educational override so you can keep progressing while you address a few important issues.

You did a solid job wiring up the profile creation route and the account email flows: the /users/{user_id}/profile/ endpoint performs token validation, authorization, S3 upload, and DB persistence, and the account routes correctly send activation and reset emails using BackgroundTasks and the injected email sender. The main problems to fix next are in src/schemas/profiles.py: allow UploadFile by adding "arbitrary_types_allowed": True to model_config, make the avatar validator use if value is None: and raise ValueError instead of HTTPException, and ensure the avatar size limit (≤ 1MB) is enforced there or clearly delegated to validate_image. In src/routes/profiles.py, double‑check that the S3 storage interface methods (upload_file, get_file_url) are truly async before using await, and either make them async or call sync methods via a thread executor to avoid runtime errors. Also, align types and contracts by typing the JWT dependency with the JWT auth interface instead of the concrete class, return a clearer message like "Invalid token." for JWTError, and make sure you don’t return avatar=None when ProfileResponseSchema.avatar is a non‑optional HttpUrl (either always provide a URL or make the schema field optional).

Overall, your structure and use of FastAPI dependencies/background tasks are strong; with these adjustments to validation, typing, and S3 async handling, your implementation will be both robust and production‑ready. Keep going—these are exactly the kinds of details that sharpen your backend engineering skills.

---

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

Footnotes

1. Rate AI review example ↩

[mateacademy-ai-mentor]

You await s3_client.upload_file(...). Ensure S3StorageInterface.upload_file is implemented as an async method. If it is synchronous, awaiting it will raise a runtime error — either make the storage API async or call it in a thread executor (or provide an async wrapper). Verify the storages interface implementation and adjust accordingly.

[mateacademy-ai-mentor]

You await s3_client.get_file_url(...). As above, make sure get_file_url is async in the storage interface. If it's synchronous, calling await will fail — either remove await or adapt the storage client to an async API.

[mateacademy-ai-mentor]

You compare current_user.group_id != UserGroupEnum.ADMIN.value. Verify what is actually stored in group_id (an integer id, or a string/name). If group_id stores the DB FK (int), comparing to the enum .value may be OK — otherwise compare using the group's name or change the check to use the consistent representation used across the codebase (see accounts routes patterns).

[mateacademy-ai-mentor]

Catching JWTError and returning the message "Invalid Authorization header format. Expected 'Bearer '" may be misleading: JWTError is raised by token decoding (invalid signature, malformed token) not header parsing. Consider returning a clearer message such as "Invalid token." or keep header-format messages only inside get_token. This will make client errors easier to understand and align with the token extraction behavior described in the task.

[mateacademy-ai-mentor]

The response builds ProfileResponseSchema and sets avatar to None when avatar_url is missing. But ProfileResponseSchema.avatar is declared as a non-optional HttpUrl in schemas/profiles.py — returning None will fail validation. Either ensure an avatar URL is always returned, or make the schema's avatar field Optional (and update tests/contracts accordingly). See the schema file for ProfileResponseSchema.

[mateacademy-ai-mentor]

Validators should raise ValueError (or TypeError) so Pydantic collects errors into a 422 response. Replace raise HTTPException(...) with raise ValueError("Avatar is required") in the avatar validator.

[mateacademy-ai-mentor]

You call validate_image(value) which is correct per requirements, but the schema must ensure avatar size ≤ 1MB. If validate_image does not enforce size, add an explicit check here: read file_bytes = await value.read() (or value.file.read() for sync), check len(file_bytes) <= 1_048_576, then value.file.seek(0). If validate_image already checks size and extensions, note that in a comment for maintainers.

[mateacademy-ai-mentor]

You .lower() first/last names in validators. The task example shows lowercase output, so this may be fine — but confirm tests expect lowercased names; if not, preserve original casing and only validate.

[mateacademy-ai-mentor]

The password reset link built here includes only the token. The reset endpoint implementation expects the user to supply an email (it selects user by data.email) — that is fine if the frontend page the link points to asks the user to enter their email along with the new password. If you want a link that allows pre-filling the email on the frontend (or matches a GET-based flow), include the user's email in the query params (e.g. ?token=...&email=...). Otherwise ensure frontend knows to send the email in the reset POST body.

[mateacademy-ai-mentor]

Good: when token records are invalid/expired you delete them using await db.run_sync(lambda s: s.delete(token_record)) (instead of await db.delete(...)). This avoids awaiting a synchronous ORM method and prevents runtime errors — keep this pattern for synchronous model deletions.