Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return to previous page after deletion #394

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gladunrv
Copy link

For example, after deleting from the page
/index?page=2
will redirect to this page again

Q A
Is bugfix? no
New feature? yes
Breaks BC? no
Tests pass? yes
Fixed issues my issues

For example, after deleting from the page
/index?page=2
will redirect to the page again
@machour
Copy link
Member

machour commented Feb 11, 2019

@gladunrv I agree with the idea but I'm not really found of the solution provided here.

We may want to use Url::remember() & $this->goBack() instead, and handle more cases, like when you edit from the index page, and are redirected to the view page instead of getting back to index.

I would:

  • Generate calls to Url::remember() in actionView/Index
  • In actionUpdate/Edit, use $this->goBack()
  • In actionDelete: If previous url is "view", then redirect to "index", else, goBack()

What do you think?

NB: This will use sessions instead of the HTTP referer..

@schmunk42
Copy link
Contributor

@machour We tried similar things in https://github.com/schmunk42/yii2-giiant, especially with Url::remember(), but this created a lot of problems when working with multiple tabs.
Moreover if you have a delete button in a frontend page (as admin), you get redirected to a non-existent page, when you go back.
Long story short, we removed all that magic and sticked with minimal logic. The proposed solution looks good to me.

@@ -147,7 +147,12 @@ public function actionDelete(<?= $actionParams ?>)
{
$this->findModel(<?= $actionParams ?>)->delete();

return $this->redirect(['index']);
list($actionID) = Yii::$app->createController(parse_url(Yii::$app->request->referrer, PHP_URL_PATH));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you're assuming that parse_url(Yii::$app->request->referrer, PHP_URL_PATH) will return correct route?

@rob006
Copy link
Contributor

rob006 commented Feb 11, 2019

This is looks brittle at best. Referrer is not always available, and assumption that URL path is a correct route and index action is somehow special, looks to fragile to me. If we really need something like this, we should check for $_GET['return'] and pass return URL directly in delete URL.

@samdark
Copy link
Member

samdark commented Feb 13, 2019

Passing previous URL in a GET variable sounds like slightly more reliable solution. @gladunrv would you like to adjust the pull request?

@schmunk42
Copy link
Contributor

How about using $_GET['return_url']?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants