From ae4febe304ea4320daf0cf1b7aab984a73830c92 Mon Sep 17 00:00:00 2001
From: Ollama <ollama@steganos.dev>
Date: Wed, 18 Mar 2026 22:05:09 +0000
Subject: [PATCH] fix: Address remaining PR review comments

- Update regex pattern to support multi-word attribute names (e.g., 'aspect ratio')
- Rename $includeDeleted to $matchDeleted for clarity (matches deleted flag value)
- Add check to skip attributes not in caller-provided definitionIds filter
- Use Unicode pattern modifier 'u' for international character support in attribute names

This addresses CodeRabbit's feedback on PR #4442.
---
 app/Models/Item.php | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/app/Models/Item.php b/app/Models/Item.php
index a666ace96..97da9bbcc 100644
--- a/app/Models/Item.php
+++ b/app/Models/Item.php
@@ -150,7 +150,7 @@ class Item extends Model
             return $result;
         }
 
-        $pattern = '/(\w+)\s*:\s*([^\s,]+)(?:\s+(?:AND|OR)\s+)?/i';
+        $pattern = '/([[:alpha:]][[:alnum:] _-]*?)\s*:\s*([^\s,]+)(?:\s+(?:AND|OR)\s+)?/iu';
         $remaining = preg_replace($pattern, '', $search);
 
         if (preg_match_all($pattern, $search, $matches, PREG_SET_ORDER)) {
@@ -175,11 +175,11 @@ class Item extends Model
      *
      * @param string $search Search term
      * @param array $definitionIds Attribute definition IDs to search within
-     * @param bool $includeDeleted Whether to include deleted items
+     * @param bool $matchDeleted Whether to match items where deleted flag equals this value
      * @param string $logic 'AND' or 'OR' for multiple attribute matching
      * @return array Array of matching item_ids
      */
-    public function searchByAttributes(string $search, array $definitionIds, bool $includeDeleted = false, string $logic = 'OR'): array
+    public function searchByAttributes(string $search, array $definitionIds, bool $matchDeleted = false, string $logic = 'OR'): array
     {
         if ($definitionIds === [] || $search === '') {
             return [];
@@ -205,6 +205,11 @@ class Item extends Model
 
                 $definitionId = $definitionNameToId[$attrName];
 
+                // Skip if this attribute is not in the caller-provided definitionIds filter
+                if (!in_array($definitionId, $definitionIds, true)) {
+                    continue;
+                }
+
                 foreach ($values as $value) {
                     $builder = $this->db->table('attribute_links');
                     $builder->select('DISTINCT attribute_links.item_id');
@@ -218,7 +223,7 @@ class Item extends Model
                     $builder->where('attribute_links.definition_id', $definitionId);
                     $builder->where('attribute_links.sale_id', null);
                     $builder->where('attribute_links.receiving_id', null);
-                    $builder->where('items.deleted', $includeDeleted);
+                    $builder->where('items.deleted', $matchDeleted);
 
                     $foundIds = array_column($builder->get()->getResultArray(), 'item_id');
 
@@ -237,7 +242,7 @@ class Item extends Model
 
         if (!empty($parsed['terms'])) {
             $term = implode(' ', $parsed['terms']);
-            $termIds = $this->searchByAttributeValue($term, $definitionIds, $includeDeleted);
+            $termIds = $this->searchByAttributeValue($term, $definitionIds, $matchDeleted);
 
             if (empty($matchingItemIds)) {
                 return $termIds;
@@ -256,10 +261,10 @@ class Item extends Model
      *
      * @param string $search Search term
      * @param array $definitionIds Attribute definition IDs to search within
-     * @param bool $includeDeleted Whether to include deleted items
+     * @param bool $matchDeleted Whether to match items where deleted flag equals this value
      * @return array Array of matching item_ids
      */
-    private function searchByAttributeValue(string $search, array $definitionIds, bool $includeDeleted = false): array
+    private function searchByAttributeValue(string $search, array $definitionIds, bool $matchDeleted = false): array
     {
         $builder = $this->db->table('attribute_links');
         $builder->select('DISTINCT attribute_links.item_id');
@@ -273,7 +278,7 @@ class Item extends Model
         $builder->whereIn('attribute_links.definition_id', $definitionIds);
         $builder->where('attribute_links.sale_id', null);
         $builder->where('attribute_links.receiving_id', null);
-        $builder->where('items.deleted', $includeDeleted);
+        $builder->where('items.deleted', $matchDeleted);
 
         return array_column($builder->get()->getResultArray(), 'item_id');
     }