Present implementation doesn't allow contrib modules to use hook_form_alter() et al to manipulate attributes without modifying module weights. Lyle and I have discussed the issue and we both feel it's necessary to add an Ubercart layer form alter hook so that contrib/other modules can easily use the drupal layer form alter hook. The reason, we decided, was because Ubercart, being a system on a system, needs to avoid limiting the Drupal layer programmability so developers can be familiar with the platform in less time. Another benefit is that low-level changes (read as evil :p), such as changing module weights, are not necessary.

CommentFileSizeAuthor
#11 521378_200907231338-0400.patch7.93 KBsammys
#8 521378_cart_form_alter.patch4.67 KBIsland Usurper
#2 521378_cart_form_alter.patch3.33 KBIsland Usurper

Comments

sammys’s picture

sammys commented

Another note: the change won't break current contrib though we recommend contrib maintainers think carefully about moving things into the Ubercart layer hook so module authors can modify the form elements they are adding to the form.

Island Usurper’s picture

Island Usurper commented
Assigned: Unassigned » Island Usurper
Status: Active » Needs review
StatusFileSize
new 521378_cart_form_alter.patch3.33 KB

I had to do a little bit of rearranging in uc_attribute_form_alter() to change it into uc_attribute_add_to_cart_form_alter() because a lot of the default data put on forms isn't available yet. I think that's the right spot for the drupal_alter(). There aren't many other good places.

sammys’s picture

sammys commented
Status: Needs review » Needs work

Patch appears to work ok for the stuff I need to do. Attributes are showing in the cart, order review and invoice. Attributes appear to be missing from the user's order view though the invoice shows it.

sammys’s picture

sammys commented
Status: Needs work » Reviewed & tested by the community

Ok... this was because I had a dirty branch. RTBC!

Island Usurper’s picture

Island Usurper commented
Status: Reviewed & tested by the community » Fixed

Alright. Committed, and updated the documentation about changes from 1.x to 2.x.

Island Usurper’s picture

Island Usurper commented
Status: Fixed » Needs work

Actually, we still need to document hook_add_to_cart_form_alter() in hooks.php.

Also, should we make the $form_state get passed by reference, the way hook_form_alter() does?

sammys’s picture

sammys commented

Yeah I agree that adding $form_state is a positive step. The same thing needs to happen across the board actually, but we can make baby steps. :)

A related patch for $form_state stuff: #435084: Using $form_state instead of $_POST in cart forms

That one is needed to support injection of content_profile (hence CCK) forms into the user information part of checkout.

Island Usurper’s picture

Island Usurper commented
Status: Needs work » Needs review
StatusFileSize
new 521378_cart_form_alter.patch4.67 KB

Actually, I had just noticed that the hook functions expected $form_state to be passed by reference, so we need to use the '__drupal_alter_by_ref' key anyway.

sammys’s picture

sammys commented
Status: Needs review » Needs work

Can we move the following two lines into an API function? Preferably one that takes the alter "name" (in this case 'add_to_cart_form') and then all remaining args get pushed along to drupal_alter() using call_user_func_array() or something?

  $form['__drupal_alter_by_ref'] = array(&$form_state);
  drupal_alter('add_to_cart_form', $form, $node);
sammys’s picture

sammys commented

Lyle and I have discussed the namespace clash with what the Drupal layer does and have decided to do the following:

  • Add API function uc_form_alter($form, &$form_state, $form_id) to uc_store. This function will facilitate any future Ubercart layer form alterations we might need.
  • Name Ubercart layer form alter hooks with the suffix '_uc_form_alter' to prevent confusion and code being invoked twice.

    • E.g uc_attributes will now have a uc_attributes_uc_form_alter() hook.

    I'm in the middle of updating the patch.

sammys’s picture

sammys commented
Status: Needs work » Needs review
StatusFileSize
new 521378_200907231338-0400.patch7.93 KB

Patch attached.

Island Usurper’s picture

Island Usurper commented
Title: Add a hook_add_to_cart_form_alter() » Add a hook_uc_form_alter()
Status: Needs review » Fixed

Works splendidly.

I guess we'll just have to figure out which forms will have alterations that need to be altered on a case-by-case basis.

Committed.

sammys’s picture

sammys commented

Jolly good sir. Another one knocked off its perch.

vitis’s picture

vitis commented

looking forward to trying the patch

is it already fixed in one of the devs?

Island Usurper’s picture

Island Usurper commented

Yes, it is.

tejaspmehta’s picture

tejaspmehta commented

hello,

do you have any idea that how form_alter will work for checkout process ? i want to store order_id in another table once order is generated. for that i am using following code but it is not working well for me 

<?php
function modulename_form_alter(&$form, &$form_state, $form_id)
{
  if($form_id = 'uc_cart_checkout_form')
  {
      $form['#submit'][] = 'modulename_getorderid';
  }
  return $form;
}
function modulename_getorderid(&$form, &$form_state)
{
//    run database query here
}
?>

Now only help i need in this code is when anyone press submit button say Review Order and confirm order at that time i want to execute modulename_getorderid function. $form['#submit'][] might be the way but its not working so if anyone have idea than please post your reply.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

summit’s picture

summit commented

Subscribing, needing also uc_form_alter for payment redirecting.
Greetings,
Martijn

torgospizza’s picture

torgospizza
he/him
Primary language English
Location Portland, OR
commented

Martijn,

This issue is CLOSED so why are you subscribing? If there is a feature you need not found in this patch, you need to open a new issue.

summit’s picture

summit commented

Ok, sorry it was late, I wanted to bookmark, not subscribe, sorry to bother you. greetings, Martijn