Get default attributes for schemas and hydrators from model fillable attribute #49
Conversation
pedroluislopez
changed the title
| * @param $record | ||
| * @return void | ||
| */ | ||
| protected function defaultHydrateAttributes(StandardObjectInterface $attributes, $record) |
There was a problem hiding this comment.
this needs to be called attributeKeys for consistency with a trait that exists (which will at some point be used in this EloquentHydrator). The method signature is correct.
There was a problem hiding this comment.
So, the function name would be defaultHydrateAttributeKeys?
There was a problem hiding this comment.
Function name would just be attributeKeys
| protected function defaultHydrateAttributes(StandardObjectInterface $attributes, $record) | ||
| { | ||
| $hydratorAttributes = $this->attributes; | ||
| if(empty($hydratorAttributes)) |
There was a problem hiding this comment.
this needs to use is_null instead of empty. I.e. an empty array can mean "hydrate no attributes" and a null value can mean "hydrate fillable attributes"
| $hydratorAttributes = []; | ||
| foreach($record->getFillable() as $attribute) | ||
| { | ||
| if(strpos($attribute, '_') !== false) |
There was a problem hiding this comment.
to convert the attribute key you should use the Str::dasherize method (the Str class already has a use statement). There's no need to us strpos - just run the attribute key through the dasherize method
| @@ -127,7 +153,7 @@ protected function hydrateAttributes(StandardObjectInterface $attributes, $recor | |||
|
|
|||
| $data = []; | |||
|
|
|||
| foreach ($this->attributes as $resourceKey => $modelKey) { | |||
| foreach ($this->defaultHydrateAttributes($attributes, $record) as $resourceKey => $modelKey) { | |||
| * @param Model $model | ||
| * @return array | ||
| */ | ||
| protected function getDefaultModelAttributes(Model $model) |
There was a problem hiding this comment.
For consistency this can also be attributeKeys
There was a problem hiding this comment.
So, the function name would be getDefaultModelAttributesKeys?
There was a problem hiding this comment.
Function name would be attributeKeys
| $schemaAttributes = $this->attributes; | ||
| if(empty($schemaAttributes)) | ||
| { | ||
| $schemaAttributes = $model->getFillable(); |
There was a problem hiding this comment.
If $model->getVisible() is not empty, that list should be used (as it is a white-list of serializable attributes).
If using $model->getFillable(), you'll need to remove any keys that are in $model->getHidden().
| protected function getDefaultModelAttributes(Model $model) | ||
| { | ||
| $schemaAttributes = $this->attributes; | ||
| if(empty($schemaAttributes)) |
There was a problem hiding this comment.
this should use is_null as per my comment on the hydrator.
| @@ -164,7 +181,7 @@ protected function getModelAttributes(Model $model) | |||
| { | |||
| $attributes = []; | |||
|
|
|||
| foreach ($this->attributes as $modelKey => $attributeKey) { | |||
| foreach ($this->getDefaultModelAttributes($model) as $modelKey => $attributeKey) { | |||
|
@lindyhopchris I make a PR from a branch only with this changes. I have made other changes about register adapters. Sorry. |
This implementation uses fillable attributes from eloquent models to avoid repetition of these attributes declaration in the schema and hydrator classes (by default, if attributes are declared in schema/hydrator class, these are used).