#9268 closed (fixed)
Pass custom form values from post-comment to preview-comment
| Reported by: | Eric Abrahamsen | Owned by: | stuartk |
|---|---|---|---|
| Component: | contrib.comments | Version: | dev |
| Severity: | Keywords: | comment preview | |
| Cc: | girzel@… | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | yes |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | UI/UX: |
Description
It's possible to add custom fields to the comment post form (to provide a 'next' value, or other custom values), but if the user opts to preview their comment, those custom values are lost. Currently the only context variables being passed to the preview view are the text of the comment and the values from the cleaned form (which don't include custom values). There may be negative implications I'm not aware of, but couldn't we just pass the 'data' variable (containing all the un-cleaned POST data) into the preview?
Attachments (5)
Change History (24)
comment:1 by , 16 years ago
| Owner: | changed from to |
|---|---|
| Status: | new → assigned |
comment:2 by , 16 years ago
| Owner: | changed from to |
|---|---|
| Status: | assigned → new |
by , 16 years ago
| Attachment: | comment_view.diff added |
|---|
comment:3 by , 16 years ago
In the patch I've passed through the 'data' variable to the preview/error page. From there, you can access 'next' using 'data.next'. In fact you could access any other arbitrary field value passed through from the original comment form.
comment:4 by , 16 years ago
| Has patch: | set |
|---|
by , 16 years ago
| Attachment: | comments_doc.diff added |
|---|
Changes to comments documentation at docs/ref/contrib/comments/index.txt
comment:5 by , 16 years ago
Diff for comments documentation added.
Diff isn't that helpful, as it makes it unclear what has changed. No changes to the original text, only an additional section right before the bottom notes section, detailing the use of the hidden 'next' tag.
comment:6 by , 16 years ago
Looks like the svn:eol-style native property is missing for that file (actually possibly the whole doc tree -- I can't find it set anywhere) so line ending differences caused the original diff to look like the whole file was changed. I posted a diff using consistent line endings so just showing what actually new.
comment:7 by , 16 years ago
I reported this originally, but since then I've started thinking that a far cleaner and more robust solution is just making your own custom CommentForm. That way custom values stick with postdata and get validated along with everything else. Right now, the only extraneous value that the post_comment view "listens" for is the "next" value: would it make sense to pass "next" along to the preview/errors form redisplay, and assume everything else is the responsibility of a custom form? It just seems so unwieldy to dump the whole POST dictionary back into the view.
Just a thought...
comment:8 by , 16 years ago
@kmtracey, thanks for sorting the diff out. I tried but couldn't get it to do what you did.
comment:9 by , 16 years ago
Perhaps a commiter could decide which is more appropriate to pass through, all post data or single value for next url?
It would be fixed either way, so I'm not fussed.
comment:11 by , 16 years ago
| milestone: | → 1.1 |
|---|---|
| Triage Stage: | Unreviewed → Accepted |
comment:12 by , 16 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
comment:13 by , 16 years ago
comment:14 by , 16 years ago
| Resolution: | fixed |
|---|---|
| Status: | closed → reopened |
I'm afraid the next parameter is still not being passed through to the preview template for me. I'm using the latest svn checkout, and have a custom comment form with the next field. Redirects work as they should if not previewing. When I try to preview, the form does not have the necessary field. Further debugging of the preview template shows that next is None in the template and therefore the field is not created. I believe this is because the above fix is incomplete. It does not read the value of next from the POST data before rendering the preview.
If we change "next": next in line 83 of comments.py to "next": form.data.get("next",next), everything works as it should and next is defined in the template: http://code.djangoproject.com/browser/django/branches/releases/1.0.X/django/contrib/comments/views/comments.py?rev=10419#L83
For the time being I am working around this "bug" by getting the value of next in the template via form.data.next
I'm very new to Django so there's a chance I may have missed something in which case I apologize for possibly muddying the waters on this.
by , 16 years ago
| Attachment: | comments.patch added |
|---|
comment:16 by , 16 years ago
| Resolution: | → fixed |
|---|---|
| Status: | reopened → closed |
comment:17 by , 16 years ago
comment:19 by , 15 years ago
| Needs documentation: | set |
|---|
I have recently come across this aswell, and again using {{ data.next }} does not work.
Instead the code is making a variable {{ next }} instead available, which is wrong in the docs. Is this what is wanted? I thought the way it was meant to be accessed was through {{ data.next }}, but this currently is not the case.
Diff against rev 9844