r113631 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r113630‎ | r113631 | r113632 >
Date:16:57, 12 March 2012
Author:nikerabbit
Status:resolved (Comments)
Tags:i18nreview 
Comment:
Committing my unfinished cleanups - should still be fully functional.
Major issue was the duplicated id parameters on different elements.
Moved them to data-attributes.
Aggregate group id is now stored only in the wrapper div.
Subgroup ids are either in the remove button span or in the selector as value.
Modified paths:
  • /trunk/extensions/Translate/Translate.php (modified) (history)
  • /trunk/extensions/Translate/resources/ext.translate.special.aggregategroups.js (modified) (history)
  • /trunk/extensions/Translate/specials/SpecialAggregateGroups.php (modified) (history)

Diff [purge]

Index: trunk/extensions/Translate/Translate.php
@@ -262,6 +262,7 @@
263263 'scripts' => 'resources/ext.translate.special.aggregategroups.js',
264264 'styles' => 'resources/ext.translate.special.aggregategroups.css',
265265 'position' => 'top',
 266+ 'dependencies' => array( 'mediawiki.util' ),
266267 ) + $resourcePaths;
267268
268269 $wgResourceModules['ext.translate.special.supportedlanguages'] = array(
Index: trunk/extensions/Translate/specials/SpecialAggregateGroups.php
@@ -4,6 +4,7 @@
55 *
66 * @file
77 * @author Santhosh Thottingal
 8+ * @author Niklas Laxström
89 * @copyright Copyright © 2012 Santhosh Thottingal
910 * @license http://www.gnu.org/copyleft/gpl.html GNU General Public License 2.0 or later
1011 */
@@ -25,97 +26,71 @@
2627 global $wgRequest, $wgOut, $wgUser;
2728 $this->user = $wgUser;
2829 $request = $wgRequest;
 30+ $out = $this->getOutput();
2931
3032 // Check permissions
3133 if ( !$this->user->isAllowed( 'translate-manage' ) ) {
32 - $wgOut->permissionRequired( 'translate-manage' );
 34+ $out->permissionRequired( 'translate-manage' );
3335 return;
3436 }
3537
3638 // Check permissions
3739 if ( $wgRequest->wasPosted() && !$this->user->matchEditToken( $wgRequest->getText( 'token' ) ) ) {
3840 self::superDebug( __METHOD__, "token failure", $this->user );
39 - $wgOut->permissionRequired( 'translate-manage' );
 41+ $out->permissionRequired( 'translate-manage' );
4042 return;
4143 }
42 - $this->showAggregateGroups();
43 -
44 - }
45 -
46 - public function loadPagesFromDB() {
47 - $dbr = wfGetDB( DB_MASTER );
48 - $tables = array( 'page', 'revtag' );
49 - $vars = array( 'page_id', 'page_title', 'page_namespace', 'page_latest', 'MAX(rt_revision) as rt_revision', 'rt_type' );
50 - $conds = array(
51 - 'page_id=rt_page',
52 - 'rt_type' => array( RevTag::getType( 'tp:mark' ), RevTag::getType( 'tp:tag' ) ),
53 - );
54 - $options = array(
55 - 'ORDER BY' => 'page_namespace, page_title',
56 - 'GROUP BY' => 'page_id, rt_type',
57 - );
58 - $res = $dbr->select( $tables, $vars, $conds, __METHOD__, $options );
59 -
60 - return $res;
61 - }
62 -
63 - protected function buildPageArray( /*db result*/ $res ) {
 44+
 45+ $groups = MessageGroups::getAllGroups();
 46+ $aggregates = array();
6447 $pages = array();
65 - foreach ( $res as $r ) {
66 - // We have multiple rows for same page, because of different tags
67 - if ( !isset( $pages[$r->page_id] ) ) {
68 - $pages[$r->page_id] = array();
69 - $title = Title::newFromRow( $r );
70 - $pages[$r->page_id]['title'] = $title;
71 - $pages[$r->page_id]['latest'] = intval( $title->getLatestRevID() );
 48+ foreach ( $groups as $group ) {
 49+ if ( $group instanceof WikiPageMessageGroup ) {
 50+ $pages[] = $group;
 51+ } elseif ( $group instanceof AggregateMessageGroup ) {
 52+ if ( TranslateMetadata::get( $group->getId(), 'subgroups' ) !== false ) {
 53+ $aggregates[] = $group;
 54+ }
7255 }
 56+ }
7357
74 - $tag = RevTag::typeToTag( $r->rt_type );
75 - $pages[$r->page_id][$tag] = intval( $r->rt_revision );
 58+ if ( !count( $pages ) ) {
 59+ // @TODO use different message
 60+ $out->addWikiMsg( 'tpt-list-nopages' );
 61+ return;
7662 }
77 - return $pages;
78 - }
7963
 64+ $this->showAggregateGroups( $aggregates, $pages );
8065
81 - protected function showAggregateGroups() {
 66+ }
 67+
 68+ protected function showAggregateGroups( array $aggregates, array $pages ) {
8269 global $wgOut;
8370 $wgOut->addModules( 'ext.translate.special.aggregategroups' );
8471
85 - $aggregategroups = MessageGroups::getAggregateGroups( );
86 - $res = $this->loadPagesFromDB();
87 - $pages = $this->buildPageArray( $res );
88 - foreach ( $aggregategroups as $id => $group ) {
89 - $wgOut->addHtml( "<div id='tpt-aggregate-group'>" );
 72+ foreach ( $aggregates as $group ) {
 73+ $id = $group->getId();
 74+ $div = Html::openElement( 'div', array(
 75+ 'class' => 'mw-tpa-group',
 76+ 'data-groupid' => $id,
 77+ 'data-id' => $this->htmlIdForGroup( $group ),
 78+ ) );
9079
91 - $removeSpan = Html::element( 'span', array(
92 - 'class' => 'tp-aggregate-remove-ag-button',
93 - 'id' => $id ) ) ;
94 - $wgOut->addHtml( "<h2 id='$id'>" . $group['name'] . $removeSpan . "</h2>" );
 80+ $wgOut->addHtml( $div );
9581
96 - $wgOut->addHtml( "<p>" . $group['description'] . "</p>" );
 82+ $remove = Html::element( 'span', array( 'class' => 'tp-aggregate-remove-ag-button' ) );
9783
98 - $wgOut->addHtml( "<ol id='tp-aggregate-groups-ol-$id'>" );
99 - $subgroups = $group['subgroups'];
100 - foreach ( $subgroups as $subgroupId => $subgroup ) {
101 - $removeSpan = Html::element( 'span', array(
102 - 'class' => 'tp-aggregate-remove-button',
103 - 'id' => $subgroupId ) );
104 - if ( $subgroup ) {
105 - $wgOut->addHtml( "<li>" .
106 - Linker::linkKnown( $subgroup->getTitle(),
107 - null,
108 - array( 'id' => $subgroupId )
109 - )
110 - . "$removeSpan </li>" );
111 - }
112 - }
113 - $wgOut->addHtml( "</ol>" );
114 -
115 - $this->groupSelector ( $pages, $group );
 84+ $hid = $this->htmlIdForGroup( $group );
 85+ $header = Html::rawElement( 'h2', null, htmlspecialchars( $group->getLabel() ) . $remove );
 86+ $wgOut->addHtml( $header );
 87+ $wgOut->addWikiText( $group->getDescription() );
 88+ $this->listSubgroups( $group );
 89+ $select = $this->getGroupSelector( $pages, $group );
 90+ $wgOut->addHtml( $select->getHtml() );
11691 $addButton = Html::element( 'input',
11792 array( 'type' => 'button',
11893 'value' => wfMsg( 'tpt-aggregategroup-add' ),
119 - 'id' => $id, 'class' => 'tp-aggregate-add-button' )
 94+ 'class' => 'tp-aggregate-add-button' )
12095 );
12196 $wgOut->addHtml( $addButton );
12297 $wgOut->addHtml( "</div>" );
@@ -147,33 +122,46 @@
148123 $wgOut->addHtml( $newGroupDiv );
149124 }
150125
151 - protected function groupSelector( $pages, $group ) {
152 - global $wgOut;
153 - $out = $wgOut;
154 - if ( !count( $pages ) ) {
155 - $wgOut->addWikiMsg( 'tpt-list-nopages' );
156 - return;
157 - }
158 - $options = "\n";
159 - $subgroups = $group['subgroups'];
160 - if ( count( $pages ) ) {
161 - foreach ( $pages as $pageId => $page ) {
162 - $title = $page['title']->getText();
163 - $pageid = TranslatablePage::getMessageGroupIdFromTitle( $page['title'] ) ;
164 - if ( ! isset( $subgroups[$pageid] ) ) {
165 - $options .= Xml::option( $title , $pageid, false , array( 'id' => $pageid ) ) . "\n";
166 - }
167 - }
168 - }
169 - $selector = Xml::tags( 'select',
 126+ protected function listSubgroups( AggregateMessageGroup $parent ) {
 127+ $out = $this->getOutput();
 128+ $sanid = Sanitizer::escapeId( $parent->getId() );
 129+
 130+ $id = $this->htmlIdForGroup( $parent, 'mw-tpa-grouplist-' );
 131+ $out->addHtml( Html::openElement( 'ol', array( 'id' => $id ) ) );
 132+
 133+ foreach ( $parent->getGroups() as $id => $group ) {
 134+ $remove = Html::element( 'span',
170135 array(
171 - 'id' => 'tp-aggregate-groups-select-' . $group['id'],
172 - 'name' => 'group',
173 - 'class' => 'tp-aggregate-group-chooser',
174 - ),
175 - $options
 136+ 'class' => 'tp-aggregate-remove-button',
 137+ 'data-groupid' => $group->getId(),
 138+ )
176139 );
177 - $out->addHtml( $selector );
 140+
 141+ $link = Linker::linkKnown( $group->getTitle(), null, array( 'id' => $id ) );
 142+ $out->addHtml( Html::rawElement( 'li', null, "$link$remove" ) );
 143+ }
 144+ $out->addHtml( Html::closeElement( 'ol' ) );
178145 }
179146
 147+ protected function getGroupSelector( $availableGroups, $parent ) {
 148+ $id = $this->htmlIdForGroup( $parent, 'mw-tpa-groupselect-' );
 149+ $select = new XmlSelect( 'group', $id );
 150+
 151+ $subgroups = $parent->getGroups();
 152+ foreach ( $availableGroups as $group ) {
 153+ $groupId = $group->getId();
 154+ // Do not include already included groups in the list
 155+ if ( isset( $subgroups[$groupId] ) ) continue;
 156+ $select->addOption( $group->getLabel(), $groupId );
 157+ }
 158+
 159+ return $select;
 160+ }
 161+
 162+ protected function htmlIdForGroup( MessageGroup $group, $prefix = '' ) {
 163+ $id = sha1( $group->getId() );
 164+ $id = substr( $id, 5, 8 );
 165+ return $prefix . $id;
 166+ }
 167+
180168 }
Index: trunk/extensions/Translate/resources/ext.translate.special.aggregategroups.js
@@ -1,142 +1,150 @@
2 -jQuery( function( $ ) {
 2+jQuery( document ).ready( function ( $ ) {
 3+ "use strict";
 4+
 5+ function getApiParams( $target ) {
 6+ return {
 7+ action: 'aggregategroups',
 8+ token: $( '#token' ).val(),
 9+ aggregategroup: $target.parents( '.mw-tpa-group' ).data( 'groupid' ),
 10+ format: "json"
 11+ };
 12+ }
313
4 - $( document ).ready( function () {
 14+ function associate( event ) {
 15+ var
 16+ $target = $( event.target ),
 17+ parentId = $target.parents( '.mw-tpa-group' ).data( 'id' ),
 18+ $selected = $( '#mw-tpa-groupselect-' + parentId + ' option:selected' ),
 19+ subgroupId = $selected.val(),
 20+ subgroupName = $selected.text();
 21+
 22+
523
6 - function associate( event ){
7 - var aggregategroup = event.target.id;
8 - var $selected = $( '#tp-aggregate-groups-select-'+ aggregategroup + ' option:selected' );
9 - var groupName = $selected.text();
10 - var groupId = $selected.val();
11 - var $select= $( 'select.tp-aggregate-group-chooser' ) ;
 24+ var successFunction = function( data, textStatus ) {
 25+ if ( data.error ) {
 26+ alert( data.error.info );
 27+ } else {
 28+ var aAttr = {
 29+ href: mw.util.wikiGetlink( subgroupName ),
 30+ title: subgroupName
 31+ };
 32+ var $a = $( '<a>', aAttr ).text( subgroupName );
1233
13 - var successFunction = function( data, textStatus ) {
14 - if ( data.error ) {
15 - alert( data.error.info );
16 - }else{
17 - $( '#tp-aggregate-groups-ol-'+ aggregategroup ).append( '<li><a id='+groupId+' href='+groupName+'>'+groupName+'</a><span class=\'tp-aggregate-remove-button\' id='+groupId+'></span></li>' );
18 - // remove this group from the select.
19 - $selected.remove();
20 - // bind click event to the dissociate(remove) button.
21 - $( 'span#'+groupId ).on ( "click", function(event){ dissociate(event); } );
22 - }
23 - };
 34+ var spanAttr = {
 35+ 'class': 'tp-aggregate-remove-button',
 36+ 'data-groupid': subgroupId
 37+ };
2438
25 - var params = {
26 - action: "aggregategroups",
27 - 'do' : 'associate',
28 - token: $( "#token" ).val(),
29 - group: groupId,
30 - aggregategroup: aggregategroup,
31 - format: "json"
32 - };
33 - $.post( mw.util.wikiScript( "api" ), params, successFunction );
34 - }
 39+ var $span = $( '<span>', spanAttr )
 40+
 41+ var $ol = $( '#mw-tpa-grouplist-' + parentId );
 42+ $ol.append( $( '<li>' ).append( $a.after( $span ) ) );
3543
36 - function dissociate(event){
37 - var groupId = event.target.id;
38 - var groupName = $( 'a#'+groupId ).text();
39 - var aggregategroup = $( 'a#'+groupId ).closest( 'div' ).find( 'h2' ).attr( 'id' );
40 - var $select = $( '#tp-aggregate-groups-select-'+ aggregategroup );
41 - var successFunction = function( data, textStatus ) {
42 - if ( data.error ) {
43 - alert( data.error.info );
44 - }else{
45 - $select .append( '<option value="'+groupId+'">'+groupName+'</option>' );
46 - $( 'span#'+ groupId ).closest( 'li' ).remove();
47 - }
48 - };
 44+ // remove this group from the select.
 45+ $selected.remove();
 46+ $span.click( dissociate );
 47+ }
 48+ };
 49+
 50+ var params = $.extend( getApiParams( $target ), {
 51+ 'do' : 'associate',
 52+ group: subgroupId,
 53+ } );
 54+ $.post( mw.util.wikiScript( 'api' ), params, successFunction );
 55+ }
4956
50 - var params = {
51 - action: "aggregategroups",
52 - 'do' : 'dissociate',
53 - token: $( "#token" ).val(),
54 - group: groupId,
55 - aggregategroup: aggregategroup,
56 - format: "json"
57 - };
58 - $.post( mw.util.wikiScript( "api" ), params, successFunction );
59 - }
 57+ function dissociate( event ) {
 58+ var
 59+ $target = $( event.target ),
 60+ parentId = $target.parents( '.mw-tpa-group' ).data( 'id' ),
 61+ $select = $( '#mw-tpa-groupselect-' + parentId );
6062
61 - function removeGroup(event){
62 - var aggregategroup = event.target.id;
63 - var successFunction = function( data, textStatus ) {
64 - if ( data.error ) {
65 - alert( data.error.info );
66 - }else{
67 - $( 'span#'+ aggregategroup ).closest('div#tpt-aggregate-group').remove();
68 - }
69 - };
 63+ function successFunction( data, textStatus ) {
 64+ if ( data.error ) {
 65+ alert( data.error.info );
 66+ } else {
 67+ $( '<option>', { value: $target.data( 'groupid' ) } )
 68+ .text( $target.parent( 'a' ).text() )
 69+ .appendTo( $select );
 70+ $target.parent( 'li' ).remove();
 71+ }
 72+ };
 73+
 74+ var params = $.extend( getApiParams( $target ), {
 75+ 'do' : 'dissociate',
 76+ group: $target.data( 'groupid' ),
 77+ } );
 78+ $.post( mw.util.wikiScript( 'api' ), params, successFunction );
 79+ }
7080
71 - var params = {
72 - action: "aggregategroups",
73 - 'do' : 'remove',
74 - token: $( "#token" ).val(),
75 - aggregategroup: aggregategroup,
76 - format: "json"
77 - };
78 - $.post( mw.util.wikiScript( "api" ), params, successFunction );
79 - }
 81+ function removeGroup( event ) {
 82+ var
 83+ $target = $( event.target ),
 84+ parentId = $target.parent( '.mw-tpa-group' ).data( 'groupid' );
8085
81 - $( 'input.tp-aggregate-add-button' ).on ( "click", function( event ){
82 - associate(event);
83 - } );
 86+ function successFunction ( data, textStatus ) {
 87+ if ( data.error ) {
 88+ alert( data.error.info );
 89+ } else {
 90+ $( event.target ).parents( '.mw-tpa-group' ).remove();
 91+ }
 92+ };
8493
85 - $( 'span.tp-aggregate-remove-button' ).on ( "click", function( event ){
86 - dissociate(event);
87 - } );
 94+ var params = $.extend( getApiParams( $target ), {'do' : 'remove' } );
 95+ $.post( mw.util.wikiScript( "api" ), params, successFunction );
 96+ }
8897
89 - $( 'span.tp-aggregate-remove-ag-button' ).on ( "click", function( event ){
90 - removeGroup(event);
91 - } );
 98+ $( '.tp-aggregate-add-button' ).click( associate );
 99+ $( '.tp-aggregate-remove-button' ).click( dissociate );
 100+ $( '.tp-aggregate-remove-ag-button' ).click( removeGroup );
 101+
 102+ $( 'a.tpt-add-new-group' ).on ( "click", function( event ){
 103+ $( 'div.tpt-add-new-group' ).removeClass( 'hidden' );
 104+ } );
92105
93 - $( 'a.tpt-add-new-group' ).on ( "click", function( event ){
94 - $( 'div.tpt-add-new-group' ).removeClass( 'hidden' );
95 - } );
 106+ $( '#tpt-aggregategroups-save' ). on ( "click", function( event ){
 107+ var aggregateGroup = $( 'input.tp-aggregategroup-add-name' ).val().toLowerCase().replace( ' ', '_');
 108+ var aggregateGroupName = $( 'input.tp-aggregategroup-add-name' ).val();
 109+ var aggregateGroupDesc = $( 'input.tp-aggregategroup-add-description' ).val();
 110+ var $select = $( 'select.tp-aggregate-group-chooser' );
96111
97 - $( '#tpt-aggregategroups-save' ). on ( "click", function( event ){
98 - var aggregateGroup = $( 'input.tp-aggregategroup-add-name' ).val().toLowerCase().replace( ' ', '_');
99 - var aggregateGroupName = $( 'input.tp-aggregategroup-add-name' ).val();
100 - var aggregateGroupDesc = $( 'input.tp-aggregategroup-add-description' ).val();
101 - var $select= $( 'select.tp-aggregate-group-chooser' ) ;
 112+ var successFunction = function( data, textStatus ) {
 113+ if ( data.error ) {
 114+ alert( data.error.info );
 115+ }else{
 116+ var $removeSpan = $( '<span>' ).attr( 'id', aggregateGroup ).addClass( 'tp-aggregate-remove-ag-button' );
 117+ var $div = $( "<div class='mw-tpa-group'>" )
 118+ .append ( $( '<h2>' ).text( aggregateGroupName )
 119+ .append ( $removeSpan ) )
 120+ .append ( $('<p>').text( aggregateGroupDesc ) )
 121+ .append ( $('<ol id=\'mw-tpa-grouplist-'+aggregateGroup+'\'>') );
102122
103 - var successFunction = function( data, textStatus ) {
104 - if ( data.error ) {
105 - alert( data.error.info );
 123+ if ( $select.length > 0 ){
 124+ var $groupSelector = $( $( 'select.tp-aggregate-group-chooser')[0] ).clone();
 125+ $groupSelector.attr('id', 'tp-aggregate-groups-select-' + aggregateGroup);
 126+ var $addButton = $( $( 'input.tp-aggregate-add-button')[0]).clone();
 127+ $addButton.attr( 'id', aggregateGroup);
 128+ $div.append( $groupSelector ).append( $addButton );
 129+ $addButton.on ( "click", function( event ){ associate(event); } );
 130+ $removeSpan.on ( "click", function( event ){ removeGroup(event); } );
 131+ $( 'div.tpt-add-new-group' ).addClass('hidden');
106132 }else{
107 - $removeSpan = $( '<span>' ).attr( 'id', aggregateGroup ).addClass( 'tp-aggregate-remove-ag-button' );
108 - $div = $( "<div id='tpt-aggregate-group'>" )
109 - .append ( $( '<h2>' ).attr( 'id', aggregateGroup ).text( aggregateGroupName )
110 - .append ( $removeSpan ) )
111 - .append ( $('<p>').text( aggregateGroupDesc ) )
112 - .append ( $('<ol id=\'tp-aggregate-groups-ol-'+aggregateGroup+'\'>') );
113 -
114 - if ( $select.length > 0 ){
115 - var $groupSelector = $( $( 'select.tp-aggregate-group-chooser')[0] ).clone();
116 - $groupSelector.attr('id', 'tp-aggregate-groups-select-' + aggregateGroup);
117 - var $addButton = $( $( 'input.tp-aggregate-add-button')[0]).clone();
118 - $addButton.attr( 'id', aggregateGroup);
119 - $div.append( $groupSelector ).append( $addButton );
120 - $addButton.on ( "click", function( event ){ associate(event); } );
121 - $removeSpan.on ( "click", function( event ){ removeGroup(event); } );
122 - $( 'div.tpt-add-new-group' ).addClass('hidden');
123 - }else{
124 - // First group in the wiki. Cannot clone the group selector, just reload this time.
125 - location.reload();
126 - }
127 - $( 'a.tpt-add-new-group' ).before ( $div ) ;
 133+ // First group in the wiki. Cannot clone the group selector, just reload this time.
 134+ location.reload();
128135 }
129 - };
 136+ $( 'a.tpt-add-new-group' ).before ( $div ) ;
 137+ }
 138+ };
130139
131 - var params = {
132 - action: "aggregategroups",
133 - 'do' : 'add',
134 - token: $( "#token" ).val(),
135 - aggregategroup: aggregateGroup,
136 - groupname : aggregateGroupName,
137 - groupdescription: aggregateGroupDesc,
138 - format: "json"
139 - };
140 - $.post( mw.util.wikiScript( "api" ), params, successFunction );
141 - } )
142 - } );
 140+ var params = {
 141+ action: "aggregategroups",
 142+ 'do' : 'add',
 143+ token: $( "#token" ).val(),
 144+ aggregategroup: aggregateGroup,
 145+ groupname : aggregateGroupName,
 146+ groupdescription: aggregateGroupDesc,
 147+ format: "json"
 148+ };
 149+ $.post( mw.util.wikiScript( "api" ), params, successFunction );
 150+ } )
143151 } );

Follow-up revisions

RevisionCommit summaryAuthorDate
r113708More cleanup of js, whitespaces....santhosh05:08, 13 March 2012
r113715Fix the broken 'add new aggregate group' in r113631...santhosh11:04, 13 March 2012

Comments

#Comment by Santhosh.thottingal (talk | contribs)   05:09, 13 March 2012

wonderful, Niklas!

Status & tagging log