[Forge][P1] Add logo selection to custom Post Type creation flow#245
[Forge][P1] Add logo selection to custom Post Type creation flow#245
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis pull request adds logo upload functionality to the custom post types feature. Users can upload PNG/JPG/WEBP/GIF logos (up to 2MB) when creating or editing custom post types. The logos persist on the post-type records and display across UI surfaces with fallback to default icons. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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)
📝 Coding Plan
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 |
Greptile SummaryThis PR adds optional logo upload support to the custom post type create/edit flows — persisting the logo on Key findings:
Confidence Score: 2/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Browser
participant CreateView as ProjectCustomPostTypesView
participant UpdateView as ProjectCustomPostTypeUpdateView
participant Form as ProjectCustomPostTypeForm
participant Model as ProjectCustomPostType
participant Storage
Note over Browser,Storage: Create flow
Browser->>CreateView: POST /project/:pk/custom-post-types (multipart)
CreateView->>Form: ProjectCustomPostTypeForm(POST, FILES)
Form->>Form: ImageField.to_python() → PIL validates image
Form->>Form: clean_logo() → checks content_type + size
alt Form invalid
CreateView-->>Browser: 200 re-render with inline errors
else Form valid
Form->>Model: save(commit=False) → project assigned
Model->>Model: full_clean() → clean() size check
Model->>Storage: upload to custom_post_type_logos/
CreateView-->>Browser: 302 redirect
end
Note over Browser,Storage: Edit/Update flow
Browser->>UpdateView: POST /project/:pk/post-type/:id (multipart)
UpdateView->>Form: ProjectCustomPostTypeForm(POST, FILES, instance=…)
Form->>Form: ImageField.to_python() → PIL validates image
Form->>Form: clean_logo() → checks content_type + size
alt Form invalid
UpdateView-->>Browser: 302 redirect + generic flash error ⚠️
else Form valid
Form->>Model: form.save()
Model->>Model: full_clean()
Model->>Storage: upload / keep existing logo
UpdateView-->>Browser: 302 redirect + success flash
end
Last reviewed commit: "Add custom post type..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| oversized = b"0" * (ProjectCustomPostType.logo_max_file_size_bytes + 1) | ||
|
|
||
| client.force_login(user) | ||
| response = client.post( | ||
| reverse("project_custom_post_types", kwargs={"pk": project.id}), | ||
| { | ||
| "name": "News", | ||
| "prompt_guidance": "Fast-paced updates.", | ||
| "logo": SimpleUploadedFile("logo.png", oversized, content_type="image/png"), | ||
| }, | ||
| ) | ||
|
|
||
| assert response.status_code == 200 | ||
| assert "Logo must be 2MB or smaller" in response.content.decode("utf-8") |
There was a problem hiding this comment.
The oversized data (b"0" * 2097153) is not a valid PNG file. Django's forms.ImageField (the form field type produced from a model ImageField) calls PIL's Image.open() + image.verify() in to_python() before the form's clean_logo method executes. PIL will reject the raw bytes with a "Upload a valid image…" error, so the "Logo must be 2MB or smaller" assertion will never be true and the test will fail.
To properly test the size path you need a structurally valid image whose byte-length exceeds 2 MB. One common approach is to embed the 1×1 PNG bytes inside a large-enough payload by stuffing a PNG comment chunk, or to mock logo.size at the form layer. For example:
# Build a >2 MB payload that PIL still accepts by wrapping TINY_PNG_BYTES in
# an in-memory file but overriding the reported size.
from unittest.mock import patch
from django.core.files.uploadedfile import SimpleUploadedFile
oversized_file = SimpleUploadedFile("logo.png", TINY_PNG_BYTES, content_type="image/png")
oversized_file._size = ProjectCustomPostType.logo_max_file_size_bytes + 1
client.force_login(user)
response = client.post(
reverse("project_custom_post_types", kwargs={"pk": project.id}),
{
"name": "News",
"prompt_guidance": "Fast-paced updates.",
"logo": oversized_file,
},
)SimpleUploadedFile exposes a writable _size attribute that the form reads via logo.size, letting you simulate an oversized file without actually allocating 2 MB or crafting a real large image.
| <div> | ||
| <label class="block mb-1 text-sm font-medium text-gray-700">Logo (optional)</label> | ||
| <input type="file" name="logo" accept="image/png,image/jpeg,image/webp,image/gif" class="block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500" /> | ||
| <p class="mt-1 text-xs text-gray-500">Accepted: PNG, JPG, WEBP, GIF · max 2MB.</p> | ||
| </div> |
There was a problem hiding this comment.
The edit form only renders a blank <input type="file">. When the form is submitted without selecting a new file, Django's FileField preserves the existing value from the instance — which is correct — but it also means there is no mechanism for a user to clear a logo they previously uploaded. They can replace it but never remove it.
A common Django pattern is to add a ClearableFileInput widget or a companion boolean checkbox clear_logo that, when checked, sets logo = None in the view before form.save(). Even a simple "Remove logo" button that POSTs to a dedicated endpoint would unblock users.
Additionally, the edit form renders the file input as raw HTML rather than through the form widget ({{ edit_form.logo }}), which means any error messages from clean_logo (wrong format, oversized) are silently dropped when the update view redirects — the user only sees the generic "Could not update custom post type. Please check the form." flash message with no indication of which field failed or why.
| logo = models.ImageField( | ||
| upload_to="custom_post_type_logos/", | ||
| blank=True, | ||
| validators=[FileExtensionValidator(allowed_extensions=["png", "jpg", "jpeg", "webp", "gif"])], | ||
| ) |
There was a problem hiding this comment.
FileExtensionValidator is weaker than the form's content-type check and could be misleading
FileExtensionValidator only inspects the file's extension (e.g., ".png"), which an attacker can trivially spoof. The form's clean_logo already enforces a stricter check via the MIME content_type attribute. Having the extension validator on the model could give a false sense of security for code paths that bypass the form (e.g., tests or management commands that create records directly), while still allowing a file named evil.png with non-image content to pass through the model-level guard.
Consider replacing it with a custom validator that also checks the declared MIME type, or relying solely on the forms.ImageField PIL validation (which is already the strongest layer). If the intent is defence-in-depth at the model layer, the validator should at least mirror what the form checks.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
core/views.py (1)
1567-1574: Add a structured log before redirecting invalid updates.These failures currently only hit flash messages, so repeated rejected logo uploads leave no server-side trace. Logging
project_id,post_type_id, andform.errors.get_json_data()here would make support/debugging much easier.As per coding guidelines, "Use Python's logging module (structlog) extensively" and "Include context in log messages with relevant fields (error, exc_info, resource IDs, etc.) to aid debugging and traceability."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/views.py` around lines 1567 - 1574, Add a structured server-side log before emitting flash messages for invalid form updates: call the module's structlog logger (e.g., logger = structlog.get_logger() or existing logger) and log at error level including fields project_id, post_type_id and the form errors via form.errors.get_json_data(), plus any contextual info like request.user.id; do this right before the existing messages.error loop so invalid update attempts are recorded with these identifiers and the error payload for debugging/traceability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/forms.py`:
- Around line 340-353: Replace the explicit logo = forms.FileField override with
Django's forms.ImageField so Pillow-based image validation is preserved (keep
the existing widget attrs and optional required=False setting), and retain any
size checks in clean_logo() but stop relying solely on client-supplied
content_type; ensure the form continues to use the ProjectCustomPostType model
and fields in Meta and leave clear_logo untouched—update references to logo and
clean_logo accordingly so ImageField.clean handles actual image verification
while you still enforce max-size server-side.
In `@frontend/templates/project/project_custom_post_types.html`:
- Around line 65-68: The label for the logo file input is not bound to the
input, so add an id to the input (e.g., id="logo") and set the label's for
attribute to that id to associate them (look for the <label> and <input
name="logo"> elements in project_custom_post_types.html); optionally ensure the
helper paragraph is referenced via aria-describedby on the input if you want
screen readers to read the "Accepted: ..." text (use the helper's id and set
input aria-describedby to that id).
---
Nitpick comments:
In `@core/views.py`:
- Around line 1567-1574: Add a structured server-side log before emitting flash
messages for invalid form updates: call the module's structlog logger (e.g.,
logger = structlog.get_logger() or existing logger) and log at error level
including fields project_id, post_type_id and the form errors via
form.errors.get_json_data(), plus any contextual info like request.user.id; do
this right before the existing messages.error loop so invalid update attempts
are recorded with these identifiers and the error payload for
debugging/traceability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bdc99c2-bb34-499f-a674-ecb07e6ed2b5
📒 Files selected for processing (9)
CHANGELOG.mdcore/forms.pycore/migrations/0065_projectcustomposttype_logo.pycore/models.pycore/tests/test_custom_post_types.pycore/views.pyfrontend/templates/components/project_navigation.htmlfrontend/templates/project/project_custom_post_type_posts.htmlfrontend/templates/project/project_custom_post_types.html
| logo = forms.FileField( | ||
| required=False, | ||
| widget=forms.FileInput( | ||
| attrs={ | ||
| "class": "block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500", | ||
| "accept": "image/png,image/jpeg,image/webp,image/gif", | ||
| } | ||
| ), | ||
| ) | ||
| clear_logo = forms.BooleanField(required=False) | ||
|
|
||
| class Meta: | ||
| model = ProjectCustomPostType | ||
| fields = ["name", "prompt_guidance"] | ||
| fields = ["name", "prompt_guidance", "logo"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "models.py" | head -20Repository: rasulkireev/TuxSEO
Length of output: 181
🏁 Script executed:
rg "class ProjectCustomPostType" -A 30 --type pyRepository: rasulkireev/TuxSEO
Length of output: 10648
🏁 Script executed:
rg -B 5 -A 15 "logo.*=.*ImageField\|logo.*=.*FileField" core/forms.pyRepository: rasulkireev/TuxSEO
Length of output: 44
🏁 Script executed:
# Get the full ProjectCustomPostTypeForm to understand the structure
sed -n '320,410p' core/forms.pyRepository: rasulkireev/TuxSEO
Length of output: 3023
🏁 Script executed:
# Check if Pillow is in dependencies and if ImageField validation actually happens
rg -i "pillow|image" pyproject.toml requirements.txt setup.py setup.cfg 2>/dev/null | head -20Repository: rasulkireev/TuxSEO
Length of output: 181
🏁 Script executed:
# Verify the model field definition and understand the validators
rg -B 5 -A 5 "class ProjectCustomPostType" core/models.pyRepository: rasulkireev/TuxSEO
Length of output: 336
Don't downgrade the model ImageField to a plain FileField.
The explicit logo = forms.FileField override removes Django's built-in image validation. The form's clean_logo() method only checks the content_type header attribute (client-supplied, spoofable) and file size. A crafted request can submit arbitrary bytes with a PNG/JPEG extension and matching Content-Type header, bypassing validation and breaking the security guarantee provided by Pillow's ImageField validation.
🖼️ Proposed fix
class ProjectCustomPostTypeForm(forms.ModelForm):
- logo = forms.FileField(
- required=False,
- widget=forms.FileInput(
- attrs={
- "class": "block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500",
- "accept": "image/png,image/jpeg,image/webp,image/gif",
- }
- ),
- )
clear_logo = forms.BooleanField(required=False)
class Meta:
model = ProjectCustomPostType
fields = ["name", "prompt_guidance", "logo"]
widgets = {
+ "logo": forms.FileInput(
+ attrs={
+ "class": "block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500",
+ "accept": "image/png,image/jpeg,image/webp,image/gif",
+ }
+ ),
"name": forms.TextInput(🧰 Tools
🪛 Ruff (0.15.6)
[warning] 353-353: Mutable default value for class attribute
(RUF012)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@core/forms.py` around lines 340 - 353, Replace the explicit logo =
forms.FileField override with Django's forms.ImageField so Pillow-based image
validation is preserved (keep the existing widget attrs and optional
required=False setting), and retain any size checks in clean_logo() but stop
relying solely on client-supplied content_type; ensure the form continues to use
the ProjectCustomPostType model and fields in Meta and leave clear_logo
untouched—update references to logo and clean_logo accordingly so
ImageField.clean handles actual image verification while you still enforce
max-size server-side.
| <div> | ||
| <label class="block mb-1 text-sm font-medium text-gray-700">Logo (optional)</label> | ||
| <input type="file" name="logo" accept="image/png,image/jpeg,image/webp,image/gif" class="block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500" /> | ||
| <p class="mt-1 text-xs text-gray-500">Accepted: PNG, JPG, WEBP, GIF · max 2MB.</p> |
There was a problem hiding this comment.
Bind the update-form label to its file input.
The new logo label is not associated with the file input, so screen readers will not announce the control name correctly and clicking the label will not focus the picker.
♿ Proposed fix
- <label class="block mb-1 text-sm font-medium text-gray-700">Logo (optional)</label>
- <input type="file" name="logo" accept="image/png,image/jpeg,image/webp,image/gif" class="block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500" />
+ <label class="block mb-1 text-sm font-medium text-gray-700" for="id_logo_{{ post_type.id }}">Logo (optional)</label>
+ <input id="id_logo_{{ post_type.id }}" type="file" name="logo" accept="image/png,image/jpeg,image/webp,image/gif" class="block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500" />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div> | |
| <label class="block mb-1 text-sm font-medium text-gray-700">Logo (optional)</label> | |
| <input type="file" name="logo" accept="image/png,image/jpeg,image/webp,image/gif" class="block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500" /> | |
| <p class="mt-1 text-xs text-gray-500">Accepted: PNG, JPG, WEBP, GIF · max 2MB.</p> | |
| <div> | |
| <label class="block mb-1 text-sm font-medium text-gray-700" for="id_logo_{{ post_type.id }}">Logo (optional)</label> | |
| <input id="id_logo_{{ post_type.id }}" type="file" name="logo" accept="image/png,image/jpeg,image/webp,image/gif" class="block w-full text-sm text-gray-900 rounded-md border border-gray-300 cursor-pointer bg-gray-50 focus:outline-none focus:ring-2 focus:ring-gray-500 focus:border-gray-500" /> | |
| <p class="mt-1 text-xs text-gray-500">Accepted: PNG, JPG, WEBP, GIF · max 2MB.</p> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/templates/project/project_custom_post_types.html` around lines 65 -
68, The label for the logo file input is not bound to the input, so add an id to
the input (e.g., id="logo") and set the label's for attribute to that id to
associate them (look for the <label> and <input name="logo"> elements in
project_custom_post_types.html); optionally ensure the helper paragraph is
referenced via aria-describedby on the input if you want screen readers to read
the "Accepted: ..." text (use the helper's id and set input aria-describedby to
that id).
1 participant
Summary
logoimage upload on custom post type create/edit formsProjectCustomPostTypewith migrationTesting
./.venv/bin/python -m compileall core/models.py core/forms.py core/views.py core/tests/test_custom_post_types.py./.venv/bin/pytest core/tests/test_custom_post_types.py -q(fails in current sandbox due unavailable Postgres hostdb; expected to run in CI environment)Summary by CodeRabbit
New Features
Tests