Stock by Attribute v4.0 addon for v1.3.5-1.3.9

[mc12345678]

In review to answer your question I noticed the same.

When I went back into my local git repository I saw that I had that prepped but hadn't loaded it.

The repository is updated; however, I still want to indicate that as of right now with the revision forthcoming to Zen Cart 1.5.7 (1.5.7c) and/or the use of PHP 8.x that this plugin will not operate correctly until its use of zen_not_null is modified. In some cases more evaluation is needed in a code segment, in other cases a completely different evaluation should be used such as !empty, isset or similar.

I am going through the code where zen_not_null has been used and making modifications, with all to be pushed as a single commit instead of by piecemeal. This is a result of my incorrect usage of that function and that the function is being further modified to provide the result(s) that had been expected for its use. In the discussion had, it was not intended to be a replacement for functions such as is_null, empty, isset, === false, etc... It has a somewhat more narrow use, primarily appearing to be for the evaluation of query data.

Will identify when that update is complete.

Among other general code updates for either internal code readability and alignment, I've updated the code as associated to the use of the Zen Cart function zen_not_null. As discussed here and a few other places in the forum as well as on github for Zen Cart, 1) I incorrectly incorporated the use of that function to (ab)use its results in a way that worked for the code. I have updated the code to generally only use the Zen Cart function in cases where text type results are expected ($_GET/$_POST if used or processing fields from a database query before the field has been "sanitized"), to evaluate query results or in some cases where an SBA function provides mixed results (though the results are: null, false, an array or a query result). The function continues to identify the following as "null": false, empty array, null, effectively no results from a query, a text result that is not empty spaces/no content, or if the value is the all capital text word 'NULL'.

In the case of an evaluated value has reasonable expectation to be set as the string word 'NULL', I believe I have incorporated appropriate checks to support/allow that value (thinking the customid for example). If I have missed such a "check", please let me know what field was desired to be set as 'NULL', at what location it was attempted to be added and during what operation that seemed to "disappear".

I also finally discovered where/how when a cart was expected to indicate that it contained a product that was out-of-stock had the potential to continue through checkout. Basically found by accident, but upon further review of the problem code area, it's "obvious"...

A while ago, I had modified the approach to indicating that a cart had out-of-stock product. Having seen other plugins introduce their own "text" or result, I didn't want SBA to clobber that information, so I tried to go a sort of reverse route by *trying* (and failing) to only make the SBA product appear out-of-stock if it was not already marked as such. What I had instead was to unmark the cart as containing out-of-stock product if all product were in stock, but was missing a forceful action of: if the cart was not marked to have out-of-stock product and it should identify that it has out-of-stock product, then tag the cart as having out-of-stock product.

For the zen_not_null revisions (supporting the currently "unreleased" Zen Cart 1.5.7c and beyond), please see commit: https://github.com/mc12345678/Stock_...3c8d7112dbc6b2

For the out-of-stock correction, please see commit: https://github.com/mc12345678/Stock_...4fccbfa5b963ca

To take advantage of several code changes and improvements on the internal operation of the plugin, please consider installing the latest files as found at:
https://github.com/mc12345678/Stock_...butes_Combined

There still is a little more work to be done to fully support PHP 8.0 as I had incorporated a code operation that is being removed in that version. Effectively I was looking back at code operation and information passed to collect data instead of reconstructing information or internally collecting the information by bypassing the code inspection and just passing the needed data along. This code problem is in the catalog side observer class for evaluating the quantity of product available. A solution is in mind; however, has not been fully implemented/tested yet.

I also have received some feedback to reduce code modification in at least one of the files touched by the module, specifically either by incorporating the Zen Cart 1.5.7c admin/attributes_controller.php file into a previous Zen Cart 1.5.7 version or in use of 1.5.7c specifically, admin/attributes_controller.php will not need any touching. This would reduce the number of admin touched files from 4 to 3 with overrides used in the catalog side to support display/use of the customid (if not using that feature, then currently the only files really touched are the templates/tpl_modules_attributes.php file to support cascade attribute selection (really could be done with javascript/jQuery but hasn't been implemented) and templates/tpl_modules_main_product_image.php to support image swap which also probably could be performed without that modification).

[mc12345678]

There still is a little more work to be done to fully support PHP 8.0 as I had incorporated a code operation that is being removed in that version. Effectively I was looking back at code operation and information passed to collect data instead of reconstructing information or internally collecting the information by bypassing the code inspection and just passing the needed data along. This code problem is in the catalog side observer class for evaluating the quantity of product available. A solution is in mind; however, has not been fully implemented/tested yet.

I also have received some feedback to reduce code modification in at least one of the files touched by the module, specifically either by incorporating the Zen Cart 1.5.7c admin/attributes_controller.php file into a previous Zen Cart 1.5.7 version or in use of 1.5.7c specifically, admin/attributes_controller.php will not need any touching. This would reduce the number of admin touched files from 4 to 3 with overrides used in the catalog side to support display/use of the customid (if not using that feature, then currently the only files really touched are the templates/tpl_modules_attributes.php file to support cascade attribute selection (really could be done with javascript/jQuery but hasn't been implemented) and templates/tpl_modules_main_product_image.php to support image swap which also probably could be performed without that modification).

Come to find out that the issue is/was not so directly PHP 8.0 nor that the changes were going to prevent the use of the code as written, it was that there has been a change to generally prevent the use of the feature taken advantage of unless something additional was implemented.

Basically, PHP 7.4 and PHP 8.0+ were both to be impacted without some additional code. I have incorporated some additional operations to permit the continued use of the code as is until either a refactor to centralize some code or to incorporate additional operations to sort it all out. Anyways, I pushed the change to the master branch, but the specific change that supports continued use in default installations of PHP 7.4+ is:
Address default php.ini setting for php 7.4+ · mc12345678/Stock_By_Attributes_Combined@9884e3d (github.com)

As discussed in: Alternative method needed for PHP 8 as will remove argument evaluation/modification · Issue #95 · mc12345678/Stock_By_Attributes_Combined (github.com)

[DrByte]

Come to find out that the issue is/was not so directly PHP 8.0 nor that the changes were going to prevent the use of the code as written, it was that there has been a change to generally prevent the use of the feature taken advantage of unless something additional was implemented.

Basically, PHP 7.4 and PHP 8.0+ were both to be impacted without some additional code. I have incorporated some additional operations to permit the continued use of the code as is until either a refactor to centralize some code or to incorporate additional operations to sort it all out. Anyways, I pushed the change to the master branch, but the specific change that supports continued use in default installations of PHP 7.4+ is:
Address default php.ini setting for php 7.4+ · mc12345678/Stock_By_Attributes_Combined@9884e3d (github.com)

As discussed in: Alternative method needed for PHP 8 as will remove argument evaluation/modification · Issue #95 · mc12345678/Stock_By_Attributes_Combined (github.com)

Seems like there's probably a better way to detect arguments, perhaps by passing more information via the core code's notifier call?

[mc12345678]

Seems like there's probably a better way to detect arguments, perhaps by passing more information via the core code's notifier call?

The core's notifier as far as I have been able to tell doesn't pick up any extra parameters sent to the function that activated the notifier...

For example:

  function zen_get_products_stock($products_id) {
    global $db;
    
    // -----
    // Give an observer the chance to modify this function's return value.
    //
    $products_quantity = 0;
    $quantity_handled = false;
    $GLOBALS['zco_notifier']->notify(
        'ZEN_GET_PRODUCTS_STOCK',
        $products_id,
        $products_quantity,
        $quantity_handled
    );
    if ($quantity_handled) {
        return $products_quantity;
    }

If it is called as such:
$returned_value = zen_get_products_stock($products_id, $attributes);

Then there is nothing it seems within the notifier that is able to pull back the second parameter that was sent to the function..

The notifier provides the class (which is the base class at this point), the notifier designation itself, the read only most likely already hashed $products_id; however, that hashed value could have been formed by any number of things and ways and there really are only a few things needed by the plugin to support evaluation.

Of course, and as I indicated in the issue, if I merge information from the base SBA class with the observer class or provide some sort of interface, then it seems that the only time that the additional data is necessary is from the extra_cart_actions and if/when called from within the base SBA class. This is because there are no other edits to core code, there doesn't appear to be any other times at which that additional information is passed though it could be of use elsewhere.

The other perspective I had (a long time ago anyways) was to take the known/present attributes from either the cart or the orders, run that through the "typical" hash routine and then if/where those matched the provided $products_id, I would have a match of sorts, even if what I had to do was to take that known information and present it in different sequences so that I could get the information to hash with equivalence... At any rate, when working towards trying to remove/eliminate other modifications, this was what fell out of that development to be able to consider further potential use regardless of a call from cart or order...

If there is something in the above that I'm missing, I'm all ears. Currently, the consideration is to be able to determine, using an observer of 'ZEN_GET_PRODUCTS_STOCK', what additional parameters were sent to it and what their value(s) are and repeat the same type evaluation from 'ZEN_CHECK_STOCK_MESSAGE'.

[DrByte]

I'm assuming that SBA is passing the extra $attributes parameter? But if the function doesn't declare that parameter in its function signature, the function won't do anything with it (hence I guess why you're trying to do backflips to read it?).
And newer PHP versions will soon begin actively rejecting the function call if it doesn't match the signature.

Perhaps there's an opportunity to improve core code.

[mc12345678]

I'm assuming that SBA is passing the extra $attributes parameter? But if the function doesn't declare that parameter in its function signature, the function won't do anything with it (hence I guess why you're trying to do backflips to read it?).
And newer PHP versions will soon begin actively rejecting the function call if it doesn't match the signature.

Perhaps there's an opportunity to improve core code.

By no means shying away from the opportunity to improve core code, I haven't found anything that suggests that the following will become a problem. Perhaps I don't have all of the tea leaves though:

function thisFunction($param1) {
    // Evaluate parameters provided, if there are extra parameters, then do something with them. 
    //    This evaluation and additional action could be before, after or during the normal action on $param1.
    // Do something with $param1
}

$data = 'Data';
$extra_parameter = 'use also';
thisFunction($data, $extra_parameter);

Where even if some sort of data type is forced for $param1 that if the original/expected data type is provided then it seems from my review of php requirements that the call to the function will still be accepted. What I have seen is issues that could occur based on some of the php 8 chosen directions and more specifically on the use or potential use of optional values within the original function (if not accounted for properly downstream), Variable Arguments or possibly use of Named Arguments. My thought though in this particular code section is that the use/operation is so "limited" that changes made to the parameters of the functions would be relatively easy to see/address with the code being changed/updated as necessary to interface properly with the core code.

That all said, the follow on question to address potential core code is where such modification may be best considered... The function as a class method, additional data collected/passed in the function call, the observer class making more information available about the code surrounding the notifier (parameters of the call to the outer function?)?

In part it seems that a lot of the data possibly to be sent could be captured within the associated orders/cart classes. My issue is/was that I wanted to have the exact information that is being attempted to be used not what was at some point determined. In other words potentially the current data from the associated object, or in the case of wanting to "test" the ability to take an action the value(s) being tested... At the same time I wanted to try to use as much of the default/core code as possible rather than generating alternative code. This would appear to make it easier to make changes that apply across the board rather than having to possibly make the same change in multiple places. Further, where might be a good place to further capture this discussion? I'm aware that changes made will likely impact many.

As for use of the hashed attribute information, pretty much all is well and good when looking at possibly generating/regenerating the possible hash value from the known attribute(s) assigned to a product except when text/a file is involved... The hash is specifically modified by the content of the associated text so it is not feasible to "guess" the hash from the possible attributes when either of those two attribute types are present. The only thing that could be done at that point is to capture/borrow the associated text from the applicable order/cart object and relate it to the associated file/text so that the potential hash values could be generated by sequencing/resequencing them in a mash up to see if the provided/identified hash is feasible within the variants assigned within SBA...

Anyways, although I saw this a good number of hours ago, I wanted to at least provide my feedback on what looks like a "scare" to upcoming coding standards... I'm thinking in my limited view that in relation to the function signature that there may have been a little extra concern than what currently is described as proposed...


I got a lot of the above from: https://www.php.net/manual/en/functions.arguments.php

[mc12345678]

Another direction that I somewhat had forgotten about for this module was to in essence "stuff" the $products_id when used specifically by the module to include whatever data is needed. Obviously then it could be managed by the module and information needed be sent back as "processed". It appears that in the cases where core code makes calls that might be of interest that the data structure appears to be in the use of the orders class which appears to also be in the global space and also consistently uses $i as the increment variable... A lot of "just right" constraints that "happen" to work out...

Course it involves recoding of some aspects but I think that it would eliminate this additional "look back" process that was thrown together to span both types of calls with potential to work across all uses. I just need to find the infrequently used function that torvista added to the attributes controller I think it was where an array is created with elements that have the same name as the variable being used... Like said though, open to making core mods if there is some concept to implement. If it means anything I've been trying for too long to find alternative attribute data models but the idea of a good one seems to be a coveted or complex concept.

[DrByte]

Complex indeed.
And ultimately shouldn't be only applicable to attributes.

[cefyn]

I have the latest SWA from Github on 1.5.7b, and am having a problem with search. Entering the product model no. into the search box brings the product into the search list box, but doesn't display it on the page. Clicking search on the product, which is now in the search list box, brings it up on the page for editing, but gives this error.

PHP Fatal error: Uncaught Error: Class 'language' not found in /admin/includes/classes/products_with_attributes_stock.php:267

function displayFilteredRows($SearchBoxOnly = null, $NumberRecordsShown = null, $ReturnedProductID = null) {
global $db, $sniffer, $languages;
if (empty($languages)) {
267 $languages = new language();
}
Can anyone help with this?

[mc12345678]

I have the latest SWA from Github on 1.5.7b, and am having a problem with search. Entering the product model no. into the search box brings the product into the search list box, but doesn't display it on the page. Clicking search on the product, which is now in the search list box, brings it up on the page for editing, but gives this error.

PHP Fatal error: Uncaught Error: Class 'language' not found in /admin/includes/classes/products_with_attributes_stock.php:267

function displayFilteredRows($SearchBoxOnly = null, $NumberRecordsShown = null, $ReturnedProductID = null) {
global $db, $sniffer, $languages;
if (empty($languages)) {
267 $languages = new language();
}
Can anyone help with this?

Didn't have this issue at times in the past with some version of ZC 1.5.7, but I agree that there is the above issue. Pushing the commit change in a moment. Don't like the solution or perhaps it is the fact that such a solution has to be generated, but it does take in consideration of the history of Zen Cart unfortunately...