Opened 15 years ago
Closed 12 years ago
#12742 closed New feature (wontfix)
File upload handler in Comments framework
Reported by: | Sebastian Żurek | Owned by: | nobody |
---|---|---|---|
Component: | contrib.comments | Version: | dev |
Severity: | Normal | Keywords: | files |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I've recently find out some annoying django.contrib.comments limitations-chain. It came out after unsuccessful custom comment form (that included FileField) submission.
This is what I mean by limitations-chain:
A)
I could not use comment model having file field (CommentSeciurityForm does not use *args, kwargs when supering forms.Form and thus files=request.FILES was not valid init kword argument in forms that base on CommentSeciurityForm)
B)
In consequence comment form post handler (view function named 'post_comment') is not using request.FILES
C)
Which is related with form template issue: check for form.is_multipart is missing (hence multipart/form-data enctype is not set)
Are those file-handling limitation intentionally created (some security reasons?)? I'm attaching a .diff with some cosmetic changes, that allow for files upload.
While browsing django.contrib.comments code and using the framework in my project I can see there's still some work to be done (there's plenty of room for some generalizations - I found API being nice, however we could make it more flexible with e.g. allowing more than one comment form definition).
However, to stay focused on one issue: could You please comment this problem (or not) with files?
Attachments (2)
Change History (10)
by , 15 years ago
Attachment: | file_upload.diff added |
---|
comment:1 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 15 years ago
milestone: | → 1.2 |
---|
comment:3 by , 15 years ago
milestone: | 1.2 |
---|
comment:4 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:5 by , 14 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Type: | Bug → New feature |
comment:6 by , 13 years ago
Easy pickings: | unset |
---|---|
UI/UX: | unset |
Related (slightly duplicate) to #10838.
by , 13 years ago
Attachment: | 12742.diff added |
---|
comment:7 by , 13 years ago
The reason for adding args and kwargs to the post_comment
and then passing these to the form would be to allow users to send additional data and (say) from there on to the model (could be a custom comment).
For example, with the deprecation of the RemoteAddrMiddleware, there is no standard way to fetch the IP address. The IP address could be passed as a kwarg to the form. This could prevent hacks like https://github.com/theju/django-comments-apps/blob/master/recaptcha_comments/views.py
Another case, could be where the form's rendering depends on the request (or the session) and these could be passed through the args and/or kwargs (they can't be handled in the current case).
For this ticket, the post_comment could be passed the request from which the request.FILES can be accessed.
This patch will also solve quite a lot of #10838 and which requires the user to be passed.
Just for the record, Alex (from a conversation on IRC) is not happy with passing args and kwargs to the post_comment view function. I have placed my arguments for the reason to include the args and kwargs and now the decision is left to you :)
comment:8 by , 12 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
django.contrib.comments
has been deprecated and is no longer supported, so I'm closing this ticket. We're encouraging users to transition to a custom solution, or to a hosted solution like Disqus.
The code itself has moved to https://github.com/django/django-contrib-comments; if you want to keep using it, you could move this bug over there.
1.2 is feature-frozen, moving this feature request off the milestone.