potteryhouse / stock_by_attribute_1.5.3

Stock by Attribute for Zen Cart 1.5.1
GNU General Public License v2.0
1 stars 0 forks source link

Test/Verify: remove all variants of a product when product is deleted from database and other code cleanup. #13

Closed mc12345678 closed 9 years ago

mc12345678 commented 9 years ago

The code of this branch adds SBA table cleanup for products that are deleted through the standard ZC product deletion code. This will remove all variants that have been populated in the products_with_attributes_stock table for the product(s) being deleted.

Additional code touchups were included to standardize some of the code to ZC available "tools" (use of the TABLE_XXXX constant instead of $DB_PREFIX . "XXXX ...") and some formatting modifications intentional and a few unintentional for code readability. Have also begun in some cases using the standards proposed in the ZC wiki which include the use of two spaces instead of tabs for code arrangement and readability.

potteryhouse commented 9 years ago

Thanks for all your work on this, I will try to look it over, test locally, and merge it this weekend. I have been out of town on vacation and just got back.

potteryhouse commented 9 years ago

I did some testing, installed on a new / clean Zen Cart 1.5.1 . The new delete seems to work properly, i.e., now when a product is deleted the associated SBA attributes are also removed.

I have a couple questions / comments.

  1. How does this file work, includes\modules\pages\checkout_success\header_php_sba.php?
  2. I need to do some additional testing to verify the change in files "invoice, orders, and packingslip",
  3. I made a change to the admin\products_with_attributes_stock.php (HTTPs_CATALOG_SERVER to HTTP_CATALOG_SERVER)
  4. Updated the ChangeLog.txt file.
mc12345678 commented 9 years ago
  1. Should behave the same as if the use of DISTINCT were added into the standard header_php.php file within that directory. The difference is that the original file is not modified, but in a way overridden. I was trying to find a way to implement the DISTINCT aspect without making a change to the "core" file and instead providing something additional to work. It may require a little upkeep to ensure that it is compatible across versions of ZC, but ideally in the future this information would either be incorporated directly or could be incorporated on the side through some other convenient process.
  2. Didn't search for other locations that may "delete" a product, only applied changes to the specific ZC code that is supposed to handle that aspect of the issue. Changes made for order deletion are a part of a different branch, but also should be looked at.
  3. Use of HTTP_CATALOG_SERVER may also cause processing delays/mixed content to be served on a page. Generally speaking may not even be necessary to be included to begin with...
  4. Yeah!!! :) If there are additional questions related that I may address, please let me know/post, etc...
potteryhouse commented 9 years ago

Since the files called by the page reside in the root of the store (includes/template/...) I needed to find a clean way to get to them, thus I used HTTP_CATALOG_SERVER . If a user has there admin folder in another subfolder (as I have seen on some installations, simply using '../' will not work. Is there a default define in Zen Cart that will bring you to the root/include folder that should be used instead? (I got this one HTTP_CATALOG_SERVER from the admin configure.php file, thought it would be correct since they use it by default to do the same thing).

mc12345678 commented 9 years ago

zen_href_link is the ZC default function to pull content from the store. There are other assistive functions to pull specific template related information depending on what it is that is being used/looked for. Language files, template files, etc... Also there is a global variable that is typically set to indicate the loaded page's type (https/http) that is called $request_type. That said, there are still a few options on how to load a page so that it will load "correctly". If the page is hosted locally, then it need only the path from the host, if as said from the store's front, then /path/file.extension will go from the uri of the host, to the said path, but that path should ensure to include any store subdirectory like if hosted in the subfolder shop. The next thing is that if the file is hosted elsewhere (or locally) //hostname/folderpath/filename.extension will apply the applicable current https/http without further ado generally speaking. There are some exceptions to that like if the item is to be retrieved using ftp or file to name a few. This last aspect is characteristic of browsers complying with the applicable RFC guidelines for uris. But, if one falls back to the zen_href_link function and builds it properly, then for items hosted on the site in question will be served properly...

mc12345678 commented 9 years ago

Okay, I had to do some href creation myself from the admin side and discovered that there are two types of href creation functions: 1) zen_href_link will build a link based off of the admin folder path. 2) zen_catalog_href_link which will build a link based off the store folder path. By default it expects a $page variable that is used to fill the right side of "index.php?main_page=" so trying to include/incorporate say a javascript file from that reference may not quite work right. Wonder if the ZC function should be modified to work differently or if there remains a current need (security?) for it to be coded the way it is.

Come to think of it though, and I also realize it is twice the storage space and twice the "duty", but why not include the necessary file(s) in both the store and admin side or some variation of that where the admin "file" actually does the loading of the store side file(s). I realize that there is also a plan for ZC to try to consolidate files from the admin side into the store side and in a way trim down on some of the uniqueness of the admin files, so that may be a reason to design this with more of a future state in mind. Just asking though. :)

potteryhouse commented 9 years ago

I ran some tests, I think we can merge this to the master.