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

Added Blacklisted Meta Keys #45

Closed
wants to merge 1 commit into from
Closed

Conversation

BenQoder
Copy link

This PR bring Blacklist to Meta Keys

It is necessary in situations where you need to call relationships counts or aggregates when it is called with the loadCount method

$post = Post::find(1);

$post->loadCount(["likers"]);

Once you call the loadCount method, it throws an error.
After adding the likers_count to the blacklist, the problem is fixed

@marijoo
Copy link
Member

marijoo commented Sep 12, 2024

Thanks for the PR. Calling loadCount() leads to an error? I have to check this, but I’m not sure this is the best way to go against it.

I will check the loadCount() problem and get back to you, okay?

@marijoo
Copy link
Member

marijoo commented Sep 12, 2024

@BenQoder I wrote a simple test and it seems to work. Can you elaborate on what you did and what exactly happened?

it('works when using loadCount', function () {
    $user = User::factory()->create();
    Post::factory()->for($user)->count(3)->create();

    expect($user->posts_count)->toBeNull();

    $user->loadCount('posts');

    expect($user->posts_count)->toBe(3);
});

marijoo added a commit that referenced this pull request Sep 12, 2024
@BenQoder
Copy link
Author

I added the meta trait to my model

use HasMeta;

and called the relationship count just like this

$business = Business::latest()->first();

 $business->bookings_count;

$business->loadCount([
    "bookings",
]);

dd($business);

Could you adjust your test to use queries like

Model::find();
// or 
Model::latest()->first()

Here's a screenshot below

Screenshot 2024-09-12 at 09 50 11

@marijoo
Copy link
Member

marijoo commented Sep 12, 2024

@BenQoder This also works as expected:

it('works when using loadCount', function () {
    Post::factory()->for(User::factory())->count(3)->create();

    $user = User::latest()->first();

    expect($user->posts_count)->toBeNull();

    $user->loadCount(['posts']);

    expect($user->posts_count)->toBe(3);
});

Would you mind providing the repository or the relevant files (the Business and Booking models at least)?

@BenQoder
Copy link
Author

Alright... At this point, I am going to close my PR because.. I now believe there is some other package that may be causing this issue.

I have resolved this on my project by creating another trait that uses the HasMeta Trait and with my fixes

Incase anyone in future is here with the same error, here is what I did to fix my issue

<?php

namespace App\Traits;

use Illuminate\Support\Str;
use Kolossal\Multiplex\HasMeta;

trait HasMetaInternal
{
    use HasMeta;

    /**
     * Determine if the given key is blacklisted from being a meta key.
     */

    public function isBlacklistedMetaKey(string $key): bool
    {
        return in_array(
            $key,
            $this->blacklistedMetaKeys ?? [],
        );
    }

    /**
     * Determine if the given key is an allowed meta key.
     */
    public function isValidMetaKey(string $key): bool
    {
        if ($this->isMetaUnguarded()) {
            return true;
        }

        if ($this->isExplicitlyAllowedMetaKey($key)) {
            return true;
        }

        if ($this->isRelationshipAggregate($key)) {
            return false;
        }

        if ($this->isModelAttribute($key)) {
            return false;
        }

        if ($this->isBlacklistedMetaKey($key)) {
            return false;
        }

        return $this->isMetaWildcardSet();
    }

    /**
     * Determine if a given key is a relationship aggregate like count, sum, avg, min, max.
     */
    public function isRelationshipAggregate(string $key): bool
    {

        if (Str::endsWith($key, "_count")) {
            return $this->isRelation(
                Str::before($key, "_count")
            );
        }

        $keywords = ["_sum_", "_avg_", "_min_", "_max_"];


        if (!Str::contains($key, $keywords)) {
            return false;
        }

        $relationshipName = null;

        foreach ($keywords as $keyword) {
            if (Str::contains($key, $keyword)) {
                $relationshipName = Str::before($key, $keyword);
                break;
            }
        }

        if (!$relationshipName) {
            return false;
        }

        return $this->isRelation(
            $relationshipName
        );
    }
}

@BenQoder BenQoder closed this Sep 12, 2024
@marijoo
Copy link
Member

marijoo commented Sep 12, 2024

Thanks! Maybe testing those models of yours in isolation (a fresh Laravel install) could clear things out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants