Database exception for View with Combined field filter with fields with no query alias

And this should fix it.

this also removed the double check in the same setting

if (!empty($field->field_alias) && !empty($field->field_alias)) {

Since the default alias is 'unknown' (no idea why this isn't just FALSE or NULL, but it isn't), something would have to set this to something empty for this to occur, so don't know if that is something that would happen, but you are right, easy enough to still check, just in case.

Updated to patch to include the empty check.

Now that I think about it....the problem that was raised by @alexpott with https://www.drupal.org/node/2370251 also might be an issue here.

With this fix, we silently skip over any fields in the filter config we don't like, with no warning to the end user that the config options are invalid/not used. So, I theory, the user could expect the view to be filtered by a certain value, but in fact it's not.

So do we need some kind of feedback on this? Or invalidate the View at some point?
I had a look at limiting the options array to fields with a field_alias, but aliases have not been set yet when buildOptionsForm() is called, so all aliases are 'unknown' at that point. So that's not something that can be used.

Any thoughts on this?

@alexpott I completely agree, but...

I had a chat with @dawehner on IRC about this as well and the problem is that it's impossible to detect which field will and which fields won't work at the time of building the options form. Like I said in #6, the criterium now used during query building can't be used because all aliases are 'unknown' at that time.

@dawehner said that when this feature was introduced it was accepted that it wouldn't work for some fields, the description for the field selector also says 'This filter doesn't work for very special field handlers.'
So to me it's now more about making sure that 'very special field handlers' are detected before exceptions are thrown. But unless we make every handler set a 'compatible with combine field filter' flag, I don't really see a criterium to test for during option building that would not give false positives or false negatives. If somebody else knows one, great!

But short of that, this is more about making sure people don't wind up facing an exception when we could have given them something more elegant.

Maybe abuse a different setting to test for a 'very special field handler'? If you can't click sort, you are probably dealing with something special right?

Not pretty, but gets the job done...

@alexpott, sorry not quite sure what you mean by 'How do we determine what to list on list of fields that can be a used as a normal filter?'

@dawehner That sounds perfect! Maybe postpone this waiting for that to land? No idea what the odds are of that making it in though.

Rerolled and updated the patch a bit.

Since #2349465: Add an isComputed method to field handlers is an API change, that won't get into 8.0.x, so the 'misuse' of clickSortable, makes sense for 8.0.x I feel. Maybe add a follow up to update it to isComputed when that becomes available.

Edit: Some extra stuff got into the interdiff, ignore the error display stuff.

Added a comment with a @todo to the isComputed issue to indicate that this is a temp fix until there is something better to check.

Also made the error a bit more verbose.

Duh, updated the message but not the test.....