UX/PERF: Update readers count when a post from another user is read. Don't fetch the post data again just to update the count. (#8078)

This commit is contained in:
Roman Rizzi 2019-09-08 22:29:15 -03:00 committed by Sam
parent d17425cbb8
commit 7d5f3c1338
7 changed files with 28 additions and 30 deletions

View File

@ -1352,17 +1352,12 @@ export default Ember.Controller.extend(bufferedProperty("model"), {
})
.then(() => refresh({ id: data.id, refreshLikes: true }));
break;
case "read":
case "read": {
postStream
.triggerChangedPost(data.id, data.updated_at, {
preserveCooked: true
})
.then(() =>
refresh({
id: data.id,
refreshReaders: topic.show_read_indicator
})
);
.triggerReadPost(data.id, data.readers_count)
.then(() => refresh({ id: data.id, refreshLikes: true }));
break;
}
case "revised":
case "rebaked": {
postStream

View File

@ -716,6 +716,19 @@ export default RestModel.extend({
return resolved;
},
triggerReadPost(postId, readersCount) {
const resolved = Ember.RSVP.Promise.resolve();
resolved.then(() => {
const post = this.findLoadedPost(postId);
if (post && readersCount > post.readers_count) {
post.set("readers_count", readersCount);
this.storePost(post);
}
});
return resolved;
},
postForPostNumber(postNumber) {
if (!this.hasPosts) {
return;

View File

@ -12,7 +12,7 @@ class PostReadersController < ApplicationController
.joins(:topic_users)
.where.not(topic_users: { last_read_post_number: nil })
.where('topic_users.topic_id = ? AND topic_users.last_read_post_number >= ?', post.topic_id, post.post_number)
.where.not(id: [current_user.id, post.user_id])
.where.not(id: post.user_id)
readers = readers.map do |r|
{

View File

@ -170,6 +170,11 @@ class Post < ActiveRecord::Base
end
end
def readers_count
read_count = reads - 1 # Excludes poster
read_count < 0 ? 0 : read_count
end
def publish_change_to_clients!(type, opts = {})
# special failsafe for posts missing topics consistency checks should fix,
# but message is safe to skip

View File

@ -357,7 +357,7 @@ SQL
if topic&.private_message?
groups = read_allowed_groups_of(topic)
post = Post.find_by(topic_id: topic.id, post_number: last_read_post_number)
trigger_post_read_count_update(post, groups)
trigger_post_read_count_update(post, groups, last_read_post_number, user_id)
update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, false)
end
end
@ -389,8 +389,9 @@ SQL
MessageBus.publish("/private-messages/unread-indicator/#{topic.id}", message, user_ids: groups_to_update.flat_map(&:members))
end
def self.trigger_post_read_count_update(post, groups)
def self.trigger_post_read_count_update(post, groups, last_read_post_number, user_id)
return if groups.empty?
post.publish_change_to_clients!(:read)
opts = { readers_count: post.readers_count, reader_id: user_id }
post.publish_change_to_clients!(:read, opts)
end
end

View File

@ -459,13 +459,6 @@ class PostSerializer < BasicPostSerializer
can_review_topic?
end
def readers_count
read_count = object.reads - 1 # Exclude logged user
read_count -= 1 unless yours
read_count < 0 ? 0 : read_count
end
private
def can_review_topic?

View File

@ -57,15 +57,6 @@ describe PostReadersController do
expect(readers).to be_empty
end
it "doesn't include current_user in the readers list" do
TopicUser.create!(user: admin, topic: @group_message, last_read_post_number: 3)
get '/post_readers.json', params: { id: @post.id }
reader = JSON.parse(response.body)['post_readers'].detect { |r| r['username'] == admin.username }
expect(reader).to be_nil
end
it "doesn't include users without reading progress on first post" do
@post.update(post_number: 1)
TopicUser.create!(user: reader, topic: @group_message, last_read_post_number: nil)