WIP: Work on improving the emails sent for a reservation by JohnVillalovos · Pull Request #1101 · LibreBooking/librebooking

JohnVillalovos · 2026-03-04T17:55:35Z

No description provided.

Extract shared reservation email template data building into ReservationEmailTemplateContext and update reservation email messages to use it. This change: - centralizes resource, repeat, reservation attribute, and created-by template data - switches reservation created emails to render from structured $Resources data instead of single-resource / name-list conditionals - updates admin reservation email classes to use BookableResource consistently - removes duplicated template-population logic from reservation email message classes Behaviorally, this prepares reservation emails for richer per-resource rendering while preserving the existing line-by-line resource details output.

…able Expand reservation email resource rendering to include additional standard resource fields and present per-resource custom attributes in a table. This change: - adds schedule name, resource id, description, notes, and resource administrator to the reservation email resource payload - formats per-resource custom attributes into email-ready table rows in ReservationEmailTemplateContext - updates reservation created user/admin email templates to show the new top-level resource metadata - changes the Resource Details section from line-by-line attribute blocks to a two-column table - preserves blank custom attributes by rendering empty value cells This improves reservation email readability and makes multi-resource reservations more informative without changing reservation-level attribute rendering.

Copilot

Pull request overview

This PR is working toward improving reservation-related emails by centralizing template data preparation in a dedicated context object and expanding the “Reservation Created” email templates to show richer per-resource details (including attributes).

Changes:

Added ReservationEmailTemplateContext to build resource/reservation variables for email templates.
Refactored ReservationEmailMessage and admin reservation email messages to use the new context (and updated some typing / constructor usage).
Updated ReservationCreated*.tpl templates to render a full $Resources list with additional resource metadata and attribute rows.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
lib/Email/Messages/ReservationRequiresApprovalEmailAdmin.php	Constructor signature updated to `BookableResource` and uses named-arg parent call.
lib/Email/Messages/ReservationEmailTemplateContext.php	New helper for assembling template variables (resources, attributes, schedule/group names, etc.).
lib/Email/Messages/ReservationEmailMessage.php	Uses `ReservationEmailTemplateContext` to populate template variables; adds `$Resources`.
lib/Email/Messages/ReservationCreatedEmailAdmin.php	Uses `ReservationEmailTemplateContext` and adds typed properties.
lang/en_us/ReservationCreatedAdmin.tpl	Renders `$Resources` list with resource details, attributes table, and image.
lang/en_us/ReservationCreated.tpl	Renders `$Resources` list with resource details, attributes table, and image.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-04T18:10:49Z

lib/Email/Messages/ReservationEmailTemplateContext.php

+        usort(
+            $resourceAttributeValues,
+            static fn (LBAttribute $left, LBAttribute $right): int => strcasecmp($left->Label(), $right->Label())
+        );
+


BuildResourceData() re-sorts resource attributes alphabetically by label. AttributeRepository::GetByCategory() already returns attributes ordered by sort_order (and then display_label) from the DB, which is the user-configured ordering used elsewhere in the app. Re-sorting here will make email attribute order inconsistent with the configured sort order; consider keeping the repository order (or sorting by SortOrder() instead).

Suggested change

usort(

$resourceAttributeValues,

static fn (LBAttribute $left, LBAttribute $right): int => strcasecmp($left->Label(), $right->Label())

);

Copilot · 2026-03-04T18:10:50Z

lib/Email/Messages/ReservationEmailTemplateContext.php

+        }
+
+        if ($attribute->Type() == CustomAttributeTypes::DATETIME) {
+            return $value->Format(Resources::GetInstance()->GetDateFormat('general_datetime'));


AttributeDisplayValue() treats DATETIME attribute values as a Date object ($value->Format(...)), but custom attribute values are stored/loaded as strings (e.g., AttributeValue casts to string). This will trigger a fatal error when a resource attribute is of type DATETIME. Parse/convert the string to a Date (or use the same formatting approach as Controls/Attributes/Date.tpl) before calling Format(), and keep it resilient if $value is already a Date instance.

Suggested change

return $value->Format(Resources::GetInstance()->GetDateFormat('general_datetime'));

$format = Resources::GetInstance()->GetDateFormat('general_datetime');

// If already a PHP DateTime-like object, use its format() method.

if ($value instanceof \DateTimeInterface) {

return $value->format($format);

}

// If it's a custom Date-like object with a Format() method, keep existing behavior.

if (is_object($value) && method_exists($value, 'Format')) {

return $value->Format($format);

}

// If it's a string, attempt to parse and format; otherwise, fall back to the raw value.

if (is_string($value)) {

try {

$date = new \DateTimeImmutable($value);

return $date->format($format);

} catch (\Exception $e) {

// If parsing fails, return the original string value.

return $value;

}

}

return strval($value);

Copilot · 2026-03-04T18:10:50Z

lib/Email/Messages/ReservationCreatedEmailAdmin.php

+        $repeatVariables = $context->RepeatVariables($this->timezone);
+        $this->Set('RepeatDates', $repeatVariables['RepeatDates']);
+        $this->Set('RepeatRanges', $repeatVariables['RepeatRanges']);


UserDto::Timezone() can be null (it’s an optional ctor arg). In that case $this->timezone will be null and RepeatVariables(string $timezone) / ToTimezone($this->timezone) will throw a TypeError/exception. Consider normalizing $this->timezone in the constructor (fallback to reservation owner timezone or date_default_timezone_get()) so template population is always safe.

Copilot · 2026-03-04T18:10:51Z

lib/Email/Messages/ReservationRequiresApprovalEmailAdmin.php

+    public function __construct(UserDto $adminDto, User $reservationOwner, ReservationSeries $reservationSeries, BookableResource $primaryResource, IAttributeRepository $attributeRepository, IUserRepository $userRepository)
    {
-        parent::__construct($adminDto, $reservationOwner, $reservationSeries, $primaryResource, $attributeRepository, $userRepository);
+        parent::__construct(
+            adminDto: $adminDto,


This file defines ReservationRequiresApprovalEmailAdmin extends ReservationCreatedEmailAdmin but does not require_once the parent class file (it currently only includes lib/Email/namespace.php). Other admin reservation email subclasses explicitly include ReservationCreatedEmailAdmin.php (e.g., ReservationDeletedEmailAdmin). Adding the require here avoids brittle dependence on include order.

Copilot · 2026-03-04T18:10:51Z

lib/Email/Messages/ReservationEmailTemplateContext.php

+            }
+
+            if ($applies) {
+                $attributeValues[] = new LBAttribute($attribute, $this->reservationSeries->GetAttributeValue($attribute->Id()));


ReservationAttributes() now includes reservation attributes with no secondary category (and may include USER / RESOURCE_TYPE scoped ones), but it doesn’t filter out AdminOnly() or IsPrivate() attributes for end-user emails. Because ReservationEmailMessage uses this for user-facing templates, this can newly expose admin-only/private attribute labels/values. Consider adding an $includeAdminOnly / $includePrivate parameter (or using the same filtering rules as AttributeService::GetReservationAttributes) and calling it with the right visibility in admin vs user emails.

Suggested change

$attributeValues[] = new LBAttribute($attribute, $this->reservationSeries->GetAttributeValue($attribute->Id()));

// Do not expose admin-only or private attributes in email templates

if ($attribute->AdminOnly() || $attribute->IsPrivate()) {

continue;

}

$attributeValues[] = new LBAttribute(

$attribute,

$this->reservationSeries->GetAttributeValue($attribute->Id())

);

Copilot · 2026-03-04T18:10:51Z

lib/Email/Messages/ReservationEmailTemplateContext.php

+        $resource = $this->PrimaryResource();
+
+        return [
+            'ResourceName' => $resource['name'],
+            'ResourceImage' => $resource['image'],


ResourceVariables() calls PrimaryResource(), which fetches all resource attribute definitions and (via BuildResourceData) also loads all schedules and resource-admin groups just to return name/image. Since PopulateTemplate() calls ResourceVariables() for every reservation email, this adds avoidable DB/repo work even for templates that only need the name/image. Consider making ResourceVariables() read directly from $this->primaryResource (and compute image path) and only load schedules/groups/attributes when Resources()/PrimaryResource() are explicitly requested.

Suggested change

$resource = $this->PrimaryResource();

return [

'ResourceName' => $resource['name'],

'ResourceImage' => $resource['image'],

$resourceName = $this->primaryResource->GetName();

$resourceImage = $this->primaryResource->GetImage();

return [

'ResourceName' => $resourceName,

'ResourceImage' => $resourceImage,

Copilot · 2026-03-04T18:10:51Z

lang/en_us/ReservationCreated.tpl

+        <strong>{$resource.name}</strong><br/>
+        {if $resource.scheduleName}<strong>Schedule:</strong> {$resource.scheduleName}<br/>{/if}
+        <strong>ResourceId:</strong> {$resource.id}<br/>
+        {if $resource.location}<strong>Location:</strong> {$resource.location}<br/>{/if}
+        {if $resource.contact}<strong>Contact:</strong> {$resource.contact}<br/>{/if}
+        {if $resource.description}<strong>Description:</strong> {$resource.description|nl2br}<br/>{/if}
+        {if $resource.notes}<strong>Notes:</strong> {$resource.notes|nl2br}<br/>{/if}
+        {if $resource.resourceAdministrator}<strong>Resource Administrator:</strong> {$resource.resourceAdministrator}<br/>{/if}
+
+        {if $resource.attributeRows|default:array()|count > 0}
+            <strong>Resource Details:</strong><br/>
+            <table cellpadding="4" cellspacing="0" border="1" style="border-collapse: collapse; margin-top: 4px;">
+                {foreach from=$resource.attributeRows item=row}
+                    <tr>
+                        <td valign="top"><strong>{$row.label}</strong></td>
+                        <td valign="top">{$row.displayValue|nl2br}</td>
+                    </tr>


The new Resources section outputs several fields ($resource.name, scheduleName, location, contact, description, notes, resourceAdministrator, and row.displayValue) directly into the HTML email without escaping. For resources and custom attributes created or updated via the JSON API (ResourcesWriteWebService/ResourceSaveController), these values can contain arbitrary HTML/JavaScript, leading to stored XSS in reservation emails when a malicious value is rendered here. Ensure all user-supplied or API-supplied strings in this block are HTML-escaped at render time (for example by applying an appropriate Smarty escape filter) before being included in the email markup.

Copilot · 2026-03-04T18:10:52Z

lang/en_us/ReservationCreatedAdmin.tpl

+        <strong>{$resource.name}</strong><br/>
+        {if $resource.scheduleName}<strong>Schedule:</strong> {$resource.scheduleName}<br/>{/if}
+        <strong>ResourceId:</strong> {$resource.id}<br/>
+        {if $resource.location}<strong>Location:</strong> {$resource.location}<br/>{/if}
+        {if $resource.contact}<strong>Contact:</strong> {$resource.contact}<br/>{/if}
+        {if $resource.description}<strong>Description:</strong> {$resource.description|nl2br}<br/>{/if}
+        {if $resource.notes}<strong>Notes:</strong> {$resource.notes|nl2br}<br/>{/if}
+        {if $resource.resourceAdministrator}<strong>Resource Administrator:</strong> {$resource.resourceAdministrator}<br/>{/if}

-{if $ResourceImage}
-	<div class="resource-image"><img alt="{$ResourceName}" src="{$ScriptUrl}/{$ResourceImage}"/></div>
-{/if}
+        {if $resource.attributeRows|default:array()|count > 0}
+            <strong>Resource Details:</strong><br/>
+            <table cellpadding="4" cellspacing="0" border="1" style="border-collapse: collapse; margin-top: 4px;">
+                {foreach from=$resource.attributeRows item=row}
+                    <tr>
+                        <td valign="top"><strong>{$row.label}</strong></td>
+                        <td valign="top">{$row.displayValue|nl2br}</td>
+                    </tr>


The expanded Resources section renders multiple fields ($resource.name, scheduleName, location, contact, description, notes, resourceAdministrator, and row.displayValue) directly into the HTML email without escaping. Because resource properties and custom attribute values coming from the JSON resource API (ResourcesWriteWebService/ResourceSaveController) are not HTML-encoded, an attacker with API access can inject HTML/JavaScript that will be stored and later executed in admin reservation emails. Apply proper HTML escaping to all of these values when rendering this block so that any characters from untrusted input are treated as text, not markup.

Copilot AI review requested due to automatic review settings March 4, 2026 17:55

JohnVillalovos marked this pull request as draft March 4, 2026 17:55

Copilot started reviewing on behalf of JohnVillalovos March 4, 2026 17:56 View session

JohnVillalovos force-pushed the jlvillal/email_table branch from c556e3b to 0a6c236 Compare March 4, 2026 18:01

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Work on improving the emails sent for a reservation#1101

WIP: Work on improving the emails sent for a reservation#1101
JohnVillalovos wants to merge 2 commits intodevelopfrom
jlvillal/email_table

JohnVillalovos commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-            return $value->Format(Resources::GetInstance()->GetDateFormat('general_datetime'));
+            $format = Resources::GetInstance()->GetDateFormat('general_datetime');
+            // If already a PHP DateTime-like object, use its format() method.
+            if ($value instanceof \DateTimeInterface) {
+                return $value->format($format);
+            }
+            // If it's a custom Date-like object with a Format() method, keep existing behavior.
+            if (is_object($value) && method_exists($value, 'Format')) {
+                return $value->Format($format);
+            }
+            // If it's a string, attempt to parse and format; otherwise, fall back to the raw value.
+            if (is_string($value)) {
+                try {
+                    $date = new \DateTimeImmutable($value);
+                    return $date->format($format);
+                } catch (\Exception $e) {
+                    // If parsing fails, return the original string value.
+                    return $value;
+                }
+            }
+            return strval($value);

Conversation

JohnVillalovos commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants