Skip to content

Commit 1b49cf3

Browse files
committed
fix(filter): normalize requiredFilters to array in Filter constructor
When the `requires` parameter of a `#drilldowninfo` filter is set, the value was stored as a plain string. The `foreach` loop in `SpecialBrowseData::createDrilldownQuery()` that checks required filters then iterated over individual characters instead of filter names, so no character ever matched a filter name and the filter was permanently hidden rather than appearing once its required filter was applied. Fix by wrapping a string value in a single-element array in `Filter::__construct()`, so `requiredFilters()` always returns an array.
1 parent 564fd88 commit 1b49cf3

3 files changed

Lines changed: 56 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ This project adheres to [Semantic Versioning](https://semver.org/) and
66

77
## [Unreleased]
88

9+
### Fixed
10+
- Fix `requires` filter option having no effect: filters with `requires=SomeFilter` were never shown because the required-filter name was iterated as individual characters instead of as a filter name ([#4](https://github.com/SemanticMediaWiki/SemanticDrilldown/issues/4))
11+
912
### Changed
1013
- Replace global config access with `getConfig()`/`GlobalVarConfig` in `SpecialBrowseData`; remove `FunctionConfigUsage` PHPCS exclude [`98a7d4e`](https://github.com/SemanticMediaWiki/SemanticDrilldown/commit/98a7d4e) ([#139](https://github.com/SemanticMediaWiki/SemanticDrilldown/issues/139))
1114

includes/Filter.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ public function __construct(
3939
$this->name = $name;
4040
$this->property = $property;
4141
$this->category = $category;
42-
$this->requiredFilters = $requiredFilters ?? [];
42+
$this->requiredFilters = is_array( $requiredFilters )
43+
? $requiredFilters
44+
: ( $requiredFilters !== null ? [ $requiredFilters ] : [] );
4345
$this->int = $int;
4446
$this->propertyType = $propertyType;
4547
$this->timePeriod = $timePeriod;

tests/phpunit/Unit/FilterTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
<?php
2+
3+
namespace SD\Tests\Unit;
4+
5+
use PHPUnit\Framework\TestCase;
6+
use SD\DbService;
7+
use SD\Filter;
8+
9+
/**
10+
* @covers \SD\Filter
11+
*/
12+
class FilterTest extends TestCase {
13+
14+
private function makeFilter( $requiredFilters ): Filter {
15+
$db = $this->createMock( DbService::class );
16+
return new Filter( $db, 'TestFilter', 'TestProperty', null, $requiredFilters, null );
17+
}
18+
19+
public function testRequiredFiltersReturnsEmptyArrayWhenNull(): void {
20+
$filter = $this->makeFilter( null );
21+
22+
$this->assertSame( [], $filter->requiredFilters() );
23+
}
24+
25+
public function testRequiredFiltersReturnsArrayWhenPassedArray(): void {
26+
$filter = $this->makeFilter( [ 'Country' ] );
27+
28+
$this->assertSame( [ 'Country' ], $filter->requiredFilters() );
29+
}
30+
31+
public function testRequiredFiltersReturnsArrayWhenPassedString(): void {
32+
$filter = $this->makeFilter( 'Country' );
33+
34+
$this->assertSame( [ 'Country' ], $filter->requiredFilters() );
35+
}
36+
37+
public function testRequiredFiltersStringIsIterableAndMatchesFilterName(): void {
38+
$filter = $this->makeFilter( 'Country' );
39+
40+
$found = false;
41+
foreach ( $filter->requiredFilters() as $required ) {
42+
if ( $required === 'Country' ) {
43+
$found = true;
44+
}
45+
}
46+
47+
$this->assertTrue( $found, 'Expected to find "Country" in requiredFilters(), not single characters' );
48+
}
49+
50+
}

0 commit comments

Comments
 (0)