#442 Overlapping bookings cause incorrect table views

[dwpoon]

The existence of overlapping bookings causes the tables for day and week views to be laid out incorrectly. Bookings are rendered as <td> elements with incorrect rowspan attributes, such that bookings that occur later than the overlapped bookings may appear in the wrong column. In day view, they appear in the wrong room; in week view, they appear on the wrong day. The table may also have missing or protruding cells.

To reproduce

1. Using MRBS 1.5.0, with the default 30-minute resolution…
2. Create an area with three rooms ("Alfa", "Bravo", "Charlie").
3. Make a booking ("X") in Alfa on October 15, 12:00 – 13:00.
4. Make a booking ("Y") in Bravo on October 15, 13:00 – 14:00.
5. Make a booking ("Z") in Alfa for 12:00 – 14:00 starting October 1, recurring weekly, ending October 8.
6. Edit series, and change the recurrence end date to October 15. (MRBS 1.5.0 allows this with no warnings. MRBS 1.7.3 has fixed this conflict-detection bug, and won't allow it.)

The resulting Day view for October 15 shows:

• A double-booking of X and Z for the 12:00 time slot in room Alfa (which was unfortunately allowed by MRBS 1.5.0, but at least it is correctly rendered).
• A double-booking of X and Z for the 12:30 time slot in Alfa… but only Z is labelled‽
• No indication that room Alfa is still booked for X from 13:00 to 14:00!
• Booking Y looks like it's in room Alfa from 13:00 to 14:00! (But if you click on it, it shows that it's for room Bravo, as it should be.)
• In the column for room Charlie, the 13:00 and 13:30 slots are missing their table cells!

After upgrading to MRBS 1.7.3, the table remains incorrectly rendered.

After upgrading to hg rev 5200:1a4495777121 (default branch), the Day view for October 15 is wrong in a different way:

• A double-booking of X and Z for the 12:00 time slot (unfortunate but correctly rendered)
• No booking at all for room Alfa for the 12:30 time slot!
• A continuation of booking Z in room Alfa during time slots 13:00 (correct), 13:30 (correct), and 14:00 (!)
• Booking Y in room Bravo from 13:00 to 14:00 (correct).
• An extra table cell past the right of room Charlie for the 14:00 time slot!

[dwpoon]

The table layout code was buggy in 1.5.0. It was revised sometime between 1.5.0 and 1.7.3, and is still buggy. It has been revised again since 1.7.3 in the default branch, and is buggy in a different way.

The fundamental problem is that the map_add_booking() function is too complicated: it's trying to adjust entries to accommodate conflicts while it's building the $column data structure, before all of the entries have been laid out. Considering all the types of interval overlaps that could occur (A starts in the middle of B, B starts in the middle of A, B resides entirely within A, B and C reside within A, etc.), it's impossible to figure out how to resolve the conflicts on the fly.

A saner approach would be to fill out all the time slots with the relevant entries, then coalesce the entries that span consecutive time slots. I've attached patches for MRBS 1.7.3 and for hg rev 5200 that fix the rendering.

[Campbell Morrison]

Thanks. I'll take a look next week.

[dwpoon]

Here's a PHP page that works with MRBS 1.7.3 that displays future conflicts that went undetected in MRBS 1.5.0.

<?php

require_once "$MRBS_ROOT/defaultincludes.inc";
require_once "$MRBS_ROOT/mrbs_sql.inc";

$future = MRBS\db()->query("
    SELECT id
        FROM $tbl_entry
        WHERE start_time >= extract(epoch FROM now())
        AND repeat_id IS NOT NULL
        ORDER BY start_time;
    ", array());

MRBS\print_header();
echo "<ul>\n";
while (($id = $future->row_keyed(0)) && $id && ($id = $id['id'])) {
    $booking = MRBS\get_entry_by_id($id);
    if ($conflicts = MRBS\mrbsCheckFree($booking, $id, $booking['repeat_id'])) {
        echo "<li>Series <a href=\"$url_base/view_entry.php?id=$id\">"
           . htmlspecialchars($booking['name'])
           . "</a> conflicts with:\n"
           . "<ul>\n";
        foreach ($conflicts as $c) {
            echo "<li>$c</li>\n";
        }
        echo "</ul></li>\n";
    }
}
echo "</ul>\n";
unset($future);
MRBS\print_footer();

[Campbell Morrison]

Many thanks. I've now committed the fix to the default branch in 897823. I haven't yet decided whether to commit the mrbs_17_branch patch. I may leave it for the moment and if anybody sufffers from this bug they can upgrade to the default branch.