-
Notifications
You must be signed in to change notification settings - Fork 74
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
Can't serialize into an array of objects #34
Comments
Same problem here. Syphon transforms the nested attributes from array of objects to an object of objects. This is unfortunate as I can not loop through the collection anymore i.e for validation of nested attributes |
Any news about that ? |
This is certainly a behavior that would be good to have. If anyone wants to open a PR, that would be awesome! @skrobek Marionette Core is currently focused on getting Marionette 2.0 out the door right now. A lot of these issues will be addressed afterwards. Sorry about the delay. |
@thejameskyle thank you for answear. |
@skrobek In my case, I wound up manually iterating over the inputs and creating an array of objects in the View. It works but is ugly and not reusable. |
@MattJones would you mind posting some of that code here for the sake of it? Is this a major roadblock for anyone? I can move this up in priority if so. It will probably still be a little while. But I'm planning on releasing v0.5 in the not too distant future. |
@thejameskyle Sure. It's more complex than it needs to be just for our situation, because it also is designed to counter the issue where jqueryui-sortable causes the DOM elements for a collection to become ordered differently than the collection itself. That's why I originally didn't post it. This is the function in my CollectionView that generates a new Collection based on the information in the DOM form: getCollectionFromDOM: function(){
var _this = this;
var fieldArr = [];
this.$(this.itemViewContainer).find('> li').each( function(something, _el){
var el = _this.$(_el);
var field = {};
field.name = el.find('#nameField').val();
field.display_name = el.find('#displayNameField').val();
field.extra_info = el.find('#extraInfoField').val();
fieldArr.push(field);
});
return new App.Entities.LoadFileStructureFieldCollection(fieldArr);
} This is the template for the model views contained in the CollectionView: <li>
<label>
<input type="hidden" name="load_file_structure_fields[][name]" id="nameField" value="{{=name}}">
<input type="hidden" name="load_file_structure_fields[][extra_info]" id="extraInfoField" value="{{=extra_info}}">
<input type="text" name="load_file_structure_fields[][display_name]" class="editBox" id="displayNameField" value="{{=display_name}}">
</label>
</li> |
Okay I've added this functionality - in coffeescript (of course). You can take inspiration from it. Syphon can almost do this out of the box, but because Syphon.serialize lines (47-50)The long term fix would be exposing if (validKeyAssignment($el, key, value)){
var keychain = config.keySplitter(key);
data = _.isFunction(Syphon.AssignKeyValue) ? Syphon.AssignKeyValue(data, keychain, value) : assignKeyValue(data, keychain, value);
} Override Syphon KeySplitterSyphon.KeySplitter = (key) ->
matches = key.match /[^\[\]]+/g
## allow us to write an empty [] next to each key
## which indicates this needs to be an array
_.map matches, (match) ->
if _.str.include(key, match + "[]") then [match] else match Override Syphon AssignKeyValueWe've now given ourselves the ability to override a public function, so we do that here. This replaces the private function Syphon.AssignKeyValue = (data, keychain, value) ->
return data if not keychain
key = keychain.shift()
## build the current object we need to store data
## only if the key is an array and its not
## currently set on our data obj
data[key] = [] if _.isArray(key) and not data[key]
## if its the last key in the chain, assign the value directly
if keychain.length is 0
switch
## when our data object is a literal array
## then we need to push an object into it
when _.isArray(data)
obj = {}
obj[key] = value
data.push(obj)
## when our data[key] is an array
## we need to push its value directly into it
when _.isArray(data[key])
data[key].push(value)
## just assign a value to the data[key]
else
data[key] = value
if keychain.length > 0
@AssignKeyValue(data[key], keychain, value)
data Okay cool so what does this allow us to do? Given we serialize these inputs... <input name="schedules[][userId]" value="1" >
<input name="schedules[][userId]" value="2" >
<input name="schedules[][userId]" value="3" > We will get this array of objects: {
schedules: [
{userId: 1},
{userId: 2},
{userId: 3}
]
} Previously Syphon would have created this worthless object: {
schedules: {
{userId: 3}
}
} So now any time you have an input that has an empty Example: <input name="schedules[]" value="1" >
<input name="schedules[]" value="2" >
<input name="schedules[]" value="3" > {
schedules: [1,2,3]
} But what about harder situations like this.... ? <input name="schedules[][userId]" value="1" >
<input name="schedules[][userId]" value="2" >
<input name="schedules[][userId]" value="3" >
<input name="schedules[][note]" value="foo" >
<input name="schedules[][note]" value="bar" >
<input name="schedules[][note]" value="baz" > Well as it stands my fix will serialize the above inputs into the following object: schedules: [
{userId: 1},
{userId: 2},
{userId: 3},
{note: "foo"},
{note: "bar"},
{note: "baz"}
] That's probably not what you wanted, you probably wanted something like this... schedules: [
{userId: 1, note: "foo"},
{userId: 2, note: "bar"},
{userId: 3, note: "baz"}
] That's actually quite challenging to do, but I have done it. It involves once again exposing a new method like From there you can bust out the ol' functional programming and loop through the data object, finding all arrays with objects, and then intelligently merging them together by reducing the array of objects, and checking by the keys whether to merge with the previous object or create a new one. Sounds like fun right? It does work, and I can post the code. To my surprise even throwing non-standard order of inputs, and even omitting ones does in fact produce what you'd expect. Though, my first example above would still create 6 objects, so you cannot order your inputs like that. However.... you're probably generating your inputs through a loop which may look more like this... <input name="schedules[][userId]" value="1" >
<input name="schedules[][note]" value="foo" >
<input name="schedules[][userId]" value="2" >
<input name="schedules[][date]" value="1986-03-14" >
<input name="schedules[][userId]" value="3" >
<input name="schedules[][note]" value="baz" > Even though there are different inputs, it still generates the correct object: schedules: [
{userId: "1", note: "foo"}
{userId: "2", date: "1986-03-14"}
{userId: "3", note: "baz"}
] You would just have to be conscientious not to include the first My question is... does anyone else have a better solution? Other than abandoning this idea entirely and leaving it up to your Backbone View/Model to just properly handle setting nested array of attributes? Should I post the additional code for merging? |
@brian-mann Thanks for your solution. That's exactly what I was looking for. |
please pull the changes of @brian-mann as this is realy a helpfull change |
I'm not sure that's exactly what we want though. Why not do it like Rails does it with nested attributes? So instead of doing <input name="schedules[][userId]" value="1" >
<input name="schedules[][note]" value="foo" >
<input name="schedules[][userId]" value="2" >
<input name="schedules[][date]" value="1986-03-14" >
<input name="schedules[][userId]" value="3" >
<input name="schedules[][note]" value="baz" > let's do this: <input name="schedules[0][userId]" value="1" >
<input name="schedules[0][note]" value="foo" >
<input name="schedules[1][userId]" value="2" >
<input name="schedules[1][date]" value="1986-03-14" >
<input name="schedules[2][userId]" value="3" >
<input name="schedules[2][note]" value="baz" > Notice the explicit index here. It should be too difficult to parse this correctly – and no fancy tuple work involved. |
Here's the code I used to make it work for the HTML above: var flattenData = function(config, data, parentKey){
var flatData = {};
_.each(data, function(value, keyName){
var hash = {};
// If there is a parent key, join it with
// the current, child key.
if (parentKey){
keyName = config.keyJoiner(parentKey, keyName);
}
// This first if branch is what I added:
if (_.isArray(value) && _.isObject(value[0])) {
_.each(value, function(object, i) {
_.each(object, function(attributeValue, attributeName) {
hash[keyName + "[" + i + "][" + attributeName + "]"] = attributeValue;
});
});
} else if (_.isArray(value)){
keyName += "[]";
hash[keyName] = value;
} else if (_.isObject(value)){
hash = flattenData(config, value, keyName);
} else {
hash[keyName] = value;
}
// Store the resulting key/value pairs in the
// final flattened data object
_.extend(flatData, hash);
});
return flatData;
}; Note that I haven't tested this against deeply nested structures or anything. I just used it for my simple use case of having a collection of child objects (in my case showing a product form and nested variants). |
Where did this end up on the priority list? It seems like a very important feature to have (it is for most apps that I work on), but I can see in version 0.5.1, the "assignKeyValue" method is still private and the issue with serializing into arrays is behaving the same way. |
There's also the other side of Syphon, taking an object and pushing that into a form. With that in mind, the proposal from @clemens does help in that regard as Syphon would have enough info to locate the appropriate fields. Would requiring a structure like Also conscious that this would be one of the only times Syphon would return or receive an array for serialize or deserialize calls |
One thing to note: Whatever the implementation, it would probably be at least somewhat opinionated and wouldn't fit all use cases. So the only way around extensive bike shedding about what constitutes The Right Way™ would be to make this logic pluggable – and potentially add a (somewhat) sane default (such as the ones proposed by Brian or myself). |
👍 I agree @clemens are you interested in taking a stab at this? |
Well said @clemens |
Such as serialize into an array of objects: marionettejs/backbone.syphon#34
The rails form serializer allows creation of arrays of key/value pairs. For example,
would get serialized to
As far as I can tell, Syphon just treats it like a single object, and overwrites all previous items in the array, and produces this:
Am I missing something, or is this just not doable with Syphon?
The text was updated successfully, but these errors were encountered: