Skip to content

Revert "fixed example to check if the session exists when getting flash ... #2631

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

Merged
merged 1 commit into from
May 26, 2013

Conversation

weaverryan
Copy link
Member

Hi guys!

This reverts #2564. In that issue, I made a comment:

Before I merge this, I want to make sure I'm not missing something. Why would the following situation not cause a problem:

  1. On one request, you set a value in the flashbag and redirect
  2. On the next request, you check for app.session.started in Twig. There is a flash message in the session, but the session hasn't been started yet, so the message is not displayed.

I apologize if I'm missing something, but it seems that this is a problem. I've tested locally (and had a training group that hit this) and with the new if statement, the flash messages don't print unless you've explicitly started the session earlier in the request (e.g. by accessing the session in the controller).

Can someone verify one way or another? I don't think it's possible to lazily ask "is there something in the flashbag?" without actually starting the session to be sure.

Thanks!

…sh messages to avoid starting sessions when not needed"

It seems that there may be something stored in the flash, but you never know, because the session isn't started, so you never check.

This reverts commit a5168ad.
@weaverryan
Copy link
Member Author

ping @Drak - could you offer me a little sanity check here :). I'd be reverting a PR of Fabien's, and you know more about sessions that anyone else. Thanks in advance!

weaverryan added a commit that referenced this pull request May 26, 2013
Revert "fixed example to check if the session exists when getting flash ...
@weaverryan weaverryan merged commit a103278 into 2.1 May 26, 2013
@weaverryan weaverryan deleted the session-flash-revert branch May 26, 2013 16:42
@TerjeBr
Copy link
Contributor

TerjeBr commented Jul 15, 2013

This is what issue #6036 is all about why PR #6388 needs to be applied. Just checking if the session is started or not will not be right, because as you have noticed then the session may not start at all when it should be started. The test that needs to be done, is a test if the session has been started in a previous request/response exchange. PR #6388 add that test to the session interface, and what is more, it automatically applies it for you so no session is actually started if you only read from a session bag or a flash bag. That would make it so much easier for the application programmer.

@TerjeBr
Copy link
Contributor

TerjeBr commented Jul 15, 2013

symfony/symfony#6388

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.

2 participants