In modules/rooms_booking/includes/rooms_booking.availability_agent.inc there is a comparison function starting at line 306 function comparebyOptionals. This is called just above in the function orderbyOptionals, to sort the units by number of options set, so that the user is shown the units with the most options first in the list. The last clause in comparebyOptionals contains a typo: 1st $a should be $b (otherwise the clause contains two $a, and the unit is compared with itself, rather than the next in the array).

For my use case, I prefer to have units offered in ascending order or unit_id, and this is what happens if units do not have any options set. However the if there are two units with matching highest number of options, the units are presented in descending order of unit_id, but subsequent pairs are sorted in ascending order of unit_id. I tried to find a way to do a nested sort (descending number of options and ascending unit_id) but could not get my head around how to achieve this. I discovered that running the uasort function twice rather than just once fixes this, although there is probably a more elegant way to do this. With these fixes, the comparison function works as expected.

Modified code

function orderbyOptionals($units) {
    foreach ($units as $type => $v) {
      foreach ($v as $price => $value) {
        uasort($value, 'comparebyOptionals');
        uasort($value, 'comparebyOptionals');
        $units[$type][$price] = $value;
      }
    }

    return $units;
  }

}

function comparebyOptionals($a, $b) {
  if (!isset($b['unit']->rooms_booking_unit_options[LANGUAGE_NONE]) && !isset($a['unit']->rooms_booking_unit_options[LANGUAGE_NONE])) {
    return 0;
  }

  if (!isset($b['unit']->rooms_booking_unit_options[LANGUAGE_NONE]) && isset($a['unit']->rooms_booking_unit_options[LANGUAGE_NONE])) {
    return -1;
  }

  if (isset($b['unit']->rooms_booking_unit_options[LANGUAGE_NONE]) && !isset($a['unit']->rooms_booking_unit_options[LANGUAGE_NONE])) {
    return 1;
  }

  return count($b['unit']->rooms_booking_unit_options[LANGUAGE_NONE]) < count($a['unit']->rooms_booking_unit_options[LANGUAGE_NONE]) ? -1 : 1;
}
CommentFileSizeAuthor
#6 screenshot.2.png57.46 KB1an_m
#6 screenshot.1.png28.71 KB1an_m
#3 sort_by_options-1994880-3.patch1.43 KB1an_m
#2 sort_by_options-1994880-2i.patch1.64 KB1an_m
#1 sort_by_options-1994880-2.patch978 bytes1an_m

Comments

1an_m’s picture

1an_m commented
StatusFileSize
new sort_by_options-1994880-2.patch978 bytes

Patch for above changes

1an_m’s picture

1an_m commented
Title: Typo in sort function. » Improved comparebyOptionals function.
StatusFileSize
new sort_by_options-1994880-2i.patch1.64 KB

Changed title to better reflect thread.

Attachment is improved comparebyOptionals function.

Previously, function did not properly address case where number of options set compared as equal. This is fixed in this patch, with expanded tests for equality, and sort by unit_id if units have equal number of options set.

Please consider including in module.

(note: patch name is non-standard because I goofed in naming the previously attached patch)

1an_m’s picture

1an_m commented
Issue summary: View changes
Status: Active » Needs review
StatusFileSize
new sort_by_options-1994880-3.patch1.43 KB

Updated patch to current dev.
Removed some whitespace errors.

Could this be considered for inclusion?

  • Commit 2ff1072 on master, 7.x-1.x by plopesc: 
    Issue #1994880 by 1an_m: Improved comparebyOptionals function.
  • Commit d462ce7 on master, 7.x-1.x by ronald_istos:
    Merge pull request #48 from plopesc/issue-1994880

Issue #1994880 by...
ronald_istos’s picture

ronald_istos commented
Status: Needs review » Closed (fixed)
1an_m’s picture

1an_m commented
StatusFileSize
new screenshot.1.png28.71 KB
new screenshot.2.png57.46 KB

In the current dev version, the compareByOptionals function has been updated.

protected static function compareByOptionals($a, $b) {
    $a_items = is_array(field_get_items('rooms_unit', $a['unit'],'rooms_booking_unit_options')) ? field_get_items('rooms_unit', $a['unit'],'rooms_booking_unit_options') : array();
    $b_items = is_array(field_get_items('rooms_unit', $b['unit'],'rooms_booking_unit_options')) ? field_get_items('rooms_unit', $b['unit'],'rooms_booking_unit_options') : array();

    if (count($a_items) == count($b_items)) {
      return $a['unit']->unit_id < $b['unit']->unit_id ? 1 : -1;
    }
    else {
      return count($a_items) < count($b_items) ? 1 : -1;
    }
  }

I have created some units for testing purposes
unit_id = 1 has 2 options
unit_id = 2 has 3 options
unit_id = 3 to 12 all have 1 option only

The comparison in the 'else' clause works as intended (units are sorted according to number of options, with the greatest number of options are presented at the top of the list) see screenshot.1
However, the preceding clause which is intended to sort by unit_id if the number of options are the same, sorts from highest unit_id to lowest. see screenshot.2 (I have modified the view to show unit_id)
I have not been able to find a way of modifying the code so that units with equal number of options are secondarily sorted from lowest unit_id to highest, which makes me wonder if the test for equivalence is never true for some reason.

Any help with this would be greatly appreciated.

1an_m’s picture

1an_m commented

I found a way to sort the units by increasing unit_id (as a secondary sort, after primary sort of most to least options per unit), instead of the default code which sorts by decreasing unit_id. In the end it was quite simple, simply change line 308 in rooms_availability.booking_event.inc from

return $a['unit']->unit_id < $b['unit']->unit_id ? 1 : -1;

to

return $b['unit']->unit_id < $a['unit']->unit_id ? 1 : -1;