jenkins-bot has submitted this change and it was merged. Change subject: Remove automatic landing page links feature ......................................................................
Remove automatic landing page links feature Bug: T98747 Change-Id: Ibd68b3dd9323a164986ca44514842748d1918fb7 --- M CentralNotice.hooks.php M CentralNotice.php M CentralNotice.sql M CentralNoticeBannerLogPager.php M i18n/en.json M i18n/qqq.json M includes/Banner.php M modules/ext.centralNotice.adminUi.bannerEditor/bannereditor.js M modules/ext.centralNotice.adminUi/centralnotice.js M modules/ext.centralNotice.bannerController/bannerController.js M special/SpecialBannerLoader.php M special/SpecialCentralNoticeBanners.php M tests/BannerTest.php M tests/CentralNoticeTest.php M tests/CentralNoticeTestFixtures.php M tests/data/allocation_fixtures_doc.md 16 files changed, 21 insertions(+), 192 deletions(-) Approvals: Awight: Looks good to me, approved jenkins-bot: Verified diff --git a/CentralNotice.hooks.php b/CentralNotice.hooks.php index b6f661b..662ddd3 100644 --- a/CentralNotice.hooks.php +++ b/CentralNotice.hooks.php @@ -331,17 +331,22 @@ // Where possible; make things mobile friendly $mc = MobileContext::singleton(); if ( $mc->shouldDisplayMobileView() ) { + // TODO Remove $wgNoticeFundraisingUrl, no longer used $wgNoticeFundraisingUrl = $mc->getMobileUrl( $wgNoticeFundraisingUrl ); $wgCentralBannerRecorder = $mc->getMobileUrl( $wgCentralBannerRecorder ); $wgCentralSelectedBannerDispatcher = $mc->getMobileUrl( $wgCentralSelectedBannerDispatcher ); } } + // TODO Remove, no longer used $vars[ 'wgNoticeFundraisingUrl' ] = $wgNoticeFundraisingUrl; + $vars[ 'wgCentralBannerRecorder' ] = $wgCentralBannerRecorder; $vars[ 'wgCentralNoticeSampleRate' ] = $wgCentralNoticeSampleRate; + // TODO Remove, no longer used $vars[ 'wgNoticeXXCountries' ] = $wgNoticeXXCountries; + $vars[ 'wgNoticeNumberOfBuckets' ] = $wgNoticeNumberOfBuckets; $vars[ 'wgNoticeBucketExpiry' ] = $wgNoticeBucketExpiry; $vars[ 'wgNoticeNumberOfControllerBuckets' ] = $wgNoticeNumberOfControllerBuckets; diff --git a/CentralNotice.php b/CentralNotice.php index e19ccf9..dbff791 100644 --- a/CentralNotice.php +++ b/CentralNotice.php @@ -96,6 +96,7 @@ $wgNoticeEnableFundraising = true; // Base URL for default fundraiser landing page (without query string) +// TODO Remove, no longer used $wgNoticeFundraisingUrl = 'https://donate.wikimedia.org/wiki/Special:LandingCheck'; // Source for live counter information @@ -182,6 +183,7 @@ // These are countries that MaxMind will give out when information is a bit fuzzy. However, // fundraising code doesn't like non ISO countries so we map them to the fictional point case 'XX' +// TODO No longer used, remove $wgNoticeXXCountries = array( 'XX', 'EU', 'AP', 'A1', 'A2', 'O1' ); // String of space delimited domains that will be able to accept data via JS message event when diff --git a/CentralNotice.sql b/CentralNotice.sql index f222ff6..49f8ed4 100644 --- a/CentralNotice.sql +++ b/CentralNotice.sql @@ -35,8 +35,8 @@ `tmp_display_anon` tinyint(1) NOT NULL DEFAULT '1', `tmp_display_account` tinyint(1) NOT NULL DEFAULT '1', `tmp_fundraising` tinyint(1) NOT NULL DEFAULT '0', - `tmp_autolink` tinyint(1) NOT NULL DEFAULT '0', - `tmp_landing_pages` varchar(255) DEFAULT NULL, + `tmp_autolink` tinyint(1) NOT NULL DEFAULT '0', -- Note: no longer used + `tmp_landing_pages` varchar(255) DEFAULT NULL, -- Note: no longer used `tmp_archived` tinyint(1) NOT NULL DEFAULT '0', `tmp_category` varchar(255) DEFAULT NULL, `tmp_preview_sandbox` tinyint(1) NOT NULL DEFAULT '0' diff --git a/CentralNoticeBannerLogPager.php b/CentralNoticeBannerLogPager.php index 564218e..a95bb35 100644 --- a/CentralNoticeBannerLogPager.php +++ b/CentralNoticeBannerLogPager.php @@ -171,6 +171,9 @@ $this->msg( 'centralnotice-category' )->text(), $row->tmplog_end_category )->text() . "<br/>"; + + // Autolink/landing pages feature has been removed, but we might as + // well show any info about it in the logs. $details .= $this->msg( 'centralnotice-log-label', $this->msg( 'centralnotice-autolink' )->text(), diff --git a/i18n/en.json b/i18n/en.json index dd71e1f..062a9b6 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -113,10 +113,6 @@ "centralnotice-banner-collapsible": "Collapsible", "centralnotice-banner-fundraising": "This is a fundraising banner", "centralnotice-banner-display-on": "Display on", - "centralnotice-banner-autolink": "Automatically create landing page link", - "centralnotice-banner-autolink-help": "When enabled CentralNotice will automatically create a localised link to $3 with a landing page selected at random. There must be an anchor tag with $1 in the banner body.", - "centralnotice-banner-landing-pages": "Landing pages (comma-separated)", - "centralnotice-banner-landing-pages-default": "e.g. JimmyAppeal01", "centralnotice-banner-mixins": "Mixins", "centralnotice-banner-mixins-help": "Check each mixin that you want to be included during banner delivery. (See [https://www.mediawiki.org/wiki/Extension:CentralNotice/Banner_mixins Banner mixins] for more information.)", "centralnotice-geo": "Geotargeted", diff --git a/i18n/qqq.json b/i18n/qqq.json index 70b6ac0..1750ed2 100644 --- a/i18n/qqq.json +++ b/i18n/qqq.json @@ -136,10 +136,6 @@ "centralnotice-banner-collapsible": "Unused at this time.", "centralnotice-banner-fundraising": "Label of checkbox", "centralnotice-banner-display-on": "A label in a form. The options to select will be different platforms, such as \"desktop\", \"Android\" etc.", - "centralnotice-banner-autolink": "Label of checkbox", - "centralnotice-banner-autolink-help": "Brief instructions on how to use the interface. Parameters:\n* $1 - a bit of HTML\n* $2 - (Unused) a title for a page\n* $3 - a URL", - "centralnotice-banner-landing-pages": "Label for an input box. Landing pages are valid MediaWiki titles that donors are redirected to upon clicking a banner.", - "centralnotice-banner-landing-pages-default": "Example valid landing pages for an input box placeholder text.", "centralnotice-banner-mixins": "Label for a list of banner \"[[mw:Extension:CentralNotice/Banner_mixins|mixins]]\", code components which can be reused between banners.", "centralnotice-banner-mixins-help": "Help text for editing a list of \"[[mw:Extension:CentralNotice/Banner_mixins|banner mixins]]\", code components which can be reused between banners.", "centralnotice-geo": "Used to label a checkbox which activates geotargeting", diff --git a/includes/Banner.php b/includes/Banner.php index ec63e1b..0a3abe2 100644 --- a/includes/Banner.php +++ b/includes/Banner.php @@ -76,14 +76,6 @@ /** @var string Category that the banner belongs to. Will be special value expanded. */ protected $category = '{{{campaign}}}'; - /** @var bool True if <a#cn-landingpage-link> will have it's href attribute set randomly from. - * @see Banner->landingLinks - */ - protected $autolink = false; - - /** @var string[] Array of href targets. See Banner->autolink */ - protected $autolinks = array(); - /** @var bool True if archived and hidden from default view. */ protected $archived = false; @@ -292,53 +284,6 @@ } /** - * If true the banner renderer should replace the contents of the <a#cn-landingpage-link> href - * with values from @see Banner->getAutoLinks() - * - * @return bool - */ - public function isAutoLinked() { - $this->populateBasicData(); - return $this->autolink; - } - - /** - * Automatic linking values for the banner renderer to use. - * - * @see Banner->isAutoLinked() - * - * @return string[] - */ - public function getAutoLinks() { - $this->populateBasicData(); - return $this->autolinks; - } - - /** - * Set autolinking behaviour. If enabled the banner renderer should replace the contents of - * the <a#cn-landingpage-link> href with values from $links - * - * @param bool $enabled - * @param string[] $links Array of raw href links. E.g. array( '//foo.bar.com/' ) - * - * @return $this - */ - public function setAutoLink( $enabled, $links ) { - $this->populateBasicData(); - - $links = (array)$links; - array_walk( $links, function ( &$x ) { $x = trim( $x ); } ); - sort( $links ); - if ( ( $this->autolink !== $enabled ) || ( $this->autolinks != $links ) ) { - $this->setBasicDataDirty(); - $this->autolink = $enabled; - $this->autolinks = $links; - } - - return $this; - } - - /** * Should the banner be considered archived and hidden from default view * * @return bool @@ -378,8 +323,6 @@ 'tmp_name', 'tmp_display_anon', 'tmp_display_account', - 'tmp_autolink', - 'tmp_landing_pages', 'tmp_archived', 'tmp_category' ), @@ -394,8 +337,6 @@ $this->name = $row->tmp_name; $this->allocateAnon = (bool)$row->tmp_display_anon; $this->allocateLoggedIn = (bool)$row->tmp_display_account; - $this->autolink = (bool)$row->tmp_autolink; - $this->autolinks = explode( ',', $row->tmp_landing_pages); $this->archived = (bool)$row->tmp_archived; $this->category = $row->tmp_category; } else { @@ -438,8 +379,6 @@ array( 'tmp_display_anon' => (int)$this->allocateAnon, 'tmp_display_account' => (int)$this->allocateLoggedIn, - 'tmp_autolink' => $this->autolink, - 'tmp_landing_pages' => implode( ',', $this->autolinks ), 'tmp_archived' => $this->archived, 'tmp_category' => $this->category, ), @@ -1082,7 +1021,6 @@ $destBanner->setAllocation( $this->allocateToAnon(), $this->allocateToLoggedIn() ); $destBanner->setCategory( $this->getCategory() ); - $destBanner->setAutoLink( $this->isAutoLinked(), $this->getAutoLinks() ); $destBanner->setDevices( $this->getDevices() ); $destBanner->setMixins( array_keys( $this->getMixins() ) ); $destBanner->setPriorityLanguages( $this->getPriorityLanguages() ); @@ -1241,8 +1179,6 @@ 'tmp_display_anon', 'tmp_display_account', 'tmp_category', - 'tmp_autolink', - 'tmp_landing_pages', 'not_name', 'not_preferred', 'asn_bucket', @@ -1272,8 +1208,6 @@ 'display_anon' => intval( $row->tmp_display_anon ), // display to anonymous users? 'display_account' => intval( $row->tmp_display_account ), // display to logged in users? 'fundraising' => intval( $row->tmp_category === 'fundraising' ), // fundraising banner? - 'autolink' => intval( $row->tmp_autolink ), // automatically create links? - 'landing_pages' => $row->tmp_landing_pages, // landing pages to link to 'device' => $row->dev_name, // device this banner can target 'campaign' => $row->not_name, // campaign the banner is assigned to 'campaign_z_index' => $row->not_preferred, // z level of the campaign @@ -1306,8 +1240,6 @@ 'account' => (int)$banner->allocateToLoggedIn(), 'fundraising' => (int)($banner->getCategory() === 'fundraising'), // TODO: Death to this! 'category' => $banner->getCategory(), - 'autolink' => (int)$banner->isAutoLinked(), - 'landingpages' => implode( ',', $banner->getAutoLinks() ), 'controller_mixin' => implode( ",", array_keys( $banner->getMixins() ) ), 'devices' => array_values( $banner->getDevices() ), ); @@ -1326,8 +1258,6 @@ * display_anon: 0/1 whether the banner is displayed to anonymous users * display_account: 0/1 same, for logged-in users * fundraising: 0/1, is in the fundraising group - * autolink: make landing page autolink? - * landing_pages: array of landing page names * device: device key */ static function getHistoricalBanner( $name, $ts ) { @@ -1357,8 +1287,6 @@ "display_anon" => "tmplog_end_anon", "display_account" => "tmplog_end_account", "fundraising" => "tmplog_end_fundraising", - "autolink" => "tmplog_end_autolink", - "landing_pages" => "tmplog_end_landingpages", ), array( "tmplog_id = {$newestLog->log_id}", @@ -1369,8 +1297,6 @@ $banner['display_account'] = (int) $row->display_account; $banner['fundraising'] = (int) $row->fundraising; - $banner['autolink'] = (int) $row->autolink; - $banner['landing_pages'] = explode( ", ", $row->landing_pages ); //XXX $banner['devices'] = array( "desktop" ); @@ -1386,8 +1312,6 @@ * @param $displayAnon integer flag for display to anonymous users * @param $displayAccount integer flag for display to logged in users * @param $fundraising integer flag for fundraising banner (optional) - * @param $autolink integer flag for automatically creating landing page links (optional) - * @param $landingPages string list of landing pages (optional) * @param $mixins array list of mixins (optional) * @param $priorityLangs array Array of priority languages for the translate extension * @param $devices array Array of device names this banner is targeted at @@ -1395,7 +1319,7 @@ * @return bool true or false depending on whether banner was successfully added */ static function addTemplate( $name, $body, $user, $displayAnon, - $displayAccount, $fundraising = 0, $autolink = 0, $landingPages = '', + $displayAccount, $fundraising = 0, $mixins = array(), $priorityLangs = array(), $devices = null, $summary = null ) { @@ -1419,10 +1343,6 @@ $banner->setDevices( $devices ); $banner->setPriorityLanguages( $priorityLangs ); $banner->setBodyContent( $body ); - - $landingPages = explode( ',', $landingPages ); - array_walk( $landingPages, function ( &$x ) { $x = trim( $x ); } ); - $banner->setAutoLink( $autolink, $landingPages ); $banner->setMixins( $mixins ); diff --git a/modules/ext.centralNotice.adminUi.bannerEditor/bannereditor.js b/modules/ext.centralNotice.adminUi.bannerEditor/bannereditor.js index 4c6e966..5d56399 100644 --- a/modules/ext.centralNotice.adminUi.bannerEditor/bannereditor.js +++ b/modules/ext.centralNotice.adminUi.bannerEditor/bannereditor.js @@ -149,18 +149,6 @@ }, /** - * Shows or hides the landing pages edit box control based on the status of - * the "Automatically create landing page link" check box. - */ - showHideLpEditBox: function() { - if ( $( '#mw-input-wpcreate-landingpage-link' ).prop( 'checked' ) ) { - $( '#mw-input-wplanding-pages' ).parent().parent().show(); - } else { - $( '#mw-input-wplanding-pages' ).parent().parent().hide(); - } - }, - - /** * Hook function from onclick of the translate language drop down -- will submit the * form in order to update the language of the preview and the displayed translations. */ @@ -212,9 +200,7 @@ $( '#mw-input-wpclone-button' ).click( mw.centralNotice.adminUi.bannerEditor.doCloneBannerDialog ); $( '#mw-input-wpsave-button' ).click( mw.centralNotice.adminUi.bannerEditor.doSaveBanner ); $( '#mw-input-wptranslate-language' ).change( mw.centralNotice.adminUi.bannerEditor.updateLanguage ); - $( '#mw-input-wpcreate-landingpage-link' ).change( mw.centralNotice.adminUi.bannerEditor.showHideLpEditBox ); // And do some initial form work - mw.centralNotice.adminUi.bannerEditor.showHideLpEditBox(); $( '#cn-js-error-warn' ).hide(); } )( jQuery, mediaWiki ); diff --git a/modules/ext.centralNotice.adminUi/centralnotice.js b/modules/ext.centralNotice.adminUi/centralnotice.js index 6df20ac..b1eaead 100644 --- a/modules/ext.centralNotice.adminUi/centralnotice.js +++ b/modules/ext.centralNotice.adminUi/centralnotice.js @@ -96,15 +96,6 @@ } }); - // Reveal the landing page interface when the autolink checkbox is checked - $( '#autolink' ).click(function () { - if ( $( '#autolink' ).prop( 'checked' ) ) { - $( '#autolinkInterface' ).fadeIn( 'fast' ); - } else { - $( '#autolinkInterface' ).fadeOut( 'fast' ); - } - }); - // Special:CentralNotice; keep data-sort-value attributes for // jquery.tablesorter in sync $( '.mw-cn-input-check-sort' ).on( 'change click blur', function () { diff --git a/modules/ext.centralNotice.bannerController/bannerController.js b/modules/ext.centralNotice.bannerController/bannerController.js index 7f77078..68b07cf 100644 --- a/modules/ext.centralNotice.bannerController/bannerController.js +++ b/modules/ext.centralNotice.bannerController/bannerController.js @@ -429,7 +429,7 @@ // will NOT be made. // mw.centralNotice.insertBanner = function ( bannerJson ) { - var url, targets, durations, cookieName, cookieVal, deleteOld, now, + var durations, cookieName, cookieVal, deleteOld, now, parsedCookie, bucket; var impressionData = $.extend( mw.centralNotice.impressionData, { @@ -500,26 +500,6 @@ $( 'div#centralNotice' ) .attr( 'class', mw.html.escape( 'cn-' + mw.centralNotice.data.category ) ) .prepend( '<!--googleoff: all-->' + bannerJson.bannerHtml + '<!--googleon: all-->' ); - - // Create landing page links if required - if ( bannerJson.autolink ) { - url = new mw.Uri( mw.config.get( 'wgNoticeFundraisingUrl' ) ); - if ( ( bannerJson.landingPages !== null ) && bannerJson.landingPages.length ) { - targets = String( bannerJson.landingPages ).split( ',' ); - if ( $.inArray( mw.centralNotice.data.country, mw.config.get( 'wgNoticeXXCountries' ) ) !== -1 ) { - mw.centralNotice.data.country = 'XX'; - } - url.extend( { - landing_page: targets[Math.floor( Math.random() * targets.length )].replace( /^\s+|\s+$/, '' ), - utm_medium: 'sitenotice', - utm_campaign: bannerJson.campaign, - utm_source: bannerJson.bannerName, - language: mw.config.get( 'wgUserLanguage' ), - country: mw.centralNotice.data.country - } ); - $( '#cn-landingpage-link' ).attr( 'href', url.toString() ); - } - } // Query the initial impression state if the banner callback exists var bannerShown = true; diff --git a/special/SpecialBannerLoader.php b/special/SpecialBannerLoader.php index 26c0695..74e2488 100644 --- a/special/SpecialBannerLoader.php +++ b/special/SpecialBannerLoader.php @@ -142,8 +142,6 @@ 'bannerHtml' => $bannerHtml, 'campaign' => $this->campaignName, 'category' => $category, - 'autolink' => $settings['autolink'], - 'landingPages' => explode( ", ", $settings['landingpages'] ), ); $bannerJson = FormatJson::encode( $bannerArray ); diff --git a/special/SpecialCentralNoticeBanners.php b/special/SpecialCentralNoticeBanners.php index d59c7de..1e33234 100644 --- a/special/SpecialCentralNoticeBanners.php +++ b/special/SpecialCentralNoticeBanners.php @@ -261,7 +261,7 @@ false, false, // Default values of a zillion parameters... - 0, 0, '', array(), array(), null, + 0, array(), array(), null, $formData['newBannerEditSummary'] ); @@ -480,31 +480,6 @@ 'default' => $assignedDevices, 'cssclass' => 'separate-form-element', ); - - $formDescriptor[ 'create-landingpage-link' ] = array( - 'section' => 'settings', - 'type' => 'toggle', - 'disabled' => !$this->editable, - 'hidelabel' => true, - 'label-message' => 'centralnotice-banner-autolink', - 'help' => $this->msg( 'centralnotice-banner-autolink-help' )-> - rawParams( 'id="cn-landingpage-link"', 'JimmyAppeal01', $wgNoticeFundraisingUrl )-> - escaped(), - 'cssclass' => 'separate-form-element', - 'default' => $bannerSettings[ 'autolink' ], - ); - - $formDescriptor[ 'landing-pages' ] = array( - 'section' => 'settings', - 'type' => 'text', - 'label-message' => 'centralnotice-banner-landing-pages', - 'placeholder' => $this->msg( 'centralnotice-banner-landing-pages-default' )->escaped(), - 'cssclass' => 'separate-form-element', - 'default' => $bannerSettings[ 'landingpages' ], - ); - if ( !$this->editable ) { - $formDescriptor[ 'landing-pages' ][ 'readonly' ] = true; - } $mixinNames = array_keys( $wgCentralNoticeBannerMixins ); $availableMixins = array_combine( $mixinNames, $mixinNames ); @@ -880,10 +855,6 @@ $banner->setDevices( $formData[ 'device-classes' ] ); $banner->setPriorityLanguages( $prioLang ); $banner->setBodyContent( $formData[ 'banner-body' ] ); - - $landingPages = explode( ',', $formData[ 'landing-pages' ] ); - array_walk( $landingPages, function ( &$x ) { $x = trim( $x ); } ); - $banner->setAutoLink( $formData[ 'create-landingpage-link' ], $landingPages ); $banner->setMixins( $formData['mixins'] ); $banner->save( $this->getUser(), $summary ); diff --git a/tests/BannerTest.php b/tests/BannerTest.php index b9f4e77..c65b899 100644 --- a/tests/BannerTest.php +++ b/tests/BannerTest.php @@ -44,8 +44,6 @@ $this->assertFalse( $banner->allocateToAnon(), 'Initial anonymous allocation is set to true' ); $this->assertFalse( $banner->allocateToLoggedIn(), 'Initial logged in allocation is set to true' ); $this->assertEquals( '{{{campaign}}}', $banner->getCategory(), 'Initial category is not equal to {{{campaign}}}' ); - $this->assertFalse( $banner->isAutoLinked(), 'Initial banner is autolinked' ); - $this->assertEquals( array(), $banner->getAutoLinks(), 'Initial banner has autolinks' ); $this->assertFalse( $banner->isArchived(), 'Initial banner is archived?' ); // More complex metadata should also be empty @@ -79,8 +77,6 @@ $this->assertFalse( $banner->allocateToAnon(), 'Initial anonymous allocation is set to true' ); $this->assertFalse( $banner->allocateToLoggedIn(), 'Initial logged in allocation is set to true' ); $this->assertEquals( '{{{campaign}}}', $banner->getCategory(), 'Initial category is not equal to {{{campaign}}}' ); - $this->assertFalse( $banner->isAutoLinked(), 'Initial banner is autolinked' ); - $this->assertEquals( array(), $banner->getAutoLinks(), 'Initial banner has autolinks' ); $this->assertFalse( $banner->isArchived(), 'Initial banner is archived?' ); // More complex metadata should also be empty @@ -102,8 +98,7 @@ // Attempt to populate basic metadata $banner->setAllocation( true, true )-> - setCategory( 'testCategory' )-> - setAutoLink( true, array( 'testlink' ) ); + setCategory( 'testCategory' ); // And the more advanced metadata $banner->setDevices( 'desktop' ); @@ -115,8 +110,6 @@ $this->assertTrue( $banner->allocateToAnon(), "Failed retrieve anon allocation from initial" ); $this->assertTrue( $banner->allocateToLoggedIn(), "Failed retrieve logged in allocation from initial" ); $this->assertEquals( 'testCategory', $banner->getCategory(), "Failed retrieve category from initial" ); - $this->assertTrue( $banner->isAutoLinked(), "Failed autolink retrieve from initial" ); - $this->assertEquals( array( 'testlink' ), $banner->getAutoLinks(), 'Failed autolinks retrieve from initial' ); $this->assertEquals( array( 'desktop' ), array_values( $banner->getDevices() ), 'Failed devices retrieve from initial' ); $this->assertEquals( array( 'en', 'ru' ), $banner->getPriorityLanguages(), "Failed prilang retrieve from initial" ); @@ -126,8 +119,6 @@ $this->assertTrue( $banner2->allocateToAnon(), "Failed anon allocation from copy" ); $this->assertTrue( $banner2->allocateToLoggedIn(), "Failed loggedin allocation from copy" ); $this->assertEquals( 'testCategory', $banner2->getCategory(), "Failed category from copy" ); - $this->assertTrue( $banner2->isAutoLinked(), "Failed autolink from copy" ); - $this->assertEquals( array( 'testlink' ), $banner2->getAutoLinks(), "Failed autolinks from copy" ); $this->assertEquals( array( 'desktop' ), array_values( $banner2->getDevices() ), "Failed devices from copy" ); diff --git a/tests/CentralNoticeTest.php b/tests/CentralNoticeTest.php index ef3950e..b7ecc8d 100644 --- a/tests/CentralNoticeTest.php +++ b/tests/CentralNoticeTest.php @@ -75,14 +75,12 @@ $displayAnon = 1; $displayAccount = 1; $fundraising = 1; - $autolink = 0; - $landingPages = 'JA1, JA2'; $campaign_z_index = 1; - $this->campaignBannersJson = '[{"name":"PHPUnitTestBanner","weight":25,"display_anon":1,"display_account":1,"fundraising":1,"autolink":0,"landing_pages":"JA1,JA2","device":"desktop","campaign":"PHPUnitTestCampaign","campaign_z_index":"1","campaign_num_buckets":1,"campaign_throttle":100,"bucket":0}]'; + $this->campaignBannersJson = '[{"name":"PHPUnitTestBanner","weight":25,"display_anon":1,"display_account":1,"fundraising":1,"device":"desktop","campaign":"PHPUnitTestCampaign","campaign_z_index":"1","campaign_num_buckets":1,"campaign_throttle":100,"bucket":0}]'; Banner::addTemplate( $bannerName, $body, $this->userUser, $displayAnon, $displayAccount, - $fundraising, $autolink, $landingPages ); + $fundraising ); Campaign::addTemplateTo( 'PHPUnitTestCampaign', 'PHPUnitTestBanner', '25' ); } diff --git a/tests/CentralNoticeTestFixtures.php b/tests/CentralNoticeTestFixtures.php index 56c6d6d..3319d6a 100644 --- a/tests/CentralNoticeTestFixtures.php +++ b/tests/CentralNoticeTestFixtures.php @@ -34,8 +34,6 @@ 'display_anon' => true, 'display_account' => true, 'fundraising' => 1, - 'autolink' => 0, - 'landingPages' => 'JA1, JA2', 'weight' => 25, ); } @@ -279,10 +277,6 @@ ); } - // autolink and landingPage properties are not relevant to tests set - // up via fixture data and may not be provided. For those - // parameters, in that case, provide the default value in the - // Banner::addTemplate method signature. foreach ( $campaign['banners'] as $bannerSpec ) { Banner::addTemplate( $bannerSpec['name'], @@ -290,9 +284,7 @@ $this->user, $bannerSpec['display_anon'], $bannerSpec['display_account'], - $bannerSpec['fundraising'], - isset( $bannerSpec['autolink'] ) ? $bannerSpec['autolink'] : 0, - isset( $bannerSpec['landingPages'] ) ? $bannerSpec['landingPages'] : '' + $bannerSpec['fundraising'] ); Campaign::addTemplateTo( diff --git a/tests/data/allocation_fixtures_doc.md b/tests/data/allocation_fixtures_doc.md index 37c48c7..1f17d99 100644 --- a/tests/data/allocation_fixtures_doc.md +++ b/tests/data/allocation_fixtures_doc.md @@ -97,8 +97,8 @@ For non-geotargeted campaigns, a default, empty countries property is added. Some properties that are not relevant to these tests are -automatically given default values (autolink, landing page, banner -body). An empty mixins property is added to choices for server-side tests. +automatically given default values (bannerbody, campaign mixins for +server-side tests). Issues -- To view, visit https://gerrit.wikimedia.org/r/227647 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: Ibd68b3dd9323a164986ca44514842748d1918fb7 Gerrit-PatchSet: 2 Gerrit-Project: mediawiki/extensions/CentralNotice Gerrit-Branch: campaign_mixins Gerrit-Owner: AndyRussG <andrew.green...@gmail.com> Gerrit-Reviewer: AndyRussG <andrew.green...@gmail.com> Gerrit-Reviewer: Awight <awi...@wikimedia.org> Gerrit-Reviewer: Cdentinger <cdentin...@wikimedia.org> Gerrit-Reviewer: Ejegg <eeggles...@wikimedia.org> Gerrit-Reviewer: Siebrand <siebr...@kitano.nl> Gerrit-Reviewer: Springle <sprin...@wikimedia.org> Gerrit-Reviewer: Ssmith <ssm...@wikimedia.org> Gerrit-Reviewer: XenoRyet <dkozlow...@wikimedia.org> Gerrit-Reviewer: jenkins-bot <> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits