Improve extendability of API hooks
| Project: | User Relationships |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | alex.k |
| Status: | closed |
Jump to:
In all of Drupal's core APIs, anything "extra" is passed around, be it in a form_submit, a node save, a comment save, a user save, etc. This makes perfect sense when you realize that FAPI allows one to modify any form whatsoever - the additional values should be passed around to all the hooks.
User Relationships doesn't do this, which makes it nearly impossible for me to extend a relationship type. If I add fields to NAME_form_user_relationships_ui_type_edit_alter(), they'll never get to hook_user_relationships_type() due to user_relationships_ui_type_edit_submit(). At first, you started out good - you forced all the $form_state['values'] into $relationship_type. If you called the hook right after that, I'd have been happy. But you don't - you drupal_write_record() then destroy all the saved values in $relationship_type by calling user_relationships_type_load(). The newly overwritten value is what's passed to the hook - without any form values. In short, there's no way for me to save the values from the form using your hook.
What I'll be trying next is running my own FAPI #submit *after* yours, so that I can load the relationship based on the submitted 'name' (since I don't have the rtid at submission time), get the rtid, and then do my saving.
Lame.
#1
Looking at user_relationship_implications module it seems to do the latter - add a #submit handler and do the work there. user_relationships_ui_type_edit_submit() saves newly received rtid back into $form_state['values'], so you should be able to see it there.
As to why it was written this way, reloading the type object before calling hooks - I have no idea. If this is really not the best Drupal practice we can drop the user_relationships_type_load call.
#2
Yeah, the #submit worked well enough for me too, but it's very un-APIish.
Basically, the API, as written, allows us to respond to events, but not to *extend* or *expand* the objects on which the events occur. That's unlike Drupal core, and thus, an "odd" API for a Drupal-based module. It's certainly workaroundable, yes, but only because Drupal form API is flexible enough. However, this also means that, unless a form was used, the underlying UR API is crippled - one couldn't programmatically (i.e., non-form) do anything useful to extend or expand the UR objects (it'd be a bit like a node-extender only doing its additions in a #submit as opposed to hook_nodeapi_insert, where it should be).
#3
I can appreciate that. Freedom to do things programmatically is definitely valuable. That I believe is a true deficiency of this implementation.
At this time, getting close to a stable release, I'm going to move this to a task to be worked for a 2.x branch. Would prefer to stick to the release plan right now.
#4
Committed a one liner that fixes it http://drupal.org/cvs?commit=285758. Looked pretty safe to remove the user_relationships_type_load call.
#5
Automatically closed -- issue fixed for 2 weeks with no activity.