withAnyTags and withAllTags doesn't ignore $type parameter when null

[martingrega]

withAnyTags and withAllTags returns empty collection when you don't specify $type param, in case Tag has type set. Possibly same problem as in #28

Is this the intended behavior? It doesn't seem to be because I didn't notice any tests for it.

The problem is in

    public static function findFromString(string $name, string $type = null, string $locale = null)
    {
        $locale = $locale ?? app()->getLocale();
        return static::query()
            ->where("name->{$locale}", $name)
            ->where('type', $type)
            ->first();
    }

shouldnt it be?

    public static function findFromString(string $name, string $type = null, string $locale = null)
    {
        $locale = $locale ?? app()->getLocale();
        return static::query()
            ->withType($type)
            ->where("name->{$locale}", $name)
            ->first();
    }

because scopeWithType ignores $type when null

[freekmurze]

This seems to be a bug yes. I'd accept a PR that fixes this. Be sure to include tests.

[martingrega]

I prepared a PR for this issue with solution mentioned above changing ->where('type', $type) to ->withType($type) in findFromString method and it passes all tests.

But after examing Tag class closer I noticed that fix is not as easy because it might create another possibly unexpected behavior with findOrCreate method.

If you use findOrCreate to create a Tag with type and then try to create Tag with the same name and no type, it will not be created but returns previous Tag with type. But if you do it the other way around, it will create 2 Tags with same names, but different types. I will try to explain on test logic.

This passes

$typedTag = Tag::findOrCreate('tagX', 'typedTag');
$nonTypedTag = Tag::findOrCreate('tagX');

$this->assertEquals($typedTag->name, $nonTypedTag->name);
$this->assertEquals($typedTag->type, $nonTypedTag->type);

This fails on assertation that types are equal.

$nonTypedTag = Tag::findOrCreate('tagX');        
$typedTag = Tag::findOrCreate('tagX', 'typedTag');
       
$this->assertEquals($typedTag->name, $nonTypedTag->name);
$this->assertEquals($typedTag->type, $nonTypedTag->type);

Possible solution is to check $type parameter (! $tag || $tag->type != $type) in findOrCreateFromString but I didn't test it yet

    protected static function findOrCreateFromString(string $name, string $type = null, string $locale = null): Tag
    {
        $locale = $locale ?? app()->getLocale();
        $tag = static::findFromString($name, $type, $locale);
        if (! $tag) {  // possible solution (! $tag || $tag->type != $type)
            $tag = static::create([
                'name' => [$locale => $name],
                'type' => $type,
            ]);
        }
        return $tag;
    }

@freekmurze what do you think?

[freekmurze]

Send in the PR and I’ll take a closer look.

[freekmurze]

See #76

[wassim]

@freekmurze can you reopen the issue please? This still occurs.