From 67f5cc1ce8da8d6301db0ab6ebcd6fc497f28455 Mon Sep 17 00:00:00 2001 From: romanrizzi Date: Tue, 20 Aug 2019 10:24:34 -0300 Subject: [PATCH] Revert "FEATURE: Publish read state on group messages. (#7989)" This reverts commit 1630dae2dbf26b2a0eec639c69ff60643cf6f8d1. --- .../components/scrolling-post-stream.js.es6 | 9 +- .../components/topic-list-item.js.es6 | 36 -------- .../discourse/controllers/topic.js.es6 | 11 --- .../discourse/lib/transform-post.js.es6 | 3 +- .../javascripts/discourse/models/group.js.es6 | 3 +- .../groups-form-interaction-fields.hbs | 10 --- .../templates/list/topic-list-item.raw.hbs | 3 - .../javascripts/discourse/templates/topic.hbs | 1 - .../discourse/widgets/post-menu.js.es6 | 86 +----------------- .../discourse/widgets/post-stream.js.es6 | 1 - .../stylesheets/common/base/_topic-list.scss | 6 -- .../stylesheets/common/base/topic-post.scss | 3 +- .../stylesheets/desktop/topic-post.scss | 4 +- app/assets/stylesheets/mobile/topic-post.scss | 1 - app/controllers/admin/groups_controller.rb | 3 +- app/controllers/groups_controller.rb | 3 +- app/controllers/post_readers_controller.rb | 26 ------ app/models/group.rb | 1 - app/models/topic_list.rb | 5 +- app/models/topic_tracking_state.rb | 36 +------- app/serializers/basic_group_serializer.rb | 3 +- app/serializers/listable_topic_serializer.rb | 15 +--- app/serializers/post_serializer.rb | 8 -- app/serializers/topic_view_serializer.rb | 7 +- app/serializers/web_hook_post_serializer.rb | 3 - .../web_hook_topic_view_serializer.rb | 4 - config/locales/client.en.yml | 3 - config/routes.rb | 1 - config/site_settings.yml | 3 +- ...0190807194043_groups_publish_read_state.rb | 7 -- lib/topic_query.rb | 27 +----- lib/topic_view.rb | 8 -- spec/components/topic_query_spec.rb | 43 --------- spec/models/topic_tracking_state_spec.rb | 72 --------------- spec/requests/post_readers_controller_spec.rb | 90 ------------------- 35 files changed, 21 insertions(+), 524 deletions(-) delete mode 100644 app/controllers/post_readers_controller.rb delete mode 100644 db/migrate/20190807194043_groups_publish_read_state.rb delete mode 100644 spec/requests/post_readers_controller_spec.rb diff --git a/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 index 89a48126f30..4121a8b0b33 100644 --- a/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 +++ b/app/assets/javascripts/discourse/components/scrolling-post-stream.js.es6 @@ -40,8 +40,7 @@ export default MountWidget.extend({ "gaps", "selectedQuery", "selectedPostsCount", - "searchService", - "showReadIndicator" + "searchService" ); }, @@ -292,12 +291,6 @@ 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(); } diff --git a/app/assets/javascripts/discourse/components/topic-list-item.js.es6 b/app/assets/javascripts/discourse/components/topic-list-item.js.es6 index 9907e0286a8..564938e0566 100644 --- a/app/assets/javascripts/discourse/components/topic-list-item.js.es6 +++ b/app/assets/javascripts/discourse/components/topic-list-item.js.es6 @@ -35,42 +35,6 @@ export const ListItemDefaults = { attributeBindings: ["data-topic-id"], "data-topic-id": Ember.computed.alias("topic.id"), - didInsertElement() { - this._super(...arguments); - - if (typeof this.get("topic.read_by_group_member") !== "undefined") { - 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 (typeof this.get("topic.read_by_group_member") !== "undefined") { - this.messageBus.unsubscribe(this.readIndicatorChannel); - } - }, - - @computed("topic.id") - readIndicatorChannel(topicId) { - return `/private-messages/group-read/${topicId}`; - }, - - @computed("topic.read_by_group_member") - unreadClass(readByGroupMember) { - return readByGroupMember ? "" : "unread"; - }, - @computed newDotText() { return this.currentUser && this.currentUser.trust_level > 0 diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index 25a635b48bf..c1983bc4c16 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -1348,17 +1348,6 @@ 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 diff --git a/app/assets/javascripts/discourse/lib/transform-post.js.es6 b/app/assets/javascripts/discourse/lib/transform-post.js.es6 index 5f882d8a933..7a1872b4fe5 100644 --- a/app/assets/javascripts/discourse/lib/transform-post.js.es6 +++ b/app/assets/javascripts/discourse/lib/transform-post.js.es6 @@ -71,8 +71,7 @@ export function transformBasicPost(post) { expandablePost: false, replyCount: post.reply_count, locked: post.locked, - userCustomFields: post.user_custom_fields, - readCount: post.readers_count + userCustomFields: post.user_custom_fields }; _additionalAttributes.forEach(a => (postAtts[a] = post[a])); diff --git a/app/assets/javascripts/discourse/models/group.js.es6 b/app/assets/javascripts/discourse/models/group.js.es6 index 04898356354..5ea8f11616f 100644 --- a/app/assets/javascripts/discourse/models/group.js.es6 +++ b/app/assets/javascripts/discourse/models/group.js.es6 @@ -178,8 +178,7 @@ 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, - publish_read_state: this.publish_read_state + membership_request_template: this.membership_request_template }; if (!this.id) { diff --git a/app/assets/javascripts/discourse/templates/components/groups-form-interaction-fields.hbs b/app/assets/javascripts/discourse/templates/components/groups-form-interaction-fields.hbs index 959e2e12d13..b2f254ca328 100644 --- a/app/assets/javascripts/discourse/templates/components/groups-form-interaction-fields.hbs +++ b/app/assets/javascripts/discourse/templates/components/groups-form-interaction-fields.hbs @@ -52,16 +52,6 @@ class="groups-form-messageable-level"}} -
- -
- {{#if showEmailSettings}}
diff --git a/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs b/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs index dba2384f786..6b6b7b2af14 100644 --- a/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs +++ b/app/assets/javascripts/discourse/templates/list/topic-list-item.raw.hbs @@ -23,9 +23,6 @@ {{~#if showTopicPostBadges}} {{~raw "topic-post-badges" unread=topic.unread newPosts=topic.displayNewPosts unseen=topic.unseen url=topic.lastUnreadUrl newDotText=newDotText}} {{~/if}} - - {{~d-icon "far-eye"}} -
{{#unless hideCategory}} diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index c10ec5d1175..f8e5ee39034 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -177,7 +177,6 @@ selectedPostsCount=selectedPostsCount selectedQuery=selectedQuery gaps=model.postStream.gaps - showReadIndicator=model.show_read_indicator showFlags=(action "showPostFlags") editPost=(action "editPost") showHistory=(route-action "showHistory") diff --git a/app/assets/javascripts/discourse/widgets/post-menu.js.es6 b/app/assets/javascripts/discourse/widgets/post-menu.js.es6 index 3d7a06801f2..8739675e2f8 100644 --- a/app/assets/javascripts/discourse/widgets/post-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post-menu.js.es6 @@ -52,36 +52,6 @@ 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: "far-eye", - before: "read-count", - addContainer: false, - disabled - }; - } -}); - function likeCount(attrs) { const count = attrs.likeCount; @@ -371,12 +341,7 @@ export default createWidget("post-menu", { }, defaultState() { - return { - collapsed: true, - likedUsers: [], - readers: [], - adminVisible: false - }; + return { collapsed: true, likedUsers: [], adminVisible: false }; }, buildKey: attrs => `post-menu-${attrs.id}`, @@ -543,19 +508,6 @@ 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; }, @@ -573,15 +525,9 @@ export default createWidget("post-menu", { showMoreActions() { this.state.collapsed = false; - const likesPromise = !this.state.likedUsers.length - ? this.getWhoLiked() - : Ember.RSVP.resolve(); - - return likesPromise.then(() => { - if (!this.state.readers.length) { - return this.getWhoRead(); - } - }); + if (!this.state.likedUsers.length) { + return this.getWhoLiked(); + } }, like() { @@ -616,12 +562,6 @@ export default createWidget("post-menu", { } }, - refreshReaders() { - if (this.state.readers.length) { - return this.getWhoRead(); - } - }, - getWhoLiked() { const { attrs, state } = this; @@ -636,15 +576,6 @@ 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) { @@ -652,14 +583,5 @@ 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(); - } } }); diff --git a/app/assets/javascripts/discourse/widgets/post-stream.js.es6 b/app/assets/javascripts/discourse/widgets/post-stream.js.es6 index 9291d8a4e67..da059dc964f 100644 --- a/app/assets/javascripts/discourse/widgets/post-stream.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post-stream.js.es6 @@ -136,7 +136,6 @@ 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 })); } diff --git a/app/assets/stylesheets/common/base/_topic-list.scss b/app/assets/stylesheets/common/base/_topic-list.scss index 0fd6593434d..4d77eb01f18 100644 --- a/app/assets/stylesheets/common/base/_topic-list.scss +++ b/app/assets/stylesheets/common/base/_topic-list.scss @@ -133,12 +133,6 @@ .raw-topic-link > * { pointer-events: none; } - - .read-indicator { - &.unread { - display: none; - } - } } .link-bottom-line { diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index 03c00c58ed5..8d6e880ddd1 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/app/assets/stylesheets/common/base/topic-post.scss @@ -641,8 +641,7 @@ blockquote > *:last-child { font-size: $font-down-1; } -.who-liked, -.who-read { +.who-liked { transition: height 0.5s; a { margin: 0 0.25em 0.5em 0; diff --git a/app/assets/stylesheets/desktop/topic-post.scss b/app/assets/stylesheets/desktop/topic-post.scss index 21b94f83761..07238d3ceaf 100644 --- a/app/assets/stylesheets/desktop/topic-post.scss +++ b/app/assets/stylesheets/desktop/topic-post.scss @@ -66,7 +66,6 @@ nav.post-controls { margin-left: 0; margin-right: 0; &.my-likes, - &.read-indicator, &.regular-likes { // Like count on posts .d-icon { @@ -839,8 +838,7 @@ a.attachment:before { } } -.who-liked, -.who-read { +.who-liked { margin-top: 20px; margin-bottom: 0; width: 100%; diff --git a/app/assets/stylesheets/mobile/topic-post.scss b/app/assets/stylesheets/mobile/topic-post.scss index 30b2f6d976b..726c6fd614d 100644 --- a/app/assets/stylesheets/mobile/topic-post.scss +++ b/app/assets/stylesheets/mobile/topic-post.scss @@ -38,7 +38,6 @@ span.badge-posts { flex: 0 1 auto; button { &.like, - &.read-indicator, &.create-flag { flex: 1 1 auto; } diff --git a/app/controllers/admin/groups_controller.rb b/app/controllers/admin/groups_controller.rb index 17c98b2d31e..48ea9822b35 100644 --- a/app/controllers/admin/groups_controller.rb +++ b/app/controllers/admin/groups_controller.rb @@ -153,8 +153,7 @@ class Admin::GroupsController < Admin::AdminController :default_notification_level, :membership_request_template, :owner_usernames, - :usernames, - :publish_read_state + :usernames ] custom_fields = Group.editable_group_custom_fields permitted << { custom_fields: custom_fields } unless custom_fields.blank? diff --git a/app/controllers/groups_controller.rb b/app/controllers/groups_controller.rb index 461a98074b8..724d9818af5 100644 --- a/app/controllers/groups_controller.rb +++ b/app/controllers/groups_controller.rb @@ -552,8 +552,7 @@ class GroupsController < ApplicationController :name, :grant_trust_level, :automatic_membership_email_domains, - :automatic_membership_retroactive, - :publish_read_state + :automatic_membership_retroactive ]) custom_fields = Group.editable_group_custom_fields diff --git a/app/controllers/post_readers_controller.rb b/app/controllers/post_readers_controller.rb deleted file mode 100644 index 76d75a70ac1..00000000000 --- a/app/controllers/post_readers_controller.rb +++ /dev/null @@ -1,26 +0,0 @@ -# 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 diff --git a/app/models/group.rb b/app/models/group.rb index a04cdef63e5..fd76e8af8ff 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -897,7 +897,6 @@ 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) diff --git a/app/models/topic_list.rb b/app/models/topic_list.rb index e546df96f04..69cedff1346 100644 --- a/app/models/topic_list.rb +++ b/app/models/topic_list.rb @@ -41,8 +41,7 @@ class TopicList :current_user, :tags, :shared_drafts, - :category, - :publish_read_state + :category ) def initialize(filter, current_user, topics, opts = nil) @@ -58,8 +57,6 @@ class TopicList if @opts[:tags] @tags = Tag.where(id: @opts[:tags]).all end - - @publish_read_state = !!@opts[:publish_read_state] end def top_tags diff --git a/app/models/topic_tracking_state.rb b/app/models/topic_tracking_state.rb index 4c26cc93b31..88eb8110b91 100644 --- a/app/models/topic_tracking_state.rb +++ b/app/models/topic_tracking_state.rb @@ -128,26 +128,19 @@ class TopicTrackingState end def self.publish_read(topic_id, last_read_post_number, user_id, notification_level = nil) - topic = Topic.select(:highest_post_number, :archetype, :id).find_by(id: topic_id) + highest_post_number = Topic.where(id: topic_id).pluck(:highest_post_number).first message = { topic_id: topic_id, message_type: "read", payload: { last_read_post_number: last_read_post_number, - highest_post_number: topic.highest_post_number, + highest_post_number: highest_post_number, topic_id: topic_id, notification_level: notification_level } } - 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 - MessageBus.publish(self.unread_channel_key(user_id), message.as_json, user_ids: [user_id]) end @@ -339,8 +332,6 @@ SQL message = { topic_id: topic.id } - groups = read_allowed_groups_of(topic) - update_topic_list_read_indicator(topic, groups, topic.highest_post_number, topic.last_post_user_id, false) channels.each do |channel, ids| MessageBus.publish( @@ -350,27 +341,4 @@ SQL ) end end - - def self.read_allowed_groups_of(topic) - topic.allowed_groups - .joins(:group_users) - .select('ARRAY_AGG(group_users.user_id) AS members', :name, :publish_read_state) - .group('groups.id') - end - - def self.update_topic_list_read_indicator(topic, groups, last_read_post_number, user_id, read_event) - groups.each do |group| - member = group.members.include?(user_id) - next unless group.publish_read_state? && last_read_post_number == topic.highest_post_number - next if (read_event && !member) || (!read_event && member) - message = { topic_id: topic.id, show_indicator: read_event }.as_json - - MessageBus.publish("/private-messages/group-read/#{topic.id}", message, user_ids: group.members) - end - end - - def self.trigger_post_read_count_update(post, groups) - return if groups.none?(&:publish_read_state?) - post.publish_change_to_clients!(:read) - end end diff --git a/app/serializers/basic_group_serializer.rb b/app/serializers/basic_group_serializer.rb index d3c012194a2..3f1cb66dab8 100644 --- a/app/serializers/basic_group_serializer.rb +++ b/app/serializers/basic_group_serializer.rb @@ -31,8 +31,7 @@ class BasicGroupSerializer < ApplicationSerializer :is_group_user, :is_group_owner, :members_visibility_level, - :can_see_members, - :publish_read_state + :can_see_members def include_display_name? object.automatic diff --git a/app/serializers/listable_topic_serializer.rb b/app/serializers/listable_topic_serializer.rb index cdd00c1f67b..0d4208c5bfd 100644 --- a/app/serializers/listable_topic_serializer.rb +++ b/app/serializers/listable_topic_serializer.rb @@ -25,8 +25,7 @@ class ListableTopicSerializer < BasicTopicSerializer :notification_level, :bookmarked, :liked, - :unicode_title, - :read_by_group_member + :unicode_title has_one :last_poster, serializer: BasicUserSerializer, embed: :objects @@ -122,18 +121,6 @@ class ListableTopicSerializer < BasicTopicSerializer PinnedCheck.unpinned?(object, object.user_data) end - def read_by_group_member - # object#minimum_unread_count is a dynamically generated attribute. - # See TopicQuery#append_read_state for more information. - return false unless object.respond_to?(:minimum_unread_count) - - object.minimum_unread_count && object.minimum_unread_count <= 0 - end - - def include_read_by_group_member? - !!object.topic_list&.publish_read_state - end - protected def unread_helper diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index d0bd771a2a2..7ab24caacc5 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -26,7 +26,6 @@ class PostSerializer < BasicPostSerializer :quote_count, :incoming_link_count, :reads, - :readers_count, :score, :yours, :topic_id, @@ -459,13 +458,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? diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 1f177645164..abcfdb8af85 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -71,8 +71,7 @@ class TopicViewSerializer < ApplicationSerializer :participant_count, :destination_category_id, :pm_with_non_human_user, - :queued_posts_count, - :show_read_indicator + :queued_posts_count ) has_one :details, serializer: TopicViewDetailsSerializer, root: false, embed: :objects @@ -249,8 +248,4 @@ 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 diff --git a/app/serializers/web_hook_post_serializer.rb b/app/serializers/web_hook_post_serializer.rb index 2f78f3ca886..d2b173485a2 100644 --- a/app/serializers/web_hook_post_serializer.rb +++ b/app/serializers/web_hook_post_serializer.rb @@ -32,7 +32,4 @@ class WebHookPostSerializer < PostSerializer object.topic ? object.topic.posts_count : 0 end - def include_readers_count? - false - end end diff --git a/app/serializers/web_hook_topic_view_serializer.rb b/app/serializers/web_hook_topic_view_serializer.rb index 0d83cee5aef..5c47ead356f 100644 --- a/app/serializers/web_hook_topic_view_serializer.rb +++ b/app/serializers/web_hook_topic_view_serializer.rb @@ -28,10 +28,6 @@ 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 diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f8161e0ba2e..d444d498a5a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2421,7 +2421,6 @@ en: reply: "begin composing a reply to this post" like: "like this post" has_liked: "you've liked this post" - read_indicator: "members who read this post" undo_like: "undo like" edit: "edit this post" edit_action: "Edit" @@ -2479,7 +2478,6 @@ en: notify_user: "sent a message" bookmark: "bookmarked this" like: "liked this" - read: "read this" like_capped: one: "and {{count}} other liked this" other: "and {{count}} others liked this" @@ -3219,7 +3217,6 @@ en: members_visibility_levels: title: "Who can see this group members?" description: "Admins can see members of all groups." - publish_read_state: "On group messages publish group read state" membership: automatic: Automatic diff --git a/config/routes.rb b/config/routes.rb index da8e98ed0f6..2ef2345bb95 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -614,7 +614,6 @@ Discourse::Application.routes.draw do get "excerpt" => "excerpt#show" resources :post_action_users - resources :post_readers, only: %i[index] resources :post_actions do collection do get "users" diff --git a/config/site_settings.yml b/config/site_settings.yml index d20a6aa4e5d..2ca78030f90 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -191,10 +191,9 @@ basic: post_menu: client: true type: list - default: "read|like|share|flag|edit|bookmark|delete|admin|reply" + default: "like|share|flag|edit|bookmark|delete|admin|reply" allow_any: false choices: - - read - like - edit - flag diff --git a/db/migrate/20190807194043_groups_publish_read_state.rb b/db/migrate/20190807194043_groups_publish_read_state.rb deleted file mode 100644 index 0849b374f2b..00000000000 --- a/db/migrate/20190807194043_groups_publish_read_state.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -class GroupsPublishReadState < ActiveRecord::Migration[5.2] - def change - add_column :groups, :publish_read_state, :boolean, null: false, default: false - end -end diff --git a/lib/topic_query.rb b/lib/topic_query.rb index e7ac08398cb..1b90719454f 100644 --- a/lib/topic_query.rb +++ b/lib/topic_query.rb @@ -344,13 +344,11 @@ class TopicQuery def list_private_messages_group(user) list = private_messages_for(user, :group) - group = Group.where('name ilike ?', @options[:group_name]).select(:id, :publish_read_state).first - publish_read_state = !!group&.publish_read_state + group_id = Group.where('name ilike ?', @options[:group_name]).pluck(:id).first list = list.joins("LEFT JOIN group_archived_messages gm ON gm.topic_id = topics.id AND - gm.group_id = #{group&.id&.to_i}") + gm.group_id = #{group_id.to_i}") list = list.where("gm.id IS NULL") - list = append_read_state(list, group) if publish_read_state - create_list(:private_messages, { publish_read_state: publish_read_state }, list) + create_list(:private_messages, {}, list) end def list_private_messages_group_archive(user) @@ -1059,23 +1057,4 @@ class TopicQuery def sanitize_sql_array(input) ActiveRecord::Base.public_send(:sanitize_sql_array, input.join(',')) end - - def append_read_state(list, group) - group_id = group&.id - return list if group_id.nil? - - selected_values = list.select_values.empty? ? ['topics.*'] : list.select_values - selected_values << "tuig.minimum_unread_count" - - # The calculation was borrowed from lib/unread.rb - minimum_unread_count = TopicUser - .joins(:topic) - .joins("INNER JOIN group_users ON group_users.user_id = topic_users.user_id") - .where(group_users: { group_id: group_id }) - .select( - "MIN(COALESCE(topics.highest_post_number, 0) - COALESCE(highest_seen_post_number, 0)) AS minimum_unread_count, topic_id" - ).group(:topic_id).to_sql - - list.joins("LEFT OUTER JOIN (#{minimum_unread_count}) tuig ON topics.id = tuig.topic_id").select(*selected_values) - end end diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 69c3ca64400..c8ac58063cc 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -109,14 +109,6 @@ class TopicView @personal_message = @topic.private_message? end - def show_read_indicator? - return false unless @user || topic.private_message? - - topic.allowed_groups.any? do |group| - group.publish_read_state? && group.users.include?(@user) - end - end - def canonical_path path = relative_url.dup path << diff --git a/spec/components/topic_query_spec.rb b/spec/components/topic_query_spec.rb index a2df1f707f4..3eccdec1420 100644 --- a/spec/components/topic_query_spec.rb +++ b/spec/components/topic_query_spec.rb @@ -1020,49 +1020,6 @@ describe TopicQuery do expect(topics).to eq([]) end - - context "Calculating minimum unread count for a topic" do - before { group.update!(publish_read_state: true) } - - let(:listed_message) do - TopicQuery.new(nil, group_name: group.name) - .list_private_messages_group(creator) - .topics.first - end - - it 'returns a positive number when noone has read the last message' do - group_message.update!(highest_post_number: 1) - TopicUser.create!(user: creator, topic: group_message) - - expect(listed_message.minimum_unread_count).to eq(1) - end - - it 'returns 0 when all posts were read' do - group_message.update!(highest_post_number: 1) - TopicUser.create!(user: creator, topic: group_message, highest_seen_post_number: 1) - - expect(listed_message.minimum_unread_count).to eq(0) - end - - it 'returns the minimum number of unread posts when there are more than one user' do - new_user = Fabricate(:user) - group.add(new_user) - group_message.update!(highest_post_number: 3) - TopicUser.create!(user: creator, topic: group_message, highest_seen_post_number: 1) - TopicUser.create!(user: new_user, topic: group_message, highest_seen_post_number: 2) - - expect(listed_message.minimum_unread_count).to eq(1) - end - - it 'returns the minimum number of unread posts when there are more than one user' do - new_user = Fabricate(:topic_allowed_user, topic: group_message).user - group_message.update!(highest_post_number: 3) - TopicUser.create!(user: creator, topic: group_message, highest_seen_post_number: 1) - TopicUser.create!(user: new_user, topic: group_message, highest_seen_post_number: 2) - - expect(listed_message.minimum_unread_count).to eq(2) - end - end end context "shared drafts" do diff --git a/spec/models/topic_tracking_state_spec.rb b/spec/models/topic_tracking_state_spec.rb index ceff1b9c95a..261c3752d27 100644 --- a/spec/models/topic_tracking_state_spec.rb +++ b/spec/models/topic_tracking_state_spec.rb @@ -254,78 +254,6 @@ describe TopicTrackingState do end end - describe '#publish_read' do - fab!(:group) { Fabricate(:group) } - let(:read_topic_key) { "/private-messages/group-read/#{@group_message.id}" } - let(:read_post_key) { "/topic/#{@group_message.id}" } - let(:latest_post_number) { 3 } - - before do - group.add(user) - @group_message = Fabricate(:private_message_topic, - allowed_groups: [group], - topic_allowed_users: [Fabricate.build(:topic_allowed_user, user: user)], - highest_post_number: latest_post_number - ) - @post = Fabricate(:post, topic: @group_message, post_number: latest_post_number) - end - - it 'does not publish the read indicator if the option is disabled' do - messages = MessageBus.track_publish(read_topic_key) do - TopicTrackingState.publish_read(@group_message.id, latest_post_number, user.id) - end - - expect(messages).to be_empty - end - - it 'does not trigger a read count update if no allowed groups have the option enabled' do - messages = MessageBus.track_publish(read_post_key) do - TopicTrackingState.publish_read(@group_message.id, latest_post_number, user.id) - end - - expect(messages).to be_empty - end - - context 'when the read indicator is enabled' do - before { group.update!(publish_read_state: true) } - - it 'does publish the read indicator' do - message = MessageBus.track_publish(read_topic_key) do - TopicTrackingState.publish_read(@group_message.id, latest_post_number, user.id) - end.first - - expect(message.data['topic_id']).to eq @group_message.id - end - - it 'does not publish the read indicator if the message is not the last one' do - not_last_post_number = latest_post_number - 1 - Fabricate(:post, topic: @group_message, post_number: not_last_post_number) - messages = MessageBus.track_publish(read_topic_key) do - TopicTrackingState.publish_read(@group_message.id, not_last_post_number, user.id) - end - - expect(messages).to be_empty - end - - it 'does not publish the read indicator if the user is not a group member' do - allowed_user = Fabricate(:topic_allowed_user, topic: @group_message) - messages = MessageBus.track_publish(read_topic_key) do - TopicTrackingState.publish_read(@group_message.id, latest_post_number, allowed_user.user_id) - end - - expect(messages).to be_empty - end - - it 'publish a read count update to every client' do - message = MessageBus.track_publish(read_post_key) do - TopicTrackingState.publish_read(@group_message.id, latest_post_number, user.id) - end.first - - expect(message.data[:type]).to eq :read - end - end - end - it "correctly handles muted categories" do user = Fabricate(:user) diff --git a/spec/requests/post_readers_controller_spec.rb b/spec/requests/post_readers_controller_spec.rb deleted file mode 100644 index fa2cf3d8728..00000000000 --- a/spec/requests/post_readers_controller_spec.rb +++ /dev/null @@ -1,90 +0,0 @@ -# frozen_string_literal: true - -require 'rails_helper' - -describe PostReadersController do - describe '#index' do - fab!(:admin) { Fabricate(:admin) } - fab!(:reader) { Fabricate(:user) } - - before { sign_in(admin) } - - before do - @group = Fabricate(:group) - @group_message = Fabricate(:private_message_topic, allowed_groups: [@group]) - @post = Fabricate(:post, topic: @group_message, post_number: 3) - end - - context 'When the user has access to readers data' do - before do - @group.update!(publish_read_state: true) - @group.add(admin) - @group.add(reader) - end - - it 'returns an empty list when nobody has read the topic' do - get '/post_readers.json', params: { id: @post.id } - - readers = JSON.parse(response.body)['post_readers'] - - expect(readers).to be_empty - end - - it 'returns an user who read until that post' do - TopicUser.create!(user: reader, topic: @group_message, last_read_post_number: 3) - - get '/post_readers.json', params: { id: @post.id } - reader_data = JSON.parse(response.body)['post_readers'].first - - assert_reader_is_correctly_serialized(reader_data, reader, @post) - end - - it 'returns an user who read pass that post' do - TopicUser.create!(user: reader, topic: @group_message, last_read_post_number: 4) - - get '/post_readers.json', params: { id: @post.id } - reader_data = JSON.parse(response.body)['post_readers'].first - - assert_reader_is_correctly_serialized(reader_data, reader, @post) - end - - it 'return an empty list when nodobody read unti that post' do - TopicUser.create!(user: reader, topic: @group_message, last_read_post_number: 1) - - get '/post_readers.json', params: { id: @post.id } - readers = JSON.parse(response.body)['post_readers'] - - 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 - end - - def assert_reader_is_correctly_serialized(reader_data, reader, post) - expect(reader_data['avatar_template']).to eq reader.avatar_template - expect(reader_data['username']).to eq reader.username - expect(reader_data['username_lower']).to eq reader.username_lower - end - - it 'returns forbidden if no group has publish_read_state enabled' do - get '/post_readers.json', params: { id: @post.id } - - expect(response).to be_forbidden - end - - it 'returns forbidden if current_user is not a member of a group with publish_read_state enabled' do - @group.update!(publish_read_state: true) - - get '/post_readers.json', params: { id: @post.id } - - expect(response).to be_forbidden - end - end -end