From 11e962c84853cc4c9502798e688ea9c24ababd76 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Tue, 11 Feb 2014 15:28:05 +1100
Subject: [PATCH] BUGFIX: improve quality of unread / new counters
---
.../javascripts/discourse/lib/screen_track.js | 8 ++++++-
.../discourse/models/topic_tracking_state.js | 23 ++++++++++++++++---
.../routes/discovery_route_builders.js | 3 +++
.../models/topic_tracking_state_test.js | 21 +++++++++++++++++
4 files changed, 51 insertions(+), 4 deletions(-)
create mode 100644 test/javascripts/models/topic_tracking_state_test.js
diff --git a/app/assets/javascripts/discourse/lib/screen_track.js b/app/assets/javascripts/discourse/lib/screen_track.js
index 7d472fb65..91a6111f7 100644
--- a/app/assets/javascripts/discourse/lib/screen_track.js
+++ b/app/assets/javascripts/discourse/lib/screen_track.js
@@ -37,6 +37,11 @@ Discourse.ScreenTrack = Ember.Object.extend({
},
stop: function() {
+ if(!this.get('topicId')) {
+ // already stopped no need to "extra stop"
+ return;
+ }
+
this.tick();
this.flush();
this.reset();
@@ -105,9 +110,10 @@ Discourse.ScreenTrack = Ember.Object.extend({
var highestSeenByTopic = Discourse.Session.currentProp('highestSeenByTopic');
if ((highestSeenByTopic[topicId] || 0) < highestSeen) {
highestSeenByTopic[topicId] = highestSeen;
- Discourse.TopicTrackingState.current().updateSeen(topicId, highestSeen);
}
+ Discourse.TopicTrackingState.current().updateSeen(topicId, highestSeen);
+
if (!$.isEmptyObject(newTimings)) {
Discourse.ajax('/topics/timings', {
data: {
diff --git a/app/assets/javascripts/discourse/models/topic_tracking_state.js b/app/assets/javascripts/discourse/models/topic_tracking_state.js
index 51700570d..d6b45d073 100644
--- a/app/assets/javascripts/discourse/models/topic_tracking_state.js
+++ b/app/assets/javascripts/discourse/models/topic_tracking_state.js
@@ -39,8 +39,9 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
},
updateSeen: function(topicId, highestSeen) {
+ if(!topicId || !highestSeen) { return; }
var state = this.states["t" + topicId];
- if(state && state.last_read_post_number < highestSeen) {
+ if(state && (!state.last_read_post_number || state.last_read_post_number < highestSeen)) {
state.last_read_post_number = highestSeen;
this.incrementMessageCount();
}
@@ -84,9 +85,24 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
sync: function(list, filter){
var tracker = this;
+ var states = this.states;
if(!list || !list.topics) { return; }
+ // compensate for delayed "new" topics
+ // client side we know they are not new, server side we think they are
+ for(var i=list.topics.length-1; i>=0; i--){
+ var state = states["t"+ list.topics[i].id];
+ if(state && state.last_read_post_number > 0){
+ if(filter === "new"){
+ list.topics.splice(i, 1);
+ } else {
+ list.topics[i].unseen = false;
+ list.topics[i].dont_sync = true;
+ }
+ }
+ }
+
if(filter === "new" && !list.more_topics_url){
// scrub all new rows and reload from list
_.each(this.states, function(state){
@@ -112,10 +128,11 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
if(topic.unseen) {
row.last_read_post_number = null;
} else if (topic.unread || topic.new_posts){
- // subtle issue here
row.last_read_post_number = topic.highest_post_number - ((topic.unread||0) + (topic.new_posts||0));
} else {
- delete tracker.states["t" + topic.id];
+ if(!topic.dont_sync) {
+ delete tracker.states["t" + topic.id];
+ }
return;
}
diff --git a/app/assets/javascripts/discourse/routes/discovery_route_builders.js b/app/assets/javascripts/discourse/routes/discovery_route_builders.js
index 86487c194..53e7ac19f 100644
--- a/app/assets/javascripts/discourse/routes/discovery_route_builders.js
+++ b/app/assets/javascripts/discourse/routes/discovery_route_builders.js
@@ -11,6 +11,9 @@ function buildTopicRoute(filter) {
},
model: function() {
+ // attempt to stop early cause we need this to be called before .sync
+ Discourse.ScreenTrack.current().stop();
+
return Discourse.TopicList.list(filter).then(function(list) {
var tracking = Discourse.TopicTrackingState.current();
if (tracking) {
diff --git a/test/javascripts/models/topic_tracking_state_test.js b/test/javascripts/models/topic_tracking_state_test.js
new file mode 100644
index 000000000..305fc1d17
--- /dev/null
+++ b/test/javascripts/models/topic_tracking_state_test.js
@@ -0,0 +1,21 @@
+module("Discourse.TopicTrackingState");
+
+test("sync", function () {
+
+ var state = Discourse.TopicTrackingState.create();
+ // fake track it
+ state.states["t111"] = {last_read_post_number: null};
+
+ state.updateSeen(111, 7);
+ var list = {topics: [{
+ highest_post_number: null,
+ id: 111,
+ unread: 10,
+ new_posts: 10
+ }]};
+
+ state.sync(list, "new");
+
+ equal(list.topics.length, 0, "expect new topic to be removed as it was seen");
+
+});