diff --git a/app/models/filter.rb b/app/models/filter.rb index ab302fc1de..63884ea022 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -27,7 +27,7 @@ def cards result = result.unassigned if assignment_status.unassigned? result = result.assigned_to(assignees.ids) if assignees.present? result = result.where(creator_id: creators.ids) if creators.present? - result = result.where(board: boards.ids) if boards.present? + result = filter_boards(result) if boards.present? result = result.tagged_with(tags.ids) if tags.present? result = result.where(cards: { created_at: creation_window }) if creation_window result = result.closed_at_window(closure_window) if closure_window @@ -66,6 +66,23 @@ def only_closed? end private + def filter_boards(relation) + relation = relation.where(cards: { account_id: creator.account_id }).where(board: boards.ids) + if joins_has_many? + relation + else + # Pin the (account_id, last_active_at, status) index so the ordered page is served by a reverse scan, not a filesort. + relation.use_index(:index_cards_on_account_id_and_last_active_at_and_status) + end + end + + # Assignee, tag, and term filters add a has-many join that fans a card into + # several rows; the ordered reverse scan loses to a different plan then, so + # the pin only applies without them. + def joins_has_many? + assignees.present? || tags.present? || terms.present? + end + def include_closed_cards? only_closed? || card_ids.present? end diff --git a/test/models/filter_test.rb b/test/models/filter_test.rb index de89161417..53d9f6a353 100644 --- a/test/models/filter_test.rb +++ b/test/models/filter_test.rb @@ -48,6 +48,15 @@ class FilterTest < ActiveSupport::TestCase assert_empty users(:david).filters.new(board_ids: [ boards(:writebook).id ]).boards end + test "board-scoped cards never leak cards from an inaccessible board in the same account" do + inaccessible_card = boards(:private).cards.create!(status: "published", creator: users(:kevin)) + + filter = users(:david).filters.new(board_ids: [ boards(:private).id, boards(:writebook).id ]) + + assert_not_includes filter.cards, inaccessible_card + filter.cards.each { |card| assert_includes users(:david).boards, card.board } + end + test "remembering equivalent filters" do assert_difference "Filter.count", +1 do filter = users(:david).filters.remember(sorted_by: "latest", assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ])