attributes + price factor + special price = incorrect totals

[mc12345678]

Did a quick search for the usage of the zen_get_discount_calc function as far as it applies to the shopping_cart it is only used to affect the price of the base attribute value. Elsewhere in the code, other calculations typically are performed before feeding it into the function and it seems that it is those calculations or how the results are used that are causing the visualized issues as seen in the attributes_controller and attributes.

[lat9]

Isn't part of the issue here that the use of the "price factor" in this case is (kind of) confusing the processing?

From "the book":

The Price Factor and Offset pricing mechanism calculates the attribute price based on a percentage of the product price.

But the product's price is based on its attributes, so it seems like the final price is kind of chasing its tail. Why not simply define each of those attributes' prices as 65.00?

While the solution provided by mc12345678 generates the expected result, should those results be expected (given the above)?

[mc12345678]

Isn't part of the issue here that the use of the "price factor" in this case is (kind of) confusing the processing?

From "the book":

The Price Factor and Offset pricing mechanism calculates the attribute price based on a percentage of the product price.

But the product's price is based on its attributes, so it seems like the final price is kind of chasing its tail. Why not simply define each of those attributes' prices as 65.00?

While the solution provided by mc12345678 generates the expected result, should those results be expected (given the above)?

What does "the book" say about pricing a product by its attributes? It ought to say something regarding the setup as setting the product's price to zero and then providing an amount for the product attribute's price... That would then identify the product's price which falls in line directly with the suggested setup. Then taking things a step further a price factor/offset could be used as is currently attempted such that (in this case) the attribute is adding 500% to the price of the product to be calculated as the final price of 90 (before the special). So yes, depending on the desired aspect of display there could be a chase scenario, but everything is pretty independently calculated so that there is no loop, just actions not yet factored into the base code pretty much whenever zen_get_attributes_price_final didn't return the same price as zen_get_discount_calc.

Found this to work for the above conditions(s) discussed where a special only is applied to a product that is priced by attributes, has a price factor, and the special price is less than or equal to the product price... A salemaker sale still offers different options/visualization at least on the product page but may be okay in the shopping cart and because of the separate ways that specials, sales, and attributes are calculated, without joining those things more together (datawise) there appears to continue to be a need to modify the associated functionality to gain more control.

So in includes/modules/YOUR_TEMPLATE/attributes.php

Find (in v1.5.5 starting at line 62):

              $discount_type = zen_get_products_sale_discount_type((int)$_GET['products_id']);
              $discount_amount = zen_get_discount_calc((int)$_GET['products_id']);

              $zv_display_select_option = 0;

And add the following line that is in color:

              $discount_type = zen_get_products_sale_discount_type((int)$_GET['products_id']);
              $discount_amount = zen_get_discount_calc((int)$_GET['products_id']);
              $products_price_is_priced_by_attributes = zen_get_products_price_is_priced_by_attributes((int)$_GET['products_id']);

              $zv_display_select_option = 0;

Then find (in v1.5.5 was line 114 before adding the above or any other changes):

                  if (((CUSTOMERS_APPROVAL == '2' and $_SESSION['customer_id'] == '') or (STORE_STATUS == '1')) or ((CUSTOMERS_APPROVAL_AUTHORIZATION == '1' or CUSTOMERS_APPROVAL_AUTHORIZATION == '2') and $_SESSION['customers_authorization'] == '') or (CUSTOMERS_APPROVAL == '2' and $_SESSION['customers_authorization'] == '2') or (CUSTOMERS_APPROVAL_AUTHORIZATION == '2' and $_SESSION['customers_authorization'] != 0) ) {

                    $new_attributes_price = '';
                    $new_options_values_price = 0;
                    $products_options_display_price = '';
                    $price_onetime = '';
                  } else {
                    // collect price information if it exists
                    if ($products_options->fields['attributes_discounted'] == 1) {
                      // apply product discount to attributes if discount is on
                      //              $new_attributes_price = $products_options->fields['options_values_price'];
                      $new_attributes_price = zen_get_attributes_price_final($products_options->fields["products_attributes_id"], 1, '', 'false');
                      $new_attributes_price = zen_get_discount_calc((int)$_GET['products_id'], true, $new_attributes_price);
                    } else {
                      // discount is off do not apply
                      $new_attributes_price = $products_options->fields['options_values_price'];
                    }

And add the following colored text to support product page display of attributes that are priced by attributes (meaning with 0 price in the product's base price), have a special (not a sale) where the attribute's price is to be discounted, and in particular have a price factor and or a price factor offset that causes a difference of price calculation between two "standard" methods.

                  if (((CUSTOMERS_APPROVAL == '2' and $_SESSION['customer_id'] == '') or (STORE_STATUS == '1')) or ((CUSTOMERS_APPROVAL_AUTHORIZATION == '1' or CUSTOMERS_APPROVAL_AUTHORIZATION == '2') and $_SESSION['customers_authorization'] == '') or (CUSTOMERS_APPROVAL == '2' and $_SESSION['customers_authorization'] == '2') or (CUSTOMERS_APPROVAL_AUTHORIZATION == '2' and $_SESSION['customers_authorization'] != 0) ) {

                    $new_attributes_price = '';
                    $new_options_values_price = 0;
                    $products_options_display_price = '';
                    $price_onetime = '';
                  } else {
                    // collect price information if it exists
                    if ($products_options->fields['attributes_discounted'] == 1) {
                      // apply product discount to attributes if discount is on
                      //              $new_attributes_price = $products_options->fields['options_values_price'];
                      $new_attributes_price = zen_get_attributes_price_final($products_options->fields["products_attributes_id"], 1, '', 'false');
                      $new_attributes_price = zen_get_discount_calc((int)$_GET['products_id'], true, $new_attributes_price);
                      $new_attributes_final = zen_get_attributes_price_final($products_options->fields["products_attributes_id"], 1, $products_options, 'false');
                      if ($discount_type == 59 && $products_price_is_priced_by_attributes && $new_attributes_price < $new_attributes_final) {
                        // Known condition (59) Special applied, no Sale. Reason an issue: priced by attributes and attribute has price factor.
                        $new_attributes_price = $new_attributes_final - (zen_get_products_base_price((int)$_GET['products_id']) - zen_get_products_special_price((int)$_GET['products_id'], true));
                      }
                    } else {
                      // discount is off do not apply
                      $new_attributes_price = $products_options->fields['options_values_price'];
                    }

I've had this solution a couple of days, but have been trying to break it. Certainly if more information were passed into the function zen_get_discount_calc it seems that more could be done to prevent modification like this, but otherwise right now it seems like there are a few "outliers" that need to be uniquely addressed such as this one.

[lat9]

"The Book" discusses identifying that the product is priced by attributes (note that the product's base price doesn't have to be 0.00) and then to use the attributes' settings to identify what each attribute "contributes" to the price.

I've got a test install of ZC1.5.5e and I'll give your suggested changes a try.

For your solution to @balihr's issue, is there any way to provide the proper result without using that constant definition? That's what threw me for a loop initially because it looks more like a band-aid than a programmatic solution.

[lat9]

@mc12345678, which issue does your most recent update address?

I've applied the changes to a just-installed v1.5.5e and ran it by the SaleMaker issue that I reported here, but no joy. Each of the attribute's prices still show $50.00 instead of $25.00 on the product_info page.

[mc12345678]

"The Book" discusses identifying that the product is priced by attributes (note that the product's base price doesn't have to be 0.00) and then to use the attributes' settings to identify what each attribute "contributes" to the price.

I've got a test install of ZC1.5.5e and I'll give your suggested changes a try.

For your solution to @balihr's issue, is there any way to provide the proper result without using that constant definition? That's what threw me for a loop initially because it looks more like a band-aid than a programmatic solution.

So someone, at one point at or before ZC 1.5.0 decided that the calculated price when using the price factor should *possibly* be based on the provided $special value or perhaps not depending on whether the constant is defined as '1' or not... The reason provided by the blame history was that the constant may already be defined before its use in the shopping_cart class function calculate...

Thing is, that currently the only place in which the constant is defined is during the use of the calculate function of the shopping_cart class.. So, perhaps in say the checkout_payment area, if the shopping_cart class were called and either the calculate function was used or any of the show_total related functions, and then the order class was created, the solution wouldn't involve editing the shopping_cart class to add the constant test in the attributes_price function. This would instead modify the order of operations in order to ensure that an obscure constant was set in advance/outside the need of it really being set and almost "by accident". This would be a real PATCH/BAND-AID as compared to the constant being addressed in the code where it might actually apply. Further such a sequencing/resequencing would not yet assign the constant as might be needed for when other code tried to use the attributes_price function of the shopping_cart class.

Another solution relates back to the function that uses that constant (similar on both store and admin side) in any number of the below suggested ways:
the function zen_get_attributes_price_factor to always use $special if it is non-zero:
seems that there was a historical reason to have it pre-determined to need to be used as such. The other "parts" I was looking at in my previous (accidentally duplicated) post was a way to add a parameter to the function, which when defined/set would override the test within the function and blanketly apply the $special when the $special was non-zero (ie. remove dependency on the constant being defined/set).

Current function starts like:

// return attributes_price_factor 
   function zen_get_attributes_price_factor($price, $special, $factor, $offset) { 
     if (ATTRIBUTES_PRICE_FACTOR_FROM_SPECIAL =='1' and $special) {        // calculate from specials_new_products_price 
       $calculated_price = $special * ($factor - $offset); 
     } else {

No use of the constant:

// return attributes_price_factor 
   function zen_get_attributes_price_factor($price, $special, $factor, $offset) { 
     if (/* ATTRIBUTES_PRICE_FACTOR_FROM_SPECIAL =='1' and */$special) {        // calculate from specials_new_products_price 
       $calculated_price = $special * ($factor - $offset); 
     } else {

Keep the constant but add a parameter that must be set to true in order to calculate the price factored price based off of the provided $special price and the $special price not evaluating to false. This solution requires additional coding to provide/identify where this function should use the price factor/factor offset against the special price (if it exists) or not which likely would require a field off of the product's information page more than anything, or perhaps on the special's page, though that also seems a little buried:

// return attributes_price_factor 
   function zen_get_attributes_price_factor($price, $special, $factor, $offset, $attributes_price_factor_from_special = false) { 
     if ((ATTRIBUTES_PRICE_FACTOR_FROM_SPECIAL =='1' || $attributes_price_factor_from_special == true) and $special) {        // calculate from specials_new_products_price 
       $calculated_price = $special * ($factor - $offset); 
     } else {

Similar, but automatically considers all provided values to use the $special value until told otherwise, which is somewhat like what is happening in the calculate function of the shopping_cart, that until someone adds some software, codes a constant, etc, then the attributes' price_factor will be calculated from the special if there is one, if not then use the $price value.

// return attributes_price_factor 
   function zen_get_attributes_price_factor($price, $special, $factor, $offset, $attributes_price_factor_from_special = true) { 
     if ((ATTRIBUTES_PRICE_FACTOR_FROM_SPECIAL =='1' || $attributes_price_factor_from_special == true) and $special) {        // calculate from specials_new_products_price 
       $calculated_price = $special * ($factor - $offset); 
     } else {

Regarding the priced-by-attributes base price being zero or not, besides a number of postings indicating that other price "adjustments" don't fair well if a price other than zero is entered, found that the shopping_cart class doesn't do well/as might have been expected for this situation if a price is entered on the product page as well as in the attribute's value and a price factor applied... Somewhere in the process the attribute's price doesn't get factored like might be expected or at least differently than when the product's price is zero.

[mc12345678]

@mc12345678, which issue does your most recent update address?

I've applied the changes to a just-installed v1.5.5e and ran it by the SaleMaker issue that I reported here, but no joy. Each of the attribute's prices still show $50.00 instead of $25.00 on the product_info page.

Doesn't address sale_maker issues.

Addresses: product priced-by-attributes, attribute has a price factor and/or price factor offset, has a special applied.

A sale is a completely different animal. The comments in the code indicate that a product that is priced by attribute and the product has a percent sale applied should resolve the final price to use the sale percent off (assuming as pointed out today by torvista that the sale price is updated after the product's price has been entered/modified).

The test(s) added by my above solution evaluate to the two prices being the same, because it seems that the subfunctions never view the resulting calculations associated with the attribute as a "difference" and therefore such evaluation (which was outside the scope of the OP) requires a different solution applied elsewhere.

[lat9]

Yes, that attributes' handling continues to make my head spin!

I'll point out that there's a demo product (id=157) that has a non-zero base price, implying that the functionality was intended ... whether or not it works correctly.

[mc12345678]

I can't believe that some form of solution has been found. It seems like it was simple to correct and I can't believe that no one has done this before.. Okay, there may be some "tests" or improvements still to be made around the code to protect the user, but... Sum and total, this is the correction(s) that were needed to go from start to finish on processing a product that is or is not priced by attributes where the attributes can be associated with the special OR a sale (OR combination)... I need someone to confirm/disprove aspects of it, but it became all too clear...

Anyways, still need the CONSTANT to support transferring the individual attribute related prices over to the checkout_ related path so:

includes/classes/shopping_cart.php needs to include the below highlighted item which seems to "fix" the issue of at least attributes + price factor + special price coming out to be the incorrect total at checkout.

function attributes_price($products_id) {
     global $db, $currencies;
     
    $total_attributes_price = 0;
     $qty = $this->contents[$products_id]['qty'];
     
    if (isset($this->contents[$products_id]['attributes'])) {     
     if (!defined('ATTRIBUTES_PRICE_FACTOR_FROM_SPECIAL')) define('ATTRIBUTES_PRICE_FACTOR_FROM_SPECIAL', 1);  // mc12345678 Added to apply the attribute's price factor and price  factor offset against the provided special price if a special is present  otherwise against the provided price when returning the value from the  function zen_get_attributes_price_factor found in  includes/functions/functions_prices.php 
    reset($this->contents[$products_id]['attributes']);
     while (list($option, $value) = each($this->contents[$products_id]['attributes'])) {
       $attributes_price = 0;

Then working from the end of the file up (so line numbers hold true throughout) in includes/functions/functions_prices.php:
Find line 1083:

      $attributes_price_final += $pre_selected->fields["options_values_price"];

and replace with:

//      $attributes_price_final += $pre_selected->fields["options_values_price"];
      $attributes_price_final += zen_get_discount_calc($pre_selected->fields["products_id"], $pre_selected->fields["products_attributes_id"], $pre_selected->fields["options_values_price"] + (zen_get_products_price_is_priced_by_attributes($pre_selected->fields["products_id"]) ? zen_products_lookup($pre_selected->fields["products_id"], 'products_price') : 0));

Then find line 1081:

      $attributes_price_final -= $pre_selected->fields["options_values_price"];

and replace with:

//      $attributes_price_final -= $pre_selected->fields["options_values_price"];
      $attributes_price_final -= zen_get_discount_calc($pre_selected->fields["products_id"], $pre_selected->fields["products_attributes_id"], (zen_get_products_price_is_priced_by_attributes($pre_selected->fields["products_id"]) ? zen_products_lookup($pre_selected->fields["products_id"], 'products_price') : 0) + (zen_get_products_price_is_priced_by_attributes($pre_selected->fields["products_id"]) ? -1 : 1) * $pre_selected->fields["options_values_price"]);

Line 560:

            $calc = ($attributes_amount * $special_price_discount);

replace with:

            $calc = ($attributes_amount * $sale_price_discount);

Line 539:

            if ($special_price_discount != 0) {
              $calc = ($attributes_amount * $special_price_discount);

replace with:

            if ($sale_price_discount != 0) {
              $calc = ($attributes_amount * $sale_price_discount);

At line 501 add/insert the following:

    if ($new_products_price != 0) {
      $sale_price_discount = ($new_sale_price != 0 ? ($new_sale_price/$new_products_price) : 1);
    } else {
      $sale_price_discount = '';
    }

And in includes/modules/YOUR_TEMPLATE/attributes.php (or rather the base install), comment out line 126:

//                      $new_attributes_price = zen_get_discount_calc((int)$_GET['products_id'], true, $new_attributes_price);

And then are some similar changes that can be made to the admin side. I was working on changing the information presented in the admin attributes_controller such that the discount calculated would be displayed, but I've set that aside for the moment...

Committed these changes to a ZC PR

[mc12345678]

Seems that the following ZC commits/pulls might correct both of the issues described here. The commits/pulls are based on changes made from ZC 1.5.5 to ZC 1.5.6 (and corrections made to the first pull to ZC 1.5.6):
https://github.com/mc12345678/zc-v1-...726016a5fe4b5a
https://github.com/zencart/zencart/pull/1404/files
https://github.com/zencart/zencart/pull/2075/files