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.
This commit is contained in:
Ollama
2026-03-18 22:05:09 +00:00
committed by jekkos
parent 2b84377e12
commit ae4febe304

View File

@@ -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');
}