From ebc1763aa5c5224b42ef79680a5d1839ebbd5a85 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Mon, 13 May 2024 14:37:17 +0300 Subject: [PATCH] FIX: Change request method for categories/search (#26976) This commit changes request method for "categories/search" from GET to POST to make sure that long filters can be passed to the server. For example, category selectors with many categories are setting the full list of selected category IDs to ensure these are filtered out from the list of choices. This can result in a long URL that exceeds the maximum length. --- .../discourse/app/models/category.js | 2 +- .../sidebar-user-categories-section-test.js | 2 +- config/routes.rb | 2 +- lib/read_only_mixin.rb | 1 + spec/requests/categories_controller_spec.rb | 42 +++++++++---------- 5 files changed, 25 insertions(+), 24 deletions(-) diff --git a/app/assets/javascripts/discourse/app/models/category.js b/app/assets/javascripts/discourse/app/models/category.js index 666b08e0424..588243daef5 100644 --- a/app/assets/javascripts/discourse/app/models/category.js +++ b/app/assets/javascripts/discourse/app/models/category.js @@ -403,7 +403,7 @@ export default class Category extends RestModel { }; const result = (CATEGORY_ASYNC_SEARCH_CACHE[JSON.stringify(data)] ||= - await ajax("/categories/search", { data })); + await ajax("/categories/search", { method: "POST", data })); if (opts.includeAncestors) { return { diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js index 438fc1a4ad1..ef385a61b1d 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-user-categories-section-test.js @@ -91,7 +91,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) { return helper.response(cloneJSON(categoryFixture["/c/1/show.json"])); }); - server.get("/categories/search", () => { + server.post("/categories/search", () => { return helper.response({ categories: [], ancestors: [] }); }); }); diff --git a/config/routes.rb b/config/routes.rb index 006b1dfb377..fb45a69cc58 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1169,7 +1169,7 @@ Discourse::Application.routes.draw do resources :categories, except: %i[show new edit] post "categories/reorder" => "categories#reorder" get "categories/find" => "categories#find" - get "categories/search" => "categories#search" + post "categories/search" => "categories#search" scope path: "category/:category_id" do post "/move" => "categories#move" diff --git a/lib/read_only_mixin.rb b/lib/read_only_mixin.rb index 74b88111ec3..5e2d38ee03d 100644 --- a/lib/read_only_mixin.rb +++ b/lib/read_only_mixin.rb @@ -52,6 +52,7 @@ module ReadOnlyMixin def block_if_readonly_mode return if request.fullpath.start_with?(path "/admin/backups") + return if request.fullpath.start_with?(path "/categories/search") return if request.get? || request.head? if @staff_writes_only_mode diff --git a/spec/requests/categories_controller_spec.rb b/spec/requests/categories_controller_spec.rb index 6d7ac724541..83cc60754cb 100644 --- a/spec/requests/categories_controller_spec.rb +++ b/spec/requests/categories_controller_spec.rb @@ -1211,9 +1211,9 @@ RSpec.describe CategoriesController do category2.upsert_custom_fields("bob" => "marley") # Warm up caches - get "/categories/search.json", params: { term: "Notfoo" } + post "/categories/search.json", params: { term: "Notfoo" } - queries = track_sql_queries { get "/categories/search.json", params: { term: "Notfoo" } } + queries = track_sql_queries { post "/categories/search.json", params: { term: "Notfoo" } } expect(queries.length).to eq(8) @@ -1225,7 +1225,7 @@ RSpec.describe CategoriesController do context "without include_ancestors" do it "doesn't return ancestors" do - get "/categories/search.json", params: { term: "Notfoo" } + post "/categories/search.json", params: { term: "Notfoo" } expect(response.parsed_body).not_to have_key("ancestors") end @@ -1233,7 +1233,7 @@ RSpec.describe CategoriesController do context "with include_ancestors=false" do it "returns ancestors" do - get "/categories/search.json", params: { term: "Notfoo", include_ancestors: false } + post "/categories/search.json", params: { term: "Notfoo", include_ancestors: false } expect(response.parsed_body).not_to have_key("ancestors") end @@ -1241,7 +1241,7 @@ RSpec.describe CategoriesController do context "with include_ancestors=true" do it "returns ancestors" do - get "/categories/search.json", params: { term: "Notfoo", include_ancestors: true } + post "/categories/search.json", params: { term: "Notfoo", include_ancestors: true } expect(response.parsed_body).to have_key("ancestors") end @@ -1249,7 +1249,7 @@ RSpec.describe CategoriesController do context "with term" do it "returns categories" do - get "/categories/search.json", params: { term: "Foo" } + post "/categories/search.json", params: { term: "Foo" } expect(response.parsed_body["categories"].size).to eq(3) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1262,7 +1262,7 @@ RSpec.describe CategoriesController do context "with parent_category_id" do it "returns categories" do - get "/categories/search.json", params: { parent_category_id: category.id } + post "/categories/search.json", params: { parent_category_id: category.id } expect(response.parsed_body["categories"].size).to eq(1) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1271,7 +1271,7 @@ RSpec.describe CategoriesController do end it "can return only top-level categories" do - get "/categories/search.json", params: { parent_category_id: -1 } + post "/categories/search.json", params: { parent_category_id: -1 } expect(response.parsed_body["categories"].size).to eq(3) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1284,7 +1284,7 @@ RSpec.describe CategoriesController do context "with include_uncategorized" do it "returns Uncategorized" do - get "/categories/search.json", params: { include_uncategorized: true } + post "/categories/search.json", params: { include_uncategorized: true } expect(response.parsed_body["categories"].size).to eq(4) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1296,7 +1296,7 @@ RSpec.describe CategoriesController do end it "does not return Uncategorized" do - get "/categories/search.json", params: { include_uncategorized: false } + post "/categories/search.json", params: { include_uncategorized: false } expect(response.parsed_body["categories"].size).to eq(3) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1309,14 +1309,14 @@ RSpec.describe CategoriesController do context "with select_category_ids" do it "returns categories" do - get "/categories/search.json", params: { select_category_ids: [category.id] } + post "/categories/search.json", params: { select_category_ids: [category.id] } expect(response.parsed_body["categories"].size).to eq(1) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly("Foo") end it "works with empty categories list" do - get "/categories/search.json", params: { select_category_ids: [""] } + post "/categories/search.json", params: { select_category_ids: [""] } expect(response.parsed_body["categories"].size).to eq(0) end @@ -1324,7 +1324,7 @@ RSpec.describe CategoriesController do context "with reject_category_ids" do it "returns categories" do - get "/categories/search.json", params: { reject_category_ids: [category2.id] } + post "/categories/search.json", params: { reject_category_ids: [category2.id] } expect(response.parsed_body["categories"].size).to eq(3) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1335,7 +1335,7 @@ RSpec.describe CategoriesController do end it "works with empty categories list" do - get "/categories/search.json", params: { reject_category_ids: [""] } + post "/categories/search.json", params: { reject_category_ids: [""] } expect(response.parsed_body["categories"].size).to eq(4) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1349,7 +1349,7 @@ RSpec.describe CategoriesController do context "with include_subcategories" do it "returns categories" do - get "/categories/search.json", params: { include_subcategories: false } + post "/categories/search.json", params: { include_subcategories: false } expect(response.parsed_body["categories"].size).to eq(3) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1360,7 +1360,7 @@ RSpec.describe CategoriesController do end it "returns categories and subcategories" do - get "/categories/search.json", params: { include_subcategories: true } + post "/categories/search.json", params: { include_subcategories: true } expect(response.parsed_body["categories"].size).to eq(4) expect(response.parsed_body["categories"].map { |c| c["name"] }).to contain_exactly( @@ -1374,7 +1374,7 @@ RSpec.describe CategoriesController do context "with prioritized_category_id" do it "returns categories" do - get "/categories/search.json", params: { prioritized_category_id: category2.id } + post "/categories/search.json", params: { prioritized_category_id: category2.id } expect(response.parsed_body["categories"].size).to eq(4) expect(response.parsed_body["categories"][0]["name"]).to eq("Notfoo") @@ -1383,7 +1383,7 @@ RSpec.describe CategoriesController do context "with limit" do it "returns categories" do - get "/categories/search.json", params: { limit: 2 } + post "/categories/search.json", params: { limit: 2 } expect(response.parsed_body["categories"].size).to eq(2) end @@ -1402,7 +1402,7 @@ RSpec.describe CategoriesController do end it "returns in correct order" do - get "/categories/search.json", params: { term: "ordered" } + post "/categories/search.json", params: { term: "ordered" } expect(response.parsed_body["categories"].map { |c| c["id"] }).to eq( [category4.id, category2.id, category3.id, category1.id], @@ -1412,7 +1412,7 @@ RSpec.describe CategoriesController do it "returns categories in the correct order when the limit is lower than the total number of categories" do categories = 4.times.flat_map do |i| - get "/categories/search.json", params: { term: "ordered", page: i + 1, limit: 1 } + post "/categories/search.json", params: { term: "ordered", page: i + 1, limit: 1 } response.parsed_body["categories"] end @@ -1425,7 +1425,7 @@ RSpec.describe CategoriesController do it "returns user fields" do sign_in(admin) - get "/categories/search.json", params: { select_category_ids: [category.id] } + post "/categories/search.json", params: { select_category_ids: [category.id] } category = response.parsed_body["categories"].first expect(category["notification_level"]).to eq(NotificationLevels.all[:regular])