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

Question: conditional attributes #409

Closed
genyded opened this issue Aug 27, 2019 · 4 comments
Closed

Question: conditional attributes #409

genyded opened this issue Aug 27, 2019 · 4 comments
Milestone

Comments

@genyded
Copy link

genyded commented Aug 27, 2019

Could not find this in the docs anywhere, but is there any way to do conditional attributes along the lines of Laravel's resource attributes $this->when?

We know that we can do something like:

public function getAttributes($resource)
{
  return array_filter([
    'shortName' => $resource->short_name,
    'longName' => (Auth::check() && Auth::user()->isAdmin()) ? $resource->long_name : false,
    'banner' => $resource->banner
  ]);
}

However that also removes any other attributes with empty values which may not be desired.

We could also do something like:

public function getAttributes($resource)
{
  $arr = [
    'shortName' => $resource->short_name,
    'longName' => $resource->long_name,
    'banner' => $resource->banner
  ];
  if (Auth::check() == false || Auth::user()->is_epa() == false) {
    unset($arr['longName']);
  }
  return $arr;
}

... which preserves other attributes with empty values, but is not very clean , especially if you have many conditional attributes. Each would need it's own IF(s). Was hoping for a clean solution or plans to implement one. In real world apps, it's not likely that all users should see all attributes for all api resources at all times.

@lindyhopchris
Copy link
Member

So I'm not massively inclined to support conditional attributes. The reason is because it is really inefficient to have to loop over all the attributes to check if they are conditional or not, and filter them out. Doing this when you're serializing lots of resources for a single response is going to take a big hit.

So for the current version of the package, you will need to use the `if statements - though I do agree they are not clean!

For v2, I'll investigate whether I can support conditional attributes - but will only do so if they do not have a performance impact. I can think of potential ways of dealing with them though, but it depends on hooking into the internals of the encoder - which is exceptionally difficult to do because it is provided by another package that has an aversion to such internal modifications!

You might want to take a look at #411 as what you're asking is kind of related to that!

@lindyhopchris lindyhopchris added this to the 2.0 milestone Aug 28, 2019
@genyded
Copy link
Author

genyded commented Aug 28, 2019

Cool - this is of paramount importance to us, but performance is a key consideration as well. Laravel seems to have provided both (albeit not in JSON API format). We'll leave it to you to decide if you want to leave this open for specific tracking or close it in favor of consideration under #411 .

@lindyhopchris
Copy link
Member

Thanks, will leave open on the 2.0 milestone so I don't forget about it!

@lindyhopchris
Copy link
Member

Conditional atttributes are fully implemented in the new package laravel-json-api/laravel - see https://laraveljsonapi.io

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

2 participants