From 136054d95e7db157ffbe286460d63ab7484228a5 Mon Sep 17 00:00:00 2001
From: Thalia <thalia.e.chan@googlemail.com>
Date: Fri, 30 Aug 2019 19:37:15 +0100
Subject: [PATCH] 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
---
 includes/block/AbstractBlock.php              |  5 +-
 includes/block/CompositeBlock.php             | 21 +++++++-
 .../includes/block/CompositeBlockTest.php     | 52 ++++++++++++-------
 3 files changed, 55 insertions(+), 23 deletions(-)

diff --git a/includes/block/AbstractBlock.php b/includes/block/AbstractBlock.php
index 4d4bb07dc5..fa91909b60 100644
--- 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 3f3e2d3a5a..6f49f170ee 100644
--- 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 8409f562b5..428440f9fe 100644
--- 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,
+			],
 		];
 	}
 
-- 
2.20.1