Skip to content

Revert "alwaysIdentifier annotation option implementation" #1696

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

Merged
merged 1 commit into from
Feb 9, 2018

Conversation

dunglas
Copy link
Member

@dunglas dunglas commented Feb 9, 2018

This reverts commit 10a009f.

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets n/a
License MIT
Doc PR n/a

Digging deeper into #1528, I think that this should be handled at the Symfony level, so I proposed symfony/symfony#26108

I would like to revert #1528 before releasing the stable release.

ping @api-platform/core-team @adelplace

@soyuka
Copy link
Member

soyuka commented Feb 9, 2018

👍 we will document the proper way of handling this if we can get symfony/symfony#26108 merged! Btw you can add a 👍 to show your interest in that PR!

@adelplace
Copy link

I think that this should be handled at the Symfony level

Yes I totally agree

@dunglas dunglas merged commit 3a1961f into api-platform:master Feb 9, 2018
@dunglas dunglas deleted the revert-1528 branch February 9, 2018 14:36
@silverbackdan
Copy link
Contributor

Is there any chance on how to implement this functionality with the PR now merged in Symfony?

Is it worth API Platform having a built-in handler that can be enabled with annotation on specific properties?

@soyuka
Copy link
Member

soyuka commented Feb 22, 2018

@silverbackdan just set this in symfony configuration:

serializer:
  max_depth_handler: App\MaxDepthHandler

Your service would look like this:

<?php
namespace App;

class MaxDepthHandler
{
  public function __invoke($obj) {
    return $obj->id; // or whatever
  }
}

@silverbackdan
Copy link
Contributor

Thanks @soyuka - so very simple :)

hoangnd25 pushed a commit to hoangnd25/core that referenced this pull request Feb 23, 2018
Revert "alwaysIdentifier annotation option implementation"
kevinreniers added a commit to jimmycleuren/fireping that referenced this pull request Apr 12, 2018
This causes significant performance impact because it will load the full embedded relationship of a parent, which again includes its subdomains, which will again load its parents, ... you get the picture.

Looked for a way to just force it to return the URI /api/domains/:id, but that does not seem to be an option anymore (used to be, but was removed in api-platform/core#1696).

This feature can be enabled through Symfony starting from version 4.1, which we are not currently at.

As such, the best solutions we currently have are:
- writing our own serializer (complex and we'd mostly be reinventing the wheel)
- omitting the $parent property from the group, and instead exposing it through /api/domains/:id/parent.

Signed-off-by: Kevin Reniers <[email protected]>
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

Successfully merging this pull request may close these issues.

4 participants