From 8fd2cc54c19fa69a6bfcd06d61d7ab7774215100 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 12 Oct 2018 01:36:51 +0200 Subject: Fix type of conversation ID in conversations API (#8961) --- app/serializers/rest/conversation_serializer.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'app/serializers/rest') diff --git a/app/serializers/rest/conversation_serializer.rb b/app/serializers/rest/conversation_serializer.rb index 08cea47d2..884253f89 100644 --- a/app/serializers/rest/conversation_serializer.rb +++ b/app/serializers/rest/conversation_serializer.rb @@ -4,4 +4,8 @@ class REST::ConversationSerializer < ActiveModel::Serializer attribute :id has_many :participant_accounts, key: :accounts, serializer: REST::AccountSerializer has_one :last_status, serializer: REST::StatusSerializer + + def id + object.id.to_s + end end -- cgit From ddd30f331c7a2af38176d72d9ce2265068984bed Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 17 Oct 2018 17:13:04 +0200 Subject: Improve support for aspects/circles (#8950) * Add silent column to mentions * Save silent mentions in ActivityPub Create handler and optimize it Move networking calls out of the database transaction * Add "limited" visibility level masked as "private" in the API Unlike DMs, limited statuses are pushed into home feeds. The access control rules between direct and limited statuses is almost the same, except for counter and conversation logic * Ensure silent column is non-null, add spec * Ensure filters don't check silent mentions for blocks/mutes As those are "this person is also allowed to see" rather than "this person is involved", therefore does not warrant filtering * Clean up code * Use Status#active_mentions to limit returned mentions * Fix code style issues * Use Status#active_mentions in Notification And remove stream_entry eager-loading from Notification --- app/lib/activitypub/activity.rb | 2 +- app/lib/activitypub/activity/create.rb | 24 +++++++++++++++++- app/lib/activitypub/tag_manager.rb | 6 ++--- app/lib/feed_manager.rb | 8 +++--- app/lib/formatter.rb | 2 +- app/lib/ostatus/atom_serializer.rb | 2 +- app/models/account_conversation.rb | 2 +- app/models/mention.rb | 8 ++++++ app/models/notification.rb | 2 +- app/models/status.rb | 15 +++++++---- app/models/stream_entry.rb | 2 +- app/policies/status_policy.rb | 8 +++--- app/serializers/activitypub/note_serializer.rb | 2 +- app/serializers/rest/status_serializer.rb | 13 +++++++++- app/services/batched_remove_status_service.rb | 2 +- app/services/fan_out_on_write_service.rb | 12 +++++++++ app/services/remove_status_service.rb | 2 +- .../stream_entries/_detailed_status.html.haml | 2 +- app/workers/activitypub/distribution_worker.rb | 2 +- .../activitypub/reply_distribution_worker.rb | 6 +---- .../20181010141500_add_silent_to_mentions.rb | 23 +++++++++++++++++ db/schema.rb | 3 ++- spec/lib/activitypub/activity/create_spec.rb | 29 ++++++++++++++++++++++ 23 files changed, 142 insertions(+), 35 deletions(-) create mode 100644 db/migrate/20181010141500_add_silent_to_mentions.rb (limited to 'app/serializers/rest') diff --git a/app/lib/activitypub/activity.rb b/app/lib/activitypub/activity.rb index 3a39b723e..999954cb5 100644 --- a/app/lib/activitypub/activity.rb +++ b/app/lib/activitypub/activity.rb @@ -96,7 +96,7 @@ class ActivityPub::Activity end def notify_about_mentions(status) - status.mentions.includes(:account).each do |mention| + status.active_mentions.includes(:account).each do |mention| next unless mention.account.local? && audience_includes?(mention.account) NotifyService.new.call(mention.account, mention) end diff --git a/app/lib/activitypub/activity/create.rb b/app/lib/activitypub/activity/create.rb index 73475bf02..7e6702a63 100644 --- a/app/lib/activitypub/activity/create.rb +++ b/app/lib/activitypub/activity/create.rb @@ -28,6 +28,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity process_status_params process_tags + process_audience ApplicationRecord.transaction do @status = Status.create!(@params) @@ -66,6 +67,27 @@ class ActivityPub::Activity::Create < ActivityPub::Activity end end + def process_audience + (as_array(@object['to']) + as_array(@object['cc'])).uniq.each do |audience| + next if audience == ActivityPub::TagManager::COLLECTIONS[:public] + + # Unlike with tags, there is no point in resolving accounts we don't already + # know here, because silent mentions would only be used for local access + # control anyway + account = account_from_uri(audience) + + next if account.nil? || @mentions.any? { |mention| mention.account_id == account.id } + + @mentions << Mention.new(account: account, silent: true) + + # If there is at least one silent mention, then the status can be considered + # as a limited-audience status, and not strictly a direct message + next unless @params[:visibility] == :direct + + @params[:visibility] = :limited + end + end + def attach_tags(status) @tags.each do |tag| status.tags << tag @@ -113,7 +135,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity return if account.nil? - @mentions << Mention.new(account: account) + @mentions << Mention.new(account: account, silent: false) end def process_emoji(tag) diff --git a/app/lib/activitypub/tag_manager.rb b/app/lib/activitypub/tag_manager.rb index 95d1cf9f3..be3a562d0 100644 --- a/app/lib/activitypub/tag_manager.rb +++ b/app/lib/activitypub/tag_manager.rb @@ -58,8 +58,8 @@ class ActivityPub::TagManager [COLLECTIONS[:public]] when 'unlisted', 'private' [account_followers_url(status.account)] - when 'direct' - status.mentions.map { |mention| uri_for(mention.account) } + when 'direct', 'limited' + status.active_mentions.map { |mention| uri_for(mention.account) } end end @@ -80,7 +80,7 @@ class ActivityPub::TagManager cc << COLLECTIONS[:public] end - cc.concat(status.mentions.map { |mention| uri_for(mention.account) }) unless status.direct_visibility? + cc.concat(status.active_mentions.map { |mention| uri_for(mention.account) }) unless status.direct_visibility? || status.limited_visibility? cc end diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index b10e5dd24..3d7db2721 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -88,7 +88,7 @@ class FeedManager end query.each do |status| - next if status.direct_visibility? || filter?(:home, status, into_account) + next if status.direct_visibility? || status.limited_visibility? || filter?(:home, status, into_account) add_to_feed(:home, into_account.id, status) end @@ -156,12 +156,12 @@ class FeedManager return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) return true if phrase_filtered?(status, receiver_id, :home) - check_for_blocks = status.mentions.pluck(:account_id) + check_for_blocks = status.active_mentions.pluck(:account_id) check_for_blocks.concat([status.account_id]) if status.reblog? check_for_blocks.concat([status.reblog.account_id]) - check_for_blocks.concat(status.reblog.mentions.pluck(:account_id)) + check_for_blocks.concat(status.reblog.active_mentions.pluck(:account_id)) end return true if blocks_or_mutes?(receiver_id, check_for_blocks, :home) @@ -188,7 +188,7 @@ class FeedManager # This filter is called from NotifyService, but already after the sender of # the notification has been checked for mute/block. Therefore, it's not # necessary to check the author of the toot for mute/block again - check_for_blocks = status.mentions.pluck(:account_id) + check_for_blocks = status.active_mentions.pluck(:account_id) check_for_blocks.concat([status.in_reply_to_account]) if status.reply? && !status.in_reply_to_account_id.nil? should_filter = blocks_or_mutes?(receiver_id, check_for_blocks, :mentions) # Filter if it's from someone I blocked, in reply to someone I blocked, or mentioning someone I blocked (or muted) diff --git a/app/lib/formatter.rb b/app/lib/formatter.rb index 35d5a09b7..d13884ec8 100644 --- a/app/lib/formatter.rb +++ b/app/lib/formatter.rb @@ -27,7 +27,7 @@ class Formatter return html.html_safe # rubocop:disable Rails/OutputSafety end - linkable_accounts = status.mentions.map(&:account) + linkable_accounts = status.active_mentions.map(&:account) linkable_accounts << status.account html = raw_content diff --git a/app/lib/ostatus/atom_serializer.rb b/app/lib/ostatus/atom_serializer.rb index 1a0a635b3..7a181fb40 100644 --- a/app/lib/ostatus/atom_serializer.rb +++ b/app/lib/ostatus/atom_serializer.rb @@ -354,7 +354,7 @@ class OStatus::AtomSerializer append_element(entry, 'summary', status.spoiler_text, 'xml:lang': status.language) if status.spoiler_text? append_element(entry, 'content', Formatter.instance.format(status).to_str || '.', type: 'html', 'xml:lang': status.language) - status.mentions.sort_by(&:id).each do |mentioned| + status.active_mentions.sort_by(&:id).each do |mentioned| append_element(entry, 'link', nil, rel: :mentioned, 'ostatus:object-type': OStatus::TagManager::TYPES[:person], href: OStatus::TagManager.instance.uri_for(mentioned.account)) end diff --git a/app/models/account_conversation.rb b/app/models/account_conversation.rb index a7205ec1a..c12c8d233 100644 --- a/app/models/account_conversation.rb +++ b/app/models/account_conversation.rb @@ -85,7 +85,7 @@ class AccountConversation < ApplicationRecord private def participants_from_status(recipient, status) - ((status.mentions.pluck(:account_id) + [status.account_id]).uniq - [recipient.id]).sort + ((status.active_mentions.pluck(:account_id) + [status.account_id]).uniq - [recipient.id]).sort end end diff --git a/app/models/mention.rb b/app/models/mention.rb index 8ab886b18..d01a88e32 100644 --- a/app/models/mention.rb +++ b/app/models/mention.rb @@ -8,6 +8,7 @@ # created_at :datetime not null # updated_at :datetime not null # account_id :bigint(8) +# silent :boolean default(FALSE), not null # class Mention < ApplicationRecord @@ -18,10 +19,17 @@ class Mention < ApplicationRecord validates :account, uniqueness: { scope: :status } + scope :active, -> { where(silent: false) } + scope :silent, -> { where(silent: true) } + delegate( :username, :acct, to: :account, prefix: true ) + + def active? + !silent? + end end diff --git a/app/models/notification.rb b/app/models/notification.rb index b9bec0808..78b180301 100644 --- a/app/models/notification.rb +++ b/app/models/notification.rb @@ -24,7 +24,7 @@ class Notification < ApplicationRecord favourite: 'Favourite', }.freeze - STATUS_INCLUDES = [:account, :application, :stream_entry, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, :application, :media_attachments, :tags, mentions: :account]].freeze + STATUS_INCLUDES = [:account, :application, :media_attachments, :tags, active_mentions: :account, reblog: [:account, :application, :media_attachments, :tags, active_mentions: :account]].freeze belongs_to :account, optional: true belongs_to :from_account, class_name: 'Account', optional: true diff --git a/app/models/status.rb b/app/models/status.rb index f61bd0fee..b18cb56b2 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -37,7 +37,7 @@ class Status < ApplicationRecord update_index('statuses#status', :proper) if Chewy.enabled? - enum visibility: [:public, :unlisted, :private, :direct], _suffix: :visibility + enum visibility: [:public, :unlisted, :private, :direct, :limited], _suffix: :visibility belongs_to :application, class_name: 'Doorkeeper::Application', optional: true @@ -51,7 +51,8 @@ class Status < ApplicationRecord has_many :favourites, inverse_of: :status, dependent: :destroy has_many :reblogs, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblog, dependent: :destroy has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread - has_many :mentions, dependent: :destroy + has_many :mentions, dependent: :destroy, inverse_of: :status + has_many :active_mentions, -> { active }, class_name: 'Mention', inverse_of: :status has_many :media_attachments, dependent: :nullify has_and_belongs_to_many :tags @@ -89,7 +90,7 @@ class Status < ApplicationRecord :status_stat, :tags, :stream_entry, - mentions: :account, + active_mentions: :account, reblog: [ :account, :application, @@ -98,7 +99,7 @@ class Status < ApplicationRecord :media_attachments, :conversation, :status_stat, - mentions: :account, + active_mentions: :account, ], thread: :account @@ -171,7 +172,11 @@ class Status < ApplicationRecord end def hidden? - private_visibility? || direct_visibility? + private_visibility? || direct_visibility? || limited_visibility? + end + + def distributable? + public_visibility? || unlisted_visibility? end def with_media? diff --git a/app/models/stream_entry.rb b/app/models/stream_entry.rb index a2f273281..1a9afc5c7 100644 --- a/app/models/stream_entry.rb +++ b/app/models/stream_entry.rb @@ -48,7 +48,7 @@ class StreamEntry < ApplicationRecord end def mentions - orphaned? ? [] : status.mentions.map(&:account) + orphaned? ? [] : status.active_mentions.map(&:account) end private diff --git a/app/policies/status_policy.rb b/app/policies/status_policy.rb index 6addc8a8a..64a5111fc 100644 --- a/app/policies/status_policy.rb +++ b/app/policies/status_policy.rb @@ -12,7 +12,7 @@ class StatusPolicy < ApplicationPolicy end def show? - if direct? + if requires_mention? owned? || mention_exists? elsif private? owned? || following_author? || mention_exists? @@ -22,7 +22,7 @@ class StatusPolicy < ApplicationPolicy end def reblog? - !direct? && (!private? || owned?) && show? && !blocking_author? + !requires_mention? && (!private? || owned?) && show? && !blocking_author? end def favourite? @@ -41,8 +41,8 @@ class StatusPolicy < ApplicationPolicy private - def direct? - record.direct_visibility? + def requires_mention? + record.direct_visibility? || record.limited_visibility? end def owned? diff --git a/app/serializers/activitypub/note_serializer.rb b/app/serializers/activitypub/note_serializer.rb index 82b7ffe95..c9d23e25f 100644 --- a/app/serializers/activitypub/note_serializer.rb +++ b/app/serializers/activitypub/note_serializer.rb @@ -68,7 +68,7 @@ class ActivityPub::NoteSerializer < ActiveModel::Serializer end def virtual_tags - object.mentions.to_a.sort_by(&:id) + object.tags + object.emojis + object.active_mentions.to_a.sort_by(&:id) + object.tags + object.emojis end def atom_uri diff --git a/app/serializers/rest/status_serializer.rb b/app/serializers/rest/status_serializer.rb index 61423f961..1f2f46b7e 100644 --- a/app/serializers/rest/status_serializer.rb +++ b/app/serializers/rest/status_serializer.rb @@ -36,6 +36,17 @@ class REST::StatusSerializer < ActiveModel::Serializer !current_user.nil? end + def visibility + # This visibility is masked behind "private" + # to avoid API changes because there are no + # UX differences + if object.limited_visibility? + 'private' + else + object.visibility + end + end + def uri OStatus::TagManager.instance.uri_for(object) end @@ -88,7 +99,7 @@ class REST::StatusSerializer < ActiveModel::Serializer end def ordered_mentions - object.mentions.to_a.sort_by(&:id) + object.active_mentions.to_a.sort_by(&:id) end class ApplicationSerializer < ActiveModel::Serializer diff --git a/app/services/batched_remove_status_service.rb b/app/services/batched_remove_status_service.rb index 2fcb3cc66..b8ab58938 100644 --- a/app/services/batched_remove_status_service.rb +++ b/app/services/batched_remove_status_service.rb @@ -12,7 +12,7 @@ class BatchedRemoveStatusService < BaseService def call(statuses) statuses = Status.where(id: statuses.map(&:id)).includes(:account, :stream_entry).flat_map { |status| [status] + status.reblogs.includes(:account, :stream_entry).to_a } - @mentions = statuses.map { |s| [s.id, s.mentions.includes(:account).to_a] }.to_h + @mentions = statuses.map { |s| [s.id, s.active_mentions.includes(:account).to_a] }.to_h @tags = statuses.map { |s| [s.id, s.tags.pluck(:name)] }.to_h @stream_entry_batches = [] diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb index 5ddddf3a9..7f2a91775 100644 --- a/app/services/fan_out_on_write_service.rb +++ b/app/services/fan_out_on_write_service.rb @@ -10,6 +10,8 @@ class FanOutOnWriteService < BaseService if status.direct_visibility? deliver_to_own_conversation(status) + elsif status.limited_visibility? + deliver_to_mentioned_followers(status) else deliver_to_self(status) if status.account.local? deliver_to_followers(status) @@ -53,6 +55,16 @@ class FanOutOnWriteService < BaseService end end + def deliver_to_mentioned_followers(status) + Rails.logger.debug "Delivering status #{status.id} to limited followers" + + status.mentions.includes(:account).each do |mention| + mentioned_account = mention.account + next if !mentioned_account.local? || !mentioned_account.following?(status.account) || FeedManager.instance.filter?(:home, status, mention.account_id) + FeedManager.instance.push_to_home(mentioned_account, status) + end + end + def render_anonymous_payload(status) @payload = InlineRenderer.render(status, nil, :status) @payload = Oj.dump(event: :update, payload: @payload) diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 1ee645e6d..11d28e783 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -8,7 +8,7 @@ class RemoveStatusService < BaseService @status = status @account = status.account @tags = status.tags.pluck(:name).to_a - @mentions = status.mentions.includes(:account).to_a + @mentions = status.active_mentions.includes(:account).to_a @reblogs = status.reblogs.to_a @stream_entry = status.stream_entry @options = options diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml index 0b204d437..6e6d0eda8 100644 --- a/app/views/stream_entries/_detailed_status.html.haml +++ b/app/views/stream_entries/_detailed_status.html.haml @@ -51,7 +51,7 @@ - if status.direct_visibility? %span.detailed-status__link< = fa_icon('envelope') - - elsif status.private_visibility? + - elsif status.private_visibility? || status.limited_visibility? %span.detailed-status__link< = fa_icon('lock') - else diff --git a/app/workers/activitypub/distribution_worker.rb b/app/workers/activitypub/distribution_worker.rb index c2bfd4f2f..17c1ef7ff 100644 --- a/app/workers/activitypub/distribution_worker.rb +++ b/app/workers/activitypub/distribution_worker.rb @@ -23,7 +23,7 @@ class ActivityPub::DistributionWorker private def skip_distribution? - @status.direct_visibility? + @status.direct_visibility? || @status.limited_visibility? end def relayable? diff --git a/app/workers/activitypub/reply_distribution_worker.rb b/app/workers/activitypub/reply_distribution_worker.rb index fe99fc05f..c0ed3a1f4 100644 --- a/app/workers/activitypub/reply_distribution_worker.rb +++ b/app/workers/activitypub/reply_distribution_worker.rb @@ -9,7 +9,7 @@ class ActivityPub::ReplyDistributionWorker @status = Status.find(status_id) @account = @status.thread&.account - return if @account.nil? || skip_distribution? + return unless @account.present? && @status.distributable? ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url| [signed_payload, @status.account_id, inbox_url] @@ -20,10 +20,6 @@ class ActivityPub::ReplyDistributionWorker private - def skip_distribution? - @status.private_visibility? || @status.direct_visibility? - end - def inboxes @inboxes ||= @account.followers.inboxes end diff --git a/db/migrate/20181010141500_add_silent_to_mentions.rb b/db/migrate/20181010141500_add_silent_to_mentions.rb new file mode 100644 index 000000000..dbb4fba26 --- /dev/null +++ b/db/migrate/20181010141500_add_silent_to_mentions.rb @@ -0,0 +1,23 @@ +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddSilentToMentions < ActiveRecord::Migration[5.2] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + def up + safety_assured do + add_column_with_default( + :mentions, + :silent, + :boolean, + allow_null: false, + default: false + ) + end + end + + def down + remove_column :mentions, :silent + end +end diff --git a/db/schema.rb b/db/schema.rb index bf6ab4e68..f79f26f16 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_10_07_025445) do +ActiveRecord::Schema.define(version: 2018_10_10_141500) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -301,6 +301,7 @@ ActiveRecord::Schema.define(version: 2018_10_07_025445) do t.datetime "created_at", null: false t.datetime "updated_at", null: false t.bigint "account_id" + t.boolean "silent", default: false, null: false t.index ["account_id", "status_id"], name: "index_mentions_on_account_id_and_status_id", unique: true t.index ["status_id"], name: "index_mentions_on_status_id" end diff --git a/spec/lib/activitypub/activity/create_spec.rb b/spec/lib/activitypub/activity/create_spec.rb index 62b9db8c2..cd20b7c7c 100644 --- a/spec/lib/activitypub/activity/create_spec.rb +++ b/spec/lib/activitypub/activity/create_spec.rb @@ -105,6 +105,31 @@ RSpec.describe ActivityPub::Activity::Create do end end + context 'limited' do + let(:recipient) { Fabricate(:account) } + + let(:object_json) do + { + id: [ActivityPub::TagManager.instance.uri_for(sender), '#bar'].join, + type: 'Note', + content: 'Lorem ipsum', + to: ActivityPub::TagManager.instance.uri_for(recipient), + } + end + + it 'creates status' do + status = sender.statuses.first + + expect(status).to_not be_nil + expect(status.visibility).to eq 'limited' + end + + it 'creates silent mention' do + status = sender.statuses.first + expect(status.mentions.first).to be_silent + end + end + context 'direct' do let(:recipient) { Fabricate(:account) } @@ -114,6 +139,10 @@ RSpec.describe ActivityPub::Activity::Create do type: 'Note', content: 'Lorem ipsum', to: ActivityPub::TagManager.instance.uri_for(recipient), + tag: { + type: 'Mention', + href: ActivityPub::TagManager.instance.uri_for(recipient), + }, } end -- cgit From a38a452481d0f5207bb27ba7a2707c0028d2ac18 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 19 Oct 2018 01:47:29 +0200 Subject: Add unread indicator to conversations (#9009) --- app/controllers/api/v1/conversations_controller.rb | 20 +++++++++++++++++-- app/controllers/api/v1/reports_controller.rb | 1 - app/javascript/mastodon/actions/conversations.js | 11 +++++++++++ .../direct_timeline/components/conversation.js | 14 ++++++++++--- .../containers/conversation_container.js | 8 +++++++- app/javascript/mastodon/reducers/conversations.js | 10 ++++++++++ app/javascript/styles/mastodon/components.scss | 5 +++++ app/models/account_conversation.rb | 2 ++ app/serializers/rest/conversation_serializer.rb | 3 ++- config/initializers/doorkeeper.rb | 2 +- config/routes.rb | 7 ++++++- ...18205649_add_unread_to_account_conversations.rb | 23 ++++++++++++++++++++++ db/schema.rb | 3 ++- 13 files changed, 98 insertions(+), 11 deletions(-) create mode 100644 db/migrate/20181018205649_add_unread_to_account_conversations.rb (limited to 'app/serializers/rest') diff --git a/app/controllers/api/v1/conversations_controller.rb b/app/controllers/api/v1/conversations_controller.rb index 736cb21ca..b19f27ebf 100644 --- a/app/controllers/api/v1/conversations_controller.rb +++ b/app/controllers/api/v1/conversations_controller.rb @@ -3,9 +3,11 @@ class Api::V1::ConversationsController < Api::BaseController LIMIT = 20 - before_action -> { doorkeeper_authorize! :read, :'read:statuses' } + before_action -> { doorkeeper_authorize! :read, :'read:statuses' }, only: :index + before_action -> { doorkeeper_authorize! :write, :'write:conversations' }, except: :index before_action :require_user! - after_action :insert_pagination_headers + before_action :set_conversation, except: :index + after_action :insert_pagination_headers, only: :index respond_to :json @@ -14,8 +16,22 @@ class Api::V1::ConversationsController < Api::BaseController render json: @conversations, each_serializer: REST::ConversationSerializer end + def read + @conversation.update!(unread: false) + render json: @conversation, serializer: REST::ConversationSerializer + end + + def destroy + @conversation.destroy! + render_empty + end + private + def set_conversation + @conversation = AccountConversation.where(account: current_account).find(params[:id]) + end + def paginated_conversations AccountConversation.where(account: current_account) .paginate_by_id(limit_param(LIMIT), params_slice(:max_id, :since_id, :min_id)) diff --git a/app/controllers/api/v1/reports_controller.rb b/app/controllers/api/v1/reports_controller.rb index 726817927..e182a9c6c 100644 --- a/app/controllers/api/v1/reports_controller.rb +++ b/app/controllers/api/v1/reports_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class Api::V1::ReportsController < Api::BaseController - before_action -> { doorkeeper_authorize! :read, :'read:reports' }, except: [:create] before_action -> { doorkeeper_authorize! :write, :'write:reports' }, only: [:create] before_action :require_user! diff --git a/app/javascript/mastodon/actions/conversations.js b/app/javascript/mastodon/actions/conversations.js index cab05c1ba..aefd2fef7 100644 --- a/app/javascript/mastodon/actions/conversations.js +++ b/app/javascript/mastodon/actions/conversations.js @@ -13,6 +13,8 @@ export const CONVERSATIONS_FETCH_SUCCESS = 'CONVERSATIONS_FETCH_SUCCESS'; export const CONVERSATIONS_FETCH_FAIL = 'CONVERSATIONS_FETCH_FAIL'; export const CONVERSATIONS_UPDATE = 'CONVERSATIONS_UPDATE'; +export const CONVERSATIONS_READ = 'CONVERSATIONS_READ'; + export const mountConversations = () => ({ type: CONVERSATIONS_MOUNT, }); @@ -21,6 +23,15 @@ export const unmountConversations = () => ({ type: CONVERSATIONS_UNMOUNT, }); +export const markConversationRead = conversationId => (dispatch, getState) => { + dispatch({ + type: CONVERSATIONS_READ, + id: conversationId, + }); + + api(getState).post(`/api/v1/conversations/${conversationId}/read`); +}; + export const expandConversations = ({ maxId } = {}) => (dispatch, getState) => { dispatch(expandConversationsRequest()); diff --git a/app/javascript/mastodon/features/direct_timeline/components/conversation.js b/app/javascript/mastodon/features/direct_timeline/components/conversation.js index f9a8d4f72..52e33c3c8 100644 --- a/app/javascript/mastodon/features/direct_timeline/components/conversation.js +++ b/app/javascript/mastodon/features/direct_timeline/components/conversation.js @@ -8,6 +8,7 @@ import DisplayName from '../../../components/display_name'; import Avatar from '../../../components/avatar'; import AttachmentList from '../../../components/attachment_list'; import { HotKeys } from 'react-hotkeys'; +import classNames from 'classnames'; export default class Conversation extends ImmutablePureComponent { @@ -19,8 +20,10 @@ export default class Conversation extends ImmutablePureComponent { conversationId: PropTypes.string.isRequired, accounts: ImmutablePropTypes.list.isRequired, lastStatus: ImmutablePropTypes.map.isRequired, + unread:PropTypes.bool.isRequired, onMoveUp: PropTypes.func, onMoveDown: PropTypes.func, + markRead: PropTypes.func.isRequired, }; handleClick = () => { @@ -28,7 +31,12 @@ export default class Conversation extends ImmutablePureComponent { return; } - const { lastStatus } = this.props; + const { lastStatus, unread, markRead } = this.props; + + if (unread) { + markRead(); + } + this.context.router.history.push(`/statuses/${lastStatus.get('id')}`); } @@ -41,7 +49,7 @@ export default class Conversation extends ImmutablePureComponent { } render () { - const { accounts, lastStatus, lastAccount } = this.props; + const { accounts, lastStatus, lastAccount, unread } = this.props; if (lastStatus === null) { return null; @@ -61,7 +69,7 @@ export default class Conversation extends ImmutablePureComponent { return ( -
+
{accounts.map(account => )}
diff --git a/app/javascript/mastodon/features/direct_timeline/containers/conversation_container.js b/app/javascript/mastodon/features/direct_timeline/containers/conversation_container.js index 4166ee2ac..e2e2e3afb 100644 --- a/app/javascript/mastodon/features/direct_timeline/containers/conversation_container.js +++ b/app/javascript/mastodon/features/direct_timeline/containers/conversation_container.js @@ -1,5 +1,6 @@ import { connect } from 'react-redux'; import Conversation from '../components/conversation'; +import { markConversationRead } from '../../../actions/conversations'; const mapStateToProps = (state, { conversationId }) => { const conversation = state.getIn(['conversations', 'items']).find(x => x.get('id') === conversationId); @@ -7,9 +8,14 @@ const mapStateToProps = (state, { conversationId }) => { return { accounts: conversation.get('accounts').map(accountId => state.getIn(['accounts', accountId], null)), + unread: conversation.get('unread'), lastStatus, lastAccount: lastStatus === null ? null : state.getIn(['accounts', lastStatus.get('account')], null), }; }; -export default connect(mapStateToProps)(Conversation); +const mapDispatchToProps = (dispatch, { conversationId }) => ({ + markRead: () => dispatch(markConversationRead(conversationId)), +}); + +export default connect(mapStateToProps, mapDispatchToProps)(Conversation); diff --git a/app/javascript/mastodon/reducers/conversations.js b/app/javascript/mastodon/reducers/conversations.js index 6b3f22d25..ea39fccee 100644 --- a/app/javascript/mastodon/reducers/conversations.js +++ b/app/javascript/mastodon/reducers/conversations.js @@ -6,6 +6,7 @@ import { CONVERSATIONS_FETCH_SUCCESS, CONVERSATIONS_FETCH_FAIL, CONVERSATIONS_UPDATE, + CONVERSATIONS_READ, } from '../actions/conversations'; import compareId from '../compare_id'; @@ -18,6 +19,7 @@ const initialState = ImmutableMap({ const conversationToMap = item => ImmutableMap({ id: item.id, + unread: item.unread, accounts: ImmutableList(item.accounts.map(a => a.id)), last_status: item.last_status.id, }); @@ -80,6 +82,14 @@ export default function conversations(state = initialState, action) { return state.update('mounted', count => count + 1); case CONVERSATIONS_UNMOUNT: return state.update('mounted', count => count - 1); + case CONVERSATIONS_READ: + return state.update('items', list => list.map(item => { + if (item.get('id') === action.id) { + return item.set('unread', false); + } + + return item; + })); default: return state; } diff --git a/app/javascript/styles/mastodon/components.scss b/app/javascript/styles/mastodon/components.scss index 129bde856..24b614a37 100644 --- a/app/javascript/styles/mastodon/components.scss +++ b/app/javascript/styles/mastodon/components.scss @@ -5503,6 +5503,11 @@ noscript { border-bottom: 1px solid lighten($ui-base-color, 8%); cursor: pointer; + &--unread { + background: lighten($ui-base-color, 8%); + border-bottom-color: lighten($ui-base-color, 12%); + } + &__header { display: flex; margin-bottom: 15px; diff --git a/app/models/account_conversation.rb b/app/models/account_conversation.rb index c12c8d233..b7447d805 100644 --- a/app/models/account_conversation.rb +++ b/app/models/account_conversation.rb @@ -10,6 +10,7 @@ # status_ids :bigint(8) default([]), not null, is an Array # last_status_id :bigint(8) # lock_version :integer default(0), not null +# unread :boolean default(FALSE), not null # class AccountConversation < ApplicationRecord @@ -58,6 +59,7 @@ class AccountConversation < ApplicationRecord def add_status(recipient, status) conversation = find_or_initialize_by(account: recipient, conversation_id: status.conversation_id, participant_account_ids: participants_from_status(recipient, status)) conversation.status_ids << status.id + conversation.unread = status.account_id != recipient.id conversation.save conversation rescue ActiveRecord::StaleObjectError diff --git a/app/serializers/rest/conversation_serializer.rb b/app/serializers/rest/conversation_serializer.rb index 884253f89..b09ca6341 100644 --- a/app/serializers/rest/conversation_serializer.rb +++ b/app/serializers/rest/conversation_serializer.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class REST::ConversationSerializer < ActiveModel::Serializer - attribute :id + attributes :id, :unread + has_many :participant_accounts, key: :accounts, serializer: REST::AccountSerializer has_one :last_status, serializer: REST::StatusSerializer diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index fe2490b32..367eead6a 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -58,6 +58,7 @@ Doorkeeper.configure do optional_scopes :write, :'write:accounts', :'write:blocks', + :'write:conversations', :'write:favourites', :'write:filters', :'write:follows', @@ -76,7 +77,6 @@ Doorkeeper.configure do :'read:lists', :'read:mutes', :'read:notifications', - :'read:reports', :'read:search', :'read:statuses', :follow, diff --git a/config/routes.rb b/config/routes.rb index a2468c9bd..b203e1329 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -261,7 +261,12 @@ Rails.application.routes.draw do resources :streaming, only: [:index] resources :custom_emojis, only: [:index] resources :suggestions, only: [:index, :destroy] - resources :conversations, only: [:index] + + resources :conversations, only: [:index, :destroy] do + member do + post :read + end + end get '/search', to: 'search#index', as: :search diff --git a/db/migrate/20181018205649_add_unread_to_account_conversations.rb b/db/migrate/20181018205649_add_unread_to_account_conversations.rb new file mode 100644 index 000000000..3c28b9a64 --- /dev/null +++ b/db/migrate/20181018205649_add_unread_to_account_conversations.rb @@ -0,0 +1,23 @@ +require Rails.root.join('lib', 'mastodon', 'migration_helpers') + +class AddUnreadToAccountConversations < ActiveRecord::Migration[5.2] + include Mastodon::MigrationHelpers + + disable_ddl_transaction! + + def up + safety_assured do + add_column_with_default( + :account_conversations, + :unread, + :boolean, + allow_null: false, + default: false + ) + end + end + + def down + remove_column :account_conversations, :unread, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index f79f26f16..046975ac9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2018_10_10_141500) do +ActiveRecord::Schema.define(version: 2018_10_18_205649) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -22,6 +22,7 @@ ActiveRecord::Schema.define(version: 2018_10_10_141500) do t.bigint "status_ids", default: [], null: false, array: true t.bigint "last_status_id" t.integer "lock_version", default: 0, null: false + t.boolean "unread", default: false, null: false t.index ["account_id", "conversation_id", "participant_account_ids"], name: "index_unique_conversations", unique: true t.index ["account_id"], name: "index_account_conversations_on_account_id" t.index ["conversation_id"], name: "index_account_conversations_on_conversation_id" -- cgit