Separate names for server-side vs. client-side feature flags

The two are different:

* CiteReferencePreviews as specified in extension.json is a feature
flag that allows us to disable the feature entirely. It could be
named "CiteReferencePreviewsFeature" or "CiteEnableReferencePreviews",
but renaming a feature flag that's already in use is hard.

* The client-side flag tells the JavaScript code "you know what, it
was kind of a mistake you got loaded, please stop". This is because
we can not make all decisions before we register the ResourceLoader
module, e.g. if the user has certain gadgets enabled.

Adding the word "Active" is not a huge improvement, but at least
makes the two different now. Suggestions welcome.

Bug: T362771
Change-Id: I0f6a911df8772616ac50c1301f402f77dbe32089
This commit is contained in:
thiemowmde 2024-04-30 11:51:38 +02:00 committed by Thiemo Kreuz (WMDE)
parent f0d7406811
commit e3fdee52aa
6 changed files with 15 additions and 14 deletions

View file

@ -15,7 +15,7 @@ const { TYPE_REFERENCE } = require( './constants.js' );
* @return {boolean|null} Null when there is no way the popup type can be enabled at run-time.
*/
function isReferencePreviewsEnabled( user, isPreviewTypeEnabled, config ) {
if ( !config.get( 'wgCiteReferencePreviews' ) ) {
if ( !config.get( 'wgCiteReferencePreviewsActive' ) ) {
return null;
}

View file

@ -6,7 +6,7 @@ const LOGGING_SCHEMA = 'event.ReferencePreviewsPopups';
* Run once the preview is initialized.
*/
function initReferencePreviewsInstrumentation() {
if ( mw.config.get( 'wgCiteReferencePreviews' ) &&
if ( mw.config.get( 'wgCiteReferencePreviewsActive' ) &&
navigator.sendBeacon &&
mw.config.get( 'wgIsArticle' ) &&
!isTracking

View file

@ -70,7 +70,7 @@ class CiteHooks implements
$out->getConfig(),
$this->userOptionsLookup
);
$vars['wgCiteReferencePreviews'] = $referencePreviewsContext->isReferencePreviewsEnabled(
$vars['wgCiteReferencePreviewsActive'] = $referencePreviewsContext->isReferencePreviewsEnabled(
$out->getUser(),
$out->getSkin()
);

View file

@ -13,9 +13,7 @@ use Skin;
class ReferencePreviewsContext {
private Config $config;
private ReferencePreviewsGadgetsIntegration $gadgetsIntegration;
private UserOptionsLookup $userOptionsLookup;
public function __construct(
@ -34,6 +32,11 @@ class ReferencePreviewsContext {
*/
public const REFERENCE_PREVIEWS_PREFERENCE_NAME = 'popups-reference-previews';
/**
* If the client-side code for Reference Previews should continue loading
* (see isReferencePreviewsEnabled.js), incorporating decisions we can only make after the
* ResourceLoader module was registered via {@see CiteHooks::onResourceLoaderRegisterModules}.
*/
public function isReferencePreviewsEnabled( User $user, Skin $skin ): bool {
if (
// T243822: Temporarily disabled in the mobile skin
@ -47,6 +50,7 @@ class ReferencePreviewsContext {
return false;
}
// Anonymous users can (de)activate the feature via a cookie at runtime, hence it must load
return !$user->isNamed() || $this->userOptionsLookup->getBoolOption(
$user, self::REFERENCE_PREVIEWS_PREFERENCE_NAME
);

View file

@ -13,19 +13,16 @@ use Wikimedia\Services\NoSuchServiceException;
/**
* Gadgets integration
*
* @package ReferencePreviews
* @license GPL-2.0-or-later
*/
class ReferencePreviewsGadgetsIntegration {
public const CONFIG_NAVIGATION_POPUPS_NAME = 'CiteReferencePreviewsConflictingNavPopupsGadgetName';
public const CONFIG_REFERENCE_TOOLTIPS_NAME = 'CiteReferencePreviewsConflictingRefTooltipsGadgetName';
private ?GadgetRepo $gadgetRepo;
private string $navPopupsGadgetName;
private string $refTooltipsGadgetName;
public function __construct( Config $config ) {

View file

@ -28,21 +28,21 @@ QUnit.test( 'relevant combinations of anonymous flags', ( assert ) => {
[
{
testCase: 'enabled for an anonymous user',
wgCiteReferencePreviews: true,
wgCiteReferencePreviewsActive: true,
isAnon: true,
enabledByAnon: true,
expected: true
},
{
testCase: 'turned off via the feature flag (anonymous user)',
wgCiteReferencePreviews: false,
wgCiteReferencePreviewsActive: false,
isAnon: true,
enabledByAnon: true,
expected: null
},
{
testCase: 'manually disabled by the anonymous user',
wgCiteReferencePreviews: true,
wgCiteReferencePreviewsActive: true,
isAnon: true,
enabledByAnon: false,
expected: false
@ -62,7 +62,7 @@ QUnit.test( 'relevant combinations of anonymous flags', ( assert ) => {
return data.enabledByAnon;
},
config = new Map();
config.set( 'wgCiteReferencePreviews', data.wgCiteReferencePreviews );
config.set( 'wgCiteReferencePreviewsActive', data.wgCiteReferencePreviewsActive );
if ( data.isAnon ) {
user.options.get = () => assert.true( false, 'not expected to be called 2' );
@ -83,7 +83,7 @@ QUnit.test( 'it should display reference previews when conditions are fulfilled'
userSettings = createStubUserSettings( false ),
config = new Map();
config.set( 'wgCiteReferencePreviews', true );
config.set( 'wgCiteReferencePreviewsActive', true );
assert.true(
require( 'ext.cite.referencePreviews' ).private.isReferencePreviewsEnabled( user, userSettings, config ),
@ -96,7 +96,7 @@ QUnit.test( 'it should not be enabled when the global is disabling it', ( assert
userSettings = createStubUserSettings( false ),
config = new Map();
config.set( 'wgCiteReferencePreviews', false );
config.set( 'wgCiteReferencePreviewsActive', false );
assert.strictEqual(
require( 'ext.cite.referencePreviews' ).private.isReferencePreviewsEnabled( user, userSettings, config ),