cviebrock / eloquent-taggable

Easily add the ability to tag your Eloquent models in Laravel.
MIT License
537 stars 72 forks source link

Call to a member function contains() on null #55

Closed leonelngande closed 7 years ago

leonelngande commented 7 years ago

I've included the package in my application as per the readme.md file and added the service provider. I then proceeded to add the Taggable trait to my Course model as follow;

[<?php namespace App;

use App\User; use Illuminate\Database\Eloquent\Model; use Cviebrock\EloquentTaggable\Taggable;

class Course extends Model {

use Taggable; ]

But when i try to tag a model (Course) like below

[<?php

namespace App\Http\Controllers; use Illuminate\Http\Request; use Illuminate\Support\Facades\DB; use App\Course; use App\Tag; use App\Type; use App\User; use Auth;

class LearningController extends Controller {

public function __construct()
{
    $this->middleware('auth');
}

public function catalog() { foreach(Course::all() as $course) { print_r($course->toArray()); $course->tag(['GCE']); // !!!! HERE !!!! } }

The the error below is thrown

[

(1/1) FatalThrowableError Call to a member function contains() on null

in Taggable.php (line 96) at Course->addOneTag('GCE')in Taggable.php (line 39) at Course->tag(array('GCE'))in LearningController.php (line 43) at LearningController->catalog() at call_user_func_array(array(object(LearningController), 'catalog'), array())in Controller.php (line 55) ]

I've tried my best to resolve it but i can't seem to. This is the output from Laravel Debugbar:

[ Call to a member function contains() on null C:\laragon\www\xamcademy\vendor\cviebrock\eloquent-taggable\src\Taggable.php#96 Symfony\Component\Debug\Exception\FatalThrowableError { $tag = app(TagService::class)->findOrCreate($tagName);

    if (!$this->tags->contains($tag->getKey())) {
        $this->tags()->attach($tag->getKey());
    }
}

]

The the error below is thrownThe the error below is thrown

[[

I think the problem is coming from !$this->tags->contains($tag->getKey() $this->tags seems to return null so contains() is called on null. Please any help will be appreciated. Thanks

cviebrock commented 7 years ago

This is the same error as #59, and I can't seem to reproduce it.

Interestingly (?) both you and the other user are running on Windows. I have no idea if that's a factor, but perhaps it is? For some reason $model->tags is returning null instead of an empty Collection.

Can you tell me the version of PHP and Laravel you are running? Also, you are using the 3.0 version of Taggable, right?

cviebrock commented 7 years ago

Crazy idea: can you try replacing this code the trait:

    protected function addOneTag(string $tagName)
    {
        $tag = app(TagService::class)->findOrCreate($tagName);

        if (!$this->tags->contains($tag->getKey())) {
            $this->tags()->attach($tag->getKey());
        }
    }

With this:

    protected function addOneTag(string $tagName)
    {
        $tag = app(TagService::class)->findOrCreate($tagName);

        $tags = $this->tags;

        if (!$tags->contains($tag->getKey())) {
            $this->tags()->attach($tag->getKey());
        }
    }

Maybe there is something about chaining it that is failing.

cviebrock commented 7 years ago

Also, first check that you don't have a column called tags in your model's table (see #1 ).

leonelngande commented 7 years ago

:-) Yea i should've checked for that too #1 just realized now. Thanks alot

What about defining the relationship on the model, i've defined it as follows;

public function courseTags() 
    {

        return $this->morphToMany(Tag::class, 'taggable', 'taggable_taggables', 'taggable_id', 'tag_id');

    }

but i keep getting this error

SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'tag_id' in field list is ambiguous (SQL: select `tag_id`, `name`, `taggable_taggables`.`taggable_id` as `pivot_taggable_id`, `taggable_taggables`.`id` as `pivot_id` from `taggable_tags` inner join `taggable_taggables` on `taggable_tags`.`tag_id` = `taggable_taggables`.`id` where `taggable_taggables`.`taggable_id` in (1, 3, 4, 7, 8, 9, 10, 11, 12, 13) and `taggable_taggables`.`taggable_type` = App\Course)

My model is App\Course. Would really appreciate any insight

cviebrock commented 7 years ago

Hmm ... which version of the package, and which version of Laravel is this?

leonelngande commented 7 years ago

Package version is 3.1.0 and am running Laravel 5.4

cviebrock commented 7 years ago

The package automatically creates the relationship tags() when you include the trait, so you shouldn't have to do anything other than use Taggable in your model.

Unfortunately, the package doesn't support using a different name for the relationship.

leonelngande commented 7 years ago

I am using the Voyager Admin package and to when creating a new Course i have to select tags for the course using Voyager's multiple-select data-type. This requires defining the many-to-many relationship between App\Course and App\Tag in my Course model, which is where am encountering this problem

cviebrock commented 7 years ago

I'm not familiar with that package. Also, you don't need App\Tag if you use my package, because it supplies Cviebrock\EloquentTaggable\Models\Tag ... and that is kinda hard coded into a bunch of places (although I could abstract it out in the next version).

So, can you get Voyager to recognize the relation with that model instead? Or maybe Voyager just handles tags all by itself, and you don't actually need my package?

leonelngande commented 7 years ago

Voyager uses eloquent's attach() method to add tags to courses. To do this Voyager needs to know the relationship, name, table, foreign key, etc which are all made available in the many-to-many relationship between \App\Course and \App\Tag. Also my Tag model extends yours

`<?php namespace App;

use Cviebrock\EloquentTaggable\Models\Tag as CviebrockTag;

class Tag extends CviebrockTag { ... }`

So this is okay right? Can you show me how you would define the many to many relation between App\Course and your package's Tag model? Maybe something isn't right in mine

leonelngande commented 7 years ago

Also i extended your package's Tag model because I had to implement basic Tag Group functionality since my app needs it too

cviebrock commented 7 years ago

The problem with extending the Tag model, is that the package's service class and trait all have references to $model->tags() and Tag::create(...), etc.. So unless you edit all of those -- at which point, you've just reimplemented the same functionality yourself, and you don't need the package.

It sounds like Voyager's admin pages already handle all the relationship attaching and detaching that my package does ... so I'm really not seeing why you would need my package still.

leonelngande commented 7 years ago

Ok.

I use your package at the user facing side of my application, that is in my own controllers to display courses, etc based on tags selected and some other functionality it provides. Voyager takes care of attaching/detaching but it needs some configuration one of which is defining the relationship.

leonelngande commented 7 years ago

Well, i think we should just close this here. Thanks for everything 👍