views price field handler needs more data so price alterations are correctly processed
| Project: | Ubercart |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed |
The uc_discounts module needs to correctly handle prices that are shown in a view.
It works by uc_discount_price_handler_alter() gets a $context, which includes a $node object. This is fine when we're actually looking at a node, but with Views, the $node is not actually a node -- it's an object of the data the view returned and no more.
So if the discount CA predicate needs a node type, or terms, these might not be there.
I can see several ways to go about fixing this, and all of them require changes to the uc_product_handler_field_price.
a. node_load the node in the discount handler.
This is simple. It's a performance cost, but lots of views handlers do a node_load for various reasons.
This requires a change to uc_product_handler_field_price to add the nid as a field with $this->additional_fields, since views that add this field on a relationship may not have it.
b. use $this->additional_fields in the field handler to add the fields we know that CA conditions will need.
This would be best done when the view is being saved:
1. ask CA about what conditions apply to nodes,
2. ask the conditions what node fields they need
3. save this in the view as a setting on the price field handler (the same way UI options are saved; I've don ethis on quite a few views handlers)
4. use $this->additional_fields to add the extra fields to the view query.
c. An alternative approach to b would be to check CA condition requirements when the handler is being validated in the UI, and insist the user add those fields to their view.
#1
Original issue on UC Discounts: #579162: Discounts don't work in views price field
#2
This possibly affects VAT calculations too, if these need the node terms to decide which VAT rate to apply.
#3
After discussing uc VAT requirements with longwave, I think the safest thing, and the thing that covers all cases easily, is to do a node_load in the handler and pass that in the $context.
Price alteration handlers expect something called a #node to be an actual node object -- if they all have to account for the special case of views then they're all repeating the same work.
I haven't time to write a patch, but basically the approach can be cribbed from this: http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/image/views...
You need to ensure the nid field in construct() and then use that with the alias in your node_load(). This ensures that the field will work on relationships.
#4
Huh, even simpler than that.
From uc_product_handler_field_addtocart.inc:
$product = node_load($values->{$this->aliases['nid']});
#5
Well, it's not quite that simple, since Views needs to make sure that the nid column has been loaded. I would have thought it always would be considering that uc_products has to join to the node table, but I get a warning about the property not existing on one of those objects.
I'm not really happy about doing it this way, but since we're already doing the same thing in the add-to-cart handler, I can't protest too much.
#6
Seems to work. Committed.
#7
Thanks! :)
Having nid as an 'additional fields' isn't that unusual; views core does it quite a few times I think.
#8
Not sure if this is the same issue, but I have upgraded ubercart to the latest version (2.0) and am now receiving this error.
Fatal error: Cannot access empty property in /Users/aaronchristian/Sites/htdocs/lowgimeals/sites/all/modules/ubercart/uc_product/views/uc_product_handler_field_price.inc on line 63
Do these two issues correspond with one another?
#9
Nah, that's another issue that might be resolvable by clearing your Views cache.
#10
Thanks rszrama,
Tired that, still receiving the error, I'll try to have a deeper look into it and post back If I find the issue.
#11
#12
Automatically closed -- issue fixed for 2 weeks with no activity.