Navigation Menu

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

Latest version of L5 breaks Notifications. #45

Closed
landjea opened this issue Oct 21, 2014 · 7 comments
Closed

Latest version of L5 breaks Notifications. #45

landjea opened this issue Oct 21, 2014 · 7 comments

Comments

@landjea
Copy link

landjea commented Oct 21, 2014

I was using the Notifications package on an L5 test site I am tinkering with and it was working last week. Today, after a composer update this morning to latest L5 though, the Notifications no longer appear using {!! Notification::showAll() !!} on a blade view. Didn't change anything else yet, so I am pretty confident it was the latest L5 update. I can see the message container in the Session but haven't sorted out why they aren't appearing. I am going to keep digging at it, but thought I would let you know.

when Notifications don't appear:

\Session::all()

array (size=4)
  '_token' => string 'A1etWkhS277ZJVQPrbGKsHzOJ3uKX8uT7gfBTbd3' (length=40)
  'flash' => 
    array (size=2)
      'old' => 
        array (size=0)
          empty
      'new' => 
        array (size=2)
          0 => string 'notifications_containers' (length=24)
          1 => string 'notifications_default_1' (length=23)
  'notifications_containers' => 
    array (size=1)
      0 => string 'default' (length=7)
  'notifications_default_1' => string '{"message":"Sorry, you can only send one message every 3 minutes","format":"<div class=\"alert alert-:type alert-dismissable\" id=\"flash_notice\">\r\n            <button type=\"button\" class=\"close\" data-dismiss=\"alert\" aria-hidden=\"true\">&times;<\/button>\r\n            :message\r\n          <\/div>","type":"warning","flashable":true,"alias":null,"position":null}' (length=374)
@edvinaskrucas
Copy link
Owner

Thanks for informing, due my limited time I will invest more time to update this package when L5 goes live.

@landjea
Copy link
Author

landjea commented Oct 21, 2014

Yeah no problem. L5 isn't live yet so not huge concern for you. I think I have narrowed it down, so far, to line 115 of \Krucas\Notification\Subscriber. The $this->getSession()->flash($sessionKey, $message->toJson()); is failing because it can't find the "flash()" method of $this->getSession() which I believe is just the Laravel Session pulled in. I was going to go check the L5 pull I have to see if the session flash method was renamed maybe.

@landjea
Copy link
Author

landjea commented Oct 21, 2014

Ugh. Brainfart. Line 115 is working because the $message=>toJson() is what appears in the Session (above). So your event of "onFlash" is firing correctly. And appears to be properly handing off the json-formated $message to the Session. Must be in that Session. Gonna try to backtrack through the flash, put, set, array_set, etc... methods to see if I can track it.

EDIT : Got lost mucking my way backwards in the Session and can't figure why it is not showing the data contained in flash.new. Better minds than mine will have to sort it out eventually.

@landjea
Copy link
Author

landjea commented Oct 21, 2014

For anyone also stuck in this problematic state, I am using this ugly blunt workaround below in my master blade to show the First message in the container. (I never have more than one). You can probably move the str_replace chunk into a foreach () loop on the notifications_containers if you want the showAll() effect.

        <!-- notifications to user  -->
        @if( \Session::has('notifications_containers') )

            {!! str_replace([ ':message', ':type' ], [ json_decode(\Session::get('notifications_default_1'))->message, json_decode(\Session::get('notifications_default_1'))->type ],json_decode(\Session::get('notifications_default_1'))->format); !!}

        @endif

@colinyoung87
Copy link

Great, cheers landjea.

I created a class to overwrite the showAll() function using your code, so no need to replace the code in any views - and it can easily be switched out when the package is updated.

@edvinaskrucas
Copy link
Owner

Ok, thanks. I am planing to update module source when L5 is out :)

@Azirius
Copy link

Azirius commented Jan 31, 2015

I understand what is causing this issue. It’s the fact that the session store is empty when passed via the Service Provider, therefore ‘flashed’ notifications don’t get loaded between requests. I find that firing the ‘boot’ event later on once the session store has been populated fixes this issue.

$events->fire(‘notification.booted’, \App::make('notification'));

Hope this helps. I’m not entirely sure how else you’d populate the NotificationsBag automatically without further digging, but this is a quick fix that isn’t too difficult to implement.

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

No branches or pull requests

4 participants