From f180a9bd59def3bfe44b91c6c34917e338bb4906 Mon Sep 17 00:00:00 2001
From: Elmar Ludwig <elmar.ludwig@uni-osnabrueck.de>
Date: Mon, 8 Nov 2021 17:43:05 +0100
Subject: [PATCH] fix checks for overlapping bookings, fixes #251
---
app/controllers/course/room_requests.php | 5 +-
app/controllers/resources/export.php | 6 +--
app/controllers/resources/statistics.php | 4 +-
app/controllers/room_management/planning.php | 6 +--
.../resources/ResourceBooking.class.php | 37 +++----------
.../resources/ResourceRequest.class.php | 44 ++++-----------
lib/resources/RoomManager.class.php | 54 ++-----------------
7 files changed, 23 insertions(+), 133 deletions(-)
diff --git a/app/controllers/course/room_requests.php b/app/controllers/course/room_requests.php
index 100dad5cdb3..8747e9b4ab5 100644
--- a/app/controllers/course/room_requests.php
+++ b/app/controllers/course/room_requests.php
@@ -630,10 +630,7 @@ class Course_RoomRequestsController extends AuthenticatedController
$request_dates_booked = 0;
foreach ($request_time_intervals as $interval) {
$booked = ResourceBookingInterval::countBySql(
- 'resource_id = :room_id
- AND
- (begin BETWEEN :begin AND :end
- OR end BETWEEN :begin AND :end)',
+ 'resource_id = :room_id AND begin < :end AND end > :begin',
[
'room_id' => $room->id,
'begin' => $interval['begin'],
diff --git a/app/controllers/resources/export.php b/app/controllers/resources/export.php
index 87d6d8a0df0..299cd3a706d 100644
--- a/app/controllers/resources/export.php
+++ b/app/controllers/resources/export.php
@@ -325,11 +325,7 @@ class Resources_ExportController extends AuthenticatedController
foreach ($resources as $resource) {
//Retrieve the bookings in the specified time range:
$intervals = ResourceBookingInterval::findBySql(
- '`resource_id` = :resource_id
- AND
- (`begin` BETWEEN :begin AND :end
- OR `end` BETWEEN :begin AND :end)
- ORDER BY `begin` ASC, `end` ASC',
+ 'resource_id = :resource_id AND begin < :end AND end > :begin ORDER BY begin, end',
[
'resource_id' => $resource->id,
'begin' => $this->begin->getTimestamp(),
diff --git a/app/controllers/resources/statistics.php b/app/controllers/resources/statistics.php
index 5c8b1744085..6ae55e11d2d 100644
--- a/app/controllers/resources/statistics.php
+++ b/app/controllers/resources/statistics.php
@@ -55,9 +55,7 @@ class Resources_StatisticsController extends AuthenticatedController
WHERE
rb.booking_type IN ( :types )
AND
- (rbi.begin BETWEEN :begin AND :end)
- OR
- (rbi.end BETWEEN :begin AND :end);"
+ rbi.begin < :end AND rbi.end > :begin"
);
$sum_stmt->execute(
diff --git a/app/controllers/room_management/planning.php b/app/controllers/room_management/planning.php
index ecfba9ee74e..06a460b00b4 100644
--- a/app/controllers/room_management/planning.php
+++ b/app/controllers/room_management/planning.php
@@ -1197,11 +1197,7 @@ class RoomManagement_PlanningController extends AuthenticatedController
ON resource_booking_intervals.booking_id = rb.id
WHERE rb.internal_comment <> ''
AND rb.resource_id IN ( :room_ids )
- AND (
- resource_booking_intervals.begin BETWEEN :begin AND :end
- OR
- resource_booking_intervals.end BETWEEN :begin AND :end
- )
+ AND resource_booking_intervals.begin < :end AND resource_booking_intervals.end > :begin
ORDER BY resource_booking_intervals.begin ASC, resource_booking_intervals.end ASC",
[
'room_ids' => $this->room_ids,
diff --git a/lib/models/resources/ResourceBooking.class.php b/lib/models/resources/ResourceBooking.class.php
index e12ce922d95..4534afc35b0 100644
--- a/lib/models/resources/ResourceBooking.class.php
+++ b/lib/models/resources/ResourceBooking.class.php
@@ -730,8 +730,7 @@ class ResourceBooking extends SimpleORMap implements PrivacyObject, Studip\Calen
if ($derived_resource->userHasPermission($this->booking_user, 'tutor', [$current_begin, $current_end])) {
//Sufficient permissions to delete bookings
//in the time frame.
- $delete_sql = 'begin < :end AND end > :begin
- AND resource_id = :resource_id ';
+ $delete_sql = 'begin < :end AND end > :begin AND resource_id = :resource_id ';
$sql_params = [
'begin' => $current_begin->getTimestamp(),
'end' => $current_end->getTimestamp(),
@@ -763,8 +762,7 @@ class ResourceBooking extends SimpleORMap implements PrivacyObject, Studip\Calen
} else {
$derived_resource = $this->resource->getDerivedClassInstance();
if ($derived_resource->userHasPermission($this->booking_user, 'autor', [$real_begin, $end])) {
- $delete_sql = 'begin < :end AND end > :begin
- AND resource_id = :resource_id ';
+ $delete_sql = 'begin < :end AND end > :begin AND resource_id = :resource_id ';
$sql_params = [
'begin' => $real_begin->getTimestamp(),
'end' => $end->getTimestamp(),
@@ -959,27 +957,11 @@ class ResourceBooking extends SimpleORMap implements PrivacyObject, Studip\Calen
public function isRepetitionInTimeframe(DateTime $begin, DateTime $end)
{
return ResourceBookingInterval::countBySql(
- "booking_id = :booking_id
- AND (
- (begin BETWEEN :begin AND :end)
- OR
- (end BETWEEN :begin AND :end)
- OR
- (begin < :begin AND end > :end)
- )
- AND NOT (
- (begin BETWEEN :booking_begin AND :booking_end)
- OR
- (end BETWEEN :booking_begin AND :booking_end)
- OR
- (begin < :booking_begin AND end > :booking_end)
- )",
+ 'booking_id = :booking_id AND begin < :end AND end > :begin',
[
'booking_id' => $this->id,
'begin' => $begin->getTimestamp(),
- 'end' => $end->getTimestamp(),
- 'booking_begin' => $this->begin,
- 'booking_end' => $this->end
+ 'end' => $end->getTimestamp()
]
) > 0;
}
@@ -1313,14 +1295,7 @@ class ResourceBooking extends SimpleORMap implements PrivacyObject, Studip\Calen
}
return ResourceBookingInterval::findBySql(
- 'booking_id = :booking_id
- AND
- (
- (begin >= :begin AND begin <= :end)
- OR
- (end >= :begin AND end <= :end)
- )
- ORDER BY begin ASC, end ASC',
+ 'booking_id = :booking_id AND begin < :end AND end > :begin ORDER BY begin, end',
[
'booking_id' => $this->id,
'begin' => $begin->getTimestamp(),
@@ -1798,7 +1773,7 @@ class ResourceBooking extends SimpleORMap implements PrivacyObject, Studip\Calen
if ($end instanceof DateTime) {
$end = $end->getTimestamp();
}
- $sql .= "AND begin <= :end AND :begin <= end";
+ $sql .= "AND begin < :end AND :begin < end";
$sql_array['begin'] = $begin;
$sql_array['end'] = $end;
}
diff --git a/lib/models/resources/ResourceRequest.class.php b/lib/models/resources/ResourceRequest.class.php
index 9782b59636a..9ac498342b3 100644
--- a/lib/models/resources/ResourceRequest.class.php
+++ b/lib/models/resources/ResourceRequest.class.php
@@ -247,18 +247,9 @@ class ResourceRequest extends SimpleORMap implements PrivacyObject, Studip\Calen
//Now we build the SQL snippet for the time intervals.
//These are repeated four times in the query below.
- //First we use one template ($time_sql_template) and
- //replace NUMBER with our counting variable $i.
//BEGIN and END are replaced below since the columns for
//BEGIN and END are different in the four cases where we
//repeat the SQL snippet for the time intervals.
- $time_sql_template = '(
- BEGIN BETWEEN :beginNUMBER AND :endNUMBER
- OR
- END BETWEEN :beginNUMBER AND :endNUMBER
- OR
- BEGIN <= :beginNUMBER AND END >= :endNUMBER
- ) ';
$time_sql = '';
if ($time_ranges) {
@@ -269,11 +260,7 @@ class ResourceRequest extends SimpleORMap implements PrivacyObject, Studip\Calen
if ($i > 1) {
$time_sql .= ' OR ';
}
- $time_sql .= str_replace(
- 'NUMBER',
- $i,
- $time_sql_template
- );
+ $time_sql .= sprintf('BEGIN < :end%d AND END > :begin%d ', $i, $i);
$sql_params[('begin' . $i)] = $time_range['begin'];
$sql_params[('end' . $i)] = $time_range['end'];
@@ -288,6 +275,7 @@ class ResourceRequest extends SimpleORMap implements PrivacyObject, Studip\Calen
//to a date, a metadate or a course.
//This is done in the rest of the SQL query:
+ // FIXME this subselect looks unnecessarily complex
$whole_sql = 'resource_requests.id IN (
SELECT id FROM resource_requests
WHERE
@@ -298,11 +286,8 @@ class ResourceRequest extends SimpleORMap implements PrivacyObject, Studip\Calen
['(CAST(begin AS SIGNED) - preparation_time)', 'end'],
$time_sql
)
- . (
- $closed_status_sql
- ? $closed_status_sql
- : ''
- ) . '
+ . $closed_status_sql
+ . '
UNION
SELECT id FROM resource_requests
INNER JOIN termine USING (termin_id)
@@ -317,11 +302,8 @@ class ResourceRequest extends SimpleORMap implements PrivacyObject, Studip\Calen
],
$time_sql
)
- . (
- $closed_status_sql
- ? $closed_status_sql
- : ''
- ) . '
+ . $closed_status_sql
+ . '
UNION
SELECT id FROM resource_requests
INNER JOIN termine USING (metadate_id)
@@ -336,11 +318,8 @@ class ResourceRequest extends SimpleORMap implements PrivacyObject, Studip\Calen
],
$time_sql
)
- . (
- $closed_status_sql
- ? $closed_status_sql
- : ''
- ) . '
+ . $closed_status_sql
+ . '
UNION
SELECT id FROM resource_requests
INNER JOIN termine
@@ -356,11 +335,8 @@ class ResourceRequest extends SimpleORMap implements PrivacyObject, Studip\Calen
],
$time_sql
)
- . (
- $closed_status_sql
- ? $closed_status_sql
- : ''
- ) . '
+ . $closed_status_sql
+ . '
GROUP BY id
) '
. $excluded_request_ids_sql;
diff --git a/lib/resources/RoomManager.class.php b/lib/resources/RoomManager.class.php
index 0932d453cee..f1a71a82353 100644
--- a/lib/resources/RoomManager.class.php
+++ b/lib/resources/RoomManager.class.php
@@ -408,20 +408,7 @@ class RoomManager
//Build the SQL query and the SQL parameters:
$sql = "INNER JOIN resource_bookings
ON resource_booking_intervals.booking_id = resource_bookings.id
- WHERE
- (
- (
- resource_booking_intervals.begin >= :begin
- AND
- resource_booking_intervals.begin <= :end
- )
- OR
- (
- resource_booking_intervals.end >= :begin
- AND
- resource_booking_intervals.end <= :end
- )
- ) ";
+ WHERE resource_booking_intervals.begin < :end AND resource_booking_intervals.end > :begin ";
$sql_array = [
'begin' => $begin->getTimestamp(),
'end' => $end->getTimestamp(),
@@ -746,44 +733,9 @@ class RoomManager
INNER JOIN resource_property_definitions rpd
ON resource_properties.property_id = rpd.property_id
WHERE TRUE"
- /*
- INNER JOIN seminare
- ON resource_bookings.range_id = seminare.seminar_id
- WHERE
- TRUE"
- /*
- (resource_categories.class_name = 'Room' OR TRUE)
- AND
- (
- (
- resource_bookings.begin >= :begin
- AND
- resource_bookings.repeat_end <= :end
- )
- OR
- (
- resource_bookings.begin >= :begin
- AND
- resource_bookings.end <= :end
- )
- OR TRUE
- )
- AND
- (rpd.name = 'seats' OR TRUE)
- AND
- (resource_properties.state < (
- SELECT COUNT(user_id) FROM seminar_user
- WHERE seminar_user.seminar_id = seminare.seminar_id
- ) OR TRUE) ORDER BY name ASC",
- [
- 'begin' => $begin->getTimestamp(),
- 'end' => $end->getTimestamp()
- ]
- */
);
return $found_bookings;
- //SimpleORMapCollection::createFromArray($found_bookings);
}
@@ -1011,8 +963,8 @@ class RoomManager
if ($i > 1) {
$sql .= 'AND ';
}
- $sql .= "(rbi.`begin` NOT BETWEEN :begin$i AND :end$i
- AND rbi.`end` NOT BETWEEN :begin$i AND :end$i)";
+ // FIXME this checks looks bogus to me
+ $sql .= "(rbi.begin >= :end$i OR rbi.end <= :begin$i)";
$sql_params["begin$i"] = $begin;
$sql_params["end$i"] = $end;
--
GitLab