Select list component display incorrectly orders items; should match component order

The keys are strings and often contain string values. Unlike core, all data is stored in the same table. There is no guarantee that views will order them in their no order.

So am I. Create a grid, store the results, and look at the table. You'll see what I mean.

Followup: You can write a views handler that casts this no column to a number, but ensure that it only runs on your select items as I suspect it will throw a SQL exception if given a alphabetic string.

I don't use IRC for webform as it's best to keep the communication public in the queue for future maintainers (who might be me) to see.

The only solution then would be to modify the select component to encode the 'no' sequence "number" as a variable key. I'm not sure what issues this might cause for other modules. For example:

0
1
...
9
A10
A11
...
A99
B100
B101
...
B999
C1000

OR
10
11
...
19
210
211
...
299
3100
3101
...
3999
41000

The former is shorter and is the same for the typical case of <=9 values. The latter case is numeric, which means that any code relying on the 'no' field will still work if PHP type-coerces the string to a number.

You'd also have to look at any other components that store multiple values to see if they are affected.

Other data in 'no' isn't numeric. I don't see how that would work. And these tables can be huge, so we need to consider any performance consequences.

I think what you mean is:

  foreach ($submissions as $submission) {
    foreach ($submission->data as &$sortable) {
      ksort($sortable);
    }
  }

With lots of components and/or lots of submission, that's a ton of calls to ksort for a 3-sigma boundary condition. Maybe 1 time in 10,000,000 would that ksort be of benefit to the user. Doesn't seem like a good idea.

Also, it doesn't help views, although I have tried the reporting to see if 'no' is even sorted upon.

I don't think webform has an obligation to return multiple select data in the order of the keys. It would be nice if it did, but fundamentally, it seems like extra credit to me.

Another alternative would be switching to Entity Form.

Sorry you're frustrated and have lost interest. I've made several suggestions, all of which were intended to be helpful. We simply differ on whether we consider it a bug. No biggie.

If you regain interest, feel free to reopen the issue. #10 is viable, I think.

Thanks @DanChadwick for answering so promptly. I asked @Upchuck to file this since I already knew what he was describing after he asked me if it was a problem.

I agree that this is probably a bug, although the "no" column is indeed varchar "by design", the inaccurate ordering of results if there are more than 10 items is certainly not intentional.

In @Upchuk's case, he can probably work around the problem using the very solution he described in #13:

  // Add this portion:
  foreach ($submissions[$row->sid]->data as &$sortable) {
    ksort($sortable);
  }
  
  foreach (module_implements('webform_submission_load') as $module) {
    $function = $module . '_webform_submission_load';
    $function($submissions);
  }

But instead of including it directly in Webform, he could move the added ksort code into a module that implements hook_webform_submission_load(). So there's also a reasonable work-around, but not ideal.

Here's one approach that might work (in most cases) and not require hardly any effort at all. What if we just remove the sort entirely from the "no" column? MySQL by default will return the rows in the order they were inserted into the database, which will keep the order the way we desire. For select lists that have string-keys, we actually are causing the results to be different than the original order defined in the select list already, because a string-sort of keys is not likely to be the same order as the select list in the configuration. So by sorting right now, we're likely causing more problems than we're fixing. Removing the sort could give users (usually) accurate sorting and reduce the cost of the SQL query in the first place.

If we wanted totally accurate sorting, we'd need to do something more extensive. That would probably be creating a numeric order column in the DB for example, or manually sorting certain components.

Hm, well maybe depending on the inserted order isn't a good idea: http://stackoverflow.com/questions/8746519/sql-what-is-the-default-order...

However, it is still the case that we're sorting on a column that may not actually match the order of the component configuration or submission order. Perhaps we could approach this differently and simply fix the presentation of select lists, which is what @Upchuck discovered as the problem in the first place:

And when the webform submission is viewed, the results are ordered by that `no` column by default and you end up with a wrong order as 9 is considered bigger than 10 in varchar terms.

In all cases, the order of the select list should be displayed the same as the configuration of the component, not in a SQL-based ordering. We have this problem both for numeric values 10 and higher as well as for string-keys.

Let's change the issue title to reflect what we're trying to solve at least. Changing the "no" column to an integer isn't the right solution here.

I'll also add that having a component order that sorts descending is also totally valid. There'd be nothing wrong with configuring a select component as:

10|10 items
9|9 items
8|8 items
7|7 items
6|6 items
5|5 items
4|4 items
3|3 items
2|2 items
1|1 item

Assuming these were checkboxes, when displayed, they should be ordered 10 first and 1 last to match the component configuration, rather than a sort from the database.

#10a. Easy, efficient, helps views.

Now I'm confused, @Upchuk. In #5 you said:

And when the webform submission is viewed, the results are ordered by that `no` column by default and you end up with a wrong order as 9 is considered bigger than 10 in varchar terms.

But things appear fine when viewing an individual submission. Select component values are already ordered as defined in the component configuration. This already is basically required because there's no way that sorting on the "no" column will really give us the correct order of keys in all cases anyway. Although you're right that for select lists, the "no" column is numeric, for a Grid component, the keys are strings and sorting on them numerically would return the wrong results.

Likewise, I tried viewing the "Analysis" listing as well for select components, it's also in the right order.

I may have gotten mixed up here, because I thought you were describing that Webform itself was displaying values in the wrong order somewhere. If it's only in the resulting list of `webform_get_submissions()` that the data is out of order, then this really may not be a serious concern. It will simply be up to your application (assuming your calling this function manually) to order the results to match the select component order before displaying them.

I also realized that even if we did have a numeric column for ordering in the first place, this still wouldn't work in all situations. Because a user can at any time change a select component into a different order after submissions have come in, the recorded order wouldn't be accurate for displaying the submission any more either.

So, long story short: There is no way (even if we added a numeric column) to reliably "sort" data as it comes out of the database. You will always have to manually *order* (not sort) it based on the component configuration, at least when it comes to select lists and grid components. If we offered a multiple-value textfield where the "no" column really was an ordered value, this would be a much larger problem.

Have I summarized everything correctly this time?

All of that said, we could also still probably remove the ORDER BY "no" from the query entirely for performance reasons which would reduce the cost of the query and should have no negative effects. As a side-effect, it would result in desirable results in your situation, but that would be simply a coincidence of MySQL InnoDB tables.

Okay, confirmed for real this time. Definitely a problem.

To reproduce:

- Add a new form with a select component.
- Select "multiple" to make them checkboxes.
- Enter in at least 10 values. You can order them ascending but I did descending just to really demonstrate the problem:

22|22
21|21
20|20
19|19
18|18
17|17
16|16
15|15
14|14
13|13
12|12
11|11
10|10
9|9
8|8
7|7
6|6
5|5
4|4
3|3
2|2
1|1

- Now view the form and select *at least 10 options*, preferably 15 or so.
- Then view the submission.

Expected:
- The results of the select list should match the order of the component.

Actual:
- The results of the select list are in no particular order at all (from the end-user standpoint). You can make the problem even worse by editing an existing submission and adding more options.

Screenshot of the submission result for a select list with the above configuration:

So based on the above, back to my modified issue title, which accurately describes the end-user facing problem.

Implement #10A and test. It's a line-line fix that addresses the root problem (rather than fixing it later). It helps views too. As other components are created which expect the 'no' field to behave like a weight, it will continue to give the desired result. It is also invertible -- if an index contains a characters, remove it to get the original numeric index back.

Webform has no obligation to adjust submitted data to suit changes to the form made after submission. Every component has issues when this happens. It's not something that webform can or should reasonably deal with.

I don't see a problem with matching the component configuration on display. It would be relatively lightweight, as it's only done on display and we already have the full component right there.

That said, fixing the "no" column to sort correctly by prefixing with letters would indeed help Views in most cases. But updating existing submissions seems like it'd be an expensive update hook. We'd also end up slightly modifying the $submission data structure with the new keys, which might end up increasing the amount of data juggling we do.

Let's consider laying the groundwork for future improvements. I can see that components may well expect that if they create a typical numerically-indexed array of values (as in $values[] = 'my value') that that component would expect to get an array back in the same order. I'm thinking not so much of multi-select, but of, say, extending other components to be multi, like file for example. OTOH, they could ksort to guarantee it, although doing this on every submission would be costly when a lot of submissions are loaded (e.g. analysis, export and to a lesser extent, views).

Further, we could set the stage for views to get the rows in the same order. This would allow a future views submissions data handler to retrieve raw data and display it without calling webform_load_submissions. Combined with webform machine names and component machine names, we could have much more native views integration, which increases the power of views.

The "no" column is actually overloaded. It means two things: delta/weight and sub-component. For a grid, the 'no' contains the key of the question. There is a long-standing feature request for multi-select grids (i.e. with checkboxes, rather than radio buttons). To implement this, the 'no' keys would have to be fudged -- 'question1_1' would be the first answer and so forth. This would make a crosstab in sql all but impossible.

Options:

1. Fix order in component before display. Only affects Select components. Could be as simple as a ksort or as complicated at #29. Short-term fix though. And does nothing to make schema better reflect the submission data.
2. Change select to use alphabetically-sorting keys a la #10b. Only affects sort components. Simple. Both a short-term fix and makes keys more suitable for sorting. Faster than 1 because only done on save and only affects multi-select components, possibly only only when > 9 answers.
3. Change the storage and retrieval of numerically indexed components (all of them) -- those with 0, 1, 2, 3... keys to be alphasorted, and decode them upon retrieval. This would be transparent to the component. We'd have to pick an escape character that sorts nicely in all collations. Maybe colon or equals? 0-9 could be left as is. 0, 1, ... , 9, :A10, :A11 ... , :A99, :B100
4. Add an integer weight/delta column. Affords new abilities to all columns. Could be used to order multi-select grid data properly. Adds 4 bytes of data plus overhead to a potentially very large table. Not sure if NULL could be allowed, but if the default has to be 0, then a hook_update would be needed. Based upon other issues, a default of 0 can probably be safely inserted into the table without fear of exceeding the PHP timeout.
5. A variation of the above, is to set a sub-component id, which in the case of the grid would be the question key. NULL is allowed. existing "no" column would be turned into an int. This would probably best suit the webform data.

Maintainer decision: ksort values before display, to fix original issue. Larger questions about schema postponed to D8. I will create an open task for D8 schema and reference this issue.

Thanks for your help, guys.

Committed to 7.x-4.x and 8.x.

Thanks Dan, this solution works in almost all situations. I found a problem that the "Table" display of values is still out of order. If we want to add a ksort() to the _webform_table_select()

If you're up for matching the component order, I've attached a patch that uses the loaded component order to match the output order. I think you're right that it's not really our responsibility to deal with modified components, but this solution is still pretty easy (though not trivial) and provides a very reliable output for our users. It turns out the Analysis display already sorts based on component order, so there's no need to modify that callback.

Not much of a fan of #35.

I *think* the theory of operation of grouped option display is to create a nested list of options so that an overriding theme function might use it if it wants, and then flatten it back out in the default theme function. If this isn't the case, then the code should be simplified to always pass the flattened list and remove the flattening code from the theme function.

So #35 creates the flat list. It uses it to sort the values. Then it throws that away and does the work all over again for a flat list. These lists might involve callbacks which do database or other time-consuming operations. Then the default theme function throws that work away again to flatten the list so it can use it.

I would rather see another ksort stuck into _webform_table_select and be done with it. I really don't think that re-ordering existing results based upon a schema change to a component after data has been added is worth the code complexity. It might even be argued that it's not a bonus as it doesn't represent what is actually in the database. For example, we show values that are no longer options.

@quicksketch -- if you really want to do all this work, I suggest one minor improvement:

  $value = (array) $value;
  $option_keys = array_keys($options);
  return array_merge(array_intersect($option_keys, $value), array_diff($value, $option_keys));  

I think it's a little cleaner than putting in duplicates and removing them with array_unique.

Too many cooks fretting over whether the cookies should be round or square. quicksketch -- commit what you prefer.

I found a problem that the "Table" display of values is still out of order. If we want to add a ksort() to the _webform_table_select()

Eeeeh. Upon further reflection, I'm not comfortable leaving table as is with half the cases fixed. I added a ksort to the table output. If quicksketch goes forward with #35, just delete the ksort before applying. In this way, the -dev release is at least consistent and if this issue gets dropped, we don't forget about the table output.

Committed to 7.x-4.x and 8.x.

Changing to Feature Request for the specific additional feature of re-ordering database data to match changes in the component schema made after the data was submitted.

I'm satisfied with just the ksort(). It'll work 90% of the time and is simple.