-
-
Notifications
You must be signed in to change notification settings - Fork 79
Conversation
With this change Serializers can provide links like:: ``` class PostSerializer implements SerializerInterface { // ... public function getLinks($post) { return ['self' => '/posts/' . $post->id]; } } ``` Partly resolves #64
@@ -143,6 +167,13 @@ public function comments($post) | |||
return new Relationship(new Collection($post->comments, new CommentSerializer)); | |||
} | |||
} | |||
class PostSerializer4WithLinks extends PostSerializer4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each class must be in a file by itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah well.. nit pick yourself ;)
@@ -115,7 +115,7 @@ $document->setMeta(['key' => 'value']); | |||
They also allow you to add links in a similar way: | |||
|
|||
```php | |||
$resource = new Resource; | |||
$resource = new Resource($data, $serializer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Resource constructor expects those parameters
FYI: This allows serializers to provide link relations (which I think is a crucial feature) partly resolving #64 – if this gets merged I'll gladly add another PR addressing the meta part |
I like. Hope this can get merged, as it's come in handy for a couple of my projects. |
$serializerLinks = $this->serializer->getLinks($this->data); | ||
if (! empty($serializerLinks)) { | ||
$links = array_merge($serializerLinks, $links); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should a Resource instance's links take precedence over the serializer links?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's what I assumed (and what is covered by a test), but frankly I wasn't really sure about it.. And after thinking about it again it might be more flexible if a serializer can overrule resource links.
In the original patch by @bnchdrff it was also the other way around - should I swap it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, didn't see the test and misread the order of array_merge
arguments – the current behaviour is fine.
Allow serializers to add links
With this change Serializers can provide links like::
Partly resolves #64