Expand event API to include partners as array string. DDFHER-119
#1743
Conversation
| @@ -63,6 +63,7 @@ public function getResponse(EventInstance $event_instance): EventsGET200Response | |||
| 'branches' => $this->getBranches(), | |||
| 'address' => $this->getAddress(), | |||
| 'tags' => $this->getTags(), | |||
| 'partners' => $this->getMultiValue('event_partners'), | |||
There was a problem hiding this comment.
Consider what happens with this value if there are no partners.
The spec requires an optional array of strings.getMultiValue() returns null or an array of strings. Will the response leave out the partners property if the value is null or automatically convert it to an empty array?
Personally I would prefer getMultiValue() to always return an array of strings. The value might then be empty.
There was a problem hiding this comment.
Fair point. I've updated it so it'll always return an array, even if it is empty.
| return NULL; | ||
| } | ||
|
|
||
| return explode(', ', $strings); |
There was a problem hiding this comment.
Please consider if this is an appropriate way to handle multivalue fields.
As I read it, if one of the value contain , then it will be treated as a multivalue. That does not seem right. Is there a better way to go about this?
If we go with this approach I would appreciate a reference to why , is an appropriate separator.
event_partners is a multivalue field and to me it is not obvious that multiple values are separated by , by Drupal Core/$field->getString().
There was a problem hiding this comment.
It was an attempt to reuse the custom getValue() method, but, i didnt consider that strings might actually contain , .
I've updated it, to use the Drupal getValue method instead, and using array_column to turn it into a simple array.
rasben
force-pushed
the
DDFHER-119_expand-event-api
branch
from
71df799
to
5a08e10
Compare
Link to issue
https://reload.atlassian.net/browse/DDFHER-119
/api/v1/events