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

Counting relationships? #166

Closed
fisharebest opened this issue Apr 24, 2018 · 14 comments
Closed

Counting relationships? #166

fisharebest opened this issue Apr 24, 2018 · 14 comments
Milestone

Comments

@fisharebest
Copy link

Laravel provides support to add counts of relations to a model. e.g.

$posts = App\Post::withCount('comments')->get();

foreach ($posts as $post) {
    echo $post->comments_count;
}

Is there a way to incorporate $model->withCount(...) into a laravel-json-api request?

@fisharebest
Copy link
Author

fisharebest commented Apr 24, 2018

To be clear, I'd like to add include=comments_count to the request, and have withCount('comments') called on the model(s).

@lindyhopchris
Copy link
Member

Hi! The include parameter is for inclusion of related resources, which a count is not. I.e. it's for including the related comments themselves, not the count.

It might be best including this in relationship meta? Or just including it as an attribute? Do you have a preference?

@fisharebest
Copy link
Author

For my application, it would be simplest just to have it as an extra attribute.

But it's only needed on a few requests, so it needs to be selectable in the request.

@fisharebest
Copy link
Author

FYI, I've got this working by extending EloquentAdapter and adding the following method:

    protected function with(Builder $query, Collection $includePaths): void
    {
        foreach ($includePaths->toArray() as $include_path) {
            if (ends_with($include_path, '_count')) {
                // Count
                $query->withCount(str_replace_last('_count', '', $include_path));
            } else {
                // Eager load
                $query->with($include_path);
            }
        }

        parent::with($query, $includePaths);
    }

I can now use api/foo?include=bar_count to give me a count of bar linked to foo.

(I think eager loading is automatic in 1.x, but I'm still using 0.x).

I fully appreciate your comment that I am abusing the include parameter, but I can't find a cleaner way to do this.

The only 'gotcha' I found was that EloquentSchema::attributeKeys() gets a list of attributes from Model::getFillable(), and so I needed to add xxx_count to the model's $fillable attribute - even though this attribute isn't actually fillable.

@dimitvassilev
Copy link

Hi, @fisharebest,
You might want to use the sparse fieldsets specification http://jsonapi.org/format/#fetching-sparse-fieldsets, i.e. sth like api/foo?include=bar&fields[bar]=bar_count to make bar_count visible in the bar relation.

@lindyhopchris
Copy link
Member

@fisharebest glad you found a solution that works for you at the moment.

@dimitvassilev Sparse fieldsets are a good suggestion, though potentially would have to list a lot of fields as you might all of them except for the count.

Having looked through the Eloquent documentation on this, I actually think this would be a useful to add to this package, but in a JSON API compliant way. My suggestion would be to add the count to the meta of the relationship, i.e.:

{
  "data": {
    "type": "posts",
    "id": "1",
    "attributes": {
      "title": "Hello World",
      "content": "..."
    },
    "relationships": {
      "comments": {
        "self": "/api/posts/1/relationships/comments",
        "related": "/api/posts/1/comments",
        "meta": {
          "count": 12
        }
      }
    }
  }
}

And use a custom parameter to allow it to be included. E.g.:

GET /api/posts/1?count=comments

Or if you wanted multiple relationships counted:

GET /api/posts/1?count=comments,tags

Would be interested to hear your thoughts!

@fisharebest
Copy link
Author

And use a custom parameter to allow it to be included.

I guess there could be other meta data to include. With a post, you may want both the number of comments and the date of the most recent. So perhaps a more flexible parameter name along the lines of includeMeta[comments.count,comments.latest]

@lindyhopchris lindyhopchris added this to the 1.1.0 milestone May 3, 2018
@lindyhopchris
Copy link
Member

@fisharebest yes, definitely a good suggestion.

I've added this to the 1.1.0 milestone as it can be worked on post the 1.0.0 release.

@JeanLucEsser
Copy link
Contributor

If I'm not mistaken, by the spec, query params must be camel cased, or prefixed by - or _.
http://jsonapi.org/format/#query-parameters

So includeMeta would work but count= would not.

Also, can I suggest using the same approach as with fields?
That is the type and then the attributes:
includeMeta[comments]=count,latest (or just _meta[comments])

Another thing, I use the meta node in each included relationship to put the pivot table attributes.
Something akin to comments->meta->attributes->is-private if my Post-Comment pivot table has such an attribute.

Would be nice to use something similar for that:
includePivotFields[comments]=is-private (or just _pivot-fields[comments])

What do you think?

-- JL

@dimitvassilev
Copy link

@JeanLucEsser I think you are mistaken about the spec - it only says that camel-casing is one of the recommended ways to specify multi-word parameters, the other (still only recommended) being by use of the - and _ characters. Neither prefix is required, nor the use of single-word parameters prohibitted.

@lindyhopchris
Copy link
Member

lindyhopchris commented Oct 29, 2018

Laravel 5.7.10 added a loadCount feature for Eloquent models:
https://laravel-news.com/laravel-5-7-10

When adding a count feature, would be good to also support this eager loading.

ED: one thing to work out is what to do if the resource is side-loaded via an include path... would need to include the count in the side-loaded resource if the client is always expecting it to be there. E.g. if a request asks for post.comments and the comments resource is always meant to have a count in one of its relationships, how to eagerly load the count on each comment model so that n+1 problems are avoided?

@webdobe
Copy link

webdobe commented May 2, 2020

So how do we do this? Using sparse fields seem to do the count queries regardless if you include them in the fields query params. Also, this was said to be added to the v1.1.0 milestone, but we are way past that. Would be good to document this somewhere if we are able to do it. Any info regarding this is appreciated!

@lindyhopchris
Copy link
Member

@webdobe thanks for your comment. this is going to have to wait until the revamp, as I need to sort out the revamp first. Sorry, but maintaining an open source project I have to prioritise, and at the moment the revamp is needed to open the pathway to closing off a lot of these issues.

@lindyhopchris
Copy link
Member

Closing in favour of implementing in the new package - laravel-json-api/laravel. See the issue linked above to track progress in that package.

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

No branches or pull requests

5 participants