Opened 15 years ago

Closed 15 years ago

Last modified 12 years ago

#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)

comment_view.diff (484 bytes ) - added by stuartk 15 years ago.
Diff against rev 9844
comments.py (4.5 KB ) - added by stuartk 15 years ago.
/django/contrib/comments/views/comments.py
comments_doc.diff (15.6 KB ) - added by stuartk 15 years ago.
Changes to comments documentation at docs/ref/contrib/comments/index.txt
comments_doc.2.diff (1.3 KB ) - added by Karen Tracey 15 years ago.
Better docs diff
comments.patch (451 bytes ) - added by leanmeandonothingmachine 15 years ago.

Download all attachments as: .zip

Change History (24)

comment:1 by anonymous, 15 years ago

Owner: changed from nobody to anonymous
Status: newassigned

comment:2 by stuartk, 15 years ago

Owner: changed from anonymous to stuartk
Status: assignednew

by stuartk, 15 years ago

Attachment: comment_view.diff added

Diff against rev 9844

by stuartk, 15 years ago

Attachment: comments.py added

/django/contrib/comments/views/comments.py

comment:3 by stuartk, 15 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 stuartk, 15 years ago

Has patch: set

by stuartk, 15 years ago

Attachment: comments_doc.diff added

Changes to comments documentation at docs/ref/contrib/comments/index.txt

comment:5 by stuartk, 15 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.

by Karen Tracey, 15 years ago

Attachment: comments_doc.2.diff added

Better docs diff

comment:6 by Karen Tracey, 15 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 Eric Abrahamsen, 15 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 stuartk, 15 years ago

@kmtracey, thanks for sorting the diff out. I tried but couldn't get it to do what you did.

comment:9 by stuartk, 15 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:10 by (none), 15 years ago

milestone: post-1.0

Milestone post-1.0 deleted

comment:11 by Jacob, 15 years ago

milestone: 1.1
Triage Stage: UnreviewedAccepted

comment:12 by Jacob, 15 years ago

Resolution: fixed
Status: newclosed

(In [10418]) Fixed #9268: pass the "next" param through in the comment preview/post view. Also updated the docs to make this a bit clearer.

comment:13 by Jacob, 15 years ago

(In [10419]) [1.0.X] Fixed #9268: pass the "next" param through in the comment preview/post view. Also updated the docs to make this a bit clearer. Backport of r10418 from trunk.

comment:14 by saqimtiaz, 15 years ago

Resolution: fixed
Status: closedreopened

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.

comment:15 by Russell Keith-Magee, 15 years ago

Duplicate report in #11125

by leanmeandonothingmachine, 15 years ago

Attachment: comments.patch added

comment:16 by Russell Keith-Magee, 15 years ago

Resolution: fixed
Status: reopenedclosed

(In [11019]) Fixed #9268 -- Ensured that the next argument is passed on when previewing comments. Thanks to leanmeandonothingmachine for the patch.

comment:17 by Russell Keith-Magee, 15 years ago

(In [11020]) [1.0.X] Fixed #9268 -- Ensured that the next argument is passed on when previewing comments. Thanks to leanmeandonothingmachine for the patch.

Merge of r11019 from trunk.

comment:19 by mfeet, 14 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.

comment:20 by Jacob, 12 years ago

milestone: 1.1

Milestone 1.1 deleted

Note: See TracTickets for help on using tickets.
Back to Top