From 3c1f4ab132041ca37ff5c35285d7808103d7f47e Mon Sep 17 00:00:00 2001 From: Daniel James Date: Tue, 14 Nov 2023 22:15:02 +0530 Subject: [PATCH] Improved Search in React & added support for persons model (#8493) * Improved Search in React & added support for persons model * Review changes --- .../app/controllers/api/v0/api_controller.rb | 4 ++ WcaOnRails/app/models/person.rb | 2 +- .../components/DelegateProbations/index.jsx | 20 +++---- .../SearchWidget/MultiSearchInput.jsx | 51 ++++++++++-------- .../components/SearchWidget/UserSearch.jsx | 52 ------------------- .../components/SearchWidget/WcaSearch.jsx | 32 ++++++++++++ .../app/webpacker/lib/hooks/useLoadedData.js | 50 +----------------- .../app/webpacker/lib/requests/routes.js.erb | 2 +- WcaOnRails/config/routes.rb | 1 + 9 files changed, 78 insertions(+), 136 deletions(-) delete mode 100644 WcaOnRails/app/webpacker/components/SearchWidget/UserSearch.jsx create mode 100644 WcaOnRails/app/webpacker/components/SearchWidget/WcaSearch.jsx diff --git a/WcaOnRails/app/controllers/api/v0/api_controller.rb b/WcaOnRails/app/controllers/api/v0/api_controller.rb index 8f60b19927..73d7632f7d 100644 --- a/WcaOnRails/app/controllers/api/v0/api_controller.rb +++ b/WcaOnRails/app/controllers/api/v0/api_controller.rb @@ -121,6 +121,10 @@ def users_search search(User) end + def persons_search + search(Person) + end + def regulations_search search(Regulation) end diff --git a/WcaOnRails/app/models/person.rb b/WcaOnRails/app/models/person.rb index bcdb204bcb..3fcdb4f182 100644 --- a/WcaOnRails/app/models/person.rb +++ b/WcaOnRails/app/models/person.rb @@ -256,7 +256,7 @@ def gender_visible? %w(m f).include? gender end - def self.search(query) + def self.search(query, params: {}) persons = Person.current.includes(:user) query.split.each do |part| persons = persons.where("name LIKE :part OR wca_id LIKE :part", part: "%#{part}%") diff --git a/WcaOnRails/app/webpacker/components/DelegateProbations/index.jsx b/WcaOnRails/app/webpacker/components/DelegateProbations/index.jsx index 5a60fcb5a9..c993dabf48 100644 --- a/WcaOnRails/app/webpacker/components/DelegateProbations/index.jsx +++ b/WcaOnRails/app/webpacker/components/DelegateProbations/index.jsx @@ -9,8 +9,7 @@ import { endDelegateProbationUrl, } from '../../lib/requests/routes.js.erb'; import useSaveAction from '../../lib/hooks/useSaveAction'; -import useInputState from '../../lib/hooks/useInputState'; -import UserSearch from '../SearchWidget/UserSearch'; +import WcaSearch from '../SearchWidget/WcaSearch'; import Errored from '../Requests/Errored'; const dateFormat = 'YYYY-MM-DD'; @@ -62,7 +61,7 @@ function ProbationListTable({ } export default function DelegateProbations() { - const [userId, setUserId] = React.useState(); + const [user, setUser] = React.useState(); const { data, loading, error, sync, } = useLoadedData(delegateProbationDataUrl); @@ -76,18 +75,19 @@ export default function DelegateProbations() { return ( <>

Delegate Probations

- diff --git a/WcaOnRails/app/webpacker/components/SearchWidget/MultiSearchInput.jsx b/WcaOnRails/app/webpacker/components/SearchWidget/MultiSearchInput.jsx index f9cc822930..64fd2dd215 100644 --- a/WcaOnRails/app/webpacker/components/SearchWidget/MultiSearchInput.jsx +++ b/WcaOnRails/app/webpacker/components/SearchWidget/MultiSearchInput.jsx @@ -1,4 +1,4 @@ -import React, { useState, useEffect, useCallback } from 'react'; +import React, { useState, useEffect } from 'react'; import { Dropdown } from 'semantic-ui-react'; import CompetitionItem from './CompetitionItem'; @@ -55,15 +55,18 @@ const createSearchItem = (search) => itemToOption({ const DEBOUNCE_MS = 300; -function MultiSearchInput({ +export default function MultiSearchInput({ url, - goToItemOnSelect, + // If multiple is true, selectedValue is an array of items, otherwise it's a single item. + selectedValue, + // If multiple is true, setSelectedValue is a function that takes an array of items, otherwise + // it's a function that takes a single item. + setSelectedValue, + showOptionToGoToSearchPage = false, placeholder, removeNoResultsMessage, - selectedItems, disabled = false, multiple = true, - onChange, }) { const [search, setSearch] = useState(''); const [results, setResults] = useState([]); @@ -71,11 +74,9 @@ function MultiSearchInput({ const debouncedSearch = useDebounce(search, DEBOUNCE_MS); - // wrap the 'onChange' handler because we want to reset the search string - const onChangeInternal = useCallback((_, { value }) => { + useEffect(() => { setSearch(''); - onChange(value); - }, [onChange, setSearch]); + }, [selectedValue]); useEffect(() => { // Do nothing if search string is empty: we're just loading the page @@ -95,21 +96,27 @@ function MultiSearchInput({ } }, [debouncedSearch, url]); - const options = [...selectedItems, ...results].map((option) => ({ + const dropDownOptions = [ + ...(showOptionToGoToSearchPage && search.length > 0 ? [createSearchItem(search)] : []), + ...(multiple ? selectedValue : []), + ...results, + ].map((option) => ({ ...option, text: , })); - // If we go to item on select, we want to give the user the option to go to - // the search page. - if (goToItemOnSelect && search.length > 0) { - options.unshift(createSearchItem(search)); - } + const onChangeInternal = (_, { value, options }) => { + const map = {}; + options.forEach((option) => { + map[option.value] = option; + }); + if (multiple) { + setSelectedValue(value.map((id) => itemToOption(map[id].item))); + } else { + setSelectedValue(map[value] ? itemToOption(map[value].item) : null); + } + }; - // FIXME: the search filter from FUI is not the greatest: when searching for - // "galerie lafa" it won't match the "galeries lafayette" competitions - // (whereas searching for "galeries lafa" does). - // We should try to set our own search method that would match word by word. return ( item.id) : selectedItems[0]?.id} + value={multiple ? selectedValue.map((item) => item.id) : selectedValue?.id} searchQuery={search} - options={options} + options={dropDownOptions} onChange={onChangeInternal} onSearchChange={(e, { searchQuery }) => setSearch(searchQuery)} loading={loading} @@ -132,5 +139,3 @@ function MultiSearchInput({ /> ); } - -export default MultiSearchInput; diff --git a/WcaOnRails/app/webpacker/components/SearchWidget/UserSearch.jsx b/WcaOnRails/app/webpacker/components/SearchWidget/UserSearch.jsx deleted file mode 100644 index f43bec9c65..0000000000 --- a/WcaOnRails/app/webpacker/components/SearchWidget/UserSearch.jsx +++ /dev/null @@ -1,52 +0,0 @@ -import React, { useCallback, useMemo } from 'react'; -import { userSearchApiUrl, userApiUrl } from '../../lib/requests/routes.js.erb'; -import Loading from '../Requests/Loading'; -import MultiSearchInput, { itemToOption } from './MultiSearchInput'; -import { useManyLoadedData } from '../../lib/hooks/useLoadedData'; - -export default function UserSearch({ - value, - onChange, - delegateOnly = false, - traineeOnly = false, - multiple = true, -}) { - const userIds = useMemo(() => { - if (multiple) return value || []; - return value ? [value] : []; - }, [value, multiple]); - - const queryParams = useMemo(() => { - const params = new URLSearchParams(); - - if (delegateOnly) params.append('only_staff_delegates', true); - if (traineeOnly) params.append('only_trainee_delegates', true); - - return params; - }, [delegateOnly, traineeOnly]); - - const userSearchApiUrlFn = useCallback((query) => `${userSearchApiUrl(query)}&${queryParams.toString()}`, [queryParams]); - - const { - data, - anyLoading, - } = useManyLoadedData(userIds, userApiUrl); - - const preSelected = useMemo( - // the users API actually returns users in the format { "user": stuff_you_are_interested_in } - () => Object.values(data).map((item) => itemToOption(item.user)), - [data], - ); - - if (anyLoading) return ; - - return ( - - ); -} diff --git a/WcaOnRails/app/webpacker/components/SearchWidget/WcaSearch.jsx b/WcaOnRails/app/webpacker/components/SearchWidget/WcaSearch.jsx new file mode 100644 index 0000000000..1ab3008210 --- /dev/null +++ b/WcaOnRails/app/webpacker/components/SearchWidget/WcaSearch.jsx @@ -0,0 +1,32 @@ +import React, { + useCallback, +} from 'react'; + +import { userSearchApiUrl, personSearchApiUrl } from '../../lib/requests/routes.js.erb'; +import MultiSearchInput from './MultiSearchInput'; + +export default function WcaSearch({ + selectedValue, + setSelectedValue, + multiple = true, + model, + params, +}) { + const urlFn = useCallback((query) => { + if (model === 'user') { + return `${userSearchApiUrl(query)}&${new URLSearchParams(params).toString()}`; + } if (model === 'person') { + return `${personSearchApiUrl(query)}&${new URLSearchParams(params).toString()}`; + } + return ''; + }, [params, model]); + + return ( + + ); +} diff --git a/WcaOnRails/app/webpacker/lib/hooks/useLoadedData.js b/WcaOnRails/app/webpacker/lib/hooks/useLoadedData.js index 9ba0ff2f8a..d0057bd17b 100644 --- a/WcaOnRails/app/webpacker/lib/hooks/useLoadedData.js +++ b/WcaOnRails/app/webpacker/lib/hooks/useLoadedData.js @@ -1,6 +1,4 @@ -import { - useState, useEffect, useCallback, useMemo, -} from 'react'; +import { useState, useEffect, useCallback } from 'react'; import { fetchJsonOrError } from '../requests/fetchWithAuthenticityToken'; // This is a hook that can be used to get a data from the website (as json) @@ -35,50 +33,4 @@ const useLoadedData = (url) => { }; }; -export const useManyLoadedData = (ids, urlFn) => { - const [data, setData] = useState({}); - const [headers, setHeaders] = useState({}); - const [error, setError] = useState({}); - - const [anyLoading, setAnyLoading] = useState(true); - - const promises = useMemo(() => ids.map(async (id) => { - const url = urlFn(id); - - try { - const response = await fetchJsonOrError(url); - setData((prevData) => ({ - ...prevData, - [id]: response.data, - })); - setHeaders((prevHeaders) => ({ - ...prevHeaders, - [id]: response.headers, - })); - } catch (err) { - setError((prevError) => ({ - ...prevError, - [id]: err.message, - })); - } - }), [ids, urlFn, setData, setHeaders, setError]); - - const syncAll = useCallback(() => { - setAnyLoading(true); - setData({}); - setError({}); - Promise.all(promises).finally(() => setAnyLoading(false)); - }, [promises]); - - useEffect(syncAll, [syncAll]); - - return { - data, - headers, - anyLoading, - error, - syncAll, - }; -}; - export default useLoadedData; diff --git a/WcaOnRails/app/webpacker/lib/requests/routes.js.erb b/WcaOnRails/app/webpacker/lib/requests/routes.js.erb index 071c51bbed..da579f642b 100644 --- a/WcaOnRails/app/webpacker/lib/requests/routes.js.erb +++ b/WcaOnRails/app/webpacker/lib/requests/routes.js.erb @@ -74,7 +74,7 @@ export const adminGenerateIds = `<%= CGI.unescape(Rails.application.routes.url_h export const personApiUrl = (wcaId) => `<%= CGI.unescape(Rails.application.routes.url_helpers.api_v0_person_path("${wcaId}"))%>`; -export const userApiUrl = (id) => `<%= CGI.unescape(Rails.application.routes.url_helpers.api_v0_user_path("${id}"))%>`; +export const personSearchApiUrl = (query) => `<%= Rails.application.routes.url_helpers.api_v0_search_persons_path %>?q=${query}`; export const userSearchApiUrl = (query) => `<%= Rails.application.routes.url_helpers.api_v0_search_users_path %>?q=${query}`; diff --git a/WcaOnRails/config/routes.rb b/WcaOnRails/config/routes.rb index c1d19ea034..a751ce5a71 100644 --- a/WcaOnRails/config/routes.rb +++ b/WcaOnRails/config/routes.rb @@ -300,6 +300,7 @@ get '/search/posts' => 'api#posts_search' get '/search/competitions' => 'api#competitions_search' get '/search/users' => 'api#users_search', as: :search_users + get '/search/persons' => 'api#persons_search', as: :search_persons get '/search/regulations' => 'api#regulations_search' get '/search/incidents' => 'api#incidents_search' get '/users/:id' => 'api#show_user_by_id', constraints: { id: /\d+/ }