From c76909a428b23e5941c60f6d9561db22aaa618da Mon Sep 17 00:00:00 2001 From: Suzanne Aitchison Date: Thu, 17 Feb 2022 14:57:40 +0000 Subject: [PATCH] Only make one network call to fetch all existing tags in editor (#16610) * use bulk get of tags in editor * prefer tag_ids if both tag_ids and tag_names are present --- app/controllers/api/v0/tags_controller.rb | 6 ++++- .../article-form/components/TagsField.jsx | 25 +++++++++---------- spec/requests/api/v0/tags_spec.rb | 19 ++++++++++++++ 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/app/controllers/api/v0/tags_controller.rb b/app/controllers/api/v0/tags_controller.rb index 461d548ac..36244fcb6 100644 --- a/app/controllers/api/v0/tags_controller.rb +++ b/app/controllers/api/v0/tags_controller.rb @@ -13,7 +13,11 @@ module Api per_page = (params[:per_page] || 10).to_i num = [per_page, 1000].min - @tags = @tags.where(id: params[:tag_ids]) if params[:tag_ids].present? + if params[:tag_ids].present? + @tags = @tags.where(id: params[:tag_ids]) + elsif params[:tag_names].present? + @tags = @tags.where(name: params[:tag_names]) + end @tags = @tags.order(taggings_count: :desc).page(page).per(num) diff --git a/app/javascript/article-form/components/TagsField.jsx b/app/javascript/article-form/components/TagsField.jsx index 9de038775..34ceaf55c 100644 --- a/app/javascript/article-form/components/TagsField.jsx +++ b/app/javascript/article-form/components/TagsField.jsx @@ -29,20 +29,19 @@ export const TagsField = ({ onInput, defaultValue, switchHelpContext }) => { // Fetching further tag data allows us to display a richer UI // This fetch only happens once on first component load if (defaultValue && defaultValue !== '' && !defaultsLoaded) { - const tagNames = defaultValue.split(', '); + const tagNamesQueryString = defaultValue + .split(', ') + .reduce((queryString, nextTagName) => { + if (nextTagName) { + return queryString + ? `${queryString}&tag_names[]=${nextTagName}` + : `tag_names[]=${nextTagName}`; + } + }, ''); - const tagRequests = tagNames.map((tagName) => - fetchSearch('tags', { name: tagName }).then(({ result = [] }) => { - const [potentialMatch = {}] = result; - return potentialMatch.name === tagName - ? potentialMatch - : { name: tagName }; - }), - ); - - Promise.all(tagRequests).then((data) => { - setDefaultSelections(data); - }); + fetch(`/api/tags.json?${tagNamesQueryString}`) + .then((res) => res.json()) + .then((data) => setDefaultSelections(data)); } setDefaultsLoaded(true); }, [defaultValue, defaultsLoaded]); diff --git a/spec/requests/api/v0/tags_spec.rb b/spec/requests/api/v0/tags_spec.rb index cee0dca01..742f13e0d 100644 --- a/spec/requests/api/v0/tags_spec.rb +++ b/spec/requests/api/v0/tags_spec.rb @@ -45,6 +45,25 @@ RSpec.describe "Api::V0::Tags", type: :request do expect(response.parsed_body.map { |t| t["id"] }).to match_array(tag_ids) end + it "finds tags from array of tag_names" do + tags = create_list(:tag, 10, taggings_count: 10) + tag_names = tags.sample(4).map(&:name) + + get api_tags_path, params: { tag_names: tag_names } + + expect(response.parsed_body.map { |t| t["name"] }).to match_array(tag_names) + end + + it "finds tags from array of tag_ids if both tag_ids and tag_names is passed" do + tags = create_list(:tag, 10, taggings_count: 10) + tag_names = tags.sample(2).map(&:name) + tag_ids = tags.sample(4).map(&:id) + + get api_tags_path, params: { tag_names: tag_names, tag_ids: tag_ids } + + expect(response.parsed_body.map { |t| t["id"] }).to match_array(tag_ids) + end + it "supports pagination" do create_list(:tag, 3)