contao / core

Contao 3 → see contao/contao for Contao 4
GNU Lesser General Public License v3.0
491 stars 213 forks source link

DC_Table cannot sort its listView() using a foreignkey.field #299

Closed ghost closed 12 years ago

ghost commented 12 years ago

In DC_Table, the ORDER for a drop-down field is always its value rather then the label in list View, e.g.

genre is a drop-down field with options defined

'options' = array (
'10' => 'Dance',
'15' => 'Electronica',
'26' => 'Reggae',
)

When choosing genre as a filter field, TL correctly retrieves the unique remote labels and sorts them in the filter drop-down. BUT when you select this field as a Grouped item (flag = 11, Group and sort ascending), TL fails to sort the field before using it to order it.

The problem is that the list view is using a simple ORDER BY genre instead of retrieving the label to sort it on. If you are using a remote table and the value contains the ID, then your sort order will always be wrong, because its sorted on ID, not label.

A possible solution would be to do something like this:

ORDER BY FIND_IN_SET(genre, 'label,label2,label3')

where labels are pre-sorted similar to the filterMenu drop-down section

This will obviously not be possible for multiple selections (grouping cannot be done, as multiple values exist)

I'm not sure if this is the same (as Google might not be translating very well)

88

Download the attachments Related issues: #1914

--- Originally created by thyon on November 27th, 2008, at 07:52am (ID 299)

ghost commented 12 years ago

A few corrections.

The example was a bad one, because the ID's are already sorted numerically. A better example would have been:

'options' = array (
'26' => 'Dance',
'10' => 'Electronica',
'5' => 'Reggae',
)

actually the code above is a bit wrong, in that it should list the IDs of the genre, but sorted by label, so the code would be.

then the code would be like this:


$sortOptions = sort($options);
$arrProcedure[] = "FIND_IN_SET(" . $field . ", '". join(',', array_keys($sortOptions)) ."')";

Simplified DB statement (after replacement:
SELECT * FROM tableName ORDER BY FIND_IN_SET(genre, '26,10,5')

IDs are not sorted by ID but by labels. This will allow any "optioned" field to be used as sorting correctly, since the label will be used for the sorting.

--- Originally created by thyon on November 28th, 2008, at 07:28am

leofeyer commented 12 years ago

There are two real simple solutions for this problem:

  1. Create the desired sorting order when you write the dca file.
  2. Use ksort() in the dca file to sort the options array.
<?php
$GLOBALS['TL_DCA']['table'] = array
(
    'fields' => array
    (
        'test' => array
        (
            'options' => array(30=>'foo', 20=>'bar')
        )
    )
);

// Sort options
ksort($GLOBALS['TL_DCA']['table']['fields']['test']['options']);
)
?>

--- Originally created on December 13th, 2008, at 09:57pm

ghost commented 12 years ago

Hey leo. I think you misunderstood your own code here:

This is the codeblock from your DC_Table.php driver:

private function listView()
...
...

if (is_array($orderBy) && strlen($orderByr0))
{
    $query .= " ORDER BY " . implode(', ', $orderBy);
}

This will order by the field's value (the stored ID) not its label, so this completely disregards the "options" you say we can pre-sort. The pulldown in the "sort;" section is created completely internally in DC_Table driver and its values are sorted by the simple ORDER BY sql command, i.e. your code is retrieving and sorting the table based on your OWN created list, NOT using the options at all.

That is why I posted this request.

--- Originally created by thyon on December 15th, 2008, at 08:43am

ghost commented 12 years ago

So there is a conflict in the two methods used in DC_Table.php.

  1. In the place where the pull-down menu options are created for the "sort;" section you ARE sorting them on label, ignoring the value (the ID)

BUT

  1. In the actual query to retrieve the table items, this Label sorting order used in the pull-down menu above is ignored, and the value is used (the ID), instead.

This conflict confuses the user.

Hope its starting to make more sense now?

--- Originally created by thyon on December 15th, 2008, at 08:50am

leofeyer commented 12 years ago

On my system, implode(', ', $orderBy) implodes the values and not the keys.

--- Originally created on December 15th, 2008, at 11:26am

ghost commented 12 years ago

Firstly, the options_callback allows a developer to store a value DIFFERENT from the label. Sorting on the value in DB will not produce an alphabetical sorting of the labels.

Example:

'options' => array (
  'tmp' => 'Customer Temporary',
  'vdm' => 'Working Method',
  'b2b' => 'Commerial Enterprise',
)

Please read my previous comment again, because it clearly explains that in filterMenu() (sorry, I mistakenly said "sort;" when it should have been "filter;") does it correctly, but retrieving the field LABELS from the field options and then sorting on them:

private function filterMenu()

...
    // Sort by option values
    if (!$blnDate)
    {
        uksort($options_sorter, 'strcasecmp');
    }
...

and in listView() this method is not used, but instead you sort of values in the DB (which does not tie-up with the above FilterMenu labels).

private function listView()
...
...

if (is_array($orderBy) && strlen($orderByr0))
{
    $query .= " ORDER BY " . implode(', ', $orderBy);
}

These methods are contradictory methods and do not produce an aligned result, since the values stored in the DB, do not relate in any way to the labels used to display them. Hence to sort an optioned value, you cannot just sort on its value, because the label is not taken into account. You'll have to create a label lookup and then sort based on that label lookup as I originally suggested using FIND_IN_SET().

--- Originally created by thyon on December 15th, 2008, at 02:16pm

leofeyer commented 12 years ago

I still do not really know what you mean. Is there any real-world example that I can reproduce on my server? Does this problem occur with any of the core modules?

--- Originally created on December 15th, 2008, at 02:50pm

ghost commented 12 years ago

yes. Create a selectmenu field in a blank DCA (you probably have a test one). Add the above array (directly above) as its options (single selection only). Make sure its filter and sort are set to true, so that its possible to see the filter drop-down and you're able to sort on this field as well. Flag type should be normal (e.g. by First letter sorting).

Enter a few records and select a different drop-down value for each, corresponding to at LEAST each option, e.g. 3 items to match the options above.

Then you will see the filter menu has retrieved the options drop-down correctly in alphabetical order (by their labels), but when you sort on the field, the grouping value (in gray above in the listview), will NOT be in alphabetical order, but sorted on the option keys (not labels), as the key is stored in the DB (not the label). This creates a confusion for the end-user, as they expect the grouping field to be alphabetically sorted.

--- Originally created by thyon on December 15th, 2008, at 03:03pm

ghost commented 12 years ago

Leo - I have a real-world example available here It makes use of the Catalog and Catalog EXT extensions, where the Catalog filter is displayed near the top (where the four columns are for Search tips, Search, Filter by, Sort by). In the "Chemical class" filter by dropdown, you will notice that everything is sorted alphabetically except the last item, "Acids". This is because all Chemical classes were entered into Taxonomy in alphabetical order. Then, Acids was added several days later.

--- Originally created by ben on December 15th, 2008, at 07:25pm

leofeyer commented 12 years ago

I see what you mean, however, thyon is referring to the back end (I think). I do understand that the database driver needs some modification, but I cannot run any tests without knowing how to set up a test case.

By the way, FIND_IN_SET() is really slow with larger sets (probably 100 or 500 values), so I do not know if that qualifies as a solution.

--- Originally created on December 15th, 2008, at 07:40pm

ghost commented 12 years ago

Hi Leo, Ben is experiencing the same problem in the back-end, which is actually why he raised it.

Well, after all that explaining on how to set-up the example, you can't do it? What part was unclear. Ben if any part was unclear, maybe help me explain better.

FIND_IN_SET is the ONLY efficient way I have found, as REGXP can return an incorrect result, alternatively you can wrap the value in a ",value," comma string and search for that in the value with commas added to the front and back as well, e.g. ",1,5,35,2,542,532," (commas at beginning and end were CONCATENATED to the DB value. I actually found FIND_IN_SET was much faster performance wise.

Hey Ben, did you know that SASOL is a South African company? I even had an interview with them when I started working, but I didn't get it.

--- Originally created by thyon on December 16th, 2008, at 02:59am

leofeyer commented 12 years ago

There is another problem with FIND_IN_SET: it is not supported by e.g. Oracle or PostgreSQL, so it cannot be used in the SQL statement. Instead, the database abstraction classes had to be extended which would be a lot of work. Still, I am going to check.

--- Originally created on December 17th, 2008, at 12:10pm

ghost commented 12 years ago

An alternative would be to wrap the items in a special character and then do a position search. The position can then be used to create the sort order, e.g.

LOCATE('4,5,1', '1') INSTR('1', '4,5,1')

but you need to be careful for situations like this, hence my addition parsing character: 3 occurs in 3, 33, 333, 3333, so

LOCATE('r4r5r1', 'r1') INSTR('r1', 'r4r5r1')

wrapping it in [] would make it search safe.

--- Originally created by thyon on December 17th, 2008, at 12:53pm

ghost commented 12 years ago

After upgrading TL from 2.6.2 to 2.6.3, I have noticed that foreign keys (items using Taxonomy) now sort properly in the backend but not the frontend. In the backend, it sorts properly in the "Filter" dropdowns as well as the dropdowns when editing a product.

In the frontend, the dropdowns in the Catalog Filter module do not sort alphabetically, though. (Notice "Acids" at the bottom of "Select Chemical class" and under "Select Supplier" that "Eastman Chemical" and "ExxonMobil Chemical" are not after "Cross Oil ..."

Is it possible that code was changed in TL 2.6.3 that now sorts things properly? Also, do you think the frontend sorting is something can be fixed in the Catalog Ext code?

--- Originally created by ben on December 19th, 2008, at 08:36am

ghost commented 12 years ago

Oops

![]( The second image in my last comment was supposed to be the one below. (Please edit my comment if possible.)

)#299:ticket-299_FE.gif!

--- Originally created by ben on December 19th, 2008, at 09:52am

ghost commented 12 years ago

actually Ben, the BE filter drop-downs were always sorted correctly as leo wrote a special routine to get the distinct DB values and then sort them on labels. So this has been available. I then logged the problem where I noticed that even though these drop-downs were correct, the actual GROUPING items are sorted by ID, because the labels are not located inside the DB where the query is listed from. Leo will have to write a lookup routing (using whatever function) to be able to sort on label and not the value stored in the DB.

I'm looking into the FE problem?

--- Originally created by thyon on December 19th, 2008, at 10:17am

ghost commented 12 years ago

Leo - is there any development with this issue and is there a possibility it will be resolved in the upcoming release of 2.6.4? Attached is a screenshot showing how select fields are sorting values properly, although data views are sorting by ID instead of foreign key values.

--- Originally created by ben on January 20th, 2009, at 08:18am

leofeyer commented 12 years ago

It seems that FIND_IN_SET() is the only possible solution, however, since it is a MySQL specific command, we actually should not use it. On the other hand, TYPOlight will not work on Oracle or PostgreSQL systems without heavy modifications of either the source code or the database drivers, so we might as well allow MySQL specific code.

Please try the following new code in system/drivers/DC_Table.php around line 2950

<?php
if (is_array($orderBy) && strlen($orderByr0))
{
    foreach ($orderBy as $k=>$v)
    {
        if ($GLOBALS['TL_DCA'][$this->strTable]['fields'][$v]['eval']['findInSet'])
        {
            $keys = $GLOBALS['TL_DCA'][$this->strTable]['fields'][$v]['options'];

            if (array_is_assoc($keys))
            {
                $keys = array_keys($keys);
            }

            $orderBy[$k] = "FIND_IN_SET(" . $v . ", '" . implode(',', $keys) . "')";
        }
    }

    $query .= " ORDER BY " . implode(', ', $orderBy);
}
?>

To make this work, you also need to set

<?php
$GLOBALS['TL_DCA'][TABLE]['fields'][FIELD]['eval']['findInSet'] = true;
?>

--- Originally created on March 14th, 2009, at 01:03pm

leofeyer commented 12 years ago

--- Originally completed on March 14th, 2009, at 01:03pm