From a9c440637ca9f36bcf051094abe3bcba1da63166 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 20 Apr 2018 02:28:48 +0200 Subject: Improve report layout (#7188) * Use table for statuses in report * Display reported account and reporter in the same table * Split accounts and general report info into two tables again * Redesign report statuses table, notes, merge notes and action log * Remove unused translations * Fix code style issue * Fix code style issue * Fix code style issue --- app/views/stream_entries/_detailed_status.html.haml | 6 +++--- app/views/stream_entries/_simple_status.html.haml | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) (limited to 'app/views/stream_entries') diff --git a/app/views/stream_entries/_detailed_status.html.haml b/app/views/stream_entries/_detailed_status.html.haml index e1122d5a2..afc66d148 100644 --- a/app/views/stream_entries/_detailed_status.html.haml +++ b/app/views/stream_entries/_detailed_status.html.haml @@ -22,11 +22,11 @@ - if !status.media_attachments.empty? - if status.media_attachments.first.video? - video = status.media_attachments.first - %div{ data: { component: 'Video', props: Oj.dump(src: video.file.url(:original), preview: video.file.url(:small), sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, width: 670, height: 380, detailed: true, inline: true) }} + = react_component :video, src: video.file.url(:original), preview: video.file.url(:small), sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, width: 670, height: 380, detailed: true, inline: true - else - %div{ data: { component: 'MediaGallery', props: Oj.dump(height: 380, sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, standalone: true, 'autoPlayGif': current_account&.user&.setting_auto_play_gif, 'reduceMotion': current_account&.user&.setting_reduce_motion, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json }) }} + = react_component :media_gallery, height: 380, sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, standalone: true, 'autoPlayGif': current_account&.user&.setting_auto_play_gif, 'reduceMotion': current_account&.user&.setting_reduce_motion, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } - elsif status.preview_cards.first - %div{ data: { component: 'Card', props: Oj.dump('maxDescription': 160, card: ActiveModelSerializers::SerializableResource.new(status.preview_cards.first, serializer: REST::PreviewCardSerializer).as_json) }} + = react_component :card, 'maxDescription': 160, card: ActiveModelSerializers::SerializableResource.new(status.preview_cards.first, serializer: REST::PreviewCardSerializer).as_json .detailed-status__meta %data.dt-published{ value: status.created_at.to_time.iso8601 } diff --git a/app/views/stream_entries/_simple_status.html.haml b/app/views/stream_entries/_simple_status.html.haml index 984691097..a6f5120fb 100644 --- a/app/views/stream_entries/_simple_status.html.haml +++ b/app/views/stream_entries/_simple_status.html.haml @@ -23,6 +23,6 @@ - unless status.media_attachments.empty? - if status.media_attachments.first.video? - video = status.media_attachments.first - %div{ data: { component: 'Video', props: Oj.dump(src: video.file.url(:original), preview: video.file.url(:small), sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, width: 610, height: 343, inline: true) }} + = react_component :video, src: video.file.url(:original), preview: video.file.url(:small), sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, width: 610, height: 343, inline: true - else - %div{ data: { component: 'MediaGallery', props: Oj.dump(height: 343, sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, 'autoPlayGif': current_account&.user&.setting_auto_play_gif, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json }) }} + = react_component :media_gallery, height: 343, sensitive: status.sensitive? && !current_account&.user&.setting_display_sensitive_media, 'autoPlayGif': current_account&.user&.setting_auto_play_gif, media: status.media_attachments.map { |a| ActiveModelSerializers::SerializableResource.new(a, serializer: REST::MediaAttachmentSerializer).as_json } -- cgit From 1258efa882b7a0eedc868640eb8e5a9075445ca0 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Tue, 24 Apr 2018 02:27:35 +0900 Subject: Paginate descendant statuses in public page (#7148) --- app/controllers/api/v1/statuses_controller.rb | 2 +- app/controllers/statuses_controller.rb | 73 +++++++++++++++++++++- app/models/concerns/status_threading_concern.rb | 17 ++--- app/views/stream_entries/_more.html.haml | 2 + app/views/stream_entries/_status.html.haml | 15 ++++- spec/controllers/statuses_controller_spec.rb | 43 +++++++++++++ .../concerns/status_threading_concern_spec.rb | 12 ++-- spec/views/stream_entries/show.html.haml_spec.rb | 4 +- 8 files changed, 146 insertions(+), 22 deletions(-) create mode 100644 app/views/stream_entries/_more.html.haml (limited to 'app/views/stream_entries') diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index e98241323..01880565c 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -18,7 +18,7 @@ class Api::V1::StatusesController < Api::BaseController def context ancestors_results = @status.in_reply_to_id.nil? ? [] : @status.ancestors(DEFAULT_STATUSES_LIMIT, current_account) - descendants_results = @status.descendants(current_account) + descendants_results = @status.descendants(DEFAULT_STATUSES_LIMIT, current_account) loaded_ancestors = cache_collection(ancestors_results, Status) loaded_descendants = cache_collection(descendants_results, Status) diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index a2943982a..01dac35e4 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -5,6 +5,8 @@ class StatusesController < ApplicationController include Authorization ANCESTORS_LIMIT = 20 + DESCENDANTS_LIMIT = 20 + DESCENDANTS_DEPTH_LIMIT = 4 layout 'public' @@ -19,9 +21,8 @@ class StatusesController < ApplicationController def show respond_to do |format| format.html do - @ancestors = @status.reply? ? cache_collection(@status.ancestors(ANCESTORS_LIMIT, current_account), Status) : [] - @next_ancestor = @ancestors.size < ANCESTORS_LIMIT ? nil : @ancestors.shift - @descendants = cache_collection(@status.descendants(current_account), Status) + set_ancestors + set_descendants render 'stream_entries/show' end @@ -51,10 +52,76 @@ class StatusesController < ApplicationController private + def create_descendant_thread(depth, statuses) + if depth < DESCENDANTS_DEPTH_LIMIT + { statuses: statuses } + else + next_status = statuses.pop + { statuses: statuses, next_status: next_status } + end + end + def set_account @account = Account.find_local!(params[:account_username]) end + def set_ancestors + @ancestors = @status.reply? ? cache_collection(@status.ancestors(ANCESTORS_LIMIT, current_account), Status) : [] + @next_ancestor = @ancestors.size < ANCESTORS_LIMIT ? nil : @ancestors.shift + end + + def set_descendants + @max_descendant_thread_id = params[:max_descendant_thread_id]&.to_i + @since_descendant_thread_id = params[:since_descendant_thread_id]&.to_i + + descendants = cache_collection( + @status.descendants( + DESCENDANTS_LIMIT, + current_account, + @max_descendant_thread_id, + @since_descendant_thread_id, + DESCENDANTS_DEPTH_LIMIT + ), + Status + ) + @descendant_threads = [] + + if descendants.present? + statuses = [descendants.first] + depth = 1 + + descendants.drop(1).each_with_index do |descendant, index| + if descendants[index].id == descendant.in_reply_to_id + depth += 1 + statuses << descendant + else + @descendant_threads << create_descendant_thread(depth, statuses) + + @descendant_threads.reverse_each do |descendant_thread| + statuses = descendant_thread[:statuses] + + index = statuses.find_index do |thread_status| + thread_status.id == descendant.in_reply_to_id + end + + if index.present? + depth += index - statuses.size + break + end + + depth -= statuses.size + end + + statuses = [descendant] + end + end + + @descendant_threads << create_descendant_thread(depth, statuses) + end + + @max_descendant_thread_id = @descendant_threads.pop[:statuses].first.id if descendants.size >= DESCENDANTS_LIMIT + end + def set_link_headers response.headers['Link'] = LinkHeader.new( [ diff --git a/app/models/concerns/status_threading_concern.rb b/app/models/concerns/status_threading_concern.rb index fffc095ee..a3fd34e94 100644 --- a/app/models/concerns/status_threading_concern.rb +++ b/app/models/concerns/status_threading_concern.rb @@ -7,8 +7,8 @@ module StatusThreadingConcern find_statuses_from_tree_path(ancestor_ids(limit), account) end - def descendants(account = nil) - find_statuses_from_tree_path(descendant_ids, account) + def descendants(limit, account = nil, max_child_id = nil, since_child_id = nil, depth = nil) + find_statuses_from_tree_path(descendant_ids(limit, max_child_id, since_child_id, depth), account) end private @@ -46,26 +46,27 @@ module StatusThreadingConcern SQL end - def descendant_ids - descendant_statuses.pluck(:id) + def descendant_ids(limit, max_child_id, since_child_id, depth) + descendant_statuses(limit, max_child_id, since_child_id, depth).pluck(:id) end - def descendant_statuses - Status.find_by_sql([<<-SQL.squish, id: id]) + def descendant_statuses(limit, max_child_id, since_child_id, depth) + Status.find_by_sql([<<-SQL.squish, id: id, limit: limit, max_child_id: max_child_id, since_child_id: since_child_id, depth: depth]) WITH RECURSIVE search_tree(id, path) AS ( SELECT id, ARRAY[id] FROM statuses - WHERE in_reply_to_id = :id + WHERE in_reply_to_id = :id AND COALESCE(id < :max_child_id, TRUE) AND COALESCE(id > :since_child_id, TRUE) UNION ALL SELECT statuses.id, path || statuses.id FROM search_tree JOIN statuses ON statuses.in_reply_to_id = search_tree.id - WHERE NOT statuses.id = ANY(path) + WHERE COALESCE(array_length(path, 1) < :depth, TRUE) AND NOT statuses.id = ANY(path) ) SELECT id FROM search_tree ORDER BY path + LIMIT :limit SQL end diff --git a/app/views/stream_entries/_more.html.haml b/app/views/stream_entries/_more.html.haml new file mode 100644 index 000000000..9b1dfe4a7 --- /dev/null +++ b/app/views/stream_entries/_more.html.haml @@ -0,0 +1,2 @@ += link_to url, class: 'more light' do + = t('statuses.show_more') diff --git a/app/views/stream_entries/_status.html.haml b/app/views/stream_entries/_status.html.haml index 2d0dafcb7..8decdf6b5 100644 --- a/app/views/stream_entries/_status.html.haml +++ b/app/views/stream_entries/_status.html.haml @@ -16,8 +16,7 @@ - if status.reply? && include_threads - if @next_ancestor .entry{ class: entry_classes } - = link_to short_account_status_url(@next_ancestor.account.username, @next_ancestor), class: 'more light' do - = t('statuses.show_more') + = render 'stream_entries/more', url: short_account_status_url(@next_ancestor.account.username, @next_ancestor) = render partial: 'stream_entries/status', collection: @ancestors, as: :status, locals: { is_predecessor: true, direct_reply_id: status.in_reply_to_id } .entry{ class: entry_classes } @@ -40,4 +39,14 @@ = render (centered ? 'stream_entries/detailed_status' : 'stream_entries/simple_status'), status: status.proper - if include_threads - = render partial: 'stream_entries/status', collection: @descendants, as: :status, locals: { is_successor: true, parent_id: status.id } + - if @since_descendant_thread_id + .entry{ class: entry_classes } + = render 'stream_entries/more', url: short_account_status_url(status.account.username, status, max_descendant_thread_id: @since_descendant_thread_id + 1) + - @descendant_threads.each do |thread| + = render partial: 'stream_entries/status', collection: thread[:statuses], as: :status, locals: { is_successor: true, parent_id: status.id } + - if thread[:next_status] + .entry{ class: entry_classes } + = render 'stream_entries/more', url: short_account_status_url(thread[:next_status].account.username, thread[:next_status]) + - if @next_descendant_thread + .entry{ class: entry_classes } + = render 'stream_entries/more', url: short_account_status_url(status.account.username, status, since_descendant_thread_id: @max_descendant_thread_id - 1) diff --git a/spec/controllers/statuses_controller_spec.rb b/spec/controllers/statuses_controller_spec.rb index 89af55688..b4f3c5a08 100644 --- a/spec/controllers/statuses_controller_spec.rb +++ b/spec/controllers/statuses_controller_spec.rb @@ -82,6 +82,49 @@ describe StatusesController do expect(assigns(:ancestors)).to eq [] end + it 'assigns @descendant_threads for a thread with several statuses' do + status = Fabricate(:status) + child = Fabricate(:status, in_reply_to_id: status.id) + grandchild = Fabricate(:status, in_reply_to_id: child.id) + + get :show, params: { account_username: status.account.username, id: status.id } + + expect(assigns(:descendant_threads)[0][:statuses].pluck(:id)).to eq [child.id, grandchild.id] + end + + it 'assigns @descendant_threads for several threads sharing the same descendant' do + status = Fabricate(:status) + child = Fabricate(:status, in_reply_to_id: status.id) + grandchildren = 2.times.map { Fabricate(:status, in_reply_to_id: child.id) } + + get :show, params: { account_username: status.account.username, id: status.id } + + expect(assigns(:descendant_threads)[0][:statuses].pluck(:id)).to eq [child.id, grandchildren[0].id] + expect(assigns(:descendant_threads)[1][:statuses].pluck(:id)).to eq [grandchildren[1].id] + end + + it 'assigns @max_descendant_thread_id for the last thread if it is hitting the status limit' do + stub_const 'StatusesController::DESCENDANTS_LIMIT', 1 + status = Fabricate(:status) + child = Fabricate(:status, in_reply_to_id: status.id) + + get :show, params: { account_username: status.account.username, id: status.id } + + expect(assigns(:descendant_threads)).to eq [] + expect(assigns(:max_descendant_thread_id)).to eq child.id + end + + it 'assigns @descendant_threads for threads with :next_status key if they are hitting the depth limit' do + stub_const 'StatusesController::DESCENDANTS_DEPTH_LIMIT', 1 + status = Fabricate(:status) + child = Fabricate(:status, in_reply_to_id: status.id) + + get :show, params: { account_username: status.account.username, id: status.id } + + expect(assigns(:descendant_threads)[0][:statuses].pluck(:id)).not_to include child.id + expect(assigns(:descendant_threads)[0][:next_status].id).to eq child.id + end + it 'returns a success' do status = Fabricate(:status) get :show, params: { account_username: status.account.username, id: status.id } diff --git a/spec/models/concerns/status_threading_concern_spec.rb b/spec/models/concerns/status_threading_concern_spec.rb index b8ebdd58c..e5736a307 100644 --- a/spec/models/concerns/status_threading_concern_spec.rb +++ b/spec/models/concerns/status_threading_concern_spec.rb @@ -89,34 +89,34 @@ describe StatusThreadingConcern do let!(:viewer) { Fabricate(:account, username: 'viewer') } it 'returns replies' do - expect(status.descendants).to include(reply1, reply2, reply3) + expect(status.descendants(4)).to include(reply1, reply2, reply3) end it 'does not return replies user is not allowed to see' do reply1.update(visibility: :private) reply3.update(visibility: :direct) - expect(status.descendants(viewer)).to_not include(reply1, reply3) + expect(status.descendants(4, viewer)).to_not include(reply1, reply3) end it 'does not return replies from blocked users' do viewer.block!(jeff) - expect(status.descendants(viewer)).to_not include(reply3) + expect(status.descendants(4, viewer)).to_not include(reply3) end it 'does not return replies from muted users' do viewer.mute!(jeff) - expect(status.descendants(viewer)).to_not include(reply3) + expect(status.descendants(4, viewer)).to_not include(reply3) end it 'does not return replies from silenced and not followed users' do jeff.update(silenced: true) - expect(status.descendants(viewer)).to_not include(reply3) + expect(status.descendants(4, viewer)).to_not include(reply3) end it 'does not return replies from blocked domains' do viewer.block_domain!('example.com') - expect(status.descendants(viewer)).to_not include(reply2) + expect(status.descendants(4, viewer)).to_not include(reply2) end end end diff --git a/spec/views/stream_entries/show.html.haml_spec.rb b/spec/views/stream_entries/show.html.haml_spec.rb index 6074bbc2e..560039ffa 100644 --- a/spec/views/stream_entries/show.html.haml_spec.rb +++ b/spec/views/stream_entries/show.html.haml_spec.rb @@ -24,6 +24,7 @@ describe 'stream_entries/show.html.haml', without_verify_partial_doubles: true d assign(:stream_entry, status.stream_entry) assign(:account, alice) assign(:type, status.stream_entry.activity_type.downcase) + assign(:descendant_threads, []) render @@ -49,7 +50,7 @@ describe 'stream_entries/show.html.haml', without_verify_partial_doubles: true d assign(:account, alice) assign(:type, reply.stream_entry.activity_type.downcase) assign(:ancestors, reply.stream_entry.activity.ancestors(1, bob) ) - assign(:descendants, reply.stream_entry.activity.descendants(bob)) + assign(:descendant_threads, [{ statuses: reply.stream_entry.activity.descendants(1)}]) render @@ -75,6 +76,7 @@ describe 'stream_entries/show.html.haml', without_verify_partial_doubles: true d assign(:stream_entry, status.stream_entry) assign(:account, alice) assign(:type, status.stream_entry.activity_type.downcase) + assign(:descendant_threads, []) render -- cgit From da61352fab2e59ba42caf59d4e2e33d62eebe060 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 30 Apr 2018 01:59:42 +0200 Subject: Fix "Show more" URL on paginated threads for remote statuses (#7285) * Fix URL of "Show more" link in paginated threads (ancestors side) Increase item limits in threads Fix #7268 * Fix "Show more" link in paginated threads (descendants side) --- app/controllers/statuses_controller.rb | 11 ++++++----- app/controllers/stream_entries_controller.rb | 1 + app/views/stream_entries/_status.html.haml | 20 +++++++++++--------- 3 files changed, 18 insertions(+), 14 deletions(-) (limited to 'app/views/stream_entries') diff --git a/app/controllers/statuses_controller.rb b/app/controllers/statuses_controller.rb index 01dac35e4..645995c2a 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -4,9 +4,9 @@ class StatusesController < ApplicationController include SignatureAuthentication include Authorization - ANCESTORS_LIMIT = 20 - DESCENDANTS_LIMIT = 20 - DESCENDANTS_DEPTH_LIMIT = 4 + ANCESTORS_LIMIT = 40 + DESCENDANTS_LIMIT = 60 + DESCENDANTS_DEPTH_LIMIT = 20 layout 'public' @@ -71,7 +71,7 @@ class StatusesController < ApplicationController end def set_descendants - @max_descendant_thread_id = params[:max_descendant_thread_id]&.to_i + @max_descendant_thread_id = params[:max_descendant_thread_id]&.to_i @since_descendant_thread_id = params[:since_descendant_thread_id]&.to_i descendants = cache_collection( @@ -84,11 +84,12 @@ class StatusesController < ApplicationController ), Status ) + @descendant_threads = [] if descendants.present? statuses = [descendants.first] - depth = 1 + depth = 1 descendants.drop(1).each_with_index do |descendant, index| if descendants[index].id == descendant.in_reply_to_id diff --git a/app/controllers/stream_entries_controller.rb b/app/controllers/stream_entries_controller.rb index 97cf85079..8568b151c 100644 --- a/app/controllers/stream_entries_controller.rb +++ b/app/controllers/stream_entries_controller.rb @@ -23,6 +23,7 @@ class StreamEntriesController < ApplicationController skip_session! expires_in 3.minutes, public: true end + render xml: OStatus::AtomSerializer.render(OStatus::AtomSerializer.new.entry(@stream_entry, true)) end end diff --git a/app/views/stream_entries/_status.html.haml b/app/views/stream_entries/_status.html.haml index 8decdf6b5..9764bc74d 100644 --- a/app/views/stream_entries/_status.html.haml +++ b/app/views/stream_entries/_status.html.haml @@ -5,18 +5,19 @@ is_successor ||= false direct_reply_id ||= false parent_id ||= false - is_direct_parent = direct_reply_id == status.id - is_direct_child = parent_id == status.in_reply_to_id - centered ||= include_threads && !is_predecessor && !is_successor - h_class = microformats_h_class(status, is_predecessor, is_successor, include_threads) - style_classes = style_classes(status, is_predecessor, is_successor, include_threads) - mf_classes = microformats_classes(status, is_direct_parent, is_direct_child) - entry_classes = h_class + ' ' + mf_classes + ' ' + style_classes + is_direct_parent = direct_reply_id == status.id + is_direct_child = parent_id == status.in_reply_to_id + centered ||= include_threads && !is_predecessor && !is_successor + h_class = microformats_h_class(status, is_predecessor, is_successor, include_threads) + style_classes = style_classes(status, is_predecessor, is_successor, include_threads) + mf_classes = microformats_classes(status, is_direct_parent, is_direct_child) + entry_classes = h_class + ' ' + mf_classes + ' ' + style_classes - if status.reply? && include_threads - if @next_ancestor .entry{ class: entry_classes } - = render 'stream_entries/more', url: short_account_status_url(@next_ancestor.account.username, @next_ancestor) + = render 'stream_entries/more', url: TagManager.instance.url_for(@next_ancestor) + = render partial: 'stream_entries/status', collection: @ancestors, as: :status, locals: { is_predecessor: true, direct_reply_id: status.in_reply_to_id } .entry{ class: entry_classes } @@ -44,9 +45,10 @@ = render 'stream_entries/more', url: short_account_status_url(status.account.username, status, max_descendant_thread_id: @since_descendant_thread_id + 1) - @descendant_threads.each do |thread| = render partial: 'stream_entries/status', collection: thread[:statuses], as: :status, locals: { is_successor: true, parent_id: status.id } + - if thread[:next_status] .entry{ class: entry_classes } - = render 'stream_entries/more', url: short_account_status_url(thread[:next_status].account.username, thread[:next_status]) + = render 'stream_entries/more', url: TagManager.instance.url_for(thread[:next_status]) - if @next_descendant_thread .entry{ class: entry_classes } = render 'stream_entries/more', url: short_account_status_url(status.account.username, status, since_descendant_thread_id: @max_descendant_thread_id - 1) -- cgit