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

Flash never getting expired #23

Closed
harikt opened this issue Aug 15, 2014 · 18 comments
Closed

Flash never getting expired #23

harikt opened this issue Aug 15, 2014 · 18 comments
Labels

Comments

@harikt
Copy link
Member

harikt commented Aug 15, 2014

Once you set a flash, and retrive the flash, still the message not expiring ( v2 )

$segment = $session->getSegment('Cocoframework\Example\ContactResponder');
$segment->setFlashNow('message', 'Thank you!. Someone will shortly get in touch with you!');
echo$session->getSegment('Cocoframework\Example\ContactResponder')->getFlash('message');
@harikt harikt added the bug label Aug 15, 2014
@pmjones
Copy link
Member

pmjones commented Aug 26, 2014

Setting the flash for "now" means it is available right now, and in the next request. This is as opposed to setting a flash normally, where it is available only in the next request and not right now. Does that makes sense?

@pmjones
Copy link
Member

pmjones commented Aug 26, 2014

As a side note, this is a change from how flashes used to work in v1. Previously, they were "Read-Once". Now instead of disappearing when you read them, they only disappear after the next request, whether you read them or not. That behavior is more in line with the common expectations of flash messages.

@harikt
Copy link
Member Author

harikt commented Aug 27, 2014

Setting the flash for "now" means it is available right now

agree

and in the next request

I don't like that concept.

Now is now. Next is next request I agree.

The problem is it never expires even when you load twice, thrice......

@pmjones
Copy link
Member

pmjones commented Aug 27, 2014

"On the next request" is the point of a flash. ;-)

Now, if it never goes away, that's a problem.

@harikt
Copy link
Member Author

harikt commented Aug 27, 2014

"On the next request" is the point of a flash. ;-)

Yes I agree. If it is for next request then what is the purpose of now ;) .

Working with zf1 flash, and aura v1 flash I feel v2 will make more confusion for people if now is available in next request also.

Eg : I show some message . "Send" , and when the user load/ refresh he will see again the same one.

Now, if it never goes away, that's a problem.

Yes this is the problem currently.

@pmjones
Copy link
Member

pmjones commented Aug 27, 2014

@harikt try the newest version, merged from #24. I've been using the following script to try it out and it seems to work:

<?php
require '/Users/pmjones/Code/pmjones/Aura.Session/autoload.php';

$session_factory = new \Aura\Session\SessionFactory;
$session = $session_factory->newInstance($_COOKIE);

$segment = $session->getSegment('test');

$foo = isset($_GET['foo'])
    ? $_GET['foo']
    : null;

if ($foo) {
    $segment->setFlash('foo', $foo);
}

var_dump($segment->getFlash('foo'));

$session->commit();
?>

Note the commit() at the end; that seems to be needed.

@harikt
Copy link
Member Author

harikt commented Aug 28, 2014

@pmjones why not keep the commit in the session destruct ... So users don't need to call commit manually. Calling commit manually is a bit weird to me.

@harikt
Copy link
Member Author

harikt commented Aug 28, 2014

will test it, currently moved to v1 session.

@harikt
Copy link
Member Author

harikt commented Aug 28, 2014

Tested and it works without commit. I was using setFlashNow . Once again I will say if setFlashNow can be read now then there is no need to carry to next request. That is what setFlash does.

I have no idea why people need flash now and on next request. Do you see any use case ?

@pmjones
Copy link
Member

pmjones commented Aug 29, 2014

I can imagine that someone may want a value to be available both now and on the next request. But if you are very much opposed, I am OK with removing. If so, we should probably remove getFlashNext() as well.

@harikt
Copy link
Member Author

harikt commented Aug 29, 2014

I can imagine that someone may want a value to be available both now and on the next request.

If they need flash for now and next request let them set both. setFlash() and setFlashNow()

But if you are very much opposed, I am OK with removing.

I don't find the need as I mentioned above. Let them set it twice if needed. Basically when I hear now, it is for now and not now and next. When I hear flash it is for next. ie why setFlashNow helps.

If so, we should probably remove getFlashNext() as well

I don't know why you need to remove getFlashNext .

Eg : Some people use in their view helper the call to getFlashNext with setting flash as setFlash() .

@pmjones
Copy link
Member

pmjones commented Aug 29, 2014

If you only need a flash "now" then you don't need a flash. You can just set a property value somewhere. The only way "now" makes sense is if it is a flash for the next request, that you can additionally read now.

@harikt
Copy link
Member Author

harikt commented Aug 29, 2014

may be your arguments are true. And I don't have trouble to close it.

But I wonder whether it is the way we learned. So basically we break the old school learning :)

@pmjones
Copy link
Member

pmjones commented Aug 29, 2014

But I wonder whether it is the way we learned. So basically we break the old school learning :)

I don't understand what you're saying here.

@harikt
Copy link
Member Author

harikt commented Aug 29, 2014

I thought v1 of zf have flash now, and was trying to find it. Not getting it anyway. You can ignore it :) .

@harikt
Copy link
Member Author

harikt commented Aug 29, 2014

You are correct on this, some of them clears flash when it is read ( http://symfony.com/doc/current/components/http_foundation/sessions.html#session-data-management ) , some have method to clear the flash when we read the current flash message ( http://apigen.juzna.cz/doc/komola/ZendFramework/source-class-Zend_Controller_Action_Helper_FlashMessenger.html#177 ) .

Thank you.

@pmjones
Copy link
Member

pmjones commented Aug 29, 2014

Very good sir; closing this, and feel free to reopen if needed.

@pmjones pmjones closed this as completed Aug 29, 2014
@harikt
Copy link
Member Author

harikt commented Oct 26, 2014

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

No branches or pull requests

2 participants