search

kiva / backbone.siren

Converts Siren JSON representations into Backbone Models
MIT License
14 stars 3 forks source link

action.type and action.method confusion #14

Closed gxxcastillo closed 11 years ago

gxxcastillo commented 11 years ago

As mentioned in #3, they aren't working correctly.

gxxcastillo commented 11 years ago

This should fix it: df52ac0. I haven't tested it yet so hope it works.

You can also override any of those default options by passing in some options of your own:

user.getActionByName('do-stuff').execute({type: 'PUT', contentType: 'multipart/form-data'});

(It can get a little confusing since Siren, jQuery, and HTML use different property names for this stuff...might be worth a ticket to Siren to have it match HTML form attributes?)

apsoto commented 11 years ago

I've skipped setting defaults that are undefined.

I may not be following good javascript code style. Comment if you seen any changes you'd like to see before I submit PR.

apsoto/backbone.siren@afada77740992018db584320b2a1dac7d867d531

gxxcastillo commented 11 years ago

Is it working as is? I can't see anything wrong with setting defaults.type = undefined. Whether defaults.type is set to undefined or whether it is left as undefined shouldn't change the fact that defaults.type === undefined. Also, in this case, just setting everything feels "cleaner" and saves a few characters :)

Aside from that, I can see how the name "defaults" might be misleading since they aren't really "defaults" but more like "action presets"...?

apsoto commented 11 years ago

If you don't set the type in the action response you get undefined which results in the content type being empty as opposed to backbone default to application/json.

When the content type is empty the server side doesn't parse the params.

gxxcastillo commented 11 years ago

Ah, you're right. Apparently, underscore.js copies over undefined values (kinda lame because jQuery extend does not). Thanks for catching this.

Go ahead and submit the pull request, it would be cool if the comment stated something about underscore copying over undefined values.

Feel free to close this issue if its resolved.

gxxcastillo commented 11 years ago

I went ahead and applied your recommended changes along with a few other bug fixes and an added feature so that you can pass an "attributes" object to .execute() as a property on the options object.