Allow CompositeBlock::appliesToRight to return null when unsure

CompositeBlock::appliesToRight checks $block->appliesToRight()
for each of the original blocks from which it is made.

AbstractBlock::appliesToRight returns:
* true if the block applies to the right
* false if the block does not apply to the right
* null if unsure

Before this, CompositeBlock::appliesToRight can only return true
or false. After this, it returns:
* false if false for all of the original blocks
* true if true for one or more original blocks
* null otherwise

Bug: T229417
Bug: T231145
Change-Id: Ie93b7691b57ac6a8f86b3641ad07a1d54babcd42

diff --git a/includes/block/AbstractBlock.php b/includes/block/AbstractBlock.php
index 4d4bb07..fa91909 100644 (file)
--- a/includes/block/AbstractBlock.php
+++ b/includes/block/AbstractBlock.php
@@ -250,8 +250,9 @@ abstract class AbstractBlock {
         * may be overridden according to global configs.
         *
         * @since 1.33
-        * @param string $right Right to check
-        * @return bool|null null if unrecognized right or unset property
+        * @param string $right
+        * @return bool|null The block applies to the right, or null if
+        *  unsure (e.g. unrecognized right or unset property)
         */
        public function appliesToRight( $right ) {
                $config = RequestContext::getMain()->getConfig();

diff --git a/includes/block/CompositeBlock.php b/includes/block/CompositeBlock.php
index 3f3e2d3..6f49f17 100644 (file)
--- a/includes/block/CompositeBlock.php
+++ b/includes/block/CompositeBlock.php
@@ -164,9 +164,28 @@ class CompositeBlock extends AbstractBlock {
 
        /**
         * @inheritDoc
+        *
+        * Determines whether the CompositeBlock applies to a right by checking
+        * whether the original blocks apply to that right. Each block can report
+        * true (applies), false (does not apply) or null (unsure). Then:
+        * - If any original blocks apply, this block applies
+        * - If no original blocks apply but any are unsure, this block is unsure
+        * - If all blocks do not apply, this block does not apply
         */
        public function appliesToRight( $right ) {
-               return $this->methodReturnsValue( __FUNCTION__, true, $right );
+               $isUnsure = false;
+
+               foreach ( $this->originalBlocks as $block ) {
+                       $appliesToRight = $block->appliesToRight( $right );
+
+                       if ( $appliesToRight ) {
+                               return true;
+                       } elseif ( $appliesToRight === null ) {
+                               $isUnsure = true;
+                       }
+               }
+
+               return $isUnsure ? null : false;
        }
 
        /**

diff --git a/tests/phpunit/includes/block/CompositeBlockTest.php b/tests/phpunit/includes/block/CompositeBlockTest.php
index 8409f56..428440f 100644 (file)
--- a/tests/phpunit/includes/block/CompositeBlockTest.php
+++ b/tests/phpunit/includes/block/CompositeBlockTest.php
@@ -207,40 +207,52 @@ class CompositeBlockTest extends MediaWikiLangTestCase {
         * @covers ::appliesToRight
         * @dataProvider provideTestBlockAppliesToRight
         */
-       public function testBlockAppliesToRight( $blocks, $right, $expected ) {
+       public function testBlockAppliesToRight( $applies, $expected ) {
                $this->setMwGlobals( [
                        'wgBlockDisablesLogin' => false,
                ] );
 
                $block = new CompositeBlock( [
-                       'originalBlocks' => $blocks,
+                       'originalBlocks' => [
+                               $this->getMockBlockForTestAppliesToRight( $applies[ 0 ] ),
+                               $this->getMockBlockForTestAppliesToRight( $applies[ 1 ] ),
+                       ],
                ] );
 
-               $this->assertSame( $block->appliesToRight( $right ), $expected );
+               $this->assertSame( $block->appliesToRight( 'right' ), $expected );
+       }
+
+       private function getMockBlockForTestAppliesToRight( $applies ) {
+               $mockBlock = $this->getMockBuilder( DatabaseBlock::class )
+                       ->setMethods( [ 'appliesToRight' ] )
+                       ->getMock();
+               $mockBlock->method( 'appliesToRight' )
+                       ->willReturn( $applies );
+               return $mockBlock;
        }
 
-       public static function provideTestBlockAppliesToRight() {
+       public function provideTestBlockAppliesToRight() {
                return [
-                       'Read is not blocked' => [
-                               [
-                                       new DatabaseBlock(),
-                                       new DatabaseBlock(),
-                               ],
-                               'read',
+                       'Block does not apply if no original blocks apply' => [
+                               [ false, false ],
                                false,
                        ],
-                       'Email is blocked if blocked by any blocks' => [
-                               [
-                                       new DatabaseBlock( [
-                                               'blockEmail' => true,
-                                       ] ),
-                                       new DatabaseBlock( [
-                                               'blockEmail' => false,
-                                       ] ),
-                               ],
-                               'sendemail',
+                       'Block applies if any original block applies (second block doesn\'t apply)' => [
+                               [ true, false ],
+                               true,
+                       ],
+                       'Block applies if any original block applies (second block unsure)' => [
+                               [ true, null ],
                                true,
                        ],
+                       'Block is unsure if all original blocks are unsure' => [
+                               [ null, null ],
+                               null,
+                       ],
+                       'Block is unsure if any original block is unsure, and no others apply' => [
+                               [ null, false ],
+                               null,
+                       ],
                ];
        }