laravel-zero / laravel-zero

A PHP framework for console artisans
https://laravel-zero.com
3.68k stars 201 forks source link

[5.8] Add notifications component #181

Closed nroussis closed 5 years ago

nroussis commented 5 years ago

Hi, Can I use laravel notifications introduced in laravel 5.3 as described here? https://laracasts.com/series/whats-new-in-laravel-5-3/episodes/9 Please advice. Thanks much

nroussis commented 5 years ago

I tried installing it.. in composer.json I added in required: "illuminate/notifications": "5.7.*", "ramsey/uuid": "^3.8",

added in config.php - service provider: Illuminate\Notifications\NotificationServiceProvider::class

the error I get is that class url does not exist.

In lumen I see similar problem https://github.com/laravel/lumen-framework/issues/315#issuecomment-173017125 but then this guys use illuminate/http, which I probably dont want.

Please advice... I really need this feature.

nroussis commented 5 years ago

I also tried adding to config.php url aliases.. still no luck

... , 'aliases' => ['URL' => Illuminate\Support\Facades\URL::class]

nunomaduro commented 5 years ago

This may helps: composer require illuminate/routing:5.7.*

nroussis commented 5 years ago

Hi Nuno, No luck still the same problem.. I think it has something to do with the aliases missing from the app.php (thats how it is in laravel) Can you provide installation like you have for logs? That would be very helpful. I would label this as enhancement rather than question. 👍 Thanks again.

nunomaduro commented 5 years ago

@nroussis Thanks for your feedback!

nroussis commented 5 years ago

if it helps... a screenshot of the error. image

nunomaduro commented 5 years ago

@nroussis After requiring the illuminate/routing, have you added the Illuminate\Routing\RoutingServiceProvider to the service providers list?

nroussis commented 5 years ago

nice catch 👍 Silly me Passing that one new one pops up.. image

nunomaduro commented 5 years ago

@nroussis Thanks for your hard work on this. Do you want to start work on the new component for this? Should be easy 👍

nroussis commented 5 years ago

sure. I'd love to contribute. Any guidelines from where to start?

nroussis commented 5 years ago

with notifications I think the email problem https://github.com/laravel-zero/laravel-zero/issues/142 can be resolved.

nunomaduro commented 5 years ago

@nroussis This is example of a previous component added: https://github.com/laravel-zero/framework/pull/292

nroussis commented 5 years ago

I was able to make it work by editing vendor/illuminate/routing/UrlGenerator.php ... If you know how can I integrate this without touching the actual routing package? line 659 public function setRequest(Request $request=null) line 120 public function __construct(RouteCollection $routes, Request $request = null, $assetRoot = null) line 304 $this->cachedSchema = $this->forceScheme ?:'https'.'://'; line 495 $this->cachedRoot = $this->forcedRoot ?: 'https';

nunomaduro commented 5 years ago

I am sure that there is a better way of doing things without touching the vendor. Take a look at the RoutingServiceProvider, you may need to create your own service provider like this: src/Components/Logo/Provider.php.

Instead of people register manually service providers, you will do it using a src/Components/Notification/Provider.php.

nunomaduro commented 5 years ago

@nroussis have news on this?

NoxxieNl commented 5 years ago

I was looking at this and if you install the illuminate\routing package you almost get the full front end of the laravel framework as well, the whole point of laravel zero its just the CLI version of it but I could be mistaken here..

nunomaduro commented 5 years ago

@nroussis This is a optional component. So it's fine.

NoxxieNl commented 5 years ago

@nunomaduro it was me :-)

well I included the routing and the view components from illuminate. Though getting the following error:

In Container.php line 945:
Target [Illuminate\Contracts\Notifications\Dispatcher] is not instantiable.
Notification::route('mail', 'xxxx')->notify(new ExceptionAccuredNotification());

No clue why its there though, still figuring that out why its saying that... included both ServiceProviders to the app.php file and added an alias for the notification.

added the fascade to the file where I am sending the notification:

use Illuminate\Support\Facades\Notification;
use App\Notifications\ExceptionAccuredNotification;

so far no luck...

edit

Found out that when you use the notification facade the background creates the AnonymousNotifiable class. Within this file the function notify is called:

    public function notify($notification)
    {
        app(Dispatcher::class)->send($this, $notification);
    }

And there is the Dispatcher its nagging about :-), Still searching where to find the Dispatcher...

edit 2 looks like the NotificationServiceProvider.php is never called, I added a die() cmd to the boot method and my script never died...

This is my providers mapping in app.php:

    'providers' => [
        App\Providers\AppServiceProvider::class,
        Illuminate\Notifications\RoutingServiceProvider::class,
        Illuminate\Notifications\NotificationServiceProvider::class
    ],
NoxxieNl commented 5 years ago

Got it!

The error:

In Container.php line 945:
Target [Illuminate\Contracts\Notifications\Dispatcher] is not instantiable.

was just a message that not all the correct ServiceProviders were called... (and also not in the correct order...)

I got the on demand notifications working: https://laravel.com/docs/5.7/notifications#on-demand-notifications

In your composer.json file add the following requirements:

"illuminate/notifications": "^5.7",
"illuminate/routing": "5.7.*",
"illuminate/translation": "^5.7",
"illuminate/view": "^5.7",
"laravel-zero/framework": "5.7.*",
"ramsey/uuid": "^3.8",
"swiftmailer/swiftmailer": "6.0",
"vlucas/phpdotenv": "^2.0"

in your app.php file add the following to your providers:

Illuminate\Bus\BusServiceProvider::class,
Illuminate\Cache\CacheServiceProvider::class,
Illuminate\Mail\MailServiceProvider::class,
Illuminate\View\ViewServiceProvider::class,
Illuminate\Routing\RoutingServiceProvider::class,
Illuminate\Notifications\NotificationServiceProvider::class,
Illuminate\Translation\TranslationServiceProvider::class

Create the following folder structure in your root directory (NOT in your app folder):

Storage

Framework

Views

In your config folder add the files view.php and mail.php

Files corresponding to (view.php): https://github.com/laravel/laravel/blob/master/config/view.php

and (mail.php): https://github.com/laravel/laravel/blob/master/config/mail.php

I use .env to configure the settings so I added the following to my .env file:

MAIL_DRIVER=smtp
MAIL_HOST=
MAIL_PORT=
MAIL_USERNAME=
MAIL_PASSWORD=
MAIL_ENCRYPTION=

You will also need to create the notifications file for your notification manually. They reside in the folder Notifications this folder does not exist so you need to create on in your ROOT directory NOT your app folder.

You can find the stub the notification package is using when you use php artisan make:notification <name> here: https://github.com/laravel-notification-channels/backport/blob/master/src/Notifications/Console/stubs/notification.stub

So your folder structure will look like:

Notifications

nameNotification.php

then at last when in the file where you want to use the notifications import the following namespaces:

use Illuminate\Support\Facades\Notification;
use App\Notifications\<name>;

(Where name is your notification name)

And add the code to shoot off a notification:

Notification::route('mail', 'xxxx')
            ->notifyNow(new <name>());

(Where name is your notification name)

And that is it! :) I used mailtrap to to test it all and I got a nice default email template in mailtrap. This is the most basic thing to get it working.

I have no clue if the other notification channels will work with this configuration (could check if any interest). But you can send notifications now within Laravel Zero :-)!

nunomaduro commented 5 years ago

At the moment, I have no plans of having this on Laravel Zero.

ghunti commented 5 years ago

Hey @nunomaduro I did all this for a previous laravel-zero install (5.6) and now I'm going trough this all over again on another laravel-zero app (5.8) Any particular reason for not having the notifications installed as a component? Is it because it needs to bring so many components ? Cheers

nunomaduro commented 5 years ago

@ghunti Exactly, I want this to be modular.

ghunti commented 5 years ago

@nunomaduro I think it will still be modular. If I'm not mistaken in 5.8 I "just" needed to add this to composer.json

        "illuminate/bus": "5.8.*",
        "illuminate/notifications": "5.8.*",
        "illuminate/view": "5.8.*",
        "ramsey/uuid": "^3.8"

and created the config/view.php file. I think these are easy tasks for a command like php <app_name> install notifications

Or you are concerned with the fact that we are importing 4 packages just for the notifications?

Cheers