Jump to content
  • Checkout
  • Login
  • Get in touch

osCommerce

The e-commerce.

[Development] Products Specifications


kymation

Recommended Posts

Hello Jim,

 

You've done a great job on the manual. However, it is not easy for me to judge since played with this contribution for quite some time now. I do have the following suggestions:

 

3.4. Specification Groups

Only the first sentence applies to specification groups. However important to mention, the other text does not explain specification groups but how specifications and filters may apply.

 

3.6. Specification Values

I would move this paragraph after paragraph 3.3 since it gives more information on thye specifications. This to avoid confusing with paragraph 3.5 dealing with filters.

 

3.5. Filters

As well it might be good to explain why specification values don't have to match the filter data.

 

Appendix A states:

See Section 3 for more detailed information on the use of these settings, shouldn't this actually be section 4.1?

 

 

Anyway, I am happy to see this contribution is reaching its first RC. Not in the least I like to thank Elijah as well for the effort the past few days. I would do too if I had more PHP knowledge.

 

Regards,

Jerome

Link to comment
Share on other sites

Thanks for the bug reports. I'm having a lot of trouble writing this. I've been working on this for far too long, so everything is just obvious, right? But I know it's not all that obvious to the rest of you, so I have to try to write it all down. I'll keep working on it.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

What did you have in mind for the SPECIFICATION_FILTER_NO_RESULT? I see in catalog/includes/boxes/products_filter.php around line 127 something that hints at a work in progress for showing / not showing filters if no results are found. I was gonna start working on a means to count the items that each filter would find so that I could put that into the filter display, but didn't want to duplicate any work if you already had something (even if just a skeleton) to show for that? Let me know!

 

Elijah

Link to comment
Share on other sites

Worked up the start of a function to pull the counts at the filter level. It doesn't yet stack the filters, so the numbers don't change when you have 2 or more filters going. Just wanted to post this as a start of the counting code development if it doesn't exist yet, so this is for sample purposes only.

 

Insert at the end of catalog/includes/functions/products_specifications.php

 

  function tep_count_filters($categoryId, $specification, $specificationId, $filterClass, $filterDisplay, $productsColumnName, $languageId)
 {
  $raw_query = "select 
				count(p.products_id) as count
			  from 
				  (products p) 
				  join (products_to_categories p2c) on (p.products_id = p2c.products_id) 
			  where 
				  p.products_status = '1' and 
				  p2c.categories_id = '" . $categoryId . "' 
				  ";

$raw_query .= tep_get_filter_sql($filterClass, $specificationId, $specification, $productsColumnName, $languageId);

  $filter_count_query = tep_db_query($raw_query);

  $filter_count_results = tep_db_fetch_array($filter_count_query);

  $count = (int)$filter_count_results['count'];

  return $count;
 }

 

Then in catalog/includes/boxes/products_filter.php I modified starting at line 108

 

From this:

		  if (SPECIFICATION_FILTER_NO_RESULT != 'normal') {

	  }

	  $filters_select_array[$filter_index] = array ('id' => $filter_id,
													'text' => $filter_text
												   );
	  $filter_index++;
	} // while ($filters_array

	if ($specs_array['filter_class'] == 'range') {
	  if ($specs_array['products_column_name'] == 'products_price') {
		$previous_filter = $currencies->format ($previous_filter);
	  }
	  $filters_select_array[$filter_index] = array ('id' => $previous_filter_id,
													'text' => $previous_filter . '+'
												   );
	}

 

to this:

		  $count = tep_count_filters($current_category_id, $filter_id, $specs_array['specifications_id'], $specs_array['filter_class'], $specs_array['filter_display'], $specs_array['products_column_name'], $languages_id);

	  if (SPECIFICATION_FILTER_NO_RESULT != 'normal') 
	  {
			// This should have the greyed out code as well, but for now I'm only doing the "don't show the option" code.
		  if ($count == 0)
		  {
			  continue;
		  }
	  }

	  $filters_select_array[$filter_index] = array ('id' => $filter_id,
													'text' => $filter_text,
													'count' => $count
												   );
	  $filter_index++;
	} // while ($filters_array

	if ($specs_array['filter_class'] == 'range') 
	{
	  if ($specs_array['products_column_name'] == 'products_price') 
	  {
		$previous_filter = $currencies->format ($previous_filter);
	  }

	  $filters_select_array[$filter_index] = array ('id' => $previous_filter_id,
													'text' => $previous_filter . '+',
													'count' => $count
												   );
	}

 

And then for each of the tep_draw_*_menu() functions in catalog/includes/functions/products_specifications.php I added the count like this...

 

in the tep_draw_links_menu function I changed

 

	  $field .= '  <a href="' . tep_href_link ($target, tep_get_array_get_params (array ($name, 'page') ) . $name . '=' . tep_output_string ($link_data['id']) ) . '">' .  tep_output_string ($link_data['text']) . '</a><br>' . "\n";

 

to this.

 

	  $field .= '  <a href="' . tep_href_link ($target, tep_get_array_get_params (array ($name, 'page') ) . $name . '=' . tep_output_string ($link_data['id']) ) . '">' .  tep_output_string ($link_data['text']) . '</a>(' . $link_data['count'] . ')<br>' . "\n";

 

 

It's an alright place to start I think, I'll keep working on the counts for stacked filters.

 

Elijah

Link to comment
Share on other sites

What did you have in mind for the SPECIFICATION_FILTER_NO_RESULT? I see in catalog/includes/boxes/products_filter.php around line 127 something that hints at a work in progress for showing / not showing filters if no results are found. I was gonna start working on a means to count the items that each filter would find so that I could put that into the filter display, but didn't want to duplicate any work if you already had something (even if just a skeleton) to show for that? Let me know!

 

Elijah

Your understanding is good. This is #9 on the wish list. A filter that would result in no products found could be omitted, greyed out, or left as is. This would require the counts that you are working on, which is something else that should be on the wish list. I was going to put this thing into Release Candidate, but I'll wait if you're working on this item. I'll put up what I have so far, which is a bunch of bugfixes and the addition of an option for filter results to include subcategories. Expect an update soon. Unless the forum goes kersplat again....

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

Version 0.5.2 has been uploaded to the official site. Since the previous version has still not been approved, I've also uploaded this release here as well. The User's Manual (PDF) is included in the distribution, so no need to download that separately.

 

The only new feature in this release is the optional ability to include subdirectories in the filter results. This could be quite a load on a large store, and return a lot of results, so test this before turning it loose on your unsuspecting customers.

 

All of the bugs that I can find have been fixed. If you are still seeing a problem that I've reported fixed, you'll probably need to give me details of your setup so that I can reproduce it. I can't fix something I can't see.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

The problem I was having with filter selections not showing up on the categories screen (and just displaying as a text box) turns out to be related to the admin skin contribution I had installed called mindsparx. Didn't think this would affect anything but it does! This uses a get variable called value which was interfering with your admin/product_specifications.php file.

 

The fix was to rename the get variable from 'value' to 'svalue' and change all instances of $value and '&value=1' in your code to $svalue and '&svalue=1'. This fixed it nicely!

 

Something you might want to build in to prevent people asking about it when it goes to general release and people start installing it on heavily modified stores..

Link to comment
Share on other sites

Variable conflicts are a pain. Thanks for reporting that one. I'm going to try to avoid conflicts as much as possible, especially with popular addons. This means creative naming -- something I'm horrible at. Yes, $value is a very bad variable name. Oh well.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

Jim, I Modified the tep_count_filters function() to work with multiple chosen filters. I don't have my brain 100% wrapped around how everything works under the hood of this mod, but it seems to produce correct counts on my test bench! :)

 

I still need to dissect the SQL query it performs to see if it can be reduced further (I took it from the top of catalog/includes/boxes/products_filter.php & added another 'where' clause). Let me know if you see anything else that should be done with this.

 

New tep_count_filters() function.

 

  function tep_count_filters($categoryId, $specification, $specificationId, $filterClass, $filterDisplay, $productsColumnName, $languageId)
 {
  global $languages_id, $current_category_id;

  $raw_query = "select 
				count(p.products_id) as count
			  from 
				  (products p) 
				  join (products_to_categories p2c) on (p.products_id = p2c.products_id) 
			  where 
				  p.products_status = '1' and 
				  p2c.categories_id = '" . $categoryId . "' 
				  ";

$raw_query .= tep_get_filter_sql($filterClass, $specificationId, $specification, $productsColumnName, '1');

// Loop through the $_GET vars looking for any set filters.
foreach($_GET as $k => $v)
{
	if (preg_match('/^f[0-9]+$/', $k))
	{
		// We found a var that smells like a filter; append the filtered sql so we can count it.
		$k = str_replace('f', '', $k);

		$specs_query_raw = "select s.specifications_id,
						   s.products_column_name,
						   s.filter_class,
						   s.filter_show_all,
						   s.filter_display,
						   sd.specification_name,
						   sd.specification_prefix,
						   sd.specification_suffix
					from " . TABLE_SPECIFICATION . " s,
						 " . TABLE_SPECIFICATION_DESCRIPTION . " sd,
						 " . TABLE_SPECIFICATION_GROUPS . " sg,
						 " . TABLE_SPECIFICATIONS_TO_CATEGORIES . " s2c
					where s.specification_group_id = sg.specification_group_id
					  and sg.specification_group_id = s2c.specification_group_id
					  and sd.specifications_id = s.specifications_id
					  and s2c.categories_id = '" . $current_category_id . "'
					  and s.show_filter = 'True'
					  and sg.show_filter = 'True'
					  and sd.language_id = '" . $languages_id . "'
					  and s.specifications_id = '" . $k . "'
				  ";

		$specs_query = tep_db_query ($specs_query_raw);

		$specs_array = tep_db_fetch_array($specs_query);

		$raw_query .= tep_get_filter_sql($specs_array['filter_class'], $specs_array['specifications_id'], $v, $specs_array['products_column_name'], $languages_id);

	} // if (preg_match
} // foreach($_GET

  $filter_count_query = tep_db_query($raw_query);

  $filter_count_results = tep_db_fetch_array($filter_count_query);

  $count = (int)$filter_count_results['count'];

  return $count;
 }

Link to comment
Share on other sites

A few thoughts on this, and coding in general. osCommerce, anyway.

 

1. Avoid the use of Global variables. They're a pain to debug, and likely to cause more problems than you want to deal with.

 

2. Avoid duplicating function names. We may want to borrow one from the Admin to use on the Catalog side and vvs.

 

3. The "f followed by a number" convention for specification variables is not safe. There could be other variables that use this in some other Addon. We already have code that does this anyway -- see lines 39-71 of catalog/products_filter.php. These lines should probably be made into a function so we can use this code in multiple places.

 

4. You need #3 before calling tep_get_filter_sql(). That function requires a filter variable. If you put it inside your loop you'll get your cumulative result for all filters.

 

5. osCommerce coding standards please. Or I'll just throw the result into Eclipse and let it format the whole thing. Not that that's a bad idea.

 

Wow, I'm getting pedantic here. Somebody remind me I'm not teaching a class on this. You don't want me as a teacher anyway. Believe me....

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

Just to let everyone know, I'm working on integrating the Products Tabs Addon. This will be an option to use in place of the current table on the Product Info page. It will incorporate the Specifications from this Addon into the Specifications tab. I'm not certain yet how much of the Products Tabs Addon that will actually be implemented -- there are some things there I don't like.

 

Anybody have any thoughts on this? Now would be a good time to speak up.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

Made a few corrections, was just meant to share the rough draft to see if it was going in the right direction.

 

1. Avoid the use of Global variables. They're a pain to debug, and likely to cause more problems than you want to deal with.

Removed, they just weren't cleaned up before I posted it here. The function as I posted it used a mix of both the global $current_category_id and the $categoryId that was being passed, which were both the same thing. *fixed

 

 

2. Avoid duplicating function names. We may want to borrow one from the Admin to use on the Catalog side and vvs.

I didn't even look in the admin functions to see if it wasn't already used. It's been renamed to tep_show_filter_count()

 

3. The "f followed by a number" convention for specification variables is not safe. There could be other variables that use this in some other Addon. We already have code that does this anyway -- see lines 39-71 of catalog/products_filter.php. These lines should probably be made into a function so we can use this code in multiple places.

4. You need #3 before calling tep_get_filter_sql(). That function requires a filter variable. If you put it inside your loop you'll get your cumulative result for all filters.

What would you suggest then for pulling the variables out of the $_GET array? Performing the query in lines 39-71 and comparing it to the $_GET vars? Sounds like a better plan to me.

 

5. osCommerce coding standards please. Or I'll just throw the result into Eclipse and let it format the whole thing. Not that that's a bad idea.

This one is tough. I'll do what I can though!

 

Here's the modified version that should mostly comply, other than #3 & #4.

 

  function tep_show_filter_count($current_category_id, $specification, $specification_id, $filter_class, $products_column_name, $languages_id)
 {
  $raw_query = "select 
				count(p.products_id) as count
			  from 
				  (products p) 
				  join (products_to_categories p2c) on (p.products_id = p2c.products_id) 
			  where 
				  p.products_status = '1' and 
				  p2c.categories_id = '" . $current_category_id . "' 
				  ";

$raw_query .= tep_get_filter_sql($filter_class, $specification_id, $specification, $products_column_name, '1');

// Loop through the $_GET vars looking for any set filters.
foreach($_GET as $k => $v) 
{
	if (preg_match('/^f[0-9]+$/', $k)) 
	{
		// We found a var that smells like a filter; append the filtered sql so we can count it.
		$k = str_replace('f', '', $k);

		$specs_query_raw = "select s.specifications_id,
						   s.products_column_name,
						   s.filter_class,
						   s.filter_show_all,
						   s.filter_display,
						   sd.specification_name,
						   sd.specification_prefix,
						   sd.specification_suffix
					from " . TABLE_SPECIFICATION . " s,
						 " . TABLE_SPECIFICATION_DESCRIPTION . " sd,
						 " . TABLE_SPECIFICATION_GROUPS . " sg,
						 " . TABLE_SPECIFICATIONS_TO_CATEGORIES . " s2c
					where s.specification_group_id = sg.specification_group_id
					  and sg.specification_group_id = s2c.specification_group_id
					  and sd.specifications_id = s.specifications_id
					  and s2c.categories_id = '" . $current_category_id . "'
					  and s.show_filter = 'True'
					  and sg.show_filter = 'True'
					  and sd.language_id = '" . $languages_id . "'
					  and s.specifications_id = '" . $k . "'
				  ";

		$specs_query = tep_db_query ($specs_query_raw);

		$specs_array = tep_db_fetch_array($specs_query);

		$raw_query .= tep_get_filter_sql($specs_array['filter_class'], $specs_array['specifications_id'], $v, $specs_array['products_column_name'], $languages_id);

	} // if (preg_match
} // foreach($_GET

  $filter_count_query = tep_db_query($raw_query);

  $filter_count_results = tep_db_fetch_array($filter_count_query);

  $count = (int)$filter_count_results['count'];

  return $count;
 }

 

 

Elijah

Link to comment
Share on other sites

Yes, the query in the lines that I referred to goes through each Specification that currently applies and looks for a _GET variable associated with that Specification. If none is found, it assigns zero. If one is found, it sanitizes the data.

 

You can use that code to get each variable in turn, and then use tep_get_filter_sql() to get the SQL string that will apply that filter. Concatenate those strings and you have what the current filters would produce. Add to that the result of the filter you are looking at and get a count. Pretty much what you have right now, as far as I have looked at it, but with a safer way of finding the _GET vars.

 

You may very well come up with a better way to do it than this. If so, do it. I seem to often find the most complicated way of doing anything, so my advice is not necessarily good.

 

#5 is indeed a pain. I ignore it sometimes as well. That tends to cause problems when integrating with other people's code, so I do try to stick to the standards for the project when possible. So ignore that if it's just unbearable.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

The updates have now appeared on the official site, so the backup links are now dead.

 

I have some time today, so I'm working on the next update, which is planned to be RC1. Anyone who objects had better speak up quick.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

I haven't been able to duplicate this. Please provide your filter settings, filters, and the data you are filtering against.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

Wondering if you have given any more thought or have come up with a better solution for the filter counts. I'm running into something of a problem with the code I've been using in that the sql queries performed grows substantially depending on the amount of filters you have.

 

For instance, I'm testing a category that contains 3000 products. This category has two spec groups associated with it. One with a spec that has 100 filters (pulldown)and a spec that has 50 filters (pulldown). The other spec group has 3 specs that have 5 filters (Link Lists) each. For each filter, an sql query needs to be ran to count how many products exist. Those queries change each time a filter is added so that you only see the products that would be visible if you were to select that filter, so you have an enormous amount of possible query configurations for each of the 165 (in our case) filters.

 

I can cache the results for a particular combination which yields me a full 3 second increase in page load time, bringing it to almost instantaneous since I'm not needing to perform the subqueries to get accurate filter counts. But keeping the cache fresh means another set of issues to work with that don't need to be discussed here right yet.

 

So in a round-about sort of way of asking, did you have something in mind for performing those counts that wouldn't involve copious amounts of caching or an enormous amount of timely sql queries? I'm considering going back to the drawing board on this otherwise. :P

 

Thanks!

 

Elijah

Link to comment
Share on other sites

Looking at what other e-commerce setups do for their specifications implementation, it would appear that caching the combination's of filters to get the counts may be the best way to do it, or at least the popular way. So from a cron job or manual option, refreshing the indices could be done whenever needed.

 

I'm open to ideas for caching the filter counts since I'm sure a pre-built solution would be better than the quick hack I did. I use an md5 hash of a serialized array containing the spec filters that would normally be sent to the tep_get_filter_sql() function and compare that to a database table containing that key and the count associated with it. If the key isn't found, then it performs the queries generated by tep_get_filter_sql() and inserts the found count into the database table so the next time that exact filter combination is chosen, the cached count is returned. It works pretty well, would just need to write something up that generates all possible filter combination's to include in the index.

 

Elijah

Link to comment
Share on other sites

You've pretty much confirmed what I suspected about calculating these counts. It's likely that the database load on a small store would be acceptable, but a store the size of yours would need some sort of caching to work. I had a vague concept of keying the cache on a string of Specification-Filter pairs, something like the way osCommerce passes attribute-option pairs. Your hash would also work, and possibly better. Either way, the Specifications and Filters need to be kept in the same order every time, or the cache would contain a huge number of duplicates.

 

That's really all I've done on this. It sounds like you've gone a lot further, plus you have that big store database to test against. Please continue what you're doing, if you have the time and will to continue. I'm mostly off herding code for something else, although I have some ideas to apply some of it here when I get the chance to work on this again.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

I'll keep going in the direction I'm in then since having the counts / disabled empty filters is something of a necessity for our shop. Getting this project off the ground and implemented into our live setup has been turned into a priority, so I have nothing but time and will to continue on! ^_^

 

I'm thinking that the counting mechanism might be best served up as an object. Keeping track of certain states outside the scope of the simple function would reduce the query load down even further without the need for global vars or returning back extra data from the counting function & re-digesting it on the next filter count ( i.e. re-fetching the specification data for each filter set in $_GET so it can be applied to tep_get_filter_sql() ). Would keep things nice and neat.

Link to comment
Share on other sites

Good -- I wasn't looking forward to sorting that can of worms. Thanks for taking on this task.

 

You could just retrieve the list of specifications and the filters and store them in arrays. Then again, an object would, as you say, keep things much neater. Your choice. I would undoubtedly do it the hard way, so anything you suggest is likely to be simpler.

 

Regards

Jim

See my profile for a list of my addons and ways to get support.

Link to comment
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Unfortunately, your content contains terms that we do not allow. Please edit your content to remove the highlighted words below.
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...