Sounds like this will be completely re-thought, so maybe comments on the
current (trunk) code aren't necessary, but here are some observations (not
sure if these are correct because I haven't tested anything, just quickly
looked at the code):
- It still looks like _auth_next will be the first URL visited during the
session, whether or not it requires login, but then it can't get
overwritten. So won't the redirect end up going to the first URL of the
session rather than the last URL right before login (we want the latter,
no)?
- _auth_next is reset to None if it starts with the current URL, but
don't we want it to be an exact match for the current URL (including args
and vars) in order to conclude that the redirect has happened?
- I'm not sure I'm following properly, but does this involve an
additional redirect -- one immediately after login, and then another when
that redirect hits Auth.__init__?
- This is repeated many times:
if is_relative(next):
next = self.url(next.replace('[id]', str(form.vars.id)))
Instead of the is_relative function, how about a replace_id function that
checks for a relative URL and does the replace? So, the above would just be:
next = replace_id(next,form)
Anthony
On Saturday, September 17, 2011 11:36:41 AM UTC-4, Massimo Di Pierro wrote:
>
> > It seems to me that there are two issues here. One is cleaning up the
> logic to make it uniform, DRY and understandable. The other is deciding
> where to put the next link (and doing proper validation of the URL).
> >
> > I understand (I think) the basic use case for @requires_login, I think.
> >
> > What's the use case for saving the return link in Auth()?
> >
> > Does it make sense to try to save a next link in cases like
> change-password? Profile editing?
> >
> > I'm fine with reverting for now, but I really think that this logic is
> due for a review and cleanup.
>
> I agree with that. I think we may have to release 1.99.1 with the
> _next solution and then try improve it.
>
> > Maybe starting with a spec: what are we really trying to do? And how do
> these dynamic _next links relate to the various next-URL links in
> auth.settings?
>
> I will try write more about this later today.
>
> Massimo
>
