feat(user): add user profile picture update functionality#306
feat(user): add user profile picture update functionality#306AnujChhikara wants to merge 3 commits intodevelopfrom
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces profile picture upload functionality by integrating Cloudinary. New components include environment variable configuration, a Cloudinary service for image uploads, a serializer for file validation, a repository method for persisting picture URLs, and a new API endpoint to handle profile picture updates with full integration tests. Changes
Sequence DiagramsequenceDiagram
participant Client
participant UserDetailView
participant UpdateProfileSerializer
participant CloudinaryService
participant Cloudinary
participant UserRepository
participant Database
Client->>UserDetailView: PATCH /users/profile (multipart file)
UserDetailView->>UpdateProfileSerializer: validate(request.data)
UpdateProfileSerializer->>UpdateProfileSerializer: validate_picture(file)<br/>(format, size check)
UpdateProfileSerializer-->>UserDetailView: validated picture file
UserDetailView->>CloudinaryService: upload_image(file_data,<br/>user_id, image_name)
CloudinaryService->>CloudinaryService: _require_config()
CloudinaryService->>Cloudinary: uploader.upload(BytesIO,<br/>folder, public_id)
Cloudinary-->>CloudinaryService: {secure_url, ...}
CloudinaryService-->>UserDetailView: secure_url
UserDetailView->>UserRepository: update_picture_by_id(user_id,<br/>picture_url)
UserRepository->>Database: findOneAndUpdate(_id,<br/>picture, updated_at)
Database-->>UserRepository: updated_user_doc
UserRepository-->>UserDetailView: UserModel
UserDetailView-->>Client: 200 OK {id, email,<br/>name, picture}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@requirements.txt`:
- Line 32: Replace the pinned dependency "cloudinary==1.37.0" in
requirements.txt with "cloudinary==1.44.1"; after updating the version string,
regenerate any dependency lockfile or run your dependency tooling (pip install
-r requirements.txt or pip-compile / poetry lock as applicable) and run the test
suite to ensure compatibility with the Cloudinary v1.44.1 upgrade.
In `@todo/repositories/user_repository.py`:
- Around line 100-115: The Mongo-only update in update_picture_by_id causes
Postgres drift; mirror the dual-write used in create_or_update by invoking the
same EnhancedDualWriteService update flow after the Mongo update. Specifically,
in update_picture_by_id (after collection.find_one_and_update returns a result
and before returning UserModel(**result)), call the EnhancedDualWriteService
method used by create_or_update to sync updates to Postgres with the user_id and
the changed fields (picture and updated_at); handle any DualWrite errors
consistently with create_or_update (log/raise or surface failure) so both stores
stay consistent and errors are not swallowed.
- Line 104: The code constructs a PyObjectId from user_id without validation, so
malformed IDs raise and return HTTP 500; wrap the PyObjectId(user_id)
construction in a try/except (or validate user_id up front) inside the method
that uses it and on error raise the same repository-level error (e.g.,
UserNotFoundException or a validation error) to mirror get_by_id behavior;
reference the PyObjectId(user_id) call and ensure you raise
UserNotFoundException (or the repository's validation exception) instead of
letting the raw exception propagate.
In `@todo/serializers/update_profile_serializer.py`:
- Around line 12-13: The validate_picture method contains a dead check: remove
the unreachable "if not value: raise serializers.ValidationError('picture file
is required')" because FileField(required=True) already enforces presence;
instead validate the actual file content-type and size, and if you need stronger
guarantees validate magic bytes (e.g., open with Pillow or check file header)
rather than trusting value.content_type which is client-supplied; keep any
remaining checks to raise serializers.ValidationError when the file fails
content/size/type validation.
In `@todo/services/cloudinary_service.py`:
- Around line 4-22: The _require_config() in CloudinaryService reads
CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET but never
applies them, so uploader.upload will fail; fix by calling
cloudinary.config(cloud_name=..., api_key=..., api_secret=...) with the values
returned from _require_config() before any upload (or move that config call to
application startup). Update CloudinaryService (where _require_config() is
invoked) to use its return tuple to call cloudinary.config(...) prior to calling
uploader.upload(), ensuring credentials are actually applied; keep the
cloudinary import and remove any unused env reads if you centralize config at
app startup.
In `@todo/tests/integration/test_user_profile_api.py`:
- Around line 31-44: The test test_update_profile_picture_persists_picture_url
fails because using BytesIO(b"fake_image_data") yields content_type
"application/octet-stream" which validate_picture rejects against
ALLOWED_IMAGE_TYPES, so replace the file fixture with a Django
SimpleUploadedFile that sets an explicit image content_type (e.g., "image/png")
so the serializer's validate_picture accepts it and
CloudinaryService.upload_image is invoked; update the patch call to send the
SimpleUploadedFile instead of BytesIO while keeping the existing patch of
CloudinaryService.upload_image.
In `@todo/views/user.py`:
- Around line 159-169: The current broad except in the profile image upload
masks configuration/validation errors and returns raw error strings; update the
try/except around CloudinaryService.upload_image to catch specific exceptions
(e.g., APIException from CloudinaryService._require_config and the
cloudinary.exceptions.* classes), log the full exception server-side using
logger.exception or similar, and return a generic client-facing Response like
{"message": "Failed to upload image"} with the appropriate 5xx status (keep
APIException/config errors as 500) without including str(e) in the payload;
ensure CloudinaryService.upload_image remains called as-is and only the
exception handling around it is changed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 705b13b5-58d2-44d8-b667-eaf5e01ba9d5
📒 Files selected for processing (8)
.env.examplerequirements.txttodo/repositories/user_repository.pytodo/serializers/update_profile_serializer.pytodo/services/cloudinary_service.pytodo/tests/integration/test_user_profile_api.pytodo/urls.pytodo/views/user.py
| from cloudinary import uploader | ||
| import cloudinary | ||
|
|
||
| from todo.exceptions.auth_exceptions import APIException | ||
|
|
||
|
|
||
| class CloudinaryService: | ||
| @staticmethod | ||
| def _require_config() -> tuple[str, str, str]: | ||
| cloud_name = os.getenv("CLOUDINARY_CLOUD_NAME") | ||
| api_key = os.getenv("CLOUDINARY_API_KEY") | ||
| api_secret = os.getenv("CLOUDINARY_API_SECRET") | ||
|
|
||
| if not cloud_name or not api_key or not api_secret: | ||
| raise APIException( | ||
| "Cloudinary is not configured. Set CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET." | ||
| ) | ||
|
|
||
| return str(cloud_name), str(api_key), str(api_secret) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
cloudinary python SDK configure credentials cloudinary.config vs CLOUDINARY_URL
💡 Result:
In the Cloudinary Python SDK, credentials can be configured in two primary ways: using the CLOUDINARY_URL environment variable or explicitly via cloudinary.config. The CLOUDINARY_URL environment variable is the recommended and most convenient method, especially for security. It follows the format cloudinary://api_key:api_secret@cloud_name and can include additional query parameters for other config options (e.g., ?secure=true). The SDK automatically detects and parses this variable upon import/initialization if present, populating the config with cloud_name, api_key, api_secret, and other params. No explicit config call is needed for basic setup. Use tools like python-dotenv to load it from a .env file. Example: # .env CLOUDINARY_URL=cloudinary://your_api_key:your_api_secret@your_cloud_name # In code from dotenv import load_dotenv load_dotenv import cloudinary # Credentials are auto-loaded; verify with: print(cloudinary.config.cloud_name) Alternatively, use cloudinary.config to set credentials programmatically, either directly or from other env vars. This overrides or supplements the auto-loaded config. Example: import cloudinary import os cloudinary.config( cloud_name="your_cloud_name", api_key=os.getenv("CLOUDINARY_API_KEY"), api_secret=os.getenv("CLOUDINARY_API_SECRET"), secure=True ) Both methods support all config params (e.g., secure, private_cdn). CLOUDINARY_URL is preferred for production as it centralizes secrets in env vars. Call config before importing uploader/api modules for full functionality. The global config is a singleton updated by both approaches.
Citations:
- 1: https://cloudinary.com/documentation/python_configuration_tutorial
- 2: https://console.cloudinary.com/documentation/python_quickstart
- 3: https://console-nightly.cloudinary.com/documentation/python_quickstart
- 4: https://cloudinary.com/documentation/django_integration
- 5: https://cloudinary.com/documentation/python_quickstart
- 6: https://github.com/cloudinary/pycloudinary/blob/master/cloudinary/__init__.py
🏁 Script executed:
cat -n todo/services/cloudinary_service.pyRepository: RealDevSquad/todo-backend
Length of output: 1764
🏁 Script executed:
rg "cloudinary\.config" --type pyRepository: RealDevSquad/todo-backend
Length of output: 51
🏁 Script executed:
rg "CLOUDINARY_URL" --type pyRepository: RealDevSquad/todo-backend
Length of output: 51
Critical: Cloudinary is not configured — uploader.upload will fail in production
_require_config() reads the environment variables and returns them, but the return value is discarded and cloudinary.config() is never called. The Cloudinary SDK requires either a CLOUDINARY_URL environment variable (which is auto-parsed) or an explicit cloudinary.config(cloud_name=..., api_key=..., api_secret=...) call; with three separate CLOUDINARY_* variables and no config call, uploader.upload will raise an authentication error at runtime. The cloudinary import is also dead code until configuration is applied.
At line 32, _require_config() is called but its return value is ignored. The credentials are read but never used.
Proposed fix: Call cloudinary.config() with the environment variables before uploader.upload() is called:
Example fix
`@staticmethod`
- def _require_config() -> tuple[str, str, str]:
+ def _require_config() -> None:
cloud_name = os.getenv("CLOUDINARY_CLOUD_NAME")
api_key = os.getenv("CLOUDINARY_API_KEY")
api_secret = os.getenv("CLOUDINARY_API_SECRET")
if not cloud_name or not api_key or not api_secret:
raise APIException(
"Cloudinary is not configured. Set CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET."
)
-
- return str(cloud_name), str(api_key), str(api_secret)
+ cloudinary.config(
+ cloud_name=cloud_name,
+ api_key=api_key,
+ api_secret=api_secret,
+ secure=True,
+ )Alternatively, move the configuration call to application startup to avoid reconfiguring on every upload.
🧰 Tools
🪛 GitHub Actions: Tests
[error] 5-5: ruff check reported unused import: cloudinary imported but unused (F401). Remove unused import: cloudinary.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@todo/services/cloudinary_service.py` around lines 4 - 22, The
_require_config() in CloudinaryService reads CLOUDINARY_CLOUD_NAME,
CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET but never applies them, so
uploader.upload will fail; fix by calling cloudinary.config(cloud_name=...,
api_key=..., api_secret=...) with the values returned from _require_config()
before any upload (or move that config call to application startup). Update
CloudinaryService (where _require_config() is invoked) to use its return tuple
to call cloudinary.config(...) prior to calling uploader.upload(), ensuring
credentials are actually applied; keep the cloudinary import and remove any
unused env reads if you centralize config at app startup.
✅ Confirmed as addressed by @AnujChhikara
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
cat -n todo/services/cloudinary_service.py
Length of output: 2016
@AnujChhikara, the fix looks good! The core issue is resolved:
- The new
_configure()method (Lines 24–31) correctly unpacks the tuple returned by_require_config()and passes the credentials tocloudinary.config(...). upload_imagenow callscls._configure()(Line 41) before invokinguploader.upload(), ensuring credentials are applied at runtime.- The
cloudinaryimport is no longer dead code.
One optional improvement: consider adding secure=True to the cloudinary.config() call to ensure all asset URLs use HTTPS by default:
cloudinary.config(
cloud_name=cloud_name,
api_key=api_key,
api_secret=api_secret,
secure=True, # ensures secure_url is always HTTPS
)But that's a non-critical enhancement — the authentication issue is fully addressed. ✅
If you found this review helpful, would you consider giving us a shout-out on X?
Thank you for using CodeRabbit!
There was a problem hiding this comment.
@AnujChhikara can you try adding secure=True as well in config?
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
- Introduced a new endpoint to update the user's profile picture. - Implemented a serializer to validate the uploaded image. - Integrated Cloudinary for image upload and storage. - Updated user repository to handle picture URL updates. - Added integration tests for the new profile picture update feature. - Updated .env.example to include Cloudinary configuration variables. - Added cloudinary dependency to requirements.txt.
AnujChhikara
force-pushed
the
anuj/feat-image-upload
branch
from
5a2b9de to
dfa9cdd
Compare
- Replaced BytesIO with SimpleUploadedFile for simulating image uploads in tests. - Added tests for invalid file types and missing files during profile picture updates. - Ensured proper handling of HTTP status codes for various scenarios.
| from cloudinary import uploader | ||
| import cloudinary | ||
|
|
||
| from todo.exceptions.auth_exceptions import APIException | ||
|
|
||
|
|
||
| class CloudinaryService: | ||
| @staticmethod | ||
| def _require_config() -> tuple[str, str, str]: | ||
| cloud_name = os.getenv("CLOUDINARY_CLOUD_NAME") | ||
| api_key = os.getenv("CLOUDINARY_API_KEY") | ||
| api_secret = os.getenv("CLOUDINARY_API_SECRET") | ||
|
|
||
| if not cloud_name or not api_key or not api_secret: | ||
| raise APIException( | ||
| "Cloudinary is not configured. Set CLOUDINARY_CLOUD_NAME, CLOUDINARY_API_KEY, and CLOUDINARY_API_SECRET." | ||
| ) | ||
|
|
||
| return str(cloud_name), str(api_key), str(api_secret) |
There was a problem hiding this comment.
@AnujChhikara can you try adding secure=True as well in config?
- Set secure uploads to true for Cloudinary integration. - Modified public ID generation to use only the image name, removing the user ID prefix.
2 participants
Date: 17 April, 2026
Developer Name: @AnujChhikara
Issue Ticket Number
Tech Doc Link
Business Doc Link
Description
Documentation Updated?
Under Feature Flag
Database Changes
Breaking Changes
Development Tested?
Screenshots
Screenshot 1
user.profile.todo.mp4
Test Coverage
Screenshot 1
Additional Notes