← All changes | app/Services/BookingReportService.php +33 -78 2.4.0 → 2.5.0 View file →

		@@ -5,50 +5,31 @@
5	5	use FluentBooking\App\Models\Booking;
6	6	use FluentBooking\Framework\Support\Arr;
7	7
8	8	/**
9		- * Aggregate queries over bookings, and the one definition of "bookings this
10		- * user is allowed to count".
	9	+ * Aggregate queries over bookings, and the one definition of which bookings a
	10	+ * user may count: scoped(), i.e. calendar ownership plus host membership.
11	11	*
12		- * That second job is why this class exists. The dashboard held two different
13		- * answers to the same question: the widget numbers scoped on the
14		- * `fcal_booking_hosts` pivot, and the graph beneath them scoped on the
15		- * `host_user_id` column — so a limited host could read a smaller number from
16		- * the graph than from the widget directly above it. `scoped()` is now the
17		- * single answer for both, and it is the union of calendar ownership and host
18		- * membership (`Booking::whereHostAccess()`).
	12	+ * The schedules list does not use it yet. SchedulesController still filters on
	13	+ * host_user_id alone, so a calendar owner who is not the named host sees fewer
	14	+ * rows there. Changing that would alter a shipped list, so it is left as is.
19	15	*
20		- * The schedules list is NOT on it. `SchedulesController::buildSchedulesQuery()`
21		- * and `addCountsForFirstPage()` still filter on `host_user_id` alone, so a host
22		- * who owns a calendar but is not the named host on its bookings sees fewer rows
23		- * there than the widgets above now count. Moving that screen onto `scoped()`
24		- * would change a shipped list's contents, so it is left as a deliberate,
25		- * recorded divergence rather than folded in here.
	16	+ * Dimensions and metrics come from fixed maps of literal SQL in this file; no
	17	+ * caller-supplied string reaches the query.
26	18	*
27		- * Aggregation is dimension-and-metric based rather than free-form: callers pick
28		- * from a fixed set of group-by dimensions and metrics, both of which map to
29		- * literal SQL fragments held in this file. No caller-supplied string ever
30		- * reaches the query.
31		- *
32	19	* @since 2.2.6
33	20	*/
34	21	class BookingReportService
35	22	{
36		- /**
37		- * A year plus a day, so "the last 12 months" and "this calendar year"
38		- * both fit without the caller having to think about it.
39		- */
	23	+ // A year plus a day, so any 12-month or calendar-year range fits.
40	24	const MAX_RANGE_DAYS = 366;
41	25
42		- /**
43		- * Ceiling on returned groups. A report is a summary; a caller that needs
44		- * every row wants the bookings list, not this.
45		- */
	26	+ // A report is a summary; callers needing every row want the bookings list.
46	27	const MAX_GROUPS = 200;
47	28
48	29	/**
49		- * Group-by dimension => [SQL expression, result key]. `%offset%` is
50		- * replaced with an integer offset in seconds; see shiftedColumn().
	30	+ * Group-by dimension => [SQL expression, label]. `%shifted%` is replaced
	31	+ * with the timezone-shifted date column; see shiftedColumn().
51	32	*
52	33	* @return array
53	34	*/
54	35	public static function dimensions()
		@@ -86,10 +67,10 @@
86	67	return ['start_time', 'created_at', 'end_time'];
87	68	}
88	69
89	70	/**
90		- * Bookings the given user is allowed to see, or all of them when they hold
91		- * read-all-bookings. The canonical scope — do not reimplement it.
	71	+ * Bookings the given user may see, or all of them with read-all-bookings.
	72	+ * The canonical scope; do not reimplement it.
92	73	*
93	74	* @param int|null $userId Defaults to the current user.
94	75	*
95	76	* @return \FluentBooking\Framework\Database\Orm\Builder
		@@ -162,10 +143,9 @@
162	143	implode(', ', self::metrics())
163	144	));
164	145	}
165	146
166		- // Two dimensions already produce a cross-product; a third turns a
167		- // summary back into a row dump, which is what this tool exists to avoid.
	147	+ // A third dimension turns a summary back into a row dump.
168	148	if (count($groupBy) > 2) {
169	149	return new \WP_Error('too_many_dimensions', __('Group by at most two dimensions.', 'fluent-booking'));
170	150	}
171	151
		@@ -189,16 +169,10 @@
189	169	$offset = self::offsetSeconds($timezone, $range['from']);
190	170
191	171	$query = self::scoped();
192	172
193		- // Both bounds describe a LOCAL window, so both are converted from local
194		- // to UTC — and each with its OWN offset, not the range's opening one.
195		- // Filtering on unshifted UTC while grouping on shifted local made the
196		- // first and last bucket of every report partial by the size of the
197		- // offset; using one offset for both bounds then reintroduces the same
198		- // error, an hour wide, on any range that crosses a DST change (a March
199		- // report for America/New_York would read its final day at -05:00 when
200		- // that day is actually -04:00, and swallow the first hour of April).
	173	+ // The bounds are local, so convert each to UTC with its own offset.
	174	+ // One shared offset would be an hour off on a range crossing DST.
201	175	$query->whereBetween($dateField, [
202	176	self::localToUtc($range['from'] . ' 00:00:00', $timezone),
203	177	self::localToUtc($range['to'] . ' 23:59:59', $timezone),
204	178	]);
		@@ -222,9 +196,9 @@
222	196	// COUNT(DISTINCT email) builds a distinct set over the whole range and
223	197	// each SUM adds per-row work, so a count-only report selects neither.
224	198	$orderMetric = (string) Arr::get($args, 'order_by', '');
225	199
226		- // Always: resolveOrder falls back to it when no order is named.
	200	+ // Always selected: resolveOrder falls back to it.
227	201	$selects[] = 'COUNT(*) as m_count';
228	202
229	203	if (in_array('distinct_attendees', $metrics, true) || $orderMetric === 'distinct_attendees') {
230	204	$selects[] = 'COUNT(DISTINCT email) as m_distinct_attendees';
		@@ -242,11 +216,10 @@
242	216	$selects[] = "SUM(CASE WHEN status = 'cancelled' THEN 1 ELSE 0 END) as m_cancelled";
243	217	}
244	218
245	219	if (self::wantsRate($metrics)) {
246		- // Rate denominator. `reserved` rows are checkout placeholders for
247		- // payments that were never completed — counting them as bookings
248		- // deflates every rate by however many people abandoned a payment form.
	220	+ // Rate denominator. `reserved` rows are abandoned checkout
	221	+ // placeholders and would deflate every rate.
249	222	$selects[] = "SUM(CASE WHEN status != 'reserved' THEN 1 ELSE 0 END) as m_real";
250	223	}
251	224
252	225	$query->selectRaw(implode(', ', $selects));
		@@ -269,10 +242,9 @@
269	242
270	243	$limit = (int) Arr::get($args, 'limit', 50);
271	244	$limit = max(1, min($limit, self::MAX_GROUPS));
272	245
273		- // Fetch one past the limit so the caller can be told the list was cut
274		- // rather than reading a truncated report as a complete one.
	246	+ // Fetch one extra row to detect truncation.
275	247	$rows = $query->limit($limit + 1)->get();
276	248
277	249	$truncated = count($rows) > $limit;
278	250
		@@ -292,11 +264,8 @@
292	264	];
293	265	}
294	266
295	267	/**
296		- * @return array
297		- */
298		- /**
299	268	* @return bool
300	269	*/
301	270	private static function wantsRate($metrics)
302	271	{
		@@ -313,11 +282,10 @@
313	282
314	283	foreach ($groupBy as $i => $key) {
315	284	$value = $row->{'dim_' . $i};
316	285
317		- // Never drop a null bucket silently. `country` is only
318		- // populated behind Cloudflare, so a report that omitted the
319		- // blanks would read as "everyone is in Germany".
	286	+ // Keep null buckets. `country` is only set behind Cloudflare,
	287	+ // so dropping blanks would skew the report.
320	288	$entry[$key] = $value === null || $value === '' ? '(unknown)' : $value;
321	289	}
322	290
323	291	$count = (int) $row->m_count;
		@@ -366,11 +334,9 @@
366	334	if ($statuses = array_filter((array) Arr::get($filters, 'status', []))) {
367	335	$query->whereIn('status', array_map('sanitize_text_field', $statuses));
368	336	}
369	337
370		- // array_key_exists, not truthiness: `host_id: 0` and `source: "0"` are
371		- // filters the caller asked for, and silently dropping them returns the
372		- // whole unfiltered set under a heading that says otherwise.
	338	+ // array_key_exists, not truthiness: `host_id: 0` is still a filter.
373	339	foreach (['event_id' => 'event_id', 'calendar_id' => 'calendar_id', 'host_id' => 'host_user_id'] as $key => $column) {
374	340	if (array_key_exists($key, $filters) && $filters[$key] !== null && $filters[$key] !== '') {
375	341	$query->where($column, (int) $filters[$key]);
376	342	}
		@@ -416,16 +382,14 @@
416	382	if (!isset($operators[$op])) {
417	383	return new \WP_Error('invalid_having', __('having.op must be one of: >=, >, <=, <, =.', 'fluent-booking'));
418	384	}
419	385
420		- // Required, not defaulted to 0: that builds `COUNT(*) >= 0`, so a
421		- // wrong-shaped having returns everything as though it had filtered.
	386	+ // Required: defaulting to 0 builds `COUNT(*) >= 0`, which filters nothing.
422	387	if (!is_numeric(Arr::get($having, 'value'))) {
423	388	return new \WP_Error('invalid_having', __('having.value is required and must be a number, e.g. {"metric":"count","op":">=","value":5}.', 'fluent-booking'));
424	389	}
425	390
426		- // Every part of this string is a literal from the maps above except the
427		- // value, which is cast to an integer.
	391	+ // Only literals from the maps above plus an integer-cast value.
428	392	return $columns[$metric] . ' ' . $operators[$op] . ' ' . (int) Arr::get($having, 'value', 0);
429	393	}
430	394
431	395	/**
		@@ -494,16 +458,11 @@
494	458	return str_replace('%shifted%', self::shiftedColumn($dateField, $offset), $expr);
495	459	}
496	460
497	461	/**
498		- * One local wall-clock instant expressed in UTC, using the offset in force
499		- * at that instant rather than a fixed one.
	462	+ * Convert a local wall-clock time to UTC using the offset in force at that
	463	+ * instant, which matters when a range crosses a DST change.
500	464	*
501		- * The range bounds get this treatment individually because a range can cross
502		- * a daylight-saving change: applying the offset from the range's opening day
503		- * to its closing day reads a March 31st in America/New_York at -05:00 when
504		- * it is actually -04:00, and quietly pulls in the first hour of April.
505		- *
506	465	* @param string $localDateTime 'Y-m-d H:i:s'
507	466	* @param string $timezone
508	467	*
509	468	* @return string 'Y-m-d H:i:s' in UTC
		@@ -520,13 +479,11 @@
520	479	}
521	480	}
522	481
523	482	/**
524		- * Times are stored in UTC. Grouping them by day or hour without shifting
525		- * would put an 11pm booking in Berlin on the wrong date — the exact class
526		- * of error this project keeps guarding against. CONVERT_TZ is not usable
527		- * because it needs MySQL's timezone tables loaded, which most hosts do not
528		- * do, so the offset is computed in PHP and applied as a fixed interval.
	483	+ * Shift the UTC column into the report timezone before grouping by day or
	484	+ * hour. CONVERT_TZ needs MySQL's timezone tables, which most hosts lack, so
	485	+ * the offset is computed in PHP and applied as a fixed interval.
529	486	*
530	487	* @param string $dateField Whitelisted column name.
531	488	* @param int $offset Seconds, already cast.
532	489	*
		@@ -541,11 +498,10 @@
541	498	return 'DATE_ADD(' . $dateField . ', INTERVAL ' . (int) $offset . ' SECOND)';
542	499	}
543	500
544	501	/**
545		- * The zone's offset at the start of the range. A range that crosses a DST
546		- * boundary uses one offset throughout, so bookings on the far side can land
547		- * an hour out; the caller is told which offset was applied.
	502	+ * The zone's offset at the start of the range. Grouping uses this one
	503	+ * offset throughout, so buckets past a DST change can be an hour out.
548	504	*
549	505	* @return int
550	506	*/
551	507	public static function offsetSeconds($timezone, $onDate)
		@@ -568,10 +524,9 @@
568	524	$to = $to ? sanitize_text_field($to) : gmdate('Y-m-d');
569	525	$from = $from ? sanitize_text_field($from) : gmdate('Y-m-d', strtotime($to . ' -29 days'));
570	526
571	527	foreach ([$from, $to] as $date) {
572		- // checkdate() as well as the shape: 2026-02-30 matches the pattern
573		- // and strtotime() then rolls it forward to March 2 silently.
	528	+ // checkdate(): strtotime() silently rolls 2026-02-30 into March.
574	529	if (!preg_match('/^(\d{4})-(\d{2})-(\d{2})$/', $date, $parts)
575	530	|| !checkdate((int) $parts[2], (int) $parts[3], (int) $parts[1])) {
576	531	return new \WP_Error(
577	532	'invalid_range',