On May 4, 10:59 am, Dario Ocles <dario.oc...@gmail.com> wrote:
> Hello everybody.
>
> Ramiro Morales and me were working on this ticket #15294 [0] and
> Julien linked this one #8001 [1]. We had a little discussion about two
> possible solution for #8001 and Julien suggested that we should hear
> your opinion (you can read all comments on ticket #15294).
>
> The discussion was about if we have to take care of backward
> compatibility on this issue.
>
> I uploaded two patches to #8001, one of them taking care of backward
> compatibility.
>
> I think that the reason for keeping backward compatibility, is just
> that it is good.
>
> And the reasons for deprecating the old behavior are:
> - Cleaner code.
> - This is not a documented part of Django's API, so we either can or
> cannot take care of backward compatibility.
> - I don't like to put any ugly try/catch.
>
> I'm sorry for my English.
>
> Regards,
> Dario Ocles (burzak).
>
> [0]http://code.djangoproject.com/ticket/15294
> [1]http://code.djangoproject.com/ticket/8001

Thanks for bringing this up, Dario. I'll try to summarise here the
discussion that we've had in those tickets.

As part of the effort in #15294 to remove all hardcoded urls from the
admin's codebase, it was suggested that the hardcoded
post_url_continue parameter in the response_add method be addressed as
well:

    def response_add(self, request, obj, post_url_continue='../%s/'):
        ...

post_url_continue is generally used to redirect you to the new
object's change page after it's been created. Since it is impossible
to reverse that object's change page url if you don't know the
object's model details and primary key, it was suggested that
post_url_continue default to None and that the url be reversed from
inside the response_add method.

Now, it was also pointed out that the current use of post_url_continue
was a bit limiting in that it assumes the value to be a formattable
string with one format parameter (i.e. the pk). There is a backwards
compatible way of dealing with this [1], and a backwards incompatible
way [2]. The former first assumes that it receives a formattable
string like in the "old" way, and if that generates an error then it
assumes it's a pre-formatted string. The latter only assumes it's a
pre-formatted string. I personally tend to prefer the latter as the
implementation is cleaner and also more flexible -- it is based on the
assumption that it is the developer's responsibility to provide a pre-
formatted string if they want to override the default behaviour,
instead of letting Django's response_add method do the formatting for
you.

If we go with [2] then this may be a small inconvenience for people
who are relying on the current behaviour of response_add, but the
upgrade path is very easy (i.e. just format the string yourself).
Also, response_add is not officially documented.

So, this all leads to the reason why this was brought up to the dev
list: shall we go ahead and break backwards compatibility? As far as I
can tell from the policy [3] we're pretty safe to go ahead, but it'd
be helpful to collect more opinions on this (and if possible also more
generally on the rest of the admin's codebase).

Thanks a lot!

Julien

[1] 
http://code.djangoproject.com/attachment/ticket/8001/8001_redirection_backward_compatibility.diff
[2] http://code.djangoproject.com/attachment/ticket/8001/8001_redirection.diff
[3] http://docs.djangoproject.com/en/dev/misc/api-stability/

-- 
You received this message because you are subscribed to the Google Groups 
"Django developers" group.
To post to this group, send email to django-developers@googlegroups.com.
To unsubscribe from this group, send email to 
django-developers+unsubscr...@googlegroups.com.
For more options, visit this group at 
http://groups.google.com/group/django-developers?hl=en.

Reply via email to