FEATURE: Publish read state on group messages. (Originally introduced in #7989) (#8025)

* Revert "Revert "FEATURE: Publish read state on group messages. (#7989) [Undo revert] (#8024)""

This reverts commit 36425eb9f0.

* Fix: Show who read only if the attribute is enabled

* PERF: Precalculate the last post  readed by a group member

* Use book-reader icon instear of far-eye

* FIX: update topic groups correctly

* DEV: Tidy up read indicator update on write
This commit is contained in:
Roman Rizzi
2019-08-27 09:09:00 -03:00
committed by GitHub
parent f2331ef07f
commit 7c741fa0d6
42 changed files with 688 additions and 21 deletions

View File

@@ -40,7 +40,8 @@ export default MountWidget.extend({
"gaps",
"selectedQuery",
"selectedPostsCount",
"searchService"
"searchService",
"showReadIndicator"
);
},
@@ -291,6 +292,12 @@ export default MountWidget.extend({
onRefresh: "refreshLikes"
});
}
if (args.refreshReaders) {
this.dirtyKeys.keyDirty(`post-menu-${args.id}`, {
onRefresh: "refreshReaders"
});
}
} else if (args.force) {
this.dirtyKeys.forceAll();
}

View File

@@ -35,6 +35,47 @@ export const ListItemDefaults = {
attributeBindings: ["data-topic-id"],
"data-topic-id": Ember.computed.alias("topic.id"),
didInsertElement() {
this._super(...arguments);
if (this.includeReadIndicator) {
this.messageBus.subscribe(this.readIndicatorChannel, data => {
const nodeClassList = document.querySelector(
`.indicator-topic-${data.topic_id}`
).classList;
if (data.show_indicator) {
nodeClassList.remove("unread");
} else {
nodeClassList.add("unread");
}
});
}
},
willDestroyElement() {
this._super(...arguments);
if (this.includeReadIndicator) {
this.messageBus.unsubscribe(this.readIndicatorChannel);
}
},
@computed("topic.id")
readIndicatorChannel(topicId) {
return `/private-messages/read-indicator/${topicId}`;
},
@computed("topic.read_by_group_member")
unreadClass(readByGroupMember) {
return readByGroupMember ? "" : "unread";
},
@computed("topic.read_by_group_member")
includeReadIndicator(readByGroupMember) {
return typeof readByGroupMember !== "undefined";
},
@computed
newDotText() {
return this.currentUser && this.currentUser.trust_level > 0

View File

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

View File

@@ -71,7 +71,8 @@ export function transformBasicPost(post) {
expandablePost: false,
replyCount: post.reply_count,
locked: post.locked,
userCustomFields: post.user_custom_fields
userCustomFields: post.user_custom_fields,
readCount: post.readers_count
};
_additionalAttributes.forEach(a => (postAtts[a] = post[a]));

View File

@@ -178,7 +178,8 @@ const Group = RestModel.extend({
allow_membership_requests: this.allow_membership_requests,
full_name: this.full_name,
default_notification_level: this.default_notification_level,
membership_request_template: this.membership_request_template
membership_request_template: this.membership_request_template,
publish_read_state: this.publish_read_state
};
if (!this.id) {

View File

@@ -52,6 +52,16 @@
class="groups-form-messageable-level"}}
</div>
<div class="control-group">
<label>
{{input type="checkbox"
checked=model.publish_read_state
class="groups-form-publish-read-state"}}
{{i18n 'admin.groups.manage.interaction.publish_read_state'}}
</label>
</div>
{{#if showEmailSettings}}
<div class="control-group">
<label class="control-label">{{i18n 'admin.groups.manage.interaction.email'}}</label>

View File

@@ -23,6 +23,11 @@
{{~#if showTopicPostBadges}}
{{~raw "topic-post-badges" unread=topic.unread newPosts=topic.displayNewPosts unseen=topic.unseen url=topic.lastUnreadUrl newDotText=newDotText}}
{{~/if}}
{{~#if includeReadIndicator}}
<span class='read-indicator indicator-topic-{{topic.id}} {{unreadClass}}'>
{{~d-icon "book-reader"}}
</span>
{{~/if}}
</span>
<div class="link-bottom-line">
{{#unless hideCategory}}

View File

@@ -177,6 +177,7 @@
selectedPostsCount=selectedPostsCount
selectedQuery=selectedQuery
gaps=model.postStream.gaps
showReadIndicator=model.show_read_indicator
showFlags=(action "showPostFlags")
editPost=(action "editPost")
showHistory=(route-action "showHistory")

View File

@@ -52,6 +52,36 @@ export function buildButton(name, widget) {
}
}
registerButton("read-count", attrs => {
if (attrs.showReadIndicator) {
const count = attrs.readCount;
if (count > 0) {
return {
action: "toggleWhoRead",
title: "post.controls.read_indicator",
className: "button-count read-indicator",
contents: count,
iconRight: true,
addContainer: false
};
}
}
});
registerButton("read", attrs => {
const disabled = attrs.readCount === 0;
if (attrs.showReadIndicator) {
return {
action: "toggleWhoRead",
title: "post.controls.read_indicator",
icon: "book-reader",
before: "read-count",
addContainer: false,
disabled
};
}
});
function likeCount(attrs) {
const count = attrs.likeCount;
@@ -341,7 +371,12 @@ export default createWidget("post-menu", {
},
defaultState() {
return { collapsed: true, likedUsers: [], adminVisible: false };
return {
collapsed: true,
likedUsers: [],
readers: [],
adminVisible: false
};
},
buildKey: attrs => `post-menu-${attrs.id}`,
@@ -508,6 +543,19 @@ export default createWidget("post-menu", {
);
}
if (state.readers.length) {
const remaining = state.totalReaders - state.readers.length;
contents.push(
this.attach("small-user-list", {
users: state.readers,
addSelf: false,
listClassName: "who-read",
description: "post.actions.people.read",
count: remaining
})
);
}
return contents;
},
@@ -525,9 +573,15 @@ export default createWidget("post-menu", {
showMoreActions() {
this.state.collapsed = false;
if (!this.state.likedUsers.length) {
return this.getWhoLiked();
}
const likesPromise = !this.state.likedUsers.length
? this.getWhoLiked()
: Ember.RSVP.resolve();
return likesPromise.then(() => {
if (!this.state.readers.length && this.attrs.showReadIndicator) {
return this.getWhoRead();
}
});
},
like() {
@@ -562,6 +616,12 @@ export default createWidget("post-menu", {
}
},
refreshReaders() {
if (this.state.readers.length) {
return this.getWhoRead();
}
},
getWhoLiked() {
const { attrs, state } = this;
@@ -576,6 +636,15 @@ export default createWidget("post-menu", {
});
},
getWhoRead() {
const { attrs, state } = this;
return this.store.find("post-reader", { id: attrs.id }).then(users => {
state.readers = users.map(avatarAtts);
state.totalReaders = users.totalRows;
});
},
toggleWhoLiked() {
const state = this.state;
if (state.likedUsers.length) {
@@ -583,5 +652,14 @@ export default createWidget("post-menu", {
} else {
return this.getWhoLiked();
}
},
toggleWhoRead() {
const state = this.state;
if (this.state.readers.length) {
state.readers = [];
} else {
return this.getWhoRead();
}
}
});

View File

@@ -136,6 +136,7 @@ export default createWidget("post-stream", {
this.attach("post-small-action", transformed, { model: post })
);
} else {
transformed.showReadIndicator = attrs.showReadIndicator;
result.push(this.attach("post", transformed, { model: post }));
}

View File

@@ -133,6 +133,12 @@
.raw-topic-link > * {
pointer-events: none;
}
.read-indicator {
&.unread {
display: none;
}
}
}
.link-bottom-line {

View File

@@ -641,7 +641,8 @@ blockquote > *:last-child {
font-size: $font-down-1;
}
.who-liked {
.who-liked,
.who-read {
transition: height 0.5s;
a {
margin: 0 0.25em 0.5em 0;

View File

@@ -66,6 +66,7 @@ nav.post-controls {
margin-left: 0;
margin-right: 0;
&.my-likes,
&.read-indicator,
&.regular-likes {
// Like count on posts
.d-icon {
@@ -838,7 +839,8 @@ a.attachment:before {
}
}
.who-liked {
.who-liked,
.who-read {
margin-top: 20px;
margin-bottom: 0;
width: 100%;

View File

@@ -38,6 +38,7 @@ span.badge-posts {
flex: 0 1 auto;
button {
&.like,
&.read-indicator,
&.create-flag {
flex: 1 1 auto;
}

View File

@@ -153,7 +153,8 @@ class Admin::GroupsController < Admin::AdminController
:default_notification_level,
:membership_request_template,
:owner_usernames,
:usernames
:usernames,
:publish_read_state
]
custom_fields = Group.editable_group_custom_fields
permitted << { custom_fields: custom_fields } unless custom_fields.blank?

View File

@@ -552,7 +552,8 @@ class GroupsController < ApplicationController
:name,
:grant_trust_level,
:automatic_membership_email_domains,
:automatic_membership_retroactive
:automatic_membership_retroactive,
:publish_read_state
])
custom_fields = Group.editable_group_custom_fields

View File

@@ -0,0 +1,26 @@
# frozen_string_literal: true
class PostReadersController < ApplicationController
requires_login
def index
post = Post.includes(topic: %i[allowed_groups]).find(params[:id])
read_state = post.topic.allowed_groups.any? { |g| g.publish_read_state? && g.users.include?(current_user) }
raise Discourse::InvalidAccess unless read_state
readers = User
.joins(:topic_users)
.where('topic_users.topic_id = ? AND COALESCE(topic_users.last_read_post_number, 1) >= ?', post.topic_id, post.post_number)
.where.not(id: [current_user.id, post.user_id])
readers = readers.map do |r|
{
id: r.id, avatar_template: r.avatar_template,
username: r.username,
username_lower: r.username_lower
}
end
render_json_dump(post_readers: readers)
end
end

View File

@@ -897,6 +897,7 @@ end
# visibility_level :integer default(0), not null
# public_exit :boolean default(FALSE), not null
# public_admission :boolean default(FALSE), not null
# publish_read_state :boolean default(FALSE), not null
# membership_request_template :text
# messageable_level :integer default(0)
# mentionable_level :integer default(0)

View File

@@ -176,6 +176,7 @@ SQL
topic_time = max_time_per_post if topic_time > max_time_per_post
TopicUser.update_last_read(current_user, topic_id, highest_seen, new_posts_read, topic_time, opts)
TopicGroup.update_last_read(current_user, topic_id, highest_seen)
if total_changed > 0
current_user.reload

71
app/models/topic_group.rb Normal file
View File

@@ -0,0 +1,71 @@
# frozen_string_literal: true
class TopicGroup < ActiveRecord::Base
belongs_to :group
belongs_to :topic
def self.update_last_read(user, topic_id, post_number)
updated_groups = update_read_count(user, topic_id, post_number)
create_topic_group(user, topic_id, post_number, updated_groups.map(&:group_id))
TopicTrackingState.publish_read_indicator_on_read(topic_id, post_number, user.id)
end
def self.new_message_update(user, topic_id, post_number)
updated_groups = update_read_count(user, topic_id, post_number)
create_topic_group(user, topic_id, post_number, updated_groups.map(&:group_id))
TopicTrackingState.publish_read_indicator_on_write(topic_id, post_number, user.id)
end
def self.update_read_count(user, topic_id, post_number)
update_query = <<~SQL
UPDATE topic_groups tg
SET
last_read_post_number = GREATEST(:post_number, tg.last_read_post_number),
updated_at = :now
FROM topic_allowed_groups tag
INNER JOIN group_users gu ON gu.group_id = tag.group_id
WHERE gu.user_id = :user_id
AND tag.topic_id = :topic_id
AND tg.topic_id = :topic_id
RETURNING
tg.group_id
SQL
updated_groups = DB.query(
update_query,
user_id: user.id, topic_id: topic_id, post_number: post_number, now: DateTime.now
)
end
def self.create_topic_group(user, topic_id, post_number, updated_group_ids)
query = <<~SQL
INSERT INTO topic_groups (topic_id, group_id, last_read_post_number, created_at, updated_at)
SELECT tag.topic_id, tag.group_id, :post_number, :now, :now
FROM topic_allowed_groups tag
INNER JOIN group_users gu ON gu.group_id = tag.group_id
WHERE gu.user_id = :user_id
AND tag.topic_id = :topic_id
SQL
query += 'AND NOT(tag.group_id IN (:already_updated_groups))' unless updated_group_ids.length.zero?
DB.exec(
query,
user_id: user.id, topic_id: topic_id, post_number: post_number, now: DateTime.now, already_updated_groups: updated_group_ids
)
end
end
# == Schema Information
#
# Table name: topic_groups
#
# id :integer not null, primary key
# group_id :integer not null
# topic_id :integer not null
# last_read_post_number :integer default(0), not null
#
# Indexes
#
# index_topic_allowed_groups_on_group_id_and_topic_id (group_id,topic_id) UNIQUE
#

View File

@@ -41,7 +41,8 @@ class TopicList
:current_user,
:tags,
:shared_drafts,
:category
:category,
:publish_read_state
)
def initialize(filter, current_user, topics, opts = nil)
@@ -57,6 +58,8 @@ class TopicList
if @opts[:tags]
@tags = Tag.where(id: @opts[:tags]).all
end
@publish_read_state = !!@opts[:publish_read_state]
end
def top_tags

View File

@@ -128,14 +128,14 @@ class TopicTrackingState
end
def self.publish_read(topic_id, last_read_post_number, user_id, notification_level = nil)
highest_post_number = Topic.where(id: topic_id).pluck(:highest_post_number).first
topic = Topic.select(:highest_post_number, :archetype, :id).find_by(id: topic_id)
message = {
topic_id: topic_id,
message_type: "read",
payload: {
last_read_post_number: last_read_post_number,
highest_post_number: highest_post_number,
highest_post_number: topic.highest_post_number,
topic_id: topic_id,
notification_level: notification_level
}
@@ -341,4 +341,56 @@ SQL
)
end
end
def self.publish_read_indicator_on_write(topic_id, last_read_post_number, user_id)
topic = Topic.includes(:allowed_groups).select(:highest_post_number, :archetype, :id).find_by(id: topic_id)
if topic.private_message?
groups = read_allowed_groups_of(topic)
update_topic_list_read_indicator(topic, groups, topic.highest_post_number, user_id, false)
end
end
def self.publish_read_indicator_on_read(topic_id, last_read_post_number, user_id)
topic = Topic.includes(:allowed_groups).select(:highest_post_number, :archetype, :id).find_by(id: topic_id)
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)
update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, true)
end
end
def self.read_allowed_groups_of(topic)
topic.allowed_groups
.joins(:group_users)
.where(publish_read_state: true)
.select('ARRAY_AGG(group_users.user_id) AS members', :name, :id)
.group('groups.id')
end
def self.update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, read_event)
return unless last_read_post_number == topic.highest_post_number
message = { topic_id: topic.id, show_indicator: read_event }.as_json
groups_to_update = []
groups.each do |group|
member = group.members.include?(user_id)
member_writing = (!read_event && member)
non_member_reading = (read_event && !member)
next if non_member_reading || member_writing
groups_to_update << group
end
return if groups_to_update.empty?
MessageBus.publish("/private-messages/read-indicator/#{topic.id}", message, user_ids: groups_to_update.flat_map(&:members))
end
def self.trigger_post_read_count_update(post, groups)
return if groups.empty?
post.publish_change_to_clients!(:read)
end
end

View File

@@ -31,7 +31,8 @@ class BasicGroupSerializer < ApplicationSerializer
:is_group_user,
:is_group_owner,
:members_visibility_level,
:can_see_members
:can_see_members,
:publish_read_state
def include_display_name?
object.automatic

View File

@@ -25,7 +25,8 @@ class ListableTopicSerializer < BasicTopicSerializer
:notification_level,
:bookmarked,
:liked,
:unicode_title
:unicode_title,
:read_by_group_member
has_one :last_poster, serializer: BasicUserSerializer, embed: :objects
@@ -121,6 +122,18 @@ class ListableTopicSerializer < BasicTopicSerializer
PinnedCheck.unpinned?(object, object.user_data)
end
def read_by_group_member
# object#last_read_post_number is an attribute selected from a joined table.
# See TopicQuery#append_read_state for more information.
return false unless object.respond_to?(:last_read_post_number)
object.last_read_post_number >= object.highest_post_number
end
def include_read_by_group_member?
!!object.topic_list&.publish_read_state
end
protected
def unread_helper

View File

@@ -26,6 +26,7 @@ class PostSerializer < BasicPostSerializer
:quote_count,
:incoming_link_count,
:reads,
:readers_count,
:score,
:yours,
:topic_id,
@@ -458,6 +459,13 @@ 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

@@ -71,7 +71,8 @@ class TopicViewSerializer < ApplicationSerializer
:participant_count,
:destination_category_id,
:pm_with_non_human_user,
:queued_posts_count
:queued_posts_count,
:show_read_indicator
)
has_one :details, serializer: TopicViewDetailsSerializer, root: false, embed: :objects
@@ -248,4 +249,8 @@ class TopicViewSerializer < ApplicationSerializer
def include_queued_posts_count?
scope.is_staff? && object.queued_posts_enabled
end
def show_read_indicator
object.show_read_indicator?
end
end

View File

@@ -32,4 +32,7 @@ class WebHookPostSerializer < PostSerializer
object.topic ? object.topic.posts_count : 0
end
def include_readers_count?
false
end
end

View File

@@ -28,6 +28,10 @@ class WebHookTopicViewSerializer < TopicViewSerializer
end
end
def include_show_read_indicator?
false
end
def created_by
BasicUserSerializer.new(object.topic.user, scope: scope, root: false)
end