Restore request in 2.3-dev doesn't work correctly

o5

Hello,

I think in @dev version is a bug with method $presenter->restoreRequest().

Prepare reproducible environment:

git clone git@github.com:nette/examples.git
cd examples/CD-collection/
composer install --prefer-dist

User story:

Sign in into app
Go to “Add new album” screen
Delete all cookies via Developers Tools
Refresh page
You have been redirect to login form with backlink param
After successfully re-sign in, you're in “Add new album” screen, but URI wasn't change and if you do refresh page, you get “Confirm Form Resubmission”.

The same story works fine (without form resubmission) in Nette v2.2.2.

Last edited by o5 (2014-06-27 13:02)

Majkl578

Probably because of this from @enumag.
See discussion below original commit here.

enumag

This behaviour is correct.

The method restoreRequest only restores the request without changing url (it can't actually change it because restoring post requests wouldn't work). The fact you got redirected before was an unintentional sideeffect caused by canonicalization in some cases (not always so it was not reliable).

We should add some method redirectToStoredRequest() or something like that which would ignore the post data in the stored request and redirect.

Last edited by enumag (2014-06-27 18:46)

Majkl578

Ignoring the fact it is masssive BC break of common use-case of most websites (and thus as well violates POST/REDIRECT/GET pattern), how do you suggest to replace the behavior? Calling restoreRequest() terminates code execution, so you can't place redirect after it. And any other way is imho hackish.

enumag

I want to introduce a new method which should be used instead of restoreRequest if the redirect is desired.

enumag

See my solution here and here.

duke

I am currently solving this problem by this code in my BasePresenter:

protected function startup()
{
	parent::startup();
	if ($this->request->hasFlag(\Nette\Application\Request::RESTORED)) {
		$this->redirect('this');
	}
}

o5

@duke: Thanks, I'll use your solution.

@enumag: I want to see your explanation for people after push this BC into stable version.

Last edited by o5 (2014-06-28 23:49)

enumag

I've already explained reasons for this change on GitHub. As I've said the old behavior (although relied on by many people) was neither intended nor reliable, it was just a side-effect of other things.

Also it's not my fault that this change got into Nette 2.2. At the time I was convinced that the next version after 2.1 will be 3.0 where such BC break would be fine.

Last edited by enumag (2014-06-28 22:19)

Majkl578

old behavior (although relied on by many people) was neither intended nor reliable, it was just a side-effect of other things

I think it was perfectly reliable and handsome.

Also it's not my fault that this change got into Nette 2.2.

It didn't.

enumag

Oh, didn't notice that the bug report was for 2.3-dev only. :-) In that case it's fine. When you're using master, things like this should be expected.

Last edited by enumag (2014-06-29 10:24)

David Grudl

This will be solved (ie. replaced in master) by https://github.com/…ation/pull/4