Suggestion to Extend Virtuemart product search capabilities

[Galt]

Hello, I'm a developer of Cherry Picker (CP) -- a product search module that's been around since VM1.1.
I want to suggest a very small addition to the core VM code that will allow us, Virtuemart extension developers, to write products that extend Virtuemart product search capabilities WITHOUT the need to hack any core files.

WHY DO WE NEED THIS

Well, first of all there is a version of CP that uses its own table structure to manage filters. Custom Fields are great, but there are certain reasons why we'd like to manage filter tables in our own way: and one of the most important is the "performance".
Also we keep adding new features and therefore tables are often being changed/modified. This way it's all sandboxed and no other parts of VM are affected.

As a result there is a need to edit /admin..(vm)/models/product.php so that when a filter in front-end is selected additional SQL JOINS and WHERES are appended to the main VM query.
What's worse is that users need to do it every time they update Virtuemart (and there are a lot of VM updates nowadays).

Currently we provide manuals or already pre-edited files that users need to overwrite in order for the search to work.
But from the point of view of usability this is a very poor user experience both for VM and CP users.



HOW IT WORKS NOW

File affected:
/administrator/components/com_virtuemart/models/product.php

Function:
function sortSearchListQuery(..)

Currently VM uses this trigger to fetch SQL parts:

Code Select Expand


JPluginHelper::importPlugin ('vmcustom');
$dispatcher = JDispatcher::getInstance ();
$PluginJoinTables = array();
$ret = $dispatcher->trigger ('plgVmAddToSearch', array(&$where, &$PluginJoinTables, $this->searchplugin));


But these parts are added in a very specific way -- we can't add arbitrary tables to JOINS:

Code Select Expand


if (!empty($PluginJoinTables)) {
$plgName = $PluginJoinTables[0];
$joinedTables[] = ' LEFT JOIN `#__virtuemart_product_custom_plg_' . $plgName . '` as ' . $plgName . ' ON ' . $plgName . '.`virtuemart_product_id` = p.`virtuemart_product_id` ';
}




WHAT DO I SUGGEST

A solution would be to add a new trigger right after all SQL JOINS and WHERES are fully assembled by Virtuemart:

Code Select Expand


...
if ($joinChildren) {
$joinedTables[] = ' LEFT OUTER JOIN `#__virtuemart_products` children ON p.`virtuemart_product_id` = children.`product_parent_id` ';
}

// NEW:
JPluginHelper::importPlugin('vmcustom');
$dispatcher = JDispatcher::getInstance();
$dispatcher->trigger('plgVmBeforeProductLoad', array(&$where, &$joinedTables, &$groupBy, &$orderBy));


Please note the following:
1. We are adding a new trigger name "plgVmBeforeProductLoad". We can't use the existing one "plgVmAddToSearch" as it will cause other extensions to execute unnecessarily again.
2. We pass all SQL parts that may of interest to observing plugin.
3. Plugin may judge now by the parts already added and append its own parts that it deems necessary. Also it may unset certain parts and replace it with its own extended copy. For example, I receive tons of inquiries from customers that need to show filters not only for the current category but also for all subcategories. And when a filter is selected in such a case it is expected that result products will be from subcategories too. It is easily achieved if the plugin replaces specific category:

Code Select Expand


$where[] = ' `pc`.`virtuemart_category_id` = ' . $virtuemart_category_id;


with a copy that adds all subcategories:

Code Select Expand


$cids = $comeclass->getSubcategoryIds($virtuemart_category_id);
$where[] = " pc.`virtuemart_category_id` IN ('". implode("', '", $cids) ."')";


This now becomes possible without any hacks to VM core through plugin extensions.



WHAT WILL IT ACHIEVE

These three lines of code will make it possible for any extension to extend VM search capabilities without the need to edit core files.


WILL IT BREAK ANY EXISTING CODE

Absolutely not. It is a simple trigger call that does not affect any existing code or extensions.


EXISTING EXAMPLES

HikaShop uses a similar way to extend queries. As a result HikaShop users do not need to hack the core.

Code Select Expand


JPluginHelper::importPlugin( 'hikashop' );
$dispatcher = JDispatcher::getInstance();
$dispatcher->trigger( 'onBeforeProductListingLoad', array( & $filters, & $order, & $this, & $select, & $select2, & $a, & $b, & $on) );



I am really excited to hear replies and suggestions from Virtuemart developers.

[Milbo]

Thank you for this detailed explanation. YEh a project is exactly meant like that. I wonder now, why no one earlier told us. I will remove the old trigger in vm3 and implement your idea. For vm2.6, hmmm actually I doubt that the old trigger is used. But you never know. So the clean way is to use your trigger idea for vm3, remove the old one and thats it.

[Milbo]

I added it this way in vm3 now

around line 550 before the if($this->_onlyQuery){

Code Select Expand


JPluginHelper::importPlugin('vmcustom');
$dispatcher = JDispatcher::getInstance();
$dispatcher->trigger('plgVmBeforeProductSearch', array(&$select, &$joinedTables, &$where, &$groupBy, &$orderBy,&$joinLang));


[Galt]

Hi Max, thanks for getting back to me so quickly.
So I presume this is how it looks now in your file:

Code Select Expand


if (count ($where) > 0) {
$whereString = ' WHERE (' . implode (' AND ', $where) . ') ';
}
else {
$whereString = '';
}
//vmdebug ( $joinedTables.' joined ? ',$select, $joinedTables, $whereString, $groupBy, $orderBy, $this->filter_order_Dir ); /* jexit();  */
$this->orderByString = $orderBy;

// NEW
JPluginHelper::importPlugin('vmcustom');
$dispatcher = JDispatcher::getInstance();
$dispatcher->trigger('plgVmBeforeProductSearch', array(&$select, &$joinedTables, &$where, &$groupBy, &$orderBy,&$joinLang));

if($this->_onlyQuery){
return (array($select,$joinedTables,$where,$orderBy,$joinLang));
}
$joinedTables = " \n".implode(" \n",$joinedTables);

//vmSetStartTime('sortSearchQuery');
$product_ids = $this->exeSortSearchListQuery (2, $select, $joinedTables, $whereString, $groupBy, $orderBy, $this->filter_order_Dir, $nbrReturnProducts);


In such a case please note that the variable $whereString is being formed before a trigger is called. So no matter what is added to $where by plugin the old version of WHEREs will go to $this->exeSortSearchListQuery().

So I think the call should go right before this part:

Code Select Expand


if (count ($where) > 0) {
$whereString = ' WHERE (' . implode (' AND ', $where) . ') ';


[Galt]

Hi Max, there is another consideration.
In this function there are many tables that participate in Joins, and there is a need to track them:

Code Select Expand


$joinCategory = FALSE;
$joinCatLang = false;
$joinMf = FALSE;
$joinMfLang = false;
$joinPrice = FALSE;
$joinCustom = FALSE;

..

// When need to JOIN
$joinCategory = TRUE;


It could be much tidier if we use bit masks:

Code Select Expand


// mask
$joinedTablesMask = 0;
// table masks
$tblCategory = 1;
$tblMf = 1 << 1;
$tblPrice = 1 << 2;
$tblLang = 1 << 3;
..


Then it could be used like this:

Code Select Expand


// Add Category Table to joined list
$joinedTablesMask |= $tblCategory;


// Remove table from list
$joinedTablesMask &= ~$tblCategory;


// Check if Category Table was added (or needs join)
if (($joinedTablesMask & $tblCategory)) {
..
}



And finally the final mask could be passed around for the benefit of observers which may get all the info just form one variable:

Code Select Expand


return (array($select,$joinedTables,$where,$orderBy, $joinedTablesMask));



If you guys use Git I could re-write this part and send a pull request.

[Milbo]

Actually I am quite happy with the array. Your idea spares some bites, but I am not sure if I like it. Atm it is so easy to read and your idea is for me mainly a form thing. But you can almost do the same in svn as in git. you can also download svn, do the changes, create a patch and sent it.

[Galt]

That's fine as long as you don't need to pass more variables like $joinLang )

As for plugin call, I hope you moved the code higher, because if it is "around line 550 before the if($this->_onlyQuery){" then it won't work. I verified it with tests and the result exactly as I suggested: is has to go before line "if (count ($where) > 0) {".

[klattr1]

Not sure I know what's going on here but I like the sound of it. Go Maks and Milbo!

[balai]

I think that it would be much better to return the query as object (JDatabaseQuery), than every single clause of it, which can lead into much more params in the future.
Then you just need the JDatabaseQuery::__get($clause) to get the appropriate clause of it.

By doing that, you have also the advantage to use all the functions of that object, which are very usefull