From 71dc1afd6ebfc16f26dae06d3cb629187b32b16e Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 12 Jan 2025 21:52:19 +0300 Subject: [PATCH 01/13] renamed check_posts_permissions to get_accessible_posts to match its function & added unit test for it --- app/helpers/search_helper.rb | 8 ++++---- test/helpers/search_helper_test.rb | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index fe21b7d35..d81c25311 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -1,12 +1,12 @@ module SearchHelper - def check_posts_permissions - (current_user&.is_moderator || current_user&.is_admin ? Post : Post.undeleted) + # @param user [User] user to check + def get_accessible_posts(user) + (user&.is_moderator || user&.is_admin ? Post : Post.undeleted) .qa_only.list_includes end def search_posts - posts = check_posts_permissions - + posts = get_accessible_posts(current_user) qualifiers = params_to_qualifiers search_string = params[:search] diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index a82e92f6f..f09395e48 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -35,4 +35,22 @@ class SearchHelperTest < ActionView::TestCase assert_equal expect, date_value_sql(input) end end + + test 'get_accessible_posts should correctly check access' do + admin_user = users(:admin) + mod_user = users(:moderator) + standard_user = users(:standard_user) + + admin_posts = get_accessible_posts(admin_user) + mod_posts = get_accessible_posts(mod_user) + user_posts = get_accessible_posts(standard_user) + + can_admin_get_deleted_posts = admin_posts.any?(&:deleted) + can_mod_get_deleted_posts = mod_posts.any?(&:deleted) + can_user_get_deleted_posts = user_posts.any?(&:deleted) + + assert_equal can_admin_get_deleted_posts, true + assert_equal can_mod_get_deleted_posts, true + assert_equal can_user_get_deleted_posts, false + end end From 6e75e6359f9cd5a1df1c464ef9706ed82ef3730a Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 12 Jan 2025 23:44:54 +0300 Subject: [PATCH 02/13] renamed get_accessible_posts to accessible_posts_for (per @ArtOfCode-'s note) as 'get' prefix is not idiomatic --- app/helpers/search_helper.rb | 4 ++-- test/helpers/search_helper_test.rb | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index d81c25311..a5a062482 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -1,12 +1,12 @@ module SearchHelper # @param user [User] user to check - def get_accessible_posts(user) + def accessible_posts_for(user) (user&.is_moderator || user&.is_admin ? Post : Post.undeleted) .qa_only.list_includes end def search_posts - posts = get_accessible_posts(current_user) + posts = accessible_posts_for(current_user) qualifiers = params_to_qualifiers search_string = params[:search] diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index f09395e48..37cd5f4e7 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -36,14 +36,14 @@ class SearchHelperTest < ActionView::TestCase end end - test 'get_accessible_posts should correctly check access' do + test 'accessible_posts_for should correctly check access' do admin_user = users(:admin) mod_user = users(:moderator) standard_user = users(:standard_user) - admin_posts = get_accessible_posts(admin_user) - mod_posts = get_accessible_posts(mod_user) - user_posts = get_accessible_posts(standard_user) + admin_posts = accessible_posts_for(admin_user) + mod_posts = accessible_posts_for(mod_user) + user_posts = accessible_posts_for(standard_user) can_admin_get_deleted_posts = admin_posts.any?(&:deleted) can_mod_get_deleted_posts = mod_posts.any?(&:deleted) From 914375d2fae54ba58b7bdfe8694ef8f684fcd60d Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Sun, 12 Jan 2025 23:59:06 +0300 Subject: [PATCH 03/13] parametrized user for search_posts & qualifiers_to_sql methods of SearchHelper --- app/controllers/categories_controller.rb | 2 +- app/controllers/search_controller.rb | 2 +- app/helpers/search_helper.rb | 10 +++++----- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/app/controllers/categories_controller.rb b/app/controllers/categories_controller.rb index c319160f3..f32c76386 100644 --- a/app/controllers/categories_controller.rb +++ b/app/controllers/categories_controller.rb @@ -194,7 +194,7 @@ def set_list_posts end end - @posts = helpers.qualifiers_to_sql(filter_qualifiers, @posts) + @posts = helpers.qualifiers_to_sql(filter_qualifiers, @posts, current_user) @filtered = filter_qualifiers.any? @posts = @posts.paginate(page: params[:page], per_page: 50).order(sort_param) end diff --git a/app/controllers/search_controller.rb b/app/controllers/search_controller.rb index 202a3636b..cdc3fb139 100644 --- a/app/controllers/search_controller.rb +++ b/app/controllers/search_controller.rb @@ -1,6 +1,6 @@ class SearchController < ApplicationController def search - @posts, @qualifiers = helpers.search_posts + @posts, @qualifiers = helpers.search_posts(current_user) @signed_out_me = @qualifiers.any? { |q| q[:param] == :user && q[:user_id].nil? } diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index a5a062482..546001c03 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -5,8 +5,8 @@ def accessible_posts_for(user) .qa_only.list_includes end - def search_posts - posts = accessible_posts_for(current_user) + def search_posts(user) + posts = accessible_posts_for(user) qualifiers = params_to_qualifiers search_string = params[:search] @@ -17,7 +17,7 @@ def search_posts search_string = search_data[:search] end - posts = qualifiers_to_sql(qualifiers, posts) + posts = qualifiers_to_sql(qualifiers, posts, user) posts = posts.paginate(page: params[:page], per_page: 25) posts = if search_string.present? @@ -187,8 +187,8 @@ def parse_qualifier_strings(qualifiers) # Consider partitioning and telling the user which filters were invalid end - def qualifiers_to_sql(qualifiers, query) - trust_level = current_user&.trust_level || 0 + def qualifiers_to_sql(qualifiers, query, user) + trust_level = user&.trust_level || 0 allowed_categories = Category.where('IFNULL(min_view_trust_level, -1) <= ?', trust_level) query = query.where(category_id: allowed_categories) From c06d4e9a50b88dcbda8aaeba63b33719edd28841 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 02:14:03 +0300 Subject: [PATCH 04/13] added unit test for :user search qualifier --- test/helpers/search_helper_test.rb | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index 37cd5f4e7..220dc04ad 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -49,8 +49,24 @@ class SearchHelperTest < ActionView::TestCase can_mod_get_deleted_posts = mod_posts.any?(&:deleted) can_user_get_deleted_posts = user_posts.any?(&:deleted) - assert_equal can_admin_get_deleted_posts, true - assert_equal can_mod_get_deleted_posts, true - assert_equal can_user_get_deleted_posts, false + assert can_admin_get_deleted_posts + assert can_mod_get_deleted_posts + assert_not can_user_get_deleted_posts + end + + test 'qualifiers_to_sql should correctly narrow by :user qualifier' do + standard_user = users(:standard_user) + editor_user = users(:editor) + + posts_query = accessible_posts_for(standard_user) + + eq_editor = [{ param: :user, operator: '=', user_id: editor_user.id }] + qualified_query = qualifiers_to_sql(eq_editor, posts_query, standard_user) + + is_owner_editor = qualified_query.to_a.all? { |p| p.user.id == editor_user.id } + + assert_not_equal posts_query.size, qualified_query.size + assert_not_equal qualified_query.size, 0 + assert is_owner_editor end end From f7a048e92b711b73f0009dab2578830caf55f303 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 02:29:51 +0300 Subject: [PATCH 05/13] added unit test for :status search qualifier --- test/helpers/search_helper_test.rb | 33 ++++++++++++++++++++++++------ 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index 220dc04ad..6ee591b56 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -59,14 +59,35 @@ class SearchHelperTest < ActionView::TestCase editor_user = users(:editor) posts_query = accessible_posts_for(standard_user) - eq_editor = [{ param: :user, operator: '=', user_id: editor_user.id }] - qualified_query = qualifiers_to_sql(eq_editor, posts_query, standard_user) + editor_query = qualifiers_to_sql(eq_editor, posts_query, standard_user) + + only_editor_posts = editor_query.to_a.all? { |p| p.user.id == editor_user.id } + + assert_not_equal posts_query.size, editor_query.size + assert_not_equal editor_query.size, 0 + assert only_editor_posts + end + + test 'qualifiers_to_sql should correctly narrow by :status qualifier' do + standard_user = users(:standard_user) + + posts_query = accessible_posts_for(standard_user) + eq_open = [{ param: :status, value: 'open' }] + eq_closed = [{ param: :status, value: 'closed' }] + + open_query = qualifiers_to_sql(eq_open, posts_query, standard_user) + closed_query = qualifiers_to_sql(eq_closed, posts_query, standard_user) + + only_open_posts = open_query.to_a.none?(&:closed) + only_closed_posts = closed_query.to_a.all?(&:closed) - is_owner_editor = qualified_query.to_a.all? { |p| p.user.id == editor_user.id } + assert_not_equal posts_query.size, open_query.size + assert_not_equal open_query.size, 0 + assert only_open_posts - assert_not_equal posts_query.size, qualified_query.size - assert_not_equal qualified_query.size, 0 - assert is_owner_editor + assert_not_equal posts_query.size, closed_query.size + assert_not_equal closed_query.size, 0 + assert only_closed_posts end end From 78a165f76507a763bbe604492ba3a844a6fcab0e Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 02:47:46 +0300 Subject: [PATCH 06/13] added unit test for :score search qualifier --- test/fixtures/posts.yml | 13 ++++++ test/helpers/search_helper_test.rb | 65 +++++++++++++++++++++--------- 2 files changed, 60 insertions(+), 18 deletions(-) diff --git a/test/fixtures/posts.yml b/test/fixtures/posts.yml index c3597fb96..a5f564c5e 100644 --- a/test/fixtures/posts.yml +++ b/test/fixtures/posts.yml @@ -306,6 +306,19 @@ deleted_answer: upvote_count: 0 downvote_count: 0 +good_answer: + post_type: answer + body: A3GA - This is the seventh answer to question number 1 (Q1). It has a good score. Posted by standard_user. + body_markdown: A7 - This is the seventh answer to question number 1 (Q1). It has a good score. Posted by standard_user. + score: 0.6 + parent: question_one + user: standard_user + community: sample + category: main + license: cc_by_sa + upvote_count: 1 + downvote_count: 0 + policy_doc: post_type: policy_doc body: PD - This is a policy document called "Terms of Service", or "tos" for short. diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index 6ee591b56..dc3de978d 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -37,17 +37,17 @@ class SearchHelperTest < ActionView::TestCase end test 'accessible_posts_for should correctly check access' do - admin_user = users(:admin) + adm_user = users(:admin) mod_user = users(:moderator) - standard_user = users(:standard_user) + std_user = users(:standard_user) - admin_posts = accessible_posts_for(admin_user) + adm_posts = accessible_posts_for(adm_user) mod_posts = accessible_posts_for(mod_user) - user_posts = accessible_posts_for(standard_user) + std_posts = accessible_posts_for(std_user) - can_admin_get_deleted_posts = admin_posts.any?(&:deleted) + can_admin_get_deleted_posts = adm_posts.any?(&:deleted) can_mod_get_deleted_posts = mod_posts.any?(&:deleted) - can_user_get_deleted_posts = user_posts.any?(&:deleted) + can_user_get_deleted_posts = std_posts.any?(&:deleted) assert can_admin_get_deleted_posts assert can_mod_get_deleted_posts @@ -55,29 +55,58 @@ class SearchHelperTest < ActionView::TestCase end test 'qualifiers_to_sql should correctly narrow by :user qualifier' do - standard_user = users(:standard_user) - editor_user = users(:editor) + std_user = users(:standard_user) + edt_user = users(:editor) - posts_query = accessible_posts_for(standard_user) - eq_editor = [{ param: :user, operator: '=', user_id: editor_user.id }] - editor_query = qualifiers_to_sql(eq_editor, posts_query, standard_user) + posts_query = accessible_posts_for(std_user) + eq_editor = [{ param: :user, operator: '=', user_id: edt_user.id }] + edt_query = qualifiers_to_sql(eq_editor, posts_query, std_user) - only_editor_posts = editor_query.to_a.all? { |p| p.user.id == editor_user.id } + only_editor_posts = edt_query.to_a.all? { |p| p.user.id == edt_user.id } - assert_not_equal posts_query.size, editor_query.size - assert_not_equal editor_query.size, 0 + assert_not_equal posts_query.size, edt_query.size + assert_not_equal edt_query.size, 0 assert only_editor_posts end + test 'qualifiers_to_sql should correctly narrow by :score qualifier' do + std_user = users(:standard_user) + + posts_query = accessible_posts_for(std_user) + bad_post = [{ param: :score, operator: '<', value: 0.5 }] + good_post = [{ param: :score, operator: '>', value: 0.5 }] + neut_post = [{ param: :score, operator: '=', value: 0.5 }] + + bad_posts_query = qualifiers_to_sql(bad_post, posts_query, std_user) + good_posts_query = qualifiers_to_sql(good_post, posts_query, std_user) + neut_posts_query = qualifiers_to_sql(neut_post, posts_query, std_user) + + only_bad_posts = bad_posts_query.to_a.all? { |p| p.score < 0.5 } + only_good_posts = good_posts_query.to_a.all? { |p| p.score > 0.5 } + only_neut_posts = neut_posts_query.to_a.all? { |p| p.score.to_d == 0.5.to_d } + + assert_not_equal posts_query.size, bad_posts_query.size + assert_not_equal bad_posts_query.size, 0 + assert only_bad_posts + + assert_not_equal posts_query.size, good_posts_query.size + assert_not_equal good_posts_query.size, 0 + assert only_good_posts + + assert_not_equal posts_query.size, neut_posts_query.size + assert_not_equal neut_posts_query.size, 0 + assert only_neut_posts + end + test 'qualifiers_to_sql should correctly narrow by :status qualifier' do - standard_user = users(:standard_user) + std_user = users(:standard_user) - posts_query = accessible_posts_for(standard_user) + posts_query = accessible_posts_for(std_user) eq_open = [{ param: :status, value: 'open' }] eq_closed = [{ param: :status, value: 'closed' }] - open_query = qualifiers_to_sql(eq_open, posts_query, standard_user) - closed_query = qualifiers_to_sql(eq_closed, posts_query, standard_user) + open_query = qualifiers_to_sql(eq_open, posts_query, std_user) + closed_query = qualifiers_to_sql(eq_closed, posts_query, std_user) only_open_posts = open_query.to_a.none?(&:closed) only_closed_posts = closed_query.to_a.all?(&:closed) From e4f7ffca536e7bb6b11eb44cf8fcd51181b7615c Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 03:01:59 +0300 Subject: [PATCH 07/13] added unit test for :upvotes qualifier --- test/helpers/search_helper_test.rb | 34 ++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index dc3de978d..4bfd66087 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -59,8 +59,8 @@ class SearchHelperTest < ActionView::TestCase edt_user = users(:editor) posts_query = accessible_posts_for(std_user) - eq_editor = [{ param: :user, operator: '=', user_id: edt_user.id }] - edt_query = qualifiers_to_sql(eq_editor, posts_query, std_user) + edt_post = [{ param: :user, operator: '=', user_id: edt_user.id }] + edt_query = qualifiers_to_sql(edt_post, posts_query, std_user) only_editor_posts = edt_query.to_a.all? { |p| p.user.id == edt_user.id } @@ -102,11 +102,11 @@ class SearchHelperTest < ActionView::TestCase std_user = users(:standard_user) posts_query = accessible_posts_for(std_user) - eq_open = [{ param: :status, value: 'open' }] - eq_closed = [{ param: :status, value: 'closed' }] + open_post = [{ param: :status, value: 'open' }] + closed_post = [{ param: :status, value: 'closed' }] - open_query = qualifiers_to_sql(eq_open, posts_query, std_user) - closed_query = qualifiers_to_sql(eq_closed, posts_query, std_user) + open_query = qualifiers_to_sql(open_post, posts_query, std_user) + closed_query = qualifiers_to_sql(closed_post, posts_query, std_user) only_open_posts = open_query.to_a.none?(&:closed) only_closed_posts = closed_query.to_a.all?(&:closed) @@ -119,4 +119,26 @@ class SearchHelperTest < ActionView::TestCase assert_not_equal closed_query.size, 0 assert only_closed_posts end + + test 'qualifiers_to_sql should correctly narrow by :upvotes qualifier' do + std_user = users(:standard_user) + + posts_query = accessible_posts_for(std_user) + upvoted_post = [{ param: :upvotes, operator: '>', value: 0 }] + neutral_post = [{ param: :upvotes, operator: '=', value: 0 }] + + upvoted_query = qualifiers_to_sql(upvoted_post, posts_query, std_user) + neutral_query = qualifiers_to_sql(neutral_post, posts_query, std_user) + + only_upvoted_posts = upvoted_query.to_a.all? { |p| p[:upvote_count].positive? } + only_neutral_posts = neutral_query.to_a.all? { |p| p[:upvote_count].zero? } + + assert_not_equal posts_query.size, upvoted_query.size + assert_not_equal upvoted_query.size, 0 + assert only_upvoted_posts + + assert_not_equal posts_query.size, neutral_query.size + assert_not_equal neutral_query.size, 0 + assert only_neutral_posts + end end From 46851fd5a434dce056fa10bf1130f06f4ef04810 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 03:04:43 +0300 Subject: [PATCH 08/13] added unit test for :downvotes qualifier --- test/helpers/search_helper_test.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index 4bfd66087..ce031bad8 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -141,4 +141,26 @@ class SearchHelperTest < ActionView::TestCase assert_not_equal neutral_query.size, 0 assert only_neutral_posts end + + test 'qualifiers_to_sql should correctly narrow by :downvotes qualifier' do + std_user = users(:standard_user) + + posts_query = accessible_posts_for(std_user) + downvoted_post = [{ param: :downvotes, operator: '>', value: 0 }] + neutral_post = [{ param: :downvotes, operator: '=', value: 0 }] + + downvoted_query = qualifiers_to_sql(downvoted_post, posts_query, std_user) + neutral_query = qualifiers_to_sql(neutral_post, posts_query, std_user) + + only_downvoted_posts = downvoted_query.to_a.all? { |p| p[:downvote_count].positive? } + only_neutral_posts = neutral_query.to_a.all? { |p| p[:downvote_count].zero? } + + assert_not_equal posts_query.size, downvoted_query.size + assert_not_equal downvoted_query.size, 0 + assert only_downvoted_posts + + assert_not_equal posts_query.size, neutral_query.size + assert_not_equal neutral_query.size, 0 + assert only_neutral_posts + end end From 78b965712490fcc52ba6eeb92b3e12d56064cc5e Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 03:20:32 +0300 Subject: [PATCH 09/13] added unit test for :net_votes qualifier --- test/fixtures/posts.yml | 13 +++++++++++++ test/helpers/search_helper_test.rb | 17 +++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/test/fixtures/posts.yml b/test/fixtures/posts.yml index a5f564c5e..ec8738541 100644 --- a/test/fixtures/posts.yml +++ b/test/fixtures/posts.yml @@ -319,6 +319,19 @@ good_answer: upvote_count: 1 downvote_count: 0 +divisive_answer: + post_type: answer + body: A3DA - This is the eighth answer to question number 1 (Q1). It has divisive votes. Posted by standard_user. + body_markdown: A8 - This is the eighth answer to question number 1 (Q1). It has divisive votes. Posted by standard_user. + score: 0.6 + parent: question_one + user: standard_user + community: sample + category: main + license: cc_by_sa + upvote_count: 4 + downvote_count: 2 + policy_doc: post_type: policy_doc body: PD - This is a policy document called "Terms of Service", or "tos" for short. diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index ce031bad8..b0b253589 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -163,4 +163,21 @@ class SearchHelperTest < ActionView::TestCase assert_not_equal neutral_query.size, 0 assert only_neutral_posts end + + test 'qualifiers_to_sql should correctly narrow by :net_votes qualifier' do + std_user = users(:standard_user) + + posts_query = accessible_posts_for(std_user) + divisive_post = [{ param: :net_votes, operator: '=', value: 2 }] + + divisive_query = qualifiers_to_sql(divisive_post, posts_query, std_user) + + only_divisive_posts = divisive_query.to_a.all? do |p| + (p[:upvote_count] - p[:downvote_count]) == 2 + end + + assert_not_equal posts_query.size, divisive_query.size + assert_not_equal divisive_query.size, 0 + assert only_divisive_posts + end end From 8f0eb44266d41cae466fb9cf23d20102124b6d58 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 11:32:09 +0300 Subject: [PATCH 10/13] moved category access check into SearchHelper#accessible_categories_for method --- app/helpers/search_helper.rb | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 546001c03..ab092fd74 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -1,4 +1,10 @@ module SearchHelper + # @param user [User] user to check + def accessible_categories_for(user) + trust_level = user&.trust_level || 0 + Category.where('IFNULL(min_view_trust_level, -1) <= ?', trust_level) + end + # @param user [User] user to check def accessible_posts_for(user) (user&.is_moderator || user&.is_admin ? Post : Post.undeleted) @@ -188,9 +194,8 @@ def parse_qualifier_strings(qualifiers) end def qualifiers_to_sql(qualifiers, query, user) - trust_level = user&.trust_level || 0 - allowed_categories = Category.where('IFNULL(min_view_trust_level, -1) <= ?', trust_level) - query = query.where(category_id: allowed_categories) + categories = accessible_categories_for(user) + query = query.where(category_id: categories) qualifiers.each do |qualifier| # rubocop:disable Metrics/BlockLength case qualifier[:param] From ae8e5f1b14064882cccb88cdfcc281fa0658657f Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 11:36:52 +0300 Subject: [PATCH 11/13] added User#mod_or_admin? QoL method for access checks --- app/helpers/search_helper.rb | 3 +-- app/models/user.rb | 4 ++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index ab092fd74..84ff0f471 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -7,8 +7,7 @@ def accessible_categories_for(user) # @param user [User] user to check def accessible_posts_for(user) - (user&.is_moderator || user&.is_admin ? Post : Post.undeleted) - .qa_only.list_includes + (user&.mod_or_admin? ? Post : Post.undeleted).qa_only.list_includes end def search_posts(user) diff --git a/app/models/user.rb b/app/models/user.rb index 46a27df1a..4a248beeb 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -138,6 +138,10 @@ def is_admin is_global_admin || community_user&.is_admin || false end + def mod_or_admin? + is_admin || is_moderator + end + # Used by network profile: does this user have a profile on that other comm? def has_profile_on(community_id) cu = community_users.where(community_id: community_id).first From 23f841d30ce8d1f581704398f1112867ff39210d Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 11:53:30 +0300 Subject: [PATCH 12/13] mods or admins should be able to access all categories when searching --- app/helpers/search_helper.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 84ff0f471..91b512cac 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -1,6 +1,10 @@ module SearchHelper # @param user [User] user to check def accessible_categories_for(user) + if user&.mod_or_admin? + return Category.all + end + trust_level = user&.trust_level || 0 Category.where('IFNULL(min_view_trust_level, -1) <= ?', trust_level) end From cdc5aea382484c48128824da2d6ef7f023b0d139 Mon Sep 17 00:00:00 2001 From: Oleg Valter Date: Mon, 13 Jan 2025 11:54:29 +0300 Subject: [PATCH 13/13] added unit test for :category qualifier --- test/helpers/search_helper_test.rb | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/test/helpers/search_helper_test.rb b/test/helpers/search_helper_test.rb index b0b253589..5dd8348f7 100644 --- a/test/helpers/search_helper_test.rb +++ b/test/helpers/search_helper_test.rb @@ -54,6 +54,32 @@ class SearchHelperTest < ActionView::TestCase assert_not can_user_get_deleted_posts end + test 'qualifiers_to_sql should correctly narrow by :category qualifier' do + main = categories(:main) + admin_only = categories(:admin_only) + + std_user = users(:standard_user) + adm_user = users(:admin) + + posts_query_std = accessible_posts_for(std_user) + posts_query_adm = accessible_posts_for(adm_user) + + std_post = [{ param: :category, operator: '=', category_id: main.id }] + adm_post = [{ param: :category, operator: '=', category_id: admin_only.id }] + + std_posts_query_standard = qualifiers_to_sql(std_post, posts_query_std, std_user) + adm_posts_query_standard = qualifiers_to_sql(adm_post, posts_query_std, std_user) + adm_posts_query_admin = qualifiers_to_sql(adm_post, posts_query_adm, adm_user) + + assert_not_equal posts_query_std.size, std_posts_query_standard.size + assert_not_equal std_posts_query_standard.size, 0 + + assert_not_equal posts_query_adm.size, adm_posts_query_admin.size + assert_not_equal adm_posts_query_admin.size, 0 + + assert_equal adm_posts_query_standard.size, 0 + end + test 'qualifiers_to_sql should correctly narrow by :user qualifier' do std_user = users(:standard_user) edt_user = users(:editor)