Salemaker Quirk/Bug

[lruskauff]

I found this by doing something accidentally which caused me quite a bit of hunting to fix my problem. I had my entire store on sale (ahhhh) but when I looked at my actual sale in salemaker, it only showed one category selected. I eventually fixed it, but I wanted to know why it happened so I don't do it again and also so if anyone else did it, they could fix it. So it is kind of a bug/quirk but there is a work around.

Zencart 1.5.4
Modules installed: a bunch
Tested on my cart, then with the classic template and had same results. So I installed a vanilla zencart, no extra modules, and could also reproduce it on this.

I created a sale in sale-maker as follows:
2$ off , deduct amount
today start date
no end date ( but also tried with end date).
I don't think the above matters but I didn't want to leave anything out because I did all my tests with the same data.

I selected one category but also hit the "entire catalog" button (I didn't know I hit it the first time).
When I checked my sale, I found everything on sale. (I didn't expect this)
Went back to see what I selected and only found only one category selected in the sale. The "entire catalog" button was not highlighted.

Problem: The "entire catalog" button should also be highlighted, then I could turn it off.

The fix: Edit the sale, click the "entire catalog" button twice (once to turn on, once to turn off) and save the sale. It now turned off the sale for the entire catalog and turned on the sale for just that one category.

I know people won't/shouldn't do this on purpose (click the "entire catalog" AND select one or more categories) but there is a good chance someone might do it like I did. And when you only see one category button selected and your entire store is on sale, you think you are going crazy.

.......hope this makes sense.

Lin

PS: I searched for this topic and didn't find it; if someone points me to a faq on it....

[frank18]

...
The fix: Edit the sale, click the "entire catalog" button twice (once to turn on, once to turn off) and save the sale. It now turned off the sale for the entire catalog and turned on the sale for just that one category....

That (pressing the button twice) should not be necessary, so if this can be replicated in different ZC versions by others then the code needs looking at.

Just my 5 cents

[mc12345678]

Would agree from review of the code that there is no check performed upon submission to prevent both selecting a category and checking the entire cart checkbox. There is code that indicates if a category had been previously selected to unselect the entire catalog button, and therefore upon saving the record a second time with the checkbox deselected, the database should now reflect the selected category(ies). Changes made from the 2011 version of the file (found in ZC 1.5.3) for ZC 1.5.4 would have no effect on the operation and therefore the issue is applicable to older versions of ZC as well.

[mc12345678]

Suggest the following (untested):

Find:

// Create a string of all affected (sub-)categories
if (zen_not_null($_POST['categories'])) {
$categories_selected = array();

and insert the following code in blue.

// Create a string of all affected (sub-)categories
if (zen_not_null($_POST['categories'])) {
// if both the all categories and a number of categories is selected, then remove the selection/option for all categories to keep just the
// categories selected... mc12345678 2015-07-17
if (sizeof($_POST['categories']) > 1 && in_array("0", $_POST['categories'], true)) {
unset($_POST['categories'][array_search("0", $_POST['categories'], true)]); 
// Code needed: Display some sort of error message regarding having chosen both the all catalog and a series of categories... 
}
$categories_selected = array();

[lruskauff]

Suggest the following (untested):

// Create a string of all affected (sub-)categories
if (zen_not_null($_POST['categories'])) {
// if both the all categories and a number of categories is selected, then remove the selection/option for all categories to keep just the
// categories selected... mc12345678 2015-07-17
if (sizeof($_POST['categories']) > 1 && in_array("0", $_POST['categories'], true)) {
unset($_POST['categories'][array_search("0", $_POST['categories'], true)]); 
// Code needed: Display some sort of error message regarding having chosen both the all catalog and a series of categories... 
}
$categories_selected = array();

While that would work (in theory, didn't try either) it might drive me crazy later without the error message. I could see myself a year from now with the opposite scenerio trying to set the all categories button and not knowing I had a category selected somewhere way down off the page wondering why it kept unhiliting my "all" button. And of course by then I would have forgotten all about this. Thank you for checking this out though.

[lruskauff]

What if we just had it set the "all" button if it was saved that way?

[mc12345678]

What if we just had it set the "all" button if it was saved that way?

Well think about it this way.. First the instructions weren't followed (Pick the all button OR choose categories) then, "accidentally" the entire store is marked as everything to be on sale... Now, spend possibly the 15 minutes of the timeout picking and choosing specific categories to be on sale... Now, hit the save button and all of that individual category choosing is lost... Ohh, and now the absolute entire store is on sale... Not just the few items/categories that were chosen.. Customer navigating the site picks a sale item and as it was the price was barely above cost before the sale.. They buy it, now at a loss to you... Why? Because a button was accidentally selected AND categories were selected at the same time..

Yes a message is necessary, at the time I wrote the above code it was to cause the functionality that seemed most conservative to not cause the problem I just described... Hate to say, too bad for aggravating a store owner, but the whole purpose is to make money not lose it... I didn't have the tools available to look through and figure out what message "method" would be best and to include a language define that doesn't yet exist (other error messages are presented from a defined constant). I've even been thinking about other ways to address the issue without using the search method(s), but instead if statements within the applicable loop to ensure the life of the code without multiple modifications.. You know keep it simple type design. :)

[lruskauff]

I meant keep everything set, both the individual ones chosen AND the "all". That way when the said idiot comes back to see what he/she did it is obvious. And I only meant it for a quick fix for now, which I guess I should quit thinking that way. The real fix would be not to allow me to do this stuff in the first place.

I'm just to the point where I am starting to "read" the code and understand it. But it takes me a bit of time to figure out how to do certain changes. Mostly I'm learning by example, so since there is not a message piece of code in that file, first I'd have to go find a file with it and then make sure I understood it, and then try to copy it for my example.

But I wasn't asking you to write me a message, only saying that since it would take me a bit of time to learn how and since this store is my store and not a customers, I should prob be spending my time on money making stuff :) Like content. And if anyone would like to start a "content" thread to offer more concrete examples, I wouldn't object.

[mc12345678]

I meant keep everything set, both the individual ones chosen AND the "all". That way when the said idiot comes back to see what he/she did it is obvious. And I only meant it for a quick fix for now, which I guess I should quit thinking that way. The real fix would be not to allow me to do this stuff in the first place.

I'm just to the point where I am starting to "read" the code and understand it. But it takes me a bit of time to figure out how to do certain changes. Mostly I'm learning by example, so since there is not a message piece of code in that file, first I'd have to go find a file with it and then make sure I understood it, and then try to copy it for my example.

But I wasn't asking you to write me a message, only saying that since it would take me a bit of time to learn how and since this store is my store and not a customers, I should prob be spending my time on money making stuff :) Like content. And if anyone would like to start a "content" thread to offer more concrete examples, I wouldn't object.

So that "last" part is kind of what it's about... In the original/latest design for the file, there is instruction given about selections. It appears that in a way the intention was to not allow selecting both sets/options, but there isn't code locked in to prevent both from being an option. There might actually be a business reason that both individual categories and the entire store is selected...

If the ability to store the information is limited to only one or the other then there wouldn't be a case to show the case of the entire store selected and the individual categories except for where the database was manually manipulated. The current code has a sort of inadvertent fix for selecting both in that if a category has been individually selected (even if it is every category) then the entire store checkbox is deselected... I think there's a couple of different checks that could be made to see if both have been selected in the current data structure after the database got updated, but would have to run through a couple of cases to see.

The other thing, to keep the current possible functionality is to simply display a message after the "damage" has been done (ie. another check in the process to simply provide information that it has been done (though sometimes I really wonder if information does any good if the original information (instructions) were not followed. )

Well, there's a number of message providing methods... Some use the messagestack "alert" at the top of the screen, others well just provide information on the page itself.. Generally it's useful just to reuse/continue using the same style throughout. This way when it comes time to "upgrade" the code, it remains consistent as well. :)

So, kinda' hard to say what the path forward is with it and may actually be more of a problem to others to "fix" it.. :) but, for those that don't want the possibility of setting everything on "sale" and selected categories at the same time, then it seems that the above will prevent that... Otherwise follow the instructions.. :) It wouldn't surprise me if similar "fixes" have been proposed to only have a reason to not completely close the gap in this section of the code...

Anyways, I'd say good catch, also good to see that you're starting to catch on again with reading the code..