candy-api
candy-api copied to clipboard
Setting `attribute_data` has inconsistent behavior based on model persistance
The HasAttributes trait allows for setting attributes on models. But the attributes are being managed differently via a mutator based on whether or not the model exists in the database.
This causes an issue when trying to manage models and their attributes manually, e.g. in an importing script.
Expected Behavior
I would expect that setting attribute_data on a model behaves consistent whether or not a model is persisted.
Current Behavior
When a model is new (not persisted), added attribute_data is being run through a mapper, before adding it to the model's attributes.
When a model exists (persisted), added attribute_data is being inserted "as is" (although, JSON encoded).
Possible Solution
A possible solution would presumably rely on (or affect) Admin Hub implementation. It would be beneficial for transparency to decide whether or not to use the mapping method consistently or to use "as is" data.
The issue with "as is" (as I see it) is ensuring portability of attribute groups and attributes to Elasticsearch and other implemented usage of these throughout the API and implementations with Admin Hub and possibly storefronts, as this introduces a possibility of implementation developers to provide wrongfully structured data.
The issue with using the mapper method is lack of transparency when setting the attributes. Without transparency, "magic" happens and that can be difficult to implement correctly.
Maybe dynamically generated JSON schemas to help fill and validate attribute_data based on Attribute Groups and their Attributes could be a simple yet powerful solution.
Steps to Reproduce
A simple sample code run in a Console Command:
$category = \GetCandy\Api\Core\Categories\Models\Category::firstOrNew();
$category->attribute_data = [
'name' => [
'en' => 'Test',
]
];
$category->save(); // attribute_data contains different data in database depending on the initial persistent state of the model
Context (Environment)
Detailed Description
Possible Implementation
Having discussed with Alec, we agreed.
The best approach would be to set the model back to using raw attribute data on inserts and updates and then have a Laravel Action to handle the mapAttributes functionality.
In the interim, don't use firstOrNew :-)