Django

Code

Ticket #9268 (closed: fixed)

Opened 1 year ago

Last modified 5 months ago

Pass custom form values from post-comment to preview-comment

Reported by: taojian Assigned to: stuartk
Milestone: 1.1 Component: django.contrib.comments
Version: SVN Keywords: comment preview
Cc: girzel@gmail.com Triage Stage: Accepted
Has patch: 1 Needs documentation: 1
Needs tests: 0 Patch needs improvement: 0

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

comment_view.diff (484 bytes) - added by stuartk on 02/18/09 04:51:26.
Diff against rev 9844
comments.py (4.5 kB) - added by stuartk on 02/18/09 04:52:10.
/django/contrib/comments/views/comments.py
comments_doc.diff (15.6 kB) - added by stuartk on 02/20/09 16:00:38.
Changes to comments documentation at docs/ref/contrib/comments/index.txt
comments_doc.2.diff (1.3 kB) - added by kmtracey on 02/20/09 20:24:50.
Better docs diff
comments.patch (451 bytes) - added by leanmeandonothingmachine on 06/16/09 16:13:49.

Change History

02/18/09 02:20:46 changed by anonymous

  • owner changed from nobody to anonymous.
  • needs_better_patch changed.
  • status changed from new to assigned.
  • needs_tests changed.
  • needs_docs changed.

02/18/09 02:21:17 changed by stuartk

  • owner changed from anonymous to stuartk.
  • status changed from assigned to new.

02/18/09 04:51:26 changed by stuartk

  • attachment comment_view.diff added.

Diff against rev 9844

02/18/09 04:52:10 changed by stuartk

  • attachment comments.py added.

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

02/18/09 07:15:11 changed by stuartk

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.

02/18/09 07:15:34 changed by stuartk

  • has_patch set to 1.

02/20/09 16:00:38 changed by stuartk

  • attachment comments_doc.diff added.

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

02/20/09 16:02:53 changed by stuartk

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.

02/20/09 20:24:50 changed by kmtracey

  • attachment comments_doc.2.diff added.

Better docs diff

02/20/09 20:28:03 changed by kmtracey

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.

02/20/09 21:18:21 changed by taojian

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...

02/22/09 17:00:34 changed by stuartk

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

02/22/09 17:10:08 changed by stuartk

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.

02/25/09 13:51:44 changed by

  • milestone deleted.

Milestone post-1.0 deleted

02/26/09 11:16:35 changed by jacob

  • stage changed from Unreviewed to Accepted.
  • milestone set to 1.1.

04/07/09 14:28:14 changed by jacob

  • status changed from new to closed.
  • resolution set to fixed.

(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.

04/07/09 14:29:34 changed by jacob

(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.

06/09/09 06:00:44 changed by saqimtiaz

  • status changed from closed to reopened.
  • resolution deleted.

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.

06/16/09 09:36:03 changed by russellm

Duplicate report in #11125

06/16/09 16:13:49 changed by leanmeandonothingmachine

  • attachment comments.patch added.

06/17/09 08:01:42 changed by russellm

  • status changed from reopened to closed.
  • resolution set to fixed.

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

06/17/09 08:13:30 changed by russellm

(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.

09/07/09 10:28:40 changed by hitcliff

thanks a lot for this great article! I really liked what this tutorial has to offer! having not much experience in tnis sphere I really appreciate every piece of advice I can get. have used to download great books with useful information from http://www.picktorrent.com , but always search for smth new. thanks!

10/07/09 14:55:02 changed by mfeet

  • needs_docs set to 1.

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.

Add/Change #9268 (Pass custom form values from post-comment to preview-comment)




Change Properties
Action