Mooeypoo has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/342284 )
Change subject: RCFilters UI: Rework conflicts to be objects in filter or group
context
......................................................................
RCFilters UI: Rework conflicts to be objects in filter or group context
Allow conflicts to be defined in either the filter or the group context
and represent a whole object rather than an array of filter names.
Bug: T152754
Change-Id: I2423eb2618aa64bf30395b1a1912589e0c71f283
---
M resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
M resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js
M resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
M resources/src/mediawiki.rcfilters/mw.rcfilters.init.js
M tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
5 files changed, 209 insertions(+), 61 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/84/342284/1
diff --git
a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
index 14a610b..d51f43d 100644
--- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
+++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterGroup.js
@@ -29,6 +29,8 @@
this.active = !!config.active;
this.fullCoverage = !!config.fullCoverage;
+ this.conflicts = config.conflicts || {};
+
this.aggregate( { update: 'filterItemUpdate' } );
this.connect( this, { filterItemUpdate: 'onFilterItemUpdate' }
);
};
@@ -81,6 +83,27 @@
return this.name;
};
+ mw.rcfilters.dm.FilterGroup.prototype.getConflicts = function () {
+ return this.conflicts;
+ };
+ mw.rcfilters.dm.FilterGroup.prototype.setConflicts = function (
conflicts ) {
+ this.conflicts = conflicts;
+ };
+
+ /**
+ * Check whether this item has a potential conflict with the given item
+ *
+ * This checks whether the given item is in the list of conflicts of
+ * the current item, but makes no judgment about whether the conflict
+ * is currently at play (either one of the items may not be selected)
+ *
+ * @param {mw.rcfilters.dm.FilterItem} filterItem Filter item
+ * @return {boolean} This item has a conflict with the given item
+ */
+ mw.rcfilters.dm.FilterGroup.prototype.existsInConflicts = function (
filterItem ) {
+ return Object.prototype.hasOwnProperty.call(
this.getConflicts(), filterItem.getName() );
+ };
+
/**
* Check whether there are any items selected
*
@@ -126,9 +149,15 @@
mw.rcfilters.dm.FilterGroup.prototype.areAllSelectedInConflictWith =
function ( filterItem ) {
var selectedItems = this.getSelectedItems( filterItem );
- return selectedItems.length > 0 && selectedItems.every(
function ( selectedFilter ) {
- return selectedFilter.existsInConflicts( filterItem );
- } );
+ return selectedItems.length > 0 &&
+ (
+ // The group as a whole is in conflict with
this item
+ this.existsInConflicts( filterItem ) ||
+ // All selected items are in conflict
individually
+ selectedItems.every( function ( selectedFilter
) {
+ return
selectedFilter.existsInConflicts( filterItem );
+ } )
+ );
};
/**
@@ -140,9 +169,14 @@
mw.rcfilters.dm.FilterGroup.prototype.areAnySelectedInConflictWith =
function ( filterItem ) {
var selectedItems = this.getSelectedItems( filterItem );
- return selectedItems.length > 0 && selectedItems.some( function
( selectedFilter ) {
- return selectedFilter.existsInConflicts( filterItem );
- } );
+ return selectedItems.length > 0 && (
+ // The group as a whole is in conflict with this item
+ this.existsInConflicts( filterItem ) ||
+ // Any selected items are in conflict individually
+ selectedItems.some( function ( selectedFilter ) {
+ return selectedFilter.existsInConflicts(
filterItem );
+ } )
+ );
};
/**
diff --git a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js
b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js
index 0df34f8..a019f6a 100644
--- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js
+++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FilterItem.js
@@ -34,7 +34,7 @@
// Interaction definitions
this.subset = config.subset || [];
- this.conflicts = config.conflicts || [];
+ this.conflicts = config.conflicts || {};
this.superset = [];
// Interaction states
@@ -195,7 +195,7 @@
* @return {string[]} Filter conflicts
*/
mw.rcfilters.dm.FilterItem.prototype.getConflicts = function () {
- return this.conflicts;
+ return $.extend( {}, this.conflicts,
this.getGroupModel().getConflicts() );
};
/**
@@ -204,7 +204,7 @@
* @param {string[]} conflicts Filter conflicts
*/
mw.rcfilters.dm.FilterItem.prototype.setConflicts = function (
conflicts ) {
- this.conflicts = conflicts || [];
+ this.conflicts = conflicts || {};
};
/**
@@ -237,7 +237,7 @@
* @return {boolean} This item has a conflict with the given item
*/
mw.rcfilters.dm.FilterItem.prototype.existsInConflicts = function (
filterItem ) {
- return this.conflicts.indexOf( filterItem.getName() ) > -1;
+ return Object.prototype.hasOwnProperty.call(
this.getConflicts(), filterItem.getName() );
};
/**
diff --git
a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
index 5be3656..65e8248 100644
--- a/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
+++ b/resources/src/mediawiki.rcfilters/dm/mw.rcfilters.dm.FiltersViewModel.js
@@ -162,9 +162,12 @@
* @param {Object} filters Filter group definition
*/
mw.rcfilters.dm.FiltersViewModel.prototype.initializeFilters = function
( filters ) {
- var i, filterItem, selectedFilterNames,
+ var i, filterItem, selectedFilterNames, filterConflictResult,
groupConflictResult,
model = this,
items = [],
+ supersetMap = {},
+ groupConflictMap = {},
+ filterConflictMap = {},
addArrayElementsUnique = function ( arr, elements ) {
elements = Array.isArray( elements ) ? elements
: [ elements ];
@@ -176,8 +179,31 @@
return arr;
},
- conflictMap = {},
- supersetMap = {};
+ expandConflictDefinitions = function ( obj ) {
+ var result = {};
+
+ $.each( obj, function ( group, conflicts ) {
+ var adjustedConflicts = {};
+ conflicts.forEach( function ( conflict
) {
+ if ( conflict.filter ) {
+ adjustedConflicts[
conflict.filter ] = conflict;
+ } else {
+ // This conflict is for
an entire group. Split it up to
+ // represent each filter
+
+ // Get the relevant
group items
+ model.groups[
conflict.group ].getItems().forEach( function ( groupItem ) {
+ // Rebuild the
conflict
+
adjustedConflicts[ groupItem.getName() ] = $.extend( {}, conflict, { filter:
groupItem.getName() } );
+ } );
+ }
+ } );
+
+ result[ group ] = adjustedConflicts;
+ } );
+
+ return result;
+ };
// Reset
this.clearItems();
@@ -191,6 +217,10 @@
separator: data.separator,
fullCoverage: !!data.fullCoverage
} );
+ }
+
+ if ( data.conflicts ) {
+ groupConflictMap[ group ] = data.conflicts;
}
selectedFilterNames = [];
@@ -217,26 +247,9 @@
} );
}
- // Conflicts are bi-directional, which means
FilterA can define having
- // a conflict with FilterB, and this conflict
should appear in **both**
- // filter definitions.
- // We need to remap all the 'conflicts' so they
reflect the entire state
- // in either direction regardless of which
filter defined the other as conflicting.
+ // Store conflicts
if ( data.filters[ i ].conflicts ) {
- conflictMap[ filterItem.getName() ] =
conflictMap[ filterItem.getName() ] || [];
- addArrayElementsUnique(
- conflictMap[
filterItem.getName() ],
- data.filters[ i ].conflicts
- );
-
- data.filters[ i ].conflicts.forEach(
function ( conflictingFilterName ) { // eslint-disable-line no-loop-func
- // Add this filter to the
conflicts of each of the filters in its list
- conflictMap[
conflictingFilterName ] = conflictMap[ conflictingFilterName ] || [];
- addArrayElementsUnique(
- conflictMap[
conflictingFilterName ],
- filterItem.getName()
- );
- } );
+ filterConflictMap[ data.filters[ i
].name ] = data.filters[ i ].conflicts;
}
if ( data.type === 'send_unselected_if_any' ) {
@@ -262,14 +275,23 @@
}
} );
- items.forEach( function ( filterItem ) {
- // Apply conflict map to the items
- // Now that we mapped all items and conflicts
bi-directionally
- // we need to apply the definition to each filter again
- filterItem.setConflicts( conflictMap[
filterItem.getName() ] );
+ // Expand conflicts
+ groupConflictResult = expandConflictDefinitions(
groupConflictMap );
+ filterConflictResult = expandConflictDefinitions(
filterConflictMap );
+ // Set conflicts for groups
+ $.each( groupConflictResult, function ( group, conflicts ) {
+ model.groups[ group ].setConflicts( conflicts );
+ } );
+
+ items.forEach( function ( filterItem ) {
// Apply the superset map
filterItem.setSuperset( supersetMap[
filterItem.getName() ] );
+
+ // set conflicts for item
+ if ( filterConflictResult[ filterItem.getName() ] ) {
+ filterItem.setConflicts( filterConflictResult[
filterItem.getName() ] );
+ }
} );
// Add items to the model
diff --git a/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js
b/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js
index a0b785d..1790b54 100644
--- a/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js
+++ b/resources/src/mediawiki.rcfilters/mw.rcfilters.init.js
@@ -39,7 +39,12 @@
name: 'hideanons',
label: mw.msg(
'rcfilters-filter-unregistered-label' ),
description: mw.msg(
'rcfilters-filter-unregistered-description' ),
- 'class':
'mw-changeslist-anon'
+ 'class':
'mw-changeslist-anon',
+ conflicts: [
+ {
+ group:
'userExpLevel'
+ }
+ ]
}
]
},
@@ -53,26 +58,29 @@
type: 'string_options',
separator: ',',
fullCoverage: false,
+ conflicts: [
+ {
+ group: 'registration',
+ filter: 'hideanons'
+ }
+ ],
filters: [
{
name: 'newcomer',
label: mw.msg(
'rcfilters-filter-userExpLevel-newcomer-label' ),
description: mw.msg(
'rcfilters-filter-userExpLevel-newcomer-description' ),
- conflicts: [
'hideanons' ],
'class':
'mw-changeslist-user-newcomer'
},
{
name: 'learner',
label: mw.msg(
'rcfilters-filter-userExpLevel-learner-label' ),
description: mw.msg(
'rcfilters-filter-userExpLevel-learner-description' ),
- conflicts: [
'hideanons' ],
'class':
'mw-changeslist-user-learner'
},
{
name: 'experienced',
label: mw.msg(
'rcfilters-filter-userExpLevel-experienced-label' ),
description: mw.msg(
'rcfilters-filter-userExpLevel-experienced-description' ),
- conflicts: [
'hideanons' ],
'class':
'mw-changeslist-user-experienced'
}
]
diff --git
a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
index ad0ed54..2fcd125 100644
---
a/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
+++
b/tests/qunit/suites/resources/mediawiki.rcfilters/dm.FiltersViewModel.test.js
@@ -1060,11 +1060,15 @@
filters: [
{
name: 'filter1',
- conflicts: [ 'filter2',
'filter4' ]
+ conflicts: [
+ { group:
'group2' }
+ ]
},
{
name: 'filter2',
- conflicts: [ 'filter6' ]
+ conflicts: [
+ { group:
'group2', filter: 'filter6' }
+ ]
},
{
name: 'filter3'
@@ -1074,16 +1078,21 @@
group2: {
title: 'Group 2',
type: 'send_unselected_if_any',
+ conflicts: [
+ { group: 'group1', filter:
'filter1' }
+ ],
filters: [
{
name: 'filter4'
},
{
- name: 'filter5',
- conflicts: [ 'filter3' ]
+ name: 'filter5'
},
{
- name: 'filter6'
+ name: 'filter6',
+ conflicts: [
+ { group:
'group1', filter: 'filter2' }
+ ]
}
]
}
@@ -1106,9 +1115,9 @@
'Initial state: no conflicts because no selections.'
);
- // Select a filter that has a conflict with another
+ // Select a filter that has a conflict with an entire group
model.toggleFiltersSelected( {
- filter1: true // conflicts: filter2, filter4
+ filter1: true // conflicts: entire of group 2 (
filter4, filter5, filter6)
} );
model.reassessFilterInteractions( model.getItemByName(
'filter1' ) );
@@ -1117,10 +1126,11 @@
model.getFullState(),
$.extend( true, {}, baseFullState, {
filter1: { selected: true },
- filter2: { conflicted: true },
- filter4: { conflicted: true }
+ filter4: { conflicted: true },
+ filter5: { conflicted: true },
+ filter6: { conflicted: true }
} ),
- 'Selecting a filter set its conflicts list as
"conflicted".'
+ 'Selecting a filter that conflicts with a group sets
all the conflicted group items as "conflicted".'
);
// Select one of the conflicts (both filters are now conflicted
and selected)
@@ -1133,28 +1143,102 @@
model.getFullState(),
$.extend( true, {}, baseFullState, {
filter1: { selected: true, conflicted: true },
- filter2: { conflicted: true },
- filter4: { selected: true, conflicted: true }
+ filter4: { selected: true, conflicted: true },
+ filter5: { conflicted: true },
+ filter6: { conflicted: true }
} ),
- 'Selecting a conflicting filter sets both sides to
conflicted and selected.'
+ 'Selecting a conflicting filter inside a group, sets
both sides to conflicted and selected.'
);
- // Select another filter from filter4 group, meaning:
- // now filter1 no longer conflicts with filter4
+ // Reset
+ model = new mw.rcfilters.dm.FiltersViewModel();
+ model.initializeFilters( definition );
+
+ // Select a filter that has a conflict with a specific filter
+ model.toggleFiltersSelected( {
+ filter2: true // conflicts: filter6
+ } );
+
+ model.reassessFilterInteractions( model.getItemByName(
'filter2' ) );
+
+ assert.deepEqual(
+ model.getFullState(),
+ $.extend( true, {}, baseFullState, {
+ filter2: { selected: true },
+ filter6: { conflicted: true }
+ } ),
+ 'Selecting a filter that conflicts with another filter
sets the other as "conflicted".'
+ );
+
+ // Select the conflicting filter
model.toggleFiltersSelected( {
filter6: true // conflicts: filter2
} );
+
model.reassessFilterInteractions( model.getItemByName(
'filter6' ) );
assert.deepEqual(
model.getFullState(),
$.extend( true, {}, baseFullState, {
- filter1: { selected: true, conflicted: false },
// No longer conflicts (filter4 is not the only in the group)
- filter2: { conflicted: true }, // While not
selected, still in conflict with filter1, which is selected
- filter4: { selected: true, conflicted: false },
// No longer conflicts with filter1
- filter6: { selected: true, conflicted: false }
+ filter2: { selected: true, conflicted: true },
+ filter6: { selected: true, conflicted: true },
+ filter1: { conflicted: true } // This is added
to the conflicts because filter6 is part of group2, who is in conflict with
filter1
} ),
- 'Selecting a non-conflicting filter from a conflicting
group removes the conflict'
+ 'Selecting a conflicting filter with an individual
filter, sets both sides to conflicted and selected.'
+ );
+
+ // Now choose a non-conflicting filter from the group
+ model.toggleFiltersSelected( {
+ filter5: true
+ } );
+
+ model.reassessFilterInteractions( model.getItemByName(
'filter5' ) );
+
+ assert.deepEqual(
+ model.getFullState(),
+ $.extend( true, {}, baseFullState, {
+ filter2: { selected: true },
+ filter6: { selected: true },
+ filter5: { selected: true }
+ // Filter1 is no longer in conflict because
filter2 is selected
+ // within its own group, which removes the
conflict
+ } ),
+ 'Selecting a non-conflicting filter within the group of
a conflicting filter removes the conflicts.'
+ );
+
+ // Followup on the previous test, unselect filter2 so filter1
is once again conflicted
+ model.toggleFiltersSelected( {
+ filter2: false
+ } );
+
+ model.reassessFilterInteractions( model.getItemByName(
'filter5' ) );
+
+ assert.deepEqual(
+ model.getFullState(),
+ $.extend( true, {}, baseFullState, {
+ filter1: { conflicted: true },
+ filter6: { selected: true },
+ filter5: { selected: true }
+ } ),
+ 'Unselecting an item that did not conflict returns the
conflict state.'
+ );
+
+ // Followup #2: Now actually select filter1, and make
everything conflicted
+ model.toggleFiltersSelected( {
+ filter1: true
+ } );
+
+ model.reassessFilterInteractions( model.getItemByName(
'filter5' ) );
+
+ assert.deepEqual(
+ model.getFullState(),
+ $.extend( true, {}, baseFullState, {
+ filter1: { selected: true, conflicted: true },
+ filter6: { selected: true, conflicted: true },
+ filter5: { selected: true, conflicted: true },
+ filter4: { conflicted: true } // Not selected
but conflicted because it's in group2
+ } ),
+ 'Selecting an item that conflicts with a whole group
makes all selections in that group conflicted.'
);
} );
--
To view, visit https://gerrit.wikimedia.org/r/342284
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I2423eb2618aa64bf30395b1a1912589e0c71f283
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <mor...@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
