search

symphonists / association_field

Association Field for Symphony CMS
http://symphonyextensions.com/extensions/association_field/
Other
8 stars 5 forks source link

mySQL 5.7 compatibility, add sortfield in distinct select #21

Closed jonmifsud closed 8 years ago

jonmifsud commented 8 years ago

It seems like mySQL 5.7.10 throws errors everytime there is a distinct with a sort clause where the sort is not included in the select, this just adds the sort order within the query select statement.

brendo commented 8 years ago

I've been noticing this as well. You can disable it at a server level if you need to, which is ultimately what I did.

jonmifsud commented 8 years ago

So far I only encountered two places where I need to make this fix, this and one in the core.

I'm not sure if it's best practice that we start asking people to disable things from MySQL so making small changes like this might make sense in the long term.

On Wed, 20 Jan 2016 at 08:55 Brendan Abbott notifications@github.com wrote:

I've been noticing this as well. You can disable it at a server level if you need to, which is ultimately what I did.

— Reply to this email directly or view it on GitHub https://github.com/symphonists/association_field/pull/21#issuecomment-173122098 .

brendo commented 8 years ago

I'm not sure if it's best practice that we start asking people to disable things from MySQL so making small changes like this might make sense in the long term.

Oh definitely, I'm not suggesting we do, but if you're in a pinch now that's a solution :)

jonmifsud commented 8 years ago

Ah :) yeah it's kinda working now I made a fix yesterday to the core - not sure it's great but works locally so far. I'd like to go through a few more of the sites to ensure that all the sites/functionality are working as they should before submitting it as a PR. It's only on my local setup right now so not a prod issue, that's a bit more behind.

On Wed, 20 Jan 2016 at 08:58 Brendan Abbott notifications@github.com wrote:

I'm not sure if it's best practice that we start asking people to disable things from MySQL so making small changes like this might make sense in the long term.

Oh definitely, I'm not suggesting we do, but if you're in a pinch now that's a solution :)

— Reply to this email directly or view it on GitHub https://github.com/symphonists/association_field/pull/21#issuecomment-173122500 .

jonmifsud commented 8 years ago

@nitriques any chance we could release this? very minor fix.

nitriques commented 8 years ago

@jonmifsud please create an entry in the meta.xml file and I'll release ASAP ! Thanks!

jonmifsud commented 8 years ago

@nitriques I've bumped the version and added a new delegate. I might be re-thinking my previous version for the 'association' filtering drawer.

The new delegate is mainly used for relationship filtering - when there are permissions and doing that via other means is not secure if you have multiple users on the same backend. I might want to give some more thought on other changes - including core ones on previous pull requests maybe I figure out a cleaner way of doing it.

nitriques commented 8 years ago

Ok. But should I release this?

jonmifsud commented 8 years ago

@nitriques this should be fine.

nitriques commented 8 years ago

16e1c30 was sitting in integration which is basically the same fix as eb463ba.

Cherry picked 806fb97 as c886dca. Released as 1.1.0. Thanks. Sorry for the mega delay.