DirectoryTree / LdapRecord-Laravel

Multi-domain LDAP Authentication & Management for Laravel.
https://ldaprecord.com/docs/laravel/v3
MIT License
492 stars 51 forks source link

[Bug] Creating listener following doc gives error (AssignTeam) #514

Closed st-claude closed 1 year ago

st-claude commented 1 year ago

Hi Steve

first of all thanks for your wonderful work :)

Environment:

Describe the bug: I've installed Laravel + Jetstream (livewire/blade) + teams support I've followed the documentation to allow users with an activedirectory account to login to my application.

What i want to do is get the department of the logged-in user from the active directory and make it the default "Team" using the teams feature from Jetstream...

(What happened is by default no team is created when a user is logging in, so i had to add some code inside the blade to verify the existence of the teams before trying to show it, if not, i would get an error, but that's not the problem here)

I then followed the doc here : https://ldaprecord.com/docs/laravel/v2/auth/database/laravel-jetstream/#default-team-assignment hoping it would solve it, but sadly i'm getting an error message which i didn't see anywhere else on the web, i was hoping you could shed some light on it :

TypeError                                                             PHP 8.1.2-1ubuntu2.6    10.0.0
App\Listeners\AssignTeam::handle(): Argument #1 ($event) must be of type App\Events\LdapRecord\Laravel\Events\Import\Imported, LdapRecord\Laravel\Events\Import\Imported given, called in /var/www/trackit-ng/vendor/laravel/framework/src/Illuminate/Events/Dispatcher.php on line 441

everything is copy/pasted like in the doc, and not being a developer myself i fought a lot to find a solution to this, but nothing... it's supposed to work if i believe this : https://github.com/DirectoryTree/LdapRecord-Laravel/issues/298#issuecomment-854881086

but i can't pass the error message.

is it me who's doing something wrong or is there really a problem ?

thanks a lot Dann

stevebauman commented 1 year ago

Hi @st-claude! Thanks for your kind words πŸ˜„

It looks like you haven't imported the Imported event with a use statement. Try:

namespace App\Listeners;

use LdapRecord\Laravel\Events\Import\Imported;

class AssignTeam
{
    /**
     * Handle the event.
     *
     * @param Imported $event
     *
     * @return void
     */
    public function handle(Imported $event)
    {
        $user = $event->eloquent;

        $user->ownedTeams()->save(Team::forceCreate([
            'user_id' => $user->id,
            'name' => explode(' ', $user->name, 2)[0]."'s Team",
            'personal_team' => true,
        ]));
    }
}
st-claude commented 1 year ago

Hi @stevebauman

thanks for the fast answer

actually i did, by using php artisan make:listener AssignTeam --event="LdapRecord\Laravel\Events\Import\Imported"

it adds this line : use App\Events\LdapRecord\Laravel\Events\Import\Imported;

i tried removing the App\Events, and now the error changed to that (so i guess it solved it) :

Class "App\Listeners\Team" not found

and pointing at this line : $user->ownedTeams()->save(Team::forceCreate([

is it still ldaprecord or the issue moved to jetstream teams ? :)

thanks

stevebauman commented 1 year ago

Hey @st-claude, you have to import the Team class as well, sorry I missed that on the first post:

use Laravel\Jetstream\Team;

Can you give that a shot?

st-claude commented 1 year ago

Hi @stevebauman

before opening this post i tried a few things and it goes back to what you suggesting, so i take it as a little victory for me lol (in the past i tried use Laravel\Jetstream\Team as JetstreamTeam; but it failed also)

i've added what you wrote, and new error :)

Cannot instantiate abstract class Laravel\Jetstream\Team

and it's pointing on the code at the line :

'user_id' => $user->id,

thanks

stevebauman commented 1 year ago

Sorry @st-claude, I forgot that Jetstream publishes it's models in the App\Models namespace πŸ™ˆ

Can you try importing this instead:

use App\Models\Team;
st-claude commented 1 year ago

you make it look so easy where i struggled for days lol

it works now of course...

one small question, i'm usually used to error messages and debugging, but how do you relate the cryptic "cannot instantiate abstract class" to some wrong include location ? is it some php knowledge i'm missing ?

anyway, do you think this solution warrants a small addendum to the docs ? because the make:listener doesn't exactly produce what you told me to add, and the jetstream thingy is also confusing (for a noob i mean)

also i used this opportunity to sponsor you a little, it's not much but i can keep it going a couple of months

thanks again for the great help and keep up the good work !

st-claude commented 1 year ago

Hi @stevebauman

if i could abuse of your kindness, i'm trying now to get the department name from active directory server, when i do that on a standard page with a livewire component, i can access the data from the user model with no issue, like this :

in the component : 
$user = \Illuminate\Support\Facades\Auth::user();

in the blade : 
{{ $user->ldap->getFirstAttribute('department'); }}

works fine, following the docs.

but when i try to do that in the event listener (the AssignTeam you helped me with), i can't access it, it returns null...

any idea how i could do that in the same way (like in the blade above) ? should i try and change user model to try and get this info when first login (and how) or another way ?

stevebauman commented 1 year ago

Thanks for your kind words and sponsorship @st-claude! I really appreciate that ❀️

one small question, i'm usually used to error messages and debugging, but how do you relate the cryptic "cannot instantiate abstract class" to some wrong include location ? is it some php knowledge i'm missing ?

That means that the class that you're calling is marked abstract. In our case above, this was because Laravel Jetstream includes an abstract Team class, and we were calling Team::create():

https://github.com/laravel/jetstream/blob/62000c56e7f26df609e117e1af875814e8c265fb/src/Team.php#L7

use Laravel\Jetstream\Team;

Team::create(); // Throws exception. "Team" is an 'abstract' class.

In PHP, classes marked as abstract, cannot be instantiated by themselves. They must have a class that extends them:

abstract class Animal
{
}

new Animal(); // Throws exception ❌
class Dog extends Animal
{
}

new Dog(); // Works βœ…

anyway, do you think this solution warrants a small addendum to the docs ? because the make:listener doesn't exactly produce what you told me to add, and the jetstream thingy is also confusing (for a noob i mean)

Yes I agree! I've just pushed a new version of the docs that includes this import and namespace, so it can referenced easily:

https://ldaprecord.com/docs/laravel/v2/auth/database/laravel-jetstream/#default-team-assignment

I appreciate all feedback you can provide, as I'd like the documentation to be as easy to follow as possible πŸ™

i'm trying now to get the department name from active directory server, when i do that on a standard page with a livewire component, i can access the data from the user model with no issue, like this : but when i try to do that in the event listener (the AssignTeam you helped me with), i can't access it, it returns null...

You will have access to your LDAP user in the listener we've created above, via the object property on the Imported event:

https://github.com/DirectoryTree/LdapRecord-Laravel/blob/5f36d4ba6edbf56d97211fb892188aa6f9aceabc/src/Events/Import/Event.php#L10-L15

public function handle(Imported $event)
{
    $event->object->getFirstAttribute('department');

    // ...
}

Can you give that a shot? Let me know how it goes! πŸ‘

st-claude commented 1 year ago

worked like a charm :+1: (after i corrected the small typo in getFirstAttribute)

and thanks for the explanation about the classes.

so to know about this object i need to read (and understand) the source code ?

stevebauman commented 1 year ago

so to know about this object i need to read (and understand) the source code?

Typically no, if you're using an IDE that is capable of IntelliSense or autocomplete (PHPStorm or VSCode with IntelliSense plugin). Though it's always a good practice to take a look behind the curtain to see how both LdapRecord and Jetstream function.

Closing as completed --let me know if you have any further questions! πŸ™