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

Problem with countable fields #194

Open
VoronovAlexander opened this issue Jun 6, 2022 · 8 comments
Open

Problem with countable fields #194

VoronovAlexander opened this issue Jun 6, 2022 · 8 comments

Comments

@VoronovAlexander
Copy link

VoronovAlexander commented Jun 6, 2022

How to move count variables from relationships.*.meta.count to data.*.attributes?

@VoronovAlexander VoronovAlexander changed the title Problem with sorting by countable fields Problem with countable fields Jun 7, 2022
@ben221199
Copy link

What do you mean by "move"?

@VoronovAlexander
Copy link
Author

What do you mean by "move"?

When I use in schema

    public function sortables(): iterable
    {
        return [
            SortCountable::make($this, 'administrators'),
        ];
    }
    
    public function fields(): array
    {
        return [
            ID::make()
                ->sortable(),

            Str::make('name')
                ->sortable(),

            DateTime::make('created_at')
                ->serializeUsing(fn($datetime) => $datetime->timestamp ?? null)
                ->sortable()
                ->readOnly(),

            DateTime::make('updated_at')
                ->serializeUsing(fn($datetime) => $datetime->timestamp ?? null)
                ->sortable()
                ->readOnly(),
                
            HasMany::make('administrators')->canCount(),
        ];
    }

And try use it in request:

curl --location -g --request GET 'http://localhost:8000/api/admin/v1/subdivisions?withCount=administrators&page[size]=1&page[number]=1' \
--header 'Accept: application/vnd.api+json' \
--header 'Content-Type: application/vnd.api+json'

I have in response:

{
    "meta": {
        "page": {
            "currentPage": 1,
            "from": 1,
            "lastPage": 6,
            "perPage": 1,
            "to": 1,
            "total": 6
        }
    },
    "jsonapi": {
        "version": "1.0"
    },
    "links": {
        "first": "http://localhost:8000/api/admin/v1/subdivisions?page%5Bnumber%5D=1&page%5Bsize%5D=1&withCount%5B0%5D=administrators",
        "last": "http://localhost:8000/api/admin/v1/subdivisions?page%5Bnumber%5D=6&page%5Bsize%5D=1&withCount%5B0%5D=administrators",
        "next": "http://localhost:8000/api/admin/v1/subdivisions?page%5Bnumber%5D=2&page%5Bsize%5D=1&withCount%5B0%5D=administrators"
    },
    "data": [
        {
            "type": "subdivisions",
            "id": "1",
            "attributes": {
                "name": "Северо-запад",
                "created_at": 1598788009,
                "updated_at": 1598788009,
            },
            "relationships": {
                "administrators": {
                    "links": {
                        "related": "http://localhost:8000/api/admin/v1/subdivisions/1/administrators",
                        "self": "http://localhost:8000/api/admin/v1/subdivisions/1/relationships/administrators"
                    },
                    "meta": {
                        "count": 1
                    }
                }
            },
            "links": {
                "self": "http://localhost:8000/api/admin/v1/subdivisions/1"
            }
        }
    ]
}

WithCount added count inside meta of relationship: data.*.relationships.administrators.count, but I want move «count» to data.*.attributes.administrators_count. Can I do it?

@lindyhopchris
Copy link
Contributor

That's not supported at the moment. The meta for the relationship that the count relates to feels like the best place to put it. It also makes sense from an implementation perspective, as the count is processed as part of the relationship.

Presumably if the API client wants to treat it as an attribute it can normalize the payload however it sees fit once it receives it from the backend?

@VoronovAlexander
Copy link
Author

VoronovAlexander commented Jun 7, 2022

That's not supported at the moment. The meta for the relationship that the count relates to feels like the best place to put it. It also makes sense from an implementation perspective, as the count is processed as part of the relationship.

Presumably if the API client wants to treat it as an attribute it can normalize the payload however it sees fit once it receives it from the backend?

I use Spraypaint Graphiti ORM on frontend for working with json:api, but it can't extract meta from relationships automatically.

@lindyhopchris
Copy link
Contributor

Ok so this would need a new feature to allow Eloquent aggregates as attributes. It's not a simple task though as it would involve making sure that the aggregate is always added to the model whenever the database is queried for that model.

I can see it would be a useful feature though, but just can't give an ETA on when it might be implemented. However the next thing I'm likely to be working on is improvements to the combination of eager loading / sparse field sets / aggregates. The 3 are all interrelated as they all affect the database query that needs to be executed.

@steven-fox
Copy link

It's not a "fluent" solution like the ->canCount() implementation is, but all should remember that Schemas can include custom attributes, so if they want the count of related administrators within the attributes object of the parent object, they can just add Number::make('administratorsCount')->readOnly() to the Schema. When the ?withCount query parameter is included in the request, this will work like normal. Technically the count will be included in 2 places (at attributes.administratorsCount and relationships.administrators.meta.count) but it will only be retrieved from the database once, so there is negligible performance penalty. When the ?withCount isn't present, the value will simply be null.

In one of our current apis, we're using Laravel's JsonResource to transform data into responses and this let's you use a MissingValue class, which when the value is empty, will filter the key and value out of the response entirely (kinda handy, as it can be somewhat confusing to see null instead of nothing at all for certain attributes that may or may not be computed based on the needs of the request).

@lindyhopchris
Copy link
Contributor

Worth mentioned that Laravel JSON:API does have support for conditional attributes, via the resource class and documented here: https://laraveljsonapi.io/docs/2.0/resources/attributes.html#conditional-attributes

So in other words, you'd be able to conditionally add the count attribute if it is set on the model.

I put this feature in to match the Laravel Eloquent resources. I hadn't realised there was also a MissingValue class, so need to add support for that.

@steven-fox
Copy link

That's awesome Chris! I haven't used your json:api package since its revamp a while ago, so I'm going through the docs, tutorial, and Github issues to get caught up again. Guess I hadn't made it the conditional attributes section quite yet.

Btw, this is an amazing package with an incredible amount of effort. If we decide to use it for a production app in the future, I'll do everything I can to assist with issues and PRs to give back, albeit by a small amount relative to the scope of the package. 👍

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

No branches or pull requests

4 participants