Mooeypoo has uploaded a new change for review.
https://gerrit.wikimedia.org/r/312552
Change subject: UnreadNotificationCounter should always normalize the count
......................................................................
UnreadNotificationCounter should always normalize the count
Make the counter normalize negative and above-cap results to be within
the approved range (0 - cap). This means we can remove the idea of
estimating a change, and instead make it change the counter and trust
that it would normalize its values on its own.
Change-Id: Ie8b81a4433e8254ee0e90f59e5b25d727158eecf
---
M modules/controller/mw.echo.Controller.js
M modules/model/mw.echo.dm.UnreadNotificationCounter.js
M tests/qunit/model/test_mw.echo.dm.UnreadNotificationCounter.js
3 files changed, 26 insertions(+), 30 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Echo
refs/changes/52/312552/1
diff --git a/modules/controller/mw.echo.Controller.js
b/modules/controller/mw.echo.Controller.js
index 3053e22..9c02178 100644
--- a/modules/controller/mw.echo.Controller.js
+++ b/modules/controller/mw.echo.Controller.js
@@ -483,7 +483,7 @@
// Update pagination count
this.manager.updateCurrentPageItemCount();
- localCounter.estimateChange( -itemIds.length );
+ localCounter.change( -itemIds.length );
return this.api.markAllRead(
source,
this.getTypes()
@@ -526,8 +526,8 @@
// Update pagination count
this.manager.updateCurrentPageItemCount();
- this.manager.getUnreadCounter().estimateChange( -itemIds.length
);
- this.manager.getLocalCounter().estimateChange( -itemIds.length
);
+ this.manager.getUnreadCounter().change( -itemIds.length );
+ this.manager.getLocalCounter().change( -itemIds.length );
return this.api.markItemsRead( itemIds, 'local', true ).then(
this.refreshUnreadCount.bind( this ) );
};
@@ -634,11 +634,11 @@
// Update pagination count
this.manager.updateCurrentPageItemCount();
- this.manager.getUnreadCounter().estimateChange( isRead ?
-allIds.length : allIds.length );
+ this.manager.getUnreadCounter().change( isRead ? -allIds.length
: allIds.length );
if ( modelName !== 'xwiki' ) {
- // For the local counter, we should only estimate the
change if the items
+ // For the local counter, we should only change if the
items
// are not cross-wiki
- this.manager.getLocalCounter().estimateChange( isRead ?
-allIds.length : allIds.length );
+ this.manager.getLocalCounter().change( isRead ?
-allIds.length : allIds.length );
}
return this.api.markItemsRead( allIds, model.getSource(),
isRead ).then( this.refreshUnreadCount.bind( this ) );
@@ -662,7 +662,7 @@
if ( !xwikiModel ) {
return $.Deferred().reject().promise();
}
- this.manager.getUnreadCounter().estimateChange( -itemIds.length
);
+ this.manager.getUnreadCounter().change( -itemIds.length );
itemIds = Array.isArray( itemIds ) ? itemIds : [ itemIds ];
@@ -693,7 +693,7 @@
return $.Deferred().reject().promise();
}
- this.manager.getUnreadCounter().estimateChange(
-xwikiModel.getCount() );
+ this.manager.getUnreadCounter().change( -xwikiModel.getCount()
);
return this.api.fetchNotificationGroups(
xwikiModel.getSourceNames(), this.manager.getTypeString() )
.then( function ( groupList ) {
diff --git a/modules/model/mw.echo.dm.UnreadNotificationCounter.js
b/modules/model/mw.echo.dm.UnreadNotificationCounter.js
index 1501692..a67d61c 100644
--- a/modules/model/mw.echo.dm.UnreadNotificationCounter.js
+++ b/modules/model/mw.echo.dm.UnreadNotificationCounter.js
@@ -57,7 +57,9 @@
* @return {number} Count with cap applied
*/
mw.echo.dm.UnreadNotificationCounter.prototype.getCappedNotificationCount =
function ( count ) {
- if ( count <= this.max ) {
+ if ( count < 0 ) {
+ return 0;
+ } else if ( count <= this.max ) {
return count;
} else {
return this.max + 1;
@@ -77,19 +79,10 @@
* Set the current count
*
* @param {number} count
- * @param {boolean} isEstimation Whether this number is estimated or
accurate
*/
- mw.echo.dm.UnreadNotificationCounter.prototype.setCount = function (
count, isEstimation ) {
- if ( isEstimation ) {
- if ( count > this.max ) {
- // this prevents toggling between 90-ish and 99+
- return;
- }
- if ( count < 0 ) {
- // wrong estimation?
- return;
- }
- }
+ mw.echo.dm.UnreadNotificationCounter.prototype.setCount = function (
count ) {
+ // Normalize
+ count = this.getCappedNotificationCount( count );
if ( count !== this.count ) {
this.count = count;
@@ -98,12 +91,12 @@
};
/**
- * Report an estimated change to this counter
+ * Report a change to this counter
*
* @param {number} delta
*/
- mw.echo.dm.UnreadNotificationCounter.prototype.estimateChange =
function ( delta ) {
- this.setCount( this.count + delta, true );
+ mw.echo.dm.UnreadNotificationCounter.prototype.change = function (
delta ) {
+ this.setCount( this.count + delta );
};
/**
@@ -124,7 +117,7 @@
this.type,
this.localOnly
) ).then( function ( actualCount ) {
- model.setCount( actualCount, false );
+ model.setCount( actualCount );
return actualCount;
} );
diff --git a/tests/qunit/model/test_mw.echo.dm.UnreadNotificationCounter.js
b/tests/qunit/model/test_mw.echo.dm.UnreadNotificationCounter.js
index 2ad98ed..75edf0e 100644
--- a/tests/qunit/model/test_mw.echo.dm.UnreadNotificationCounter.js
+++ b/tests/qunit/model/test_mw.echo.dm.UnreadNotificationCounter.js
@@ -11,7 +11,8 @@
cases = [
{ input: 5, output: 5 },
{ input: 20, output: 11 },
- { input: 10, output: 10 }
+ { input: 10, output: 10 },
+ { input: -10, output: 0 }
];
for ( i = 0; i < cases.length; i++ ) {
@@ -40,13 +41,15 @@
// Trigger events
model.setCount( 50 ); // [ 50 ]
- model.setCount( 300, true ); // (estimate, above max, no event)
[ 50 ]
- model.setCount( -1, true ); // (estimate, below zero, no event)
[ 50 ]
- model.setCount( 10, true ); // [ 50, 10 ]
+ model.setCount( 300 ); // [ 50, 100 ] ( capped )
+ model.setCount( 300, true ); // (above max, capped to same
value as before, no event) [ 50, 100 ]
+ model.setCount( -1, true ); // (below zero, capping to 0) [ 50,
100, 0 ]
+ model.setCount( -99, true ); // (below zero, capping to 0, same
value as before, no event) [ 50, 100, 0 ]
+ model.setCount( 10, true ); // [ 50, 100, 0, 10 ]
assert.deepEqual(
results,
- [ 50, 10 ],
+ [ 50, 100, 0, 10 ],
'countChange events emitted.'
);
} );
--
To view, visit https://gerrit.wikimedia.org/r/312552
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie8b81a4433e8254ee0e90f59e5b25d727158eecf
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/extensions/Echo
Gerrit-Branch: master
Gerrit-Owner: Mooeypoo <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
