Currently the results are displayed in the order in which they are in the database. Although only 10 are shown, the time taken to choose a specific result can be decreased if the list is in alphabetical order.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hass’s picture

Priority: Normal » Major

Should be default

bleen’s picture

Status: Active » Needs review
FileSize
7.69 KB

This should do it ... creates an "Order" select list on the widget form ...

(I also snuck in a bit of code cleanup and a couple "todo's".... shhhh dont tell anyone)

hass’s picture

This looks wrong logic to me. If you sort parts of the query result it's too late. You may miss titles... I believe the query needs order by... Isn't it? I may be wrong as I have not tested the patch, this is all code wise.

bleen’s picture

Not all of the autocomplete widgets use a db query so I chose to simply sort the final array that is created regardless of the source... Would be great if you could try out the patch and post results.

:)

mrtoner’s picture

I'll check this out in a bit. Sorting the results is what I expected (and that's what ORDER BY does, BTW, at least for MySQL). I've never seen an autocomplete that didn't sort the results in ascending alphabetical order, though, so the sorting option seems unnecessary and an example of preference creep.

mrtoner’s picture

Since I'm just learning Git it's likely I just made a mistake, but I'm seeing two hunks that are rejected in the patch.

hass’s picture

But it cannot work reliable if the sql request is not ordered by name. No need to test, it cannot work correctly. In your example we may need to sort both, sql and non-sql.

bleen’s picture

Re #5: imagine the case where someone is providing suggestion (one of the types of ac widgets available) in that case they may have been explicit in ordering the suggestions or they may not have and they want them ordered alphabetically ... I agree that in the case of a user who is using the "existing field data" that most of the time they will want everything sorted. In this case there is no extra work for the user since the default setting is sort ASC.

Re #6: I know what I did wrong ... I'll put up a new patch when I get to work

Re #7: I see what you are saying now and I think you are correct: I will need to add the order by to the SQL as well

bleen’s picture

FileSize
20.2 KB

Lets see how we do with this patch

mrtoner’s picture

@bleen18: Ah, yes, that's right.

@hass: I'm not sure I understand why the ORDER BY needs to be in the query. ORDER BY doesn't sort the column before the SELECT is performed, it sorts the results that are returned. So ORDER BY shouldn't affect which rows are returned.

mrtoner’s picture

Patch applies, but I'm getting an error now:

An AJAX HTTP error occurred.
HTTP Result Code: 500
Debugging information follows.
Path: http://example.com/autocomplete_widgets/node/track/field_writer
StatusText: Internal Server Error
ResponseText: PDOException: SQLSTATE[42000]: Syntax error or access violation: 1064 You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'ASC ASC
LIMIT 10 OFFSET 0' at line 2: SELECT DISTINCT fd.field_writer_value AS field_writer_value
FROM 
{field_data_field_writer} fd
WHERE  (field_writer_value LIKE :db_condition_placeholder_0 ESCAPE '\\') 
ORDER BY ASC ASC
LIMIT 10 OFFSET 0; Array
(
[:db_condition_placeholder_0] => %jon%
)
in _autocomplete_widgets_get_options_flddata() (line 164 of /home/.../public_html/sites/all/modules/autocomplete_widgets/autocomplete_widgets.common.inc).
mrtoner’s picture

BTW, "A to Z"/"Z to A" may not always be appropriate: the user may be using numerical fields. I suggest "Ascending," "Descending," and "None."

bleen’s picture

#10: Its because there might be 20 possible results but we are only limiting to 10. In that case we want to have MYSQL orderby first and then limit to 10.

#11: Huh?? The error there is this: ORDER BY ASC ASC
But I have no idea why that might be happening. My brain be hurtin. Ill play a bit though.

#12: Agreed

mrtoner’s picture

Oh, right: LIMIT returns the first 10 from the unordered results without ORDER BY. With "contains" matching that probably doesn't make a difference, but with "starts with" matching you'll miss a lot more relevant matches.

Umm:

$select->orderBy($order);

Doesn't that just insert "ORDER BY ASC"? It needs to order by the column:

$select->orderBy($col, $order);

bleen’s picture

FileSize
20.2 KB

DOH!!!!! Of course you need a column. And when I did my spot check I used "suggested".

How bout this patch

mrtoner’s picture

No change #15 from #9.

bleen’s picture

FileSize
20.29 KB

Ooops ... uploaded the old file. Sorry bout that

mrtoner’s picture

Status: Needs review » Reviewed & tested by the community

Ding, ding, ding! We have a winner.

bleen’s picture

Status: Reviewed & tested by the community » Fixed
FileSize
12.12 KB
9.82 KB

Thanks for your help with this mrtoner & hass...

I committed both of the patches below. One was the basic code cleanup (apply first), the other handled the actual reordering (apply second)

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.