search

twomice / com.joineryhq.jsumfields

Provides additional fields under the Summary Fields framework.
Other
1 stars 10 forks source link

Fix DB Error unknown field $yearOffset #14

Closed seamuslee001 closed 3 years ago

seamuslee001 commented 3 years ago

On a client site when adding a new contribution I was getting a DB error unknown column $yearOffset in ON clause. This fixes this. The problem was that some of the SQL was quoted in single quotes which meant that it was treating $yearOffset as literal rather than as a variable.

@twomice ping @joeMurray

twomice commented 3 years ago

Looks like this was a regression in e9f6ae66 (submitted by @lucky091588 and merged by me). @seamuslee001 I'm concerned that there may be others from that commit; will look and get back to you.

JoeMurray commented 3 years ago

Thanks, @twomice .

twomice commented 3 years ago

@seamuslee001 Could you try #17 and let me know if it addresses your issue? I prefer that approach as it is a direct reversal of a regression, so I'm inclined to use that and would value your feedback. Thanks.

JoeMurray commented 3 years ago

@seamuslee001 just a style/maintainability question: wouldn't it be better to consistently use either {$yearOffset} or ' . $yearOffset . ' rather than switch between them? I have a preference for the first style.

JoeMurray commented 3 years ago

Looking at https://github.com/twomice/com.joineryhq.jsumfields/pull/17 I think that might be the best if it works. @seamuslee001 will report back when testing complete. Thanks @twomice and @lucky091588 .