Skip to content

Commit

Permalink
FIX: more reliable topic list counts
Browse files Browse the repository at this point in the history
- unread was not incrementing when you read last post on topic
- new notifications were being inserted even if they existed in list
- terminology was all mixed up "1 new posts", split to 3 messages
- latest behaves as expected, updating count of new and updated topics
  • Loading branch information
SamSaffron committed Aug 5, 2014
1 parent 1958b02 commit 4536f77
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 12 deletions.
2 changes: 1 addition & 1 deletion app/assets/javascripts/discourse/helpers/i18n_helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ Ember.Handlebars.registerHelper('countI18n', function(key, options) {
shouldRerender: Discourse.View.renderIfChanged('count'),

render: function(buffer) {
buffer.push(I18n.t(key, { count: this.get('count') }));
buffer.push(I18n.t(key + (this.get('suffix') || ""), { count: this.get('count') }));
}

});
Expand Down
27 changes: 22 additions & 5 deletions app/assets/javascripts/discourse/models/topic_tracking_state.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,17 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
tracker.incrementMessageCount();
}

if (data.message_type === "new_topic"){
if (data.message_type === "new_topic" || data.message_type === "latest") {
var ignored_categories = Discourse.User.currentProp("muted_category_ids");
if(_.include(ignored_categories, data.payload.category_id)){
return;
}
}

if (data.message_type === "latest"){
tracker.notify(data);
}

if (data.message_type === "new_topic" || data.message_type === "unread" || data.message_type === "read") {
tracker.notify(data);
var old = tracker.states["t" + data.topic_id];
Expand All @@ -36,6 +40,7 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
};

Discourse.MessageBus.subscribe("/new", process);
Discourse.MessageBus.subscribe("/latest", process);
var currentUser = Discourse.User.current();
if(currentUser) {
Discourse.MessageBus.subscribe("/unread/" + currentUser.id, process);
Expand All @@ -55,17 +60,29 @@ Discourse.TopicTrackingState = Discourse.Model.extend({
if (!this.newIncoming) { return; }

if ((this.filter === "all" ||this.filter === "latest" || this.filter === "new") && data.message_type === "new_topic" ) {
this.newIncoming.push(data.topic_id);
this.addIncoming(data.topic_id);
}
if ((this.filter === "all" ||this.filter === "latest" || this.filter === "unread") && data.message_type === "unread") {

if ((this.filter === "all" || this.filter === "unread") && data.message_type === "unread") {
var old = this.states["t" + data.topic_id];
if(!old) {
this.newIncoming.push(data.topic_id);
if(!old || old.highest_post_number === old.last_read_post_number) {
this.addIncoming(data.topic_id);
}
}

if(this.filter === "latest" && data.message_type === "latest") {
this.addIncoming(data.topic_id);
}

this.set("incomingCount", this.newIncoming.length);
},

addIncoming: function(topicId) {
if(this.newIncoming.indexOf(topicId) === -1){
this.newIncoming.push(topicId);
}
},

resetTracking: function(){
this.newIncoming = [];
this.set("incomingCount", 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@
<tr>
<td colspan="9">
<div class='alert alert-info clickable' {{action showInserted}}>
{{countI18n new_topics_inserted count=topicTrackingState.incomingCount}}
{{i18n show_new_topics}}
{{countI18n topic_count_ suffix=topicTrackingState.filter count=topicTrackingState.incomingCount}}
{{i18n click_to_show}}
</div>
</td>
</tr>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
<tr>
<td>
<div class='alert alert-info' {{action showInserted}}>
{{countI18n new_topics_inserted countBinding="topicTrackingState.incomingCount"}}
{{i18n show_new_topics}}
{{countI18n topic_count_ suffix=topicTrackingState.filter count=topicTrackingState.incomingCount}}
{{i18n click_to_show}}
</div>
</td>
</tr>
Expand Down
5 changes: 5 additions & 0 deletions app/assets/javascripts/discourse/views/topic.js.es6
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ export default Discourse.View.extend(AddCategoryClass, Discourse.Scrolling, {
@method scrolled
**/
scrolled: function(){

if(this.isDestroyed || this.isDestroying) {

This comment has been minimized.

Copy link
@eviltrout

eviltrout Aug 5, 2014

Contributor

How does this end up getting called when the view is destroyed?

This comment has been minimized.

Copy link
@SamSaffron

SamSaffron via email Aug 6, 2014

Author Member
return;
}

var offset = window.pageYOffset || $('html').scrollTop();
if (!this.get('docAt')) {
var title = $('#topic-title');
Expand Down
17 changes: 17 additions & 0 deletions app/models/topic_tracking_state.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,23 @@ def self.publish_new(topic)
publish_read(topic.id, 1, topic.user_id)
end

def self.publish_latest(topic)
return unless topic.archetype == "regular"

message = {
topic_id: topic.id,
message_type: "latest",
payload: {
bumped_at: topic.bumped_at,
topic_id: topic.id,
category_id: topic.category_id
}
}

group_ids = topic.category && topic.category.secure_group_ids
MessageBus.publish("/latest", message.as_json, group_ids: group_ids)
end

def self.publish_unread(post)
# TODO at high scale we are going to have to defer this,
# perhaps cut down to users that are around in the last 7 days as well
Expand Down
15 changes: 13 additions & 2 deletions config/locales/client.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -150,8 +150,19 @@ en:
last_read: "this is the last post you've read; click to bookmark it"
remove: "Remove Bookmark"

new_topics_inserted: "{{count}} new posts."
show_new_topics: "Click to show."
topic_count_latest:
one: "{{count}} new or updated topic."
other: "{{count}} new or updated topics."

topic_count_unread:
one: "{{count}} unread topic."
other: "{{count}} unread topics."

topic_count_new:
one: "{{count}} new topic."
other: "{{count}} new topics."

click_to_show: "Click to show."
preview: "preview"
cancel: "cancel"

Expand Down
1 change: 1 addition & 0 deletions lib/post_jobs_enqueuer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ def trigger_post_post_process

def after_post_create
TopicTrackingState.publish_unread(@post) if @post.post_number > 1
TopicTrackingState.publish_latest(@topic)

Jobs.enqueue_in(
SiteSetting.email_time_window_mins.minutes,
Expand Down
1 change: 1 addition & 0 deletions lib/post_revisor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ def revise_and_create_new_version
def bump_topic
unless Post.where('post_number > ? and topic_id = ?', @post.post_number, @post.topic_id).exists?
@post.topic.update_column(:bumped_at, Time.now)
TopicTrackingState.publish_latest(@post.topic)
end
end

Expand Down

0 comments on commit 4536f77

Please sign in to comment.