Version 1.2 - Bluemarine theme bug

Project:Ajax Attribute Calculations
Component:Code
Category:
Priority:normal
Assigned:Unassigned
Status:active
Description
Project: 
uc_aac

Thanks for your work on this, cYu.

For some reason version 1.2 is not updating the price fields for me when an attribute with a price adjustment is selected. I am using UC alpha 8. The AAC module is enabled, but there is no change in the store behavior when it is.

Any ideas?

Thanks.

cYu
cYu's picture
Offline
Bug FinderGetting busy with the Ubercode.
Joined: 11/19/2007
Juice: 850
Re: Version 1.2

I'm unable to duplicate the problem with a fresh install of Drupal 5.5 and UC alpha 8. I created one product with 2 attributes containing 3 options each with price adjustments. Do you have a more complex situation that I should test? Are you getting any JS errors?

cYu
cYu's picture
Offline
Bug FinderGetting busy with the Ubercode.
Joined: 11/19/2007
Juice: 850
Re: Re: Version 1.2

Ok, found the problem. The code was assuming both the display and sell price were present on the page as in the default configuration. Taking one of these off causes the error. Taking both of these off will leave the module with no price to update.

I'll fix this up and upload a version 1.3. Thanks for tracking down that bug.

cYu
cYu's picture
Offline
Bug FinderGetting busy with the Ubercode.
Joined: 11/19/2007
Juice: 850
Re: Re: Re: Version 1.2

Actually....am I misunderstanding display price and sell price? Should display price not be affected by the attribute change whereas sell price should? Can one of the uberfolks explain the different meanings for those two prices?

Lyle's picture
Offline
AdministratoreLiTe!
Joined: 08/07/2007
Juice: 6846
Re: Re: Re: Re: Version 1.2
Assigned to:cYu» Lyle

They're supposed to be the same, they just look different because the price of the product should be displayed prominently. I also wanted the price mixed in with the rest of the product fields, so I left the sell_price there too.

cYu
cYu's picture
Offline
Bug FinderGetting busy with the Ubercode.
Joined: 11/19/2007
Juice: 850
Re: Re: Re: Re: Re: Version 1.2
Assigned to:Lyle» cYu

Thanks Lyle, that makes sense. I guess there is still one more buggy thing that is preventing this from working for TimK. The AAC module assumes that a div is generated with the node id like...

<div id="node-3" class="node">

but on Tim's application he is only getting

<div class="node">

Looking at Livetest I'm seeing the same identifiers as on my installs, so I'm not sure why Tim's site isn't working the same way.

Lyle's picture
Offline
AdministratoreLiTe!
Joined: 08/07/2007
Juice: 6846
Re: Re: Re: Re: Re: Re: Version 1.2
Assigned to:cYu» Lyle

It's got to be the node.tpl.php file for his theme. I don't know why any theme wouldn't use a standard identifier for nodes, though.

cYu
cYu's picture
Offline
Bug FinderGetting busy with the Ubercode.
Joined: 11/19/2007
Juice: 850
Re: Re: Re: Re: Re: Re: Re: Version 1.2
Assigned to:Lyle» cYu

Tim messaged me to let me know he was using the bluemarine theme. I enabled it on my copy, and sure enough I got the same result with no id on the node div.

I changed the first line of node.tpl.php in the themes/bluemarine folder of my drupal installation from

  <div class="node<?php if ($sticky) { print " sticky"; } ?><?php if (!$status) { print " node-unpublished"; } ?>">

to

  <div id="node-<?php print $node->nid; ?>" class="node<?php if ($sticky) { print " sticky"; } ?><?php if (!$status) { print " node-unpublished"; } ?>">

and then the most recent version of AAC worked fine for me.

TimK's picture
Offline
Joined: 08/18/2007
Juice: 147
Re: Re: Re: Re: Re: Re: Re: Re: Version 1.2
Assigned to:cYu» TimK

cYu, I just made the change you suggested to the Blue Marine theme, and your module works like a charm. Thanks for all your help.

cYu
cYu's picture
Offline
Bug FinderGetting busy with the Ubercode.
Joined: 11/19/2007
Juice: 850
Re: Re: Re: Re: Re: Re: Re: Re: Re: Version 1.2
Assigned to:TimK» cYu

Cool, cool. Hacking a default template isn't a very good solution but I don't see a very good alternative at the moment. The problem is that I need a unique identifier for pages displaying multiple nodes each having their own attribute set. Could Ubercart perhaps wrap these nodes so that this code isn't theme specific?

Ryan's picture
Offline
Joined: 08/07/2007
Juice: 15438
Re: Re: Re: Re: Re: Re: Re: Re: Re: Re: Version 1.2
Assigned to:cYu» Ryan

You might consider posting your fix for the theme to Drupal.org, honestly. It seems like an oversight on bluemarine's part, esp. if it's something standard in other themes.

cYu
cYu's picture
Offline
Bug FinderGetting busy with the Ubercode.
Joined: 11/19/2007
Juice: 850
Re: Re: Re: Re: Re: Re: Re: Re: Re: Re: Re: Version 1.2
Assigned to:Ryan» cYu

Yeah, I might look around for the appropriate place on Drupal.org to post something. The default phptemplate node.tpl.php includes a node id as does garland, but bluemarine overrides node.tpl.php and does not set an id in the containing div.

I still don't feel comfortable depending on that id being there and being formatted in a specific way. I haven't seen anything to suggest there are defined standards for something like this. I'm not going to put much more thought into it, but if I did it over again I probably would try a different approach.