Feature/attribute scope #902

lucs7 · 2025-12-17T18:20:59Z

This pull request introduces support for custom attribute scopes for ressources. Meaning based on specific resource or resource type custom attributes are shown similar to how its handled for reservation.

I previously committed this code but some final testing and changes were required
While going through the custom attributes I also added some types where suggested

Copilot

Pull request overview

This pull request introduces support for custom attribute scopes for resources, allowing attributes to be filtered by specific resources or resource types, similar to how it's handled for reservations. The PR also includes type annotation improvements throughout the codebase.

Key changes:

New GetResourceAttributes method in AttributeService to filter attributes based on resource ID and resource type ID
Enhanced template logic to display attributes conditionally based on secondary entity assignments
Type-safe comparison operators using strict equality (===) for CustomAttributeCategory comparisons

Reviewed changes

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

Show a summary per file

File	Description
tpl/Admin/Resources/manage_resources.tpl	Implements conditional attribute rendering based on resource type and direct resource assignment
tpl/Admin/Attributes/manage_attributes.tpl	Updates form fields with better labeling, tooltip help text, and visibility rules configuration
lib/Application/Attributes/AttributeService.php	Adds GetResourceAttributes method with filtering logic and stub methods for interface compatibility
lib/Application/Schedule/ScheduleResourceFilter.php	Updates type comparisons to use type-safe casting
lib/Application/Reservation/Validation/CustomAttributeValidationRule.php	Converts loose equality to strict equality for category comparisons
Domain/CustomAttribute.php	Extends WithSecondaryEntities to support RESOURCE category in addition to RESERVATION
Domain/Access/AttributeFilter.php	Adds type casting for attribute type comparisons
Pages/Admin/ManageResourcesPage.php	Updates attribute type comparison with type casting
Pages/Admin/ManageAttributesPage.php	Adds SetCategoryVisibilityRules method to define field visibility per category
Presenters/Admin/ManageAttributesPresenter.php	Calls SetCategoryVisibilityRules on page load
WebServices/Controllers/AttributeSaveController.php	Updates validation with strict type comparisons and adds DATETIME type support
Web/scripts/admin/attributes.js	Implements visibility rules and secondary category management (contains duplicate code)
tests/fakes/FakeAttributeService.php	Adds stub implementations for new interface methods
tests/Presenters/Admin/ManageAttributesPresenterTest.php	Adds comprehensive test cases for resource and resource type secondary entities
lang/*.php	Adds help text translations for AdminOnly, Private, and LimitAttributeScope in 7 languages

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

lang/it_it.php

lib/Application/Schedule/ScheduleResourceFilter.php

Web/scripts/admin/attributes.js

lib/Application/Attributes/AttributeService.php

Adds the scoping feature to resources by resource and resource type Allows to collect custom attributes just for specific limited scopes fix: scopefield on edit still visible

Better description of attribute properties and options in admin management page

…pes and CustomAttributeCategory fixes phpstan issues for wrong type comparisson

Copilot

Pull request overview

Copilot reviewed 24 out of 24 changed files in this pull request and generated 5 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-19T18:31:40Z

tpl/Admin/Resources/manage_resources.tpl

+
+																	{* If attribute has secondary entities, check them based on category *}
+																	{if $attribute->HasSecondaryEntities()}
+																		{if $attribute->SecondaryCategory() == CustomAttributeCategory::RESOURCE_TYPE}


Inconsistent comparison operator used in Smarty template. This uses loose equality (==) to compare SecondaryCategory() with CustomAttributeCategory constants. While Smarty templates don't support strict comparisons like PHP, for better type safety and consistency with the PHP code changes in this PR, consider documenting that these comparisons expect integer values or adding explicit type handling if the method can return different types.

Copilot · 2026-01-19T18:31:41Z

tpl/Admin/Resources/manage_resources.tpl

+													<div class="mb-4">
+														<span class="fs-6 fw-bold">{translate key='CustomAttributes'}</span>
+														<a id="customAttributesControl" class="link-primary" href="#"
+															data-bs-toggle="collapse"
+															data-bs-target="#customAttributes{$id}">
+															<i id="customAttributesIcon{$id}"
+																class="bi bi-chevron-down"></i>
+														</a>
+														<div id="customAttributes{$id}" class="collapse show">
+															<div class="row">
+																{* All attributes - check resource type filtering first, then direct assignment *}
+																{foreach from=$AttributeList item=attribute name=attrLoop}
+																	{assign var=showAttribute value=false}
+
+																	{* If attribute has secondary entities, check them based on category *}
+																	{if $attribute->HasSecondaryEntities()}
+																		{if $attribute->SecondaryCategory() == CustomAttributeCategory::RESOURCE_TYPE}
+																			{* Check if resource's type matches any secondary entity (resource type) *}
+																			{foreach from=$attribute->SecondaryEntityIds() item=secondaryId}
+																				{if $secondaryId == $resource->GetResourceTypeId()}
+																					{assign var=showAttribute value=true}
+																					{break}
+																				{/if}
+																			{/foreach}
+																		{elseif $attribute->SecondaryCategory() == CustomAttributeCategory::RESOURCE}
+																			{* Check if this specific resource matches any secondary entity (resource) *}
+																			{foreach from=$attribute->SecondaryEntityIds() item=secondaryId}
+																				{if $secondaryId == $id}
+																					{assign var=showAttribute value=true}
+																					{break}
+																				{/if}
+																			{/foreach}
 																		{/if}
-																	{/foreach}
-																	{if $hasResults}
-																		{* Content at the end of the iteration *}
-																	</div>
-																</div>
+																	{* Otherwise, show if attribute applies directly to this specific resource *}
+																	{elseif $attribute->AppliesToEntity($id)}
+																		{assign var=showAttribute value=true}
+																	{* Or show if attribute has no specific entity assignments (applies to all) *}
+																	{elseif $attribute->EntityIds()|count == 0}
+																		{assign var=showAttribute value=true}
+																	{/if}
+
+																	{if $showAttribute}
+																		{include file='Admin/InlineAttributeEdit.tpl' id=$id attribute=$attribute value=$resource->GetAttributeValue($attribute->Id())}
+																	{/if}
+																{/foreach}
 															</div>
-														{/if}
-													{/if}
+														</div>
+													</div>


The "Custom Attributes" section is now always displayed, even when there are no attributes to show. The previous implementation used a hasResults flag to conditionally display the entire section only when attributes existed. This change means users will see an empty "Custom Attributes" collapsible section when no attributes apply to a resource, which could be confusing. Consider adding a check to only display this section when at least one attribute will be shown.

Copilot · 2026-01-19T18:31:41Z

Web/scripts/admin/attributes.js

+        $(".cancel").click(function () {
+            $(this).closest('.dialog').dialog("close");
        });


Duplicate event handler for .cancel elements. The same click handler is registered twice (lines 105-107 and 129-131), which could cause the dialog close action to be triggered multiple times. Remove one of these duplicate handlers.

Suggested change

$(".cancel").click(function () {

$(this).closest('.dialog').dialog("close");

});

Copilot · 2026-01-19T18:31:41Z

lib/Application/Schedule/ScheduleResourceFilter.php

        }

-        if ($value->Type() == CustomAttributeTypes::SINGLE_LINE_TEXTBOX || $value->Type() == CustomAttributeTypes::MULTI_LINE_TEXTBOX) {
+        if ((int)$value->Type() == CustomAttributeTypes::SINGLE_LINE_TEXTBOX || (int)$value->Type() == CustomAttributeTypes::MULTI_LINE_TEXTBOX) {


Inconsistent comparison operator. This line uses loose equality (==) instead of strict equality (===). For consistency with the rest of the changes in this PR where type casting is used with strict comparison, this should be changed to use === for type safety.

Suggested change

if ((int)$value->Type() == CustomAttributeTypes::SINGLE_LINE_TEXTBOX || (int)$value->Type() == CustomAttributeTypes::MULTI_LINE_TEXTBOX) {

if ((int)$value->Type() === CustomAttributeTypes::SINGLE_LINE_TEXTBOX || (int)$value->Type() === CustomAttributeTypes::MULTI_LINE_TEXTBOX) {

Copilot · 2026-01-19T18:31:41Z

lib/Application/Attributes/AttributeService.php

+    public function GetResourceAttributes(UserSession $userSession, $resourceId = 0, $resourceTypeId = 0)
+    {
+        $attributes = [];
+        $customAttributes = $this->GetByCategory(CustomAttributeCategory::RESOURCE);
+
+        foreach ($customAttributes as $attribute) {
+            $secondaryCategory = $attribute->SecondaryCategory();
+            $shouldInclude = false;
+
+            if (empty($secondaryCategory)) {
+                // No secondary filtering, but don't include here - these are handled statically
+                continue;
+            } else {
+                if ((int)$secondaryCategory === CustomAttributeCategory::RESOURCE_TYPE && $resourceTypeId > 0) {
+                    // Check if this attribute applies to the specific resource type
+                    $shouldInclude = in_array($resourceTypeId, $attribute->SecondaryEntityIds());
+                } elseif ((int)$secondaryCategory === CustomAttributeCategory::RESOURCE && $resourceId > 0) {
+                    // Check if this attribute applies to the specific resource
+                    $shouldInclude = in_array($resourceId, $attribute->SecondaryEntityIds());
+
+                    // Also check if user has permission to see this resource
+                    if ($shouldInclude) {
+                        $allowedResources = $this->GetAllowedResources($userSession);
+                        $shouldInclude = array_key_exists($resourceId, $allowedResources);
+                    }
+                }
+            }
+
+            if ($shouldInclude) {
+                // Check admin-only attributes
+                $viewableForAdmin = (!$attribute->AdminOnly() || ($attribute->AdminOnly() && $userSession->IsAdmin));
+
+                // Check private attributes
+                $viewableForPrivate = (!$attribute->IsPrivate() || ($attribute->IsPrivate() && $userSession->IsAdmin));
+
+                if ($viewableForAdmin && $viewableForPrivate) {
+                    $attributes[] = new LBAttribute($attribute, null);
+                }
+            }
+        }
+
+        return $attributes;
+    }


The new GetResourceAttributes method in AttributeService is added to the IAttributeService interface but doesn't appear to be called anywhere in the codebase. While it might be intended for future use or external API consumption, consider adding tests that exercise this method to ensure it works correctly. Currently, only the FakeAttributeService stub implementation exists in the test files.

Copilot AI review requested due to automatic review settings December 17, 2025 18:21

Copilot started reviewing on behalf of lucs7 December 17, 2025 18:21 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

lucs7 force-pushed the feature/attribute-scope branch 2 times, most recently from c82fe7c to 18bf701 Compare December 17, 2025 19:44

JohnVillalovos force-pushed the feature/attribute-scope branch from 18bf701 to f02596d Compare December 23, 2025 18:57

lucs7 added 3 commits January 19, 2026 10:25

feat: Custom attribute scope for resources

9c58e06

Adds the scoping feature to resources by resource and resource type Allows to collect custom attributes just for specific limited scopes fix: scopefield on edit still visible

feat: Added tooltips for attributes management

f780cb1

Better description of attribute properties and options in admin management page

fix: Update type comparisons to strict equality for CustomAttributeTy…

827091e

…pes and CustomAttributeCategory fixes phpstan issues for wrong type comparisson

JohnVillalovos force-pushed the feature/attribute-scope branch from f02596d to 827091e Compare January 19, 2026 18:25

JohnVillalovos requested a review from Copilot January 19, 2026 18:25

Copilot started reviewing on behalf of JohnVillalovos January 19, 2026 18:25 View session

Copilot AI reviewed Jan 19, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Feature/attribute scope #902

Feature/attribute scope #902

Uh oh!

lucs7 commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Copilot AI Jan 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	$(".cancel").click(function () {
	$(this).closest('.dialog').dialog("close");
	});

	if ((int)$value->Type() == CustomAttributeTypes::SINGLE_LINE_TEXTBOX \|\| (int)$value->Type() == CustomAttributeTypes::MULTI_LINE_TEXTBOX) {
	if ((int)$value->Type() === CustomAttributeTypes::SINGLE_LINE_TEXTBOX \|\| (int)$value->Type() === CustomAttributeTypes::MULTI_LINE_TEXTBOX) {

Feature/attribute scope #902

Are you sure you want to change the base?

Feature/attribute scope #902

Uh oh!

Conversation

lucs7 commented Dec 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant