Recent changes module sort by date, but not time

If I look at the recent changes list, I see that the time component is ignored (only the date is taken into account), but that the "sort by date, oldest first" and "sort by date, newest first" options are working OK.

Are you sure they need reversing - or have I missed something?

Caveat: I'm not able to see the code at the moment so the following is from memory!!

If the recent changes are sorted by date then I think it's more by luck that judgement - In the print_changes_list function $arr is the thing to be sorted and it's necessary to set the some 'dummy' entries to allow the sort routines to work on this.

1. $arr[$n]['jd'] is currently set to 1 for name sorting and an incrementing number ($n) for date sorting
2. $arr[$n]['anniv'] is set to a number representing the change date/time

To make the date sort properly we need to sort on the ['anniv'] entry. To do this the ['jd'] entries need to match hence the change to always set $arr[$n]['jd'] to 1.

Having got the sort to work properly for dates, it seems that the acending/descending options are reversed, hence the need to move the array_reverse().

Dave

PS I've made the change on my site, you can see the result at http://webtrees.drury.me.uk

I simply don't know. My sorting, using SVN code, is already identical to yours, David.
I select: SORT BY DATE - NEWEST FIRST as the configuration and it works as expected.
Stephen

@Stephen: <<I simply don't know. My sorting, using SVN code, is already identical to yours, David.
I select: SORT BY DATE - NEWEST FIRST as the configuration and it works as expected.>>

Me too. I'm just not seeing any problem.

@Greg<<If I look at the recent changes list, I see that the time component is ignored (only the date is taken into account), but that the "sort by date, oldest first" and "sort by date, newest first" options are working OK.

Are you sure they need reversing - or have I missed something?>>

Greg, are you sure? On mine everything is in descending order by date AND time.

Just checking but you are displaying as a list rather than table aren't you?

Sorting within the table is unaffected

Dave

I do not use the LIST display for this information as it is not consistent with other blocks on the MY PAGE index page.
I do not use the recent changes on the HOME page since it brings with it a huge hit in privacy calculations and frankly, IMO, is no one else's business but my users.
I do insert a LAST CHANGED/MODIFIED datestamp within the FAMILY TREE STATS block, so anyone looking can see that the family tree is being modified.
-Stephen

HOWEVER, I can confirm David's bug, if I were to place a LIST formatted block on the Home page.

On "My Page" the sorting on my site (running latest svn) is identical whether using list or table display.

Like Stephen I would never put this data on the Home page. I query whether it should even be an option.

(Getting away from the bug details)

I don't usually display this module on the home page but having posted a link to my site earlier in this thread It dawned on me that you'd have to log in to see it. So I added it temporarily to the home page. In general I also question whether it should be permitted to display this on the home page.

As to the bug, I still insist that the code as it stands does not sort the data properly. If the results looks ok then the natural order of the data must be (more or less) already in date order.

I've had another look at this - and I still cannot reproduce the problem. I have

1) added a "recent changes" block to my home page
2) selected "display as list"
3) selected "sort by date, newest first"

I see a number of changes, which are all sorted as expected (by both date and time).

I changed the sort to "sort by date, oldest first", and it still sorts as expected.

If it's ok by you, I'll try to walk through the code again, but I can't do it for a few days. Off the top of my head I can't remember the exact problem I encountered (apart from the sort not working)

Dave

OK,

I think I can see why some people cannot see a problem and others (me!!) can.

1. There is definitely a problem with the sort logic in function print_changes_list (line 1308 in file print_changes_lists.php) that means the sort by date is broken - it actually sorts by the value of the variable $n; this is incremented as each element of the input array is processed.

2. This function is called from the module with the 1st parameter being an array of individual ids that meet the changed date criteria and this array IS ALREADY sorted in descending date order. therefore the broken sort which actually does nothing doesn't matter.

3. If the sort order within the module is changed to "sort by date, oldest first" then the broken sort routine now does wieird things depending on the sequencing of the already sorted array.

I'll try to produce a paper with a sequence of dates that show the problem.

In the mean time, the revised function I attached near the beginning of this bug really does work

Dave

Greg, this is clearly a difficult problem to reproduce. I'd be tempted to reverse normal logic and ask "Is there any reason NOT to make the amendment Dave suggests? It looks fine to me, so I'm inclined to implement it, especially as we have only a day or so now before next release.

Worst case scenario would be a tiny minority of users rarely seeing a problem, as (I believe) displaying recent changes in list format rather than table is not a common choice.

<<I'd be tempted to reverse normal logic and ask "Is there any reason NOT to make the amendment Dave suggests?>>

I dislike simply "covering up" bugs, and leaving the root cause unknown.

OK,
Lets see if I can demonstrate why the current sort is broken.

Below are two segments of code; function print_changes_list (in functions_print_lists.php line 1308) is called directly from the module with parameter 1 being a list of IDs (not only individuals as I said earlier but family, media etc.) meeting the required criteria and lets also assume the $sort parameter is set to either 'date_asc' or 'date_desc'. Function event_sort is found in functions.php at line 988.

The foreach loop at line 5 builds the array $arr needed later; at line 12 $arr[$n]['jd'] is assigned the value of $n (bacause $sort does not equal 'name' (the value of $n is initially 0 and is incremented each time through the foreach at line 14)

When the foreach has completed, the switch statements for one of the date cases will be invoked thus 'uasort'ing the array $arr with event_sort as the compare function. Line 29 checks if $a['jd'] == $b['jd']. They cannot be equal as demonstrated above (they were each assigned a different value of $n), therefore the else statement at line 36 is executed and the result of comparing the two numbers is returned. The code that would actually compare the change dates at lines 30-38 is never invoked.

1. function print_changes_list($change_ids, $sort, $show_parents=false) {
2. global $SHOW_MARRIED_NAMES;
3. $n = 0;
4. $arr=array();
5. foreach ($change_ids as $change_id) {
6. $record = WT_GedcomRecord::getInstance($change_id);
7. if (!$record || !$record->canDisplayDetails()) {
8. continue;
9. }
10. // setup sorting parameters
11. $arr[$n]['record'] = $record;
12. $arr[$n]['jd'] = ($sort == 'name') ? 1 : $n;
13. $arr[$n]['anniv'] = $record->LastChangeTimestamp(false, true);
14. $arr[$n++]['fact'] = $record->getSortName(); // in case two changes have same timestamp
15. }
16.
17. switch ($sort) {
18. case 'name':
19. uasort($arr, 'event_sort_name');
20. break;
21. case 'date_asc':
22. uasort($arr, 'event_sort');
23. $arr = array_reverse($arr);
24. break;
25. case 'date_desc':
26. uasort($arr, 'event_sort');
27. }

 etc...

28. function event_sort($a, $b) {
29. if ($a['jd']==$b['jd']) {
30. if ($a['anniv']==$b['anniv']) {
31. return utf8_strcasecmp($a['fact'], $b['fact']);
32. }
33. else {
34. return $a['anniv']-$b['anniv'];
35. }
36. } else {
37. return $a['jd']-$b['jd'];
38. }
39.}

Now whilst the code I submitted earlier does work, I realised last night that it is unnecessarily complicated, I therefore now submit a revised print_changes_list function and also a modified module.php which introduces an extra sort option to allow sorting by name A-Z or name Z-A. Should you not wish to change the module, the changes to the print_changes_list function will still work

Phew!
Dave

Added the wrong files to comment 17 - managed to delete them but couldn't find out how to replace with correct versions so added here

The data is fetched from the DB in get_recent_changes(), which sorts the results by the date of the CHAN record (but not the time).

Then, in print_changes_list(), we populate the ['jd'] field with $n. This simply maintains the sort order, as the original $jd value is no longer available.

Then, to sort by date, we use event_sort() - which sorts by the ['jd'] value.

This sort is, I guess, redundant. We simply want to maintain the original sort order, as it was sorted by the database.

We only need to apply a secondary sort if the user has selected "sort by name".

I can see how this might get the *time* portion of the sort wrong, but not the *date*. This was the point I made right at the begninning in post #2.

You have been me that the *date* is wrong, and
(a) you have not provided an example
(b) it doesn't happen for me
(c) I can't see from the code how this would be possible

But, if this is the case (date correct, time incorrect), then a better fix is surely something like fetching the list of changes from the wt_change table (which has a timestamp) instead of the wt_date table (which has only a date).

This is why I am keen to understand the actual problem.

Ah,

I see the point of confusion (mine!). When I've said sort by date, I've meant sort by date/time and you're correct in that the errors I've seen have been in the time element.

sorry about that

Dave

[Expired for webtrees because there has been no activity for 60 days.]

Fix released in webtrees 1.2.7