Skip to content
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.

Sign up for GitHub

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

Add item to GROUP BY for new MySQL 5.7 default compatibility. #236

Merged
merged 3 commits into from
Apr 4, 2018
Merged

Add item to GROUP BY for new MySQL 5.7 default compatibility. #236

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99617ad
Add item to GROUP BY for new MySQL 5.7 default compatibility.
t4k Jun 13, 2017
fe3cf1b
Fix another GROUP BY issue.
t4k Jun 13, 2017
606a9d5
Revert "Fix another GROUP BY issue."
t4k Jun 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions organizations/admin/classes/domain/Organization.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,7 +543,7 @@ public function search($whereAdd, $orderBy, $limit){
LEFT JOIN ContactRoleProfile CRP ON C.contactID = CRP.contactID
LEFT JOIN ContactRole CR ON CR.contactRoleID = CRP.contactRoleID
" . $whereStatement . "
GROUP By O.organizationID
GROUP BY O.organizationID, OHP.parentOrganizationID
ORDER BY " . $orderBy . $limitStatement;


Expand Down Expand Up @@ -639,7 +639,7 @@ public function getAlphabeticalList(){
$alphArray = array();
$result = mysqli_query($this->db->getDatabase(), "SELECT DISTINCT UPPER(SUBSTR(TRIM(LEADING 'The ' FROM name),1,1)) letter, COUNT(SUBSTR(TRIM(LEADING 'The ' FROM name),1,1)) letter_count
FROM Organization O
GROUP BY SUBSTR(TRIM(LEADING 'The ' FROM name),1,1)
GROUP BY SUBSTR(TRIM(LEADING 'The ' FROM name),1,1), O.name
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change doesn't produce what we want. This is trying to find which letters of the alphabet are represented by existing Organizations, and how many Organizations start with that letter. This change causes "letter_count" to be 1 for each letter. I don't have an easy test environment with MySQL 5.7, but I suggest trying "GROUP BY UPPER(SUBSTR(TRIM(LEADING 'The ' FROM name),1,1))" (that is, adding the "UPPER").

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. It was working in my UI and I was hung up on the solution to the other issue in this pull request, but your approach is working and correct.

I’m going to remove this commit from this PR because it doesn’t have anything to do with the fatal error. I’ll add the fix to a different PR for Organizations where I am fixing some other issues.

ORDER BY 1;");

while ($row = mysqli_fetch_assoc($result)){
Expand Down