Moodle

modified original post to use the correct sort method. Should be uasort() to maintain the keys.

Hello! I have made a patch with the suggested solution. To apply the patch please use:

cd moodle
patch p0 < patchMDL-18600.dff

The patch was made against the current CVS code tagged as MOODLE_19_WEEKLY.

(Without looking at the code) my questions for this patch would be:

a) Could this sort not be done in some earlier sql sort?
b) Does the string sorting work with utf8? (I've never looked into this)

The proposed fix may not sort correctly in some cases. For example, a student with the name "Li, ted" (who happened to enter his name with a lower case "t") would be sorted after "Lim, Jack", instead of before.

Dan:

a) Someone with more experience in the Moodle code should be able to properly answer the question. However, by looking into it myself, the solution might be to simply change the $sort argument given to get_users_by_capability() function. Now sorting is 'u.firstname ASC'. That could be changed to 'u.lastname ASC, u.firstname ASC'.

However, note that the reporter mentions that users who have not answered are listed sorted by first name, but those who responded are unordered. This might mean that the code in the choice_get_response_data() changes the order, so even if we change $sort for get_users_by_capability() we do not properly fix the problem.

I do not have sufficient experience with the code, to make other drastic changes, to the way $allresponses array is populated.

b) The strcmp() and strcasecmp() functions do not support utf8 as we would like. For example my last name (?ucan) ends up being after some last name starting with Z/z.

The only way is to convert the strings to ascii, using character collation (?ucan becomes Sucan). Then strcasecmp() would work fine after that.

The problem is the mbstring functions do not provide support for string collation (AFAIK). Based on experience, iconv() allows string collation. Can I use it?

Hubert:

Good point, thanks. I switched to strnatcasecmp() which does natural order sorting, and it's case insensitive.

Attached updated patch.

The problem is not just the case sensitivity issue (although that could be considered a problem too). If there are three students, "Li, bob", "Li, ted", and "Lim, jack", then the patch will sort them as: "Li, bob", "Lim, jack", "Li, ted", instead of the correct: "Li, bob", "Li, ted", "Lim, jack".

Hi Guys - Dan P's comments above are correct -this should be fixable updating the sort order of the sql - Mihai - your solution of ordering by 'u.lastname ASC, u.firstname ASC' should be correct, and the code shouldn't be updating the order anywhere else. I'll test this later today and push the change upstream.

thanks!

Dan

fix now in 19_STABLE and HEAD - thanks for the report and fix!

I appreciate the discussion and critique of my original solution. I usually work exclusively with the USA alphabet, so I don't think about encoding in other languages. Besides that (as Hubert points out), it was wrong anyway. I should have known better than to do that.

I don't think that the changed sort argument in get_users_by_capability() is the complete solution, though.

The foreach() loop iterates over $rawresponses, which has elements in an order other than by name. Because the elements of $rawresponses are not sorted, when elements of $allresponses[0] are moved to the other elements of $allresponses (by calls to clone() and unset()), the other elements of $alresponses are ordered by whatever order $rawresponses was in - not the sorted order of $allresponses[0].

By itself, modifying the SQL sort string sorts the users with no response, but does not sort the users who did respond.

I've looked at several discussions on various forums about sorting utf8 strings in PHP. The conclusion appears to be that it won't work reliably - especially on Windows servers (like mine). It looks like we do have to rely on the SQL sort string to get the order right.

The next problem is that $rawresponses uses a key that is not the userid (which is the key in $allresponses[0]). My new approach is: make a clone of $rawresponses with the userid as the key. Then run the foreach loop over $allresponses[0] (which is sorted the way we want it). For each user in $allresponses, if the user has a response in $rawresponses then move it to the correct part of $allresponses and remove it from $allresponses[0] as before.

It's not as pretty as I would like, but it works in more cases than my first - naive - try.

if ($rawresponses) {
$rawResponsesByUid = array();
foreach ($rawresponses as $resp) { $rawResponsesByUid[$resp->userid] = clone($resp); }

foreach ($allresponses[0] as $user) {
if (isset($rawResponsesByUid[$user->id])) { // This person has responded $response = $rawResponsesByUid[$user->id]; $allresponses[0][$response->userid]->timemodified = $response->timemodified; $allresponses[$response->optionid][$response->userid] = clone($allresponses[0][$response->userid]); unset($allresponses[0][$response->userid]); // Remove from unanswered column }
}
}

Is there a more direct way? What about doing a JOIN between table 'choice_answers' and table 'user' (joined on user id's), so that we can then sort on the user names? That seems better to me, but I don't know enough about the Moodle database calls to know how to do it. Are there any performance issues with JOIN on a large user list?

bkg

I just updated and the Not answered yet people are alphabetized...which is great. The answered response columns are not alphabetized and seem to be in no particular order.

It would look very nice if everything were alphabetized.

I found an example of what I wanted to do with a JOIN statement elsewhere in lib.php.

In function choice_get_response_data(), I replaced:

$rawresponses = get_records('choice_answers', 'choiceid', $choice->id);

with:

$rawresponses = get_records_sql("
SELECT
ca.* , u.lastname, u.firstname
FROM
{$CFG->prefix}choice_answers ca
INNER JOIN {$CFG->prefix}user u ON ca.userid=u.id
WHERE
ca.choiceid=$choice->id
ORDER BY u.lastname ASC, u.firstname ASC");

This works for me, and is cleaner that what I came up with last night.

Is there a problem with this?

bkg

thanks