Opened 6 years ago

Closed 6 years ago

Last modified 4 years ago

#10801 closed (fixed)

tutorial part 4 vote view needs to redirect in the error case

Reported by: bruce@… Owned by: jacob
Component: Documentation Version: 1.0
Severity: Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

In part 4 of the tutorial the vote view has the following code for the case that no choice was picked:

 return render_to_response('polls/detail.html', {
            'poll': p,
            'error_message': "You didn't select a choice.",
        })

However, the resulting URL is of the form "http://127.0.0.1:8000/polls/1/vote/" rather than what is should be "http://127.0.0.1:8000/polls/1/". So a redirect is needed. I don't know the best way to do that.

Change History (13)

comment:1 Changed 6 years ago by kmtracey

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

No, in the case of an error in the submitted data you want to re-display the poll form with the error message. The tutorial code is correct. Please post to django-users if you are having trouble understanding something; the tutorial has gotten plenty of attention and people using it, so it is unlikely to have an error at this point.

comment:2 follow-up: Changed 6 years ago by anonymous

if you say so... but if you vote on the resulting page you get a 404 error for the URL "http://127.0.0.1:8000/polls/1/vote/vote/". Doesn't seem right to me.

comment:3 in reply to: ↑ 2 Changed 6 years ago by kmtracey

  • Resolution invalid deleted
  • Status changed from closed to reopened

Replying to anonymous:

if you say so... but if you vote on the resulting page you get a 404 error for the URL "http://127.0.0.1:8000/polls/1/vote/vote/". Doesn't seem right to me.

You might have mentioned that detail in the original report! That actually points to something that was recently changed -- the form action in the template was changed from /polls/{{ poll.id }}/vote/ to just vote/ as part of changeset r10371. That changeset fixed a bajillion tickets so I'm not sure yet what problem exactly that it was supposed to address, but it does seem to have introduced a problem in the error path for the view.

comment:4 Changed 6 years ago by kmtracey

  • milestone set to 1.1

#9711 was the ticket for which this change was made. I don't know that making the tutorial polls app more pluggable warrants re-writing the view so my initial impression is the #9711 change ought just be reverted.

comment:5 Changed 6 years ago by mtredinnick

  • Owner changed from nobody to mtredinnick
  • Status changed from reopened to new

That looks like a typo in the change. Form action should have been ../vote/, which does make things a little simpler (maybe; also a bit trappier in other ways). I'll poke at this today if nobody else gets to it.

comment:6 Changed 6 years ago by kmtracey

Sorry, #9771 is the right original ticket number, somehow my mind tends to morph numbers when copying. I'm not sure ../vote/ is going to work either, given per that ticket description the form is displayed on a page with a url such as /polls/123/. I haven't looked in detail at the tutorial though to see if other stuff has changed to make a ../vote/ action work....

comment:7 Changed 6 years ago by mtredinnick

Yeah, don't worry ... I'm going to work through the whole tutorial to check it. It's not clear to me that this change is an improvement at all at the moment and since it's the beginner's tutorial, covering every single portable application practice is clearly out of scope in the interests of pragmatism. Reverting the change is definitely still an option for you.

comment:8 Changed 6 years ago by anonymous

Just a suggestion, but maybe the form's action is a good place to introduce {% url %} template tag in the tutorial?

comment:9 Changed 6 years ago by jacob

  • Triage Stage changed from Unreviewed to Accepted

comment:10 Changed 6 years ago by kmtracey

#10914 reported this again.

comment:11 Changed 6 years ago by jacob

  • Owner changed from mtredinnick to jacob
  • Status changed from new to assigned

comment:12 Changed 6 years ago by russellm

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [10973]) Fixed #10801 -- Reverted a portion of [10371]. Practicality beats purity in this case. Thanks to bruce@… for the report. Refs #9771.

comment:13 Changed 4 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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