diff options
| -rw-r--r-- | app/controllers/api/v1/statuses_controller.rb | 2 | ||||
| -rw-r--r-- | app/controllers/statuses_controller.rb | 7 | ||||
| -rw-r--r-- | app/javascript/styles/mastodon/stream_entries.scss | 14 | ||||
| -rw-r--r-- | app/models/concerns/status_threading_concern.rb | 24 | ||||
| -rw-r--r-- | app/views/stream_entries/_status.html.haml | 4 | ||||
| -rw-r--r-- | spec/models/concerns/status_threading_concern_spec.rb | 38 | ||||
| -rw-r--r-- | spec/views/stream_entries/show.html.haml_spec.rb | 2 |
7 files changed, 70 insertions, 21 deletions
diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 28c28592a..e98241323 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -17,7 +17,7 @@ class Api::V1::StatusesController < Api::BaseController end def context - ancestors_results = @status.in_reply_to_id.nil? ? [] : @status.ancestors(current_account) + ancestors_results = @status.in_reply_to_id.nil? ? [] : @status.ancestors(DEFAULT_STATUSES_LIMIT, current_account) descendants_results = @status.descendants(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 45226c8d2..41f098a43 100644 --- a/app/controllers/statuses_controller.rb +++ b/app/controllers/statuses_controller.rb @@ -4,6 +4,8 @@ class StatusesController < ApplicationController include SignatureAuthentication include Authorization + ANCESTORS_LIMIT = 20 + layout 'public' before_action :set_account @@ -16,8 +18,9 @@ class StatusesController < ApplicationController def show respond_to do |format| format.html do - @ancestors = @status.reply? ? cache_collection(@status.ancestors(current_account), Status) : [] - @descendants = cache_collection(@status.descendants(current_account), Status) + @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) render 'stream_entries/show' end diff --git a/app/javascript/styles/mastodon/stream_entries.scss b/app/javascript/styles/mastodon/stream_entries.scss index 442b143a0..dfdc48d06 100644 --- a/app/javascript/styles/mastodon/stream_entries.scss +++ b/app/javascript/styles/mastodon/stream_entries.scss @@ -6,7 +6,8 @@ background: $simple-background-color; .detailed-status.light, - .status.light { + .status.light, + .more.light { border-bottom: 1px solid $ui-secondary-color; animation: none; } @@ -290,6 +291,17 @@ text-decoration: underline; } } + + .more { + color: $classic-primary-color; + display: block; + padding: 14px; + text-align: center; + + &:not(:hover) { + text-decoration: none; + } + } } .embed { diff --git a/app/models/concerns/status_threading_concern.rb b/app/models/concerns/status_threading_concern.rb index b539ba10e..fffc095ee 100644 --- a/app/models/concerns/status_threading_concern.rb +++ b/app/models/concerns/status_threading_concern.rb @@ -3,8 +3,8 @@ module StatusThreadingConcern extend ActiveSupport::Concern - def ancestors(account = nil) - find_statuses_from_tree_path(ancestor_ids, account) + def ancestors(limit, account = nil) + find_statuses_from_tree_path(ancestor_ids(limit), account) end def descendants(account = nil) @@ -13,14 +13,21 @@ module StatusThreadingConcern private - def ancestor_ids - Rails.cache.fetch("ancestors:#{id}") do - ancestor_statuses.pluck(:id) + def ancestor_ids(limit) + key = "ancestors:#{id}" + ancestors = Rails.cache.fetch(key) + + if ancestors.nil? || ancestors[:limit] < limit + ids = ancestor_statuses(limit).pluck(:id).reverse! + Rails.cache.write key, limit: limit, ids: ids + ids + else + ancestors[:ids].last(limit) end end - def ancestor_statuses - Status.find_by_sql([<<-SQL.squish, id: in_reply_to_id]) + def ancestor_statuses(limit) + Status.find_by_sql([<<-SQL.squish, id: in_reply_to_id, limit: limit]) WITH RECURSIVE search_tree(id, in_reply_to_id, path) AS ( SELECT id, in_reply_to_id, ARRAY[id] @@ -34,7 +41,8 @@ module StatusThreadingConcern ) SELECT id FROM search_tree - ORDER BY path DESC + ORDER BY path + LIMIT :limit SQL end diff --git a/app/views/stream_entries/_status.html.haml b/app/views/stream_entries/_status.html.haml index e2e1fdd12..2d0dafcb7 100644 --- a/app/views/stream_entries/_status.html.haml +++ b/app/views/stream_entries/_status.html.haml @@ -14,6 +14,10 @@ entry_classes = h_class + ' ' + mf_classes + ' ' + style_classes - 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 partial: 'stream_entries/status', collection: @ancestors, as: :status, locals: { is_predecessor: true, direct_reply_id: status.in_reply_to_id } .entry{ class: entry_classes } diff --git a/spec/models/concerns/status_threading_concern_spec.rb b/spec/models/concerns/status_threading_concern_spec.rb index 62f5f6e31..b8ebdd58c 100644 --- a/spec/models/concerns/status_threading_concern_spec.rb +++ b/spec/models/concerns/status_threading_concern_spec.rb @@ -14,34 +14,34 @@ describe StatusThreadingConcern do let!(:viewer) { Fabricate(:account, username: 'viewer') } it 'returns conversation history' do - expect(reply3.ancestors).to include(status, reply1, reply2) + expect(reply3.ancestors(4)).to include(status, reply1, reply2) end it 'does not return conversation history user is not allowed to see' do reply1.update(visibility: :private) status.update(visibility: :direct) - expect(reply3.ancestors(viewer)).to_not include(reply1, status) + expect(reply3.ancestors(4, viewer)).to_not include(reply1, status) end it 'does not return conversation history from blocked users' do viewer.block!(jeff) - expect(reply3.ancestors(viewer)).to_not include(reply1) + expect(reply3.ancestors(4, viewer)).to_not include(reply1) end it 'does not return conversation history from muted users' do viewer.mute!(jeff) - expect(reply3.ancestors(viewer)).to_not include(reply1) + expect(reply3.ancestors(4, viewer)).to_not include(reply1) end it 'does not return conversation history from silenced and not followed users' do jeff.update(silenced: true) - expect(reply3.ancestors(viewer)).to_not include(reply1) + expect(reply3.ancestors(4, viewer)).to_not include(reply1) end it 'does not return conversation history from blocked domains' do viewer.block_domain!('example.com') - expect(reply3.ancestors(viewer)).to_not include(reply2) + expect(reply3.ancestors(4, viewer)).to_not include(reply2) end it 'ignores deleted records' do @@ -49,10 +49,32 @@ describe StatusThreadingConcern do second_status = Fabricate(:status, thread: first_status, account: alice) # Create cache and delete cached record - second_status.ancestors + second_status.ancestors(4) first_status.destroy - expect(second_status.ancestors).to eq([]) + expect(second_status.ancestors(4)).to eq([]) + end + + it 'can return more records than previously requested' do + first_status = Fabricate(:status, account: bob) + second_status = Fabricate(:status, thread: first_status, account: alice) + third_status = Fabricate(:status, thread: second_status, account: alice) + + # Create cache + second_status.ancestors(1) + + expect(third_status.ancestors(2)).to eq([first_status, second_status]) + end + + it 'can return fewer records than previously requested' do + first_status = Fabricate(:status, account: bob) + second_status = Fabricate(:status, thread: first_status, account: alice) + third_status = Fabricate(:status, thread: second_status, account: alice) + + # Create cache + second_status.ancestors(2) + + expect(third_status.ancestors(1)).to eq([second_status]) 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 59ea40990..6074bbc2e 100644 --- a/spec/views/stream_entries/show.html.haml_spec.rb +++ b/spec/views/stream_entries/show.html.haml_spec.rb @@ -48,7 +48,7 @@ describe 'stream_entries/show.html.haml', without_verify_partial_doubles: true d assign(:stream_entry, reply.stream_entry) assign(:account, alice) assign(:type, reply.stream_entry.activity_type.downcase) - assign(:ancestors, reply.stream_entry.activity.ancestors(bob) ) + assign(:ancestors, reply.stream_entry.activity.ancestors(1, bob) ) assign(:descendants, reply.stream_entry.activity.descendants(bob)) render |