-
-
Notifications
You must be signed in to change notification settings - Fork 436
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[BUGFIX] Count doesn't work with group by columns. This fix keeps the group by #1404
Conversation
Thank you, looks like a good addition. |
…ection. When using definition term and definition description this items should be located in a definition list.
The second commit contains a fix for the onepage checkout. The base template used a fieldset as wrapper for payment methods in checkout. That is not correct because payment methods are defined as dt and dd. |
Good observation, @Flyingmana, the group part is usually quoted (in $group = array();
foreach ($this->_parts[self::GROUP] as $term) {
$group[] = $this->_adapter->quoteIdentifier($term, true);
}
$sql .= ' ' . self::SQL_GROUP_BY . ' ' . implode(",\n\t", $group); |
@colinmollenhour What would you suggest or what would you change? |
Can you please remove the commits that are unrelated? |
…yments section. When using definition term and definition description this items should be located in a definition list." This reverts commit 345ab99.
@colinmollenhour Done |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
@Flyingmana Could you take a look? |
Some extensions like the Amasty Product Alerts Reports didn't worked correct because they use some group by statements in their queries. I've attached two screenshots.
Here is a stock alerts report before the fix
Here is a stock alert report after the fix
The second commit contains a fix for the onepage checkout. The base template used a fieldset as wrapper for payment methods in checkout. That is not correct because payment methods are defined as dt and dd.