A/B test of search in header for logged in users
A new config flag is added that buckets 50% of users into the old header and the 50% into the new header. Bug: T249363 Change-Id: I8b4fa475f9cd7e61ad2989e2a1485e7e64c8ab3f
This commit is contained in:
parent
d72b0daa24
commit
7449c7fdf6
|
@ -79,6 +79,11 @@ final class Constants {
|
||||||
*/
|
*/
|
||||||
public const CONFIG_SEARCH_IN_HEADER = 'VectorIsSearchInHeader';
|
public const CONFIG_SEARCH_IN_HEADER = 'VectorIsSearchInHeader';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @var string
|
||||||
|
*/
|
||||||
|
public const CONFIG_SEARCH_IN_HEADER_AB = 'VectorIsSearchInHeaderABTest';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @var string
|
* @var string
|
||||||
*/
|
*/
|
||||||
|
|
|
@ -31,6 +31,9 @@ use Vector\FeatureManagement\Requirement;
|
||||||
* the tabs (as in the old design).
|
* the tabs (as in the old design).
|
||||||
* The search in header is enabled if:
|
* The search in header is enabled if:
|
||||||
* - the associated feature flag has been enabled
|
* - the associated feature flag has been enabled
|
||||||
|
* - the feature flag for the A/B test is enabled,
|
||||||
|
* and the user is logged and bucketed,
|
||||||
|
* in which case 50% of logged in users will see the search in the header
|
||||||
*
|
*
|
||||||
* @unstable
|
* @unstable
|
||||||
*
|
*
|
||||||
|
@ -43,14 +46,21 @@ final class SearchInHeaderRequirement implements Requirement {
|
||||||
*/
|
*/
|
||||||
private $config;
|
private $config;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @var \User
|
||||||
|
*/
|
||||||
|
private $user;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* This constructor accepts all dependencies needed to determine
|
* This constructor accepts all dependencies needed to determine
|
||||||
* whether search in header is enabled for current user and config.
|
* whether search in header is enabled for current user and config.
|
||||||
*
|
*
|
||||||
* @param \Config $config
|
* @param \Config $config
|
||||||
|
* @param \User $user
|
||||||
*/
|
*/
|
||||||
public function __construct( Config $config ) {
|
public function __construct( \Config $config, \User $user ) {
|
||||||
$this->config = $config;
|
$this->config = $config;
|
||||||
|
$this->user = $user;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -60,11 +70,27 @@ final class SearchInHeaderRequirement implements Requirement {
|
||||||
return Constants::REQUIREMENT_SEARCH_IN_HEADER;
|
return Constants::REQUIREMENT_SEARCH_IN_HEADER;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* If A/B test is enabled check whether the user is logged in and bucketed
|
||||||
|
* @return bool
|
||||||
|
*/
|
||||||
|
private function isBucketed() {
|
||||||
|
$isABTestEnabled = (bool)$this->config->get( Constants::CONFIG_SEARCH_IN_HEADER_AB );
|
||||||
|
|
||||||
|
if ( $isABTestEnabled ) {
|
||||||
|
return $this->user->getId() % 2 === 0;
|
||||||
|
} else {
|
||||||
|
// if A/B test is disabled then resort to using CONFIG_SEARCH_IN_HEADER
|
||||||
|
return (bool)$this->config->get( Constants::CONFIG_SEARCH_IN_HEADER );
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @inheritDoc
|
* @inheritDoc
|
||||||
* @throws \ConfigException
|
* @throws \ConfigException
|
||||||
*/
|
*/
|
||||||
public function isMet() : bool {
|
public function isMet() : bool {
|
||||||
return (bool)$this->config->get( Constants::CONFIG_SEARCH_IN_HEADER );
|
return $this->user->isRegistered() ?
|
||||||
|
$this->isBucketed() : (bool)$this->config->get( Constants::CONFIG_SEARCH_IN_HEADER );
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -71,7 +71,8 @@ return [
|
||||||
// ========================================
|
// ========================================
|
||||||
$featureManager->registerRequirement(
|
$featureManager->registerRequirement(
|
||||||
new SearchInHeaderRequirement(
|
new SearchInHeaderRequirement(
|
||||||
$services->getMainConfig()
|
$services->getMainConfig(),
|
||||||
|
$context->getUser()
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
|
||||||
|
|
|
@ -189,6 +189,9 @@
|
||||||
"VectorIsSearchInHeader": {
|
"VectorIsSearchInHeader": {
|
||||||
"value": false
|
"value": false
|
||||||
},
|
},
|
||||||
|
"VectorIsSearchInHeaderABTest": {
|
||||||
|
"value": false
|
||||||
|
},
|
||||||
"VectorDefaultSidebarVisibleForAuthorisedUser": {
|
"VectorDefaultSidebarVisibleForAuthorisedUser": {
|
||||||
"value": true
|
"value": true
|
||||||
},
|
},
|
||||||
|
|
|
@ -0,0 +1,127 @@
|
||||||
|
<?php
|
||||||
|
/**
|
||||||
|
* This program is free software; you can redistribute it and/or modify
|
||||||
|
* it under the terms of the GNU General Public License as published by
|
||||||
|
* the Free Software Foundation; either version 2 of the License, or
|
||||||
|
* (at your option) any later version.
|
||||||
|
*
|
||||||
|
* This program is distributed in the hope that it will be useful,
|
||||||
|
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||||
|
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||||
|
* GNU General Public License for more details.
|
||||||
|
*
|
||||||
|
* You should have received a copy of the GNU General Public License along
|
||||||
|
* with this program; if not, write to the Free Software Foundation, Inc.,
|
||||||
|
* 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
|
||||||
|
* http://www.gnu.org/copyleft/gpl.html
|
||||||
|
*
|
||||||
|
* @file
|
||||||
|
* @since 1.36
|
||||||
|
*/
|
||||||
|
|
||||||
|
use Vector\Constants;
|
||||||
|
use Vector\FeatureManagement\Requirements\SearchInHeaderRequirement;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @group Vector
|
||||||
|
* @coversDefaultClass \Vector\FeatureManagement\Requirements\SearchInHeaderRequirement
|
||||||
|
*/
|
||||||
|
class SearchInHeaderRequirementTest extends \MediaWikiTestCase {
|
||||||
|
|
||||||
|
public function providerSearchInHeaderRequirement() {
|
||||||
|
return [
|
||||||
|
[
|
||||||
|
// Is enabled for anons
|
||||||
|
false,
|
||||||
|
// is A-B test enabled
|
||||||
|
false,
|
||||||
|
// note 0 = anon user
|
||||||
|
0,
|
||||||
|
false,
|
||||||
|
'If nothing enabled nobody gets search in header'
|
||||||
|
],
|
||||||
|
[
|
||||||
|
// Is enabled for anons
|
||||||
|
true,
|
||||||
|
// is A-B test enabled
|
||||||
|
false,
|
||||||
|
// note 0 = anon user
|
||||||
|
0,
|
||||||
|
true,
|
||||||
|
'All anons should get search in header if enable but when A/B test disabled'
|
||||||
|
],
|
||||||
|
[
|
||||||
|
// Is enabled for anons
|
||||||
|
true,
|
||||||
|
// is A-B test enabled
|
||||||
|
false,
|
||||||
|
0,
|
||||||
|
true,
|
||||||
|
'All even logged in users should get search in header when A/B test disabled'
|
||||||
|
],
|
||||||
|
[
|
||||||
|
// Is enabled for anons
|
||||||
|
true,
|
||||||
|
// is A-B test enabled
|
||||||
|
false,
|
||||||
|
1,
|
||||||
|
true,
|
||||||
|
'All odd logged in users should get search in header when A/B test disabled'
|
||||||
|
],
|
||||||
|
[
|
||||||
|
// Is enabled for anons
|
||||||
|
true,
|
||||||
|
// is A-B test enabled
|
||||||
|
true,
|
||||||
|
// note 0 = anon user
|
||||||
|
0,
|
||||||
|
true,
|
||||||
|
'All anons get search in header even when A/B enabled'
|
||||||
|
],
|
||||||
|
[
|
||||||
|
// Is enabled for anons
|
||||||
|
true,
|
||||||
|
// is A-B test enabled
|
||||||
|
true,
|
||||||
|
2,
|
||||||
|
true,
|
||||||
|
'Bucketed users get search in header when A/B test enabled'
|
||||||
|
],
|
||||||
|
[
|
||||||
|
// Is enabled for anons
|
||||||
|
true,
|
||||||
|
// is A-B test enabled
|
||||||
|
true,
|
||||||
|
1,
|
||||||
|
false,
|
||||||
|
'Non-Bucketed users do not get search in header when A/B test enabled'
|
||||||
|
],
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @covers ::isMet
|
||||||
|
* @dataProvider providerSearchInHeaderRequirement
|
||||||
|
* @param bool $searchInHeaderConfigValue
|
||||||
|
* @param bool $abValue
|
||||||
|
* @param int $userId
|
||||||
|
* @param bool $expected
|
||||||
|
* @param string $msg
|
||||||
|
*/
|
||||||
|
public function testSearchInHeaderRequirement(
|
||||||
|
$searchInHeaderConfigValue, $abValue, $userId, $expected, $msg
|
||||||
|
) {
|
||||||
|
$config = new HashConfig( [
|
||||||
|
Constants::CONFIG_SEARCH_IN_HEADER => $searchInHeaderConfigValue,
|
||||||
|
Constants::CONFIG_SEARCH_IN_HEADER_AB => $abValue,
|
||||||
|
] );
|
||||||
|
$user = $this->getTestUser()->getUser();
|
||||||
|
$user->setId( $userId );
|
||||||
|
|
||||||
|
$requirement = new SearchInHeaderRequirement(
|
||||||
|
$config, $user
|
||||||
|
);
|
||||||
|
|
||||||
|
$this->assertSame( $requirement->isMet(), $expected, $msg );
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue