r105475 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r105474‎ | r105475 | r105476 >
Date:22:54, 7 December 2011
Author:jeroendedauw
Status:deferred (Comments)
Tags:
Comment:
move incorrectly place adding of licenses to fix bug 32860
Modified paths:
  • /trunk/extensions/UploadWizard/UploadWizardHooks.php (modified) (history)
  • /trunk/extensions/UploadWizard/includes/UploadWizardCampaign.php (modified) (history)
  • /trunk/extensions/UploadWizard/includes/specials/SpecialUploadCampaign.php (modified) (history)

Diff [purge]

Index: trunk/extensions/UploadWizard/UploadWizardHooks.php
——@@ -543,7 +543,7 @@
544544 *
545545 * @return string
546546 */
547 - public static function getLicenseMessage( $licenseName, array &$licenseConfig ) {
 547+ public static function getLicenseMessage( $licenseName, array $licenseConfig ) {
548548 if ( array_key_exists( 'url', $licenseConfig[$licenseName] ) ) {
549549 return wfMsgExt( $licenseConfig[$licenseName]['msg'], 'parseinline', '', $licenseConfig[$licenseName]['url'] );
550550 }
Index: trunk/extensions/UploadWizard/includes/specials/SpecialUploadCampaign.php
——@@ -79,13 +79,6 @@
8080 $fields['Campaignname'] = array ( 'type' => 'text', 'default' => $this->subPage, 'label-message' => 'mwe-upwiz-campaign-name' );
8181 $fields['Campaignenabled'] = array ( 'type' => 'check', 'default' => $enabled, 'label-message' => 'mwe-upwiz-campaign-enabled' );
8282
83 - // show _ALL_ the licenses!
84 - $standardConfig = UploadWizardConfig::getConfig();
85 - foreach ( $standardConfig['licenses'] as $key => $license ) {
86 - $configFields['licensesOwnWork']['options'][ wfMsg( $license['msg'] ) ] = $key;
87 - $configFields['defaultOwnWorkLicence']['options'][ wfMsg( $license['msg'] ) ] = $key;
88 - }
89 -
9083 foreach ( $configFields as $name => $data ) {
9184 $data['label-message'] = 'mwe-upwiz-campaign-conf-' . $name;
9285
Index: trunk/extensions/UploadWizard/includes/UploadWizardCampaign.php
——@@ -213,11 +213,11 @@
214214 ),
215215 );
216216
217 - foreach ( $globalConfig['licensesOwnWork']['licenses'] as $license ) {
218 - $licenceMsg = UploadWizardHooks::getLicenseMessage( $license, $globalConfig['licenses'] );
219 - $config['licensesOwnWork']['options'][$licenceMsg] = $license;
 217+ foreach ( $globalConfig['licenses'] as $licenseName => $licenseDate ) {
 218+ $licenceMsg = UploadWizardHooks::getLicenseMessage( $licenseName, $globalConfig['licenses'] );
 219+ $config['licensesOwnWork']['options'][$licenceMsg] = $licenseName;
220220 }
221 -
 221+
222222 $config['defaultOwnWorkLicence']['options'] = $config['licensesOwnWork']['options'];
223223
224224 return $config;

Comments

#Comment by NeilK (talk | contribs)   03:38, 9 December 2011

Seems to work but I'm not entirely sure why. In UploadWizardCampaign you iterate over $globalConfig['licenses'] but then add the values to 'licensesOwnWork' ?

#Comment by NeilK (talk | contribs)   03:41, 9 December 2011

The interface definitely looks wrong in Special:UploadCampaigns... all the licenses are there for own-work, none for third party?

#Comment by Jeroen De Dauw (talk | contribs)   14:11, 9 December 2011

I don't think this rev changed anything other then the message stuff.

> you iterate over $globalConfig['licenses'] but then add the values to 'licensesOwnWork'

Yeah, what's wrong with this? The licensesOwnWork thing is a list of options for the license selection thing, not the list of own work licenses that should be shown in the UW UI itself. It might be better if the license definitions distinguished between which ones belong the the own work category and which ones belong to the third party one, so the code here could omit these that are not relevant. My original code actually just added the own work licenses enabled by default in the config file, but you changed this, as we need the other licences to be available as options as well.

> all the licenses are there for own-work, none for third party?

Ther above comment explains why all licences are under the own work options. There is no third party option, since this was never needed for WLM. I could add one if that is useful.

#Comment by NeilK (talk | contribs)   18:35, 13 December 2011

deferring this for now as we are deploying. Probably not crucial.

Status & tagging log