Skip to content

Commit 9812211

Browse files
Merge pull request #2971 from basecamp/entropy-sweep-materialization
Speed up the card list for large accounts
2 parents 4650909 + 33d98a2 commit 9812211

2 files changed

Lines changed: 27 additions & 1 deletion

File tree

app/models/filter.rb

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ def cards
2727
result = result.unassigned if assignment_status.unassigned?
2828
result = result.assigned_to(assignees.ids) if assignees.present?
2929
result = result.where(creator_id: creators.ids) if creators.present?
30-
result = result.where(board: boards.ids) if boards.present?
30+
result = filter_boards(result) if boards.present?
3131
result = result.tagged_with(tags.ids) if tags.present?
3232
result = result.where(cards: { created_at: creation_window }) if creation_window
3333
result = result.closed_at_window(closure_window) if closure_window
@@ -66,6 +66,23 @@ def only_closed?
6666
end
6767

6868
private
69+
def filter_boards(relation)
70+
relation = relation.where(cards: { account_id: creator.account_id }).where(board: boards.ids)
71+
if joins_has_many?
72+
relation
73+
else
74+
# Pin the (account_id, last_active_at, status) index so the ordered page is served by a reverse scan, not a filesort.
75+
relation.use_index(:index_cards_on_account_id_and_last_active_at_and_status)
76+
end
77+
end
78+
79+
# Assignee, tag, and term filters add a has-many join that fans a card into
80+
# several rows; the ordered reverse scan loses to a different plan then, so
81+
# the pin only applies without them.
82+
def joins_has_many?
83+
assignees.present? || tags.present? || terms.present?
84+
end
85+
6986
def include_closed_cards?
7087
only_closed? || card_ids.present?
7188
end

test/models/filter_test.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ class FilterTest < ActiveSupport::TestCase
4848
assert_empty users(:david).filters.new(board_ids: [ boards(:writebook).id ]).boards
4949
end
5050

51+
test "board-scoped cards never leak cards from an inaccessible board in the same account" do
52+
inaccessible_card = boards(:private).cards.create!(status: "published", creator: users(:kevin))
53+
54+
filter = users(:david).filters.new(board_ids: [ boards(:private).id, boards(:writebook).id ])
55+
56+
assert_not_includes filter.cards, inaccessible_card
57+
filter.cards.each { |card| assert_includes users(:david).boards, card.board }
58+
end
59+
5160
test "remembering equivalent filters" do
5261
assert_difference "Filter.count", +1 do
5362
filter = users(:david).filters.remember(sorted_by: "latest", assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ])

0 commit comments

Comments
 (0)