From 6a012e9fe21096f6a94281a085a16921e14ffa96 Mon Sep 17 00:00:00 2001 From: Daniel James Date: Thu, 2 Nov 2023 13:34:57 +0530 Subject: [PATCH] Replace WCA ID input with user search (#8422) * Replace WCA ID input with user search * Brought forward the UserSearch from #7459 * Review changes --- .../app/controllers/delegates_controller.rb | 5 +- .../components/DelegateProbations/index.jsx | 18 ++- .../SearchWidget/MultiSearchInput.jsx | 136 ++++++++++++++++++ .../components/SearchWidget/UserSearch.jsx | 52 +++++++ .../app/webpacker/lib/hooks/useLoadedData.js | 50 ++++++- .../app/webpacker/lib/requests/routes.js.erb | 4 + WcaOnRails/config/routes.rb | 4 +- .../controllers/delegates_controller_spec.rb | 8 +- 8 files changed, 263 insertions(+), 14 deletions(-) create mode 100644 WcaOnRails/app/webpacker/components/SearchWidget/MultiSearchInput.jsx create mode 100644 WcaOnRails/app/webpacker/components/SearchWidget/UserSearch.jsx diff --git a/WcaOnRails/app/controllers/delegates_controller.rb b/WcaOnRails/app/controllers/delegates_controller.rb index 82bcfa52f1..272371d249 100644 --- a/WcaOnRails/app/controllers/delegates_controller.rb +++ b/WcaOnRails/app/controllers/delegates_controller.rb @@ -36,10 +36,9 @@ def delegate_probation_data def start_delegate_probation respond_to do |format| format.json do - wca_id = params[:wcaId] - user = User.find_by_wca_id!(wca_id) + user_id = params[:userId] Role.create!( - user_id: user.id, + user_id: user_id, group_id: UserGroup.find_by!(name: "Delegate Probation").id, start_date: Date.today, ) diff --git a/WcaOnRails/app/webpacker/components/DelegateProbations/index.jsx b/WcaOnRails/app/webpacker/components/DelegateProbations/index.jsx index d2a9113bab..5a60fcb5a9 100644 --- a/WcaOnRails/app/webpacker/components/DelegateProbations/index.jsx +++ b/WcaOnRails/app/webpacker/components/DelegateProbations/index.jsx @@ -1,5 +1,5 @@ import React from 'react'; -import { Button, Input, Table } from 'semantic-ui-react'; +import { Button, Table } from 'semantic-ui-react'; import DatePicker from 'react-datepicker'; import UserBadge from '../UserBadge'; import useLoadedData from '../../lib/hooks/useLoadedData'; @@ -10,6 +10,7 @@ import { } from '../../lib/requests/routes.js.erb'; import useSaveAction from '../../lib/hooks/useSaveAction'; import useInputState from '../../lib/hooks/useInputState'; +import UserSearch from '../SearchWidget/UserSearch'; import Errored from '../Requests/Errored'; const dateFormat = 'YYYY-MM-DD'; @@ -61,7 +62,7 @@ function ProbationListTable({ } export default function DelegateProbations() { - const [wcaId, setWcaId] = useInputState(''); + const [userId, setUserId] = React.useState(); const { data, loading, error, sync, } = useLoadedData(delegateProbationDataUrl); @@ -75,9 +76,18 @@ 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 new file mode 100644 index 0000000000..f9cc822930 --- /dev/null +++ b/WcaOnRails/app/webpacker/components/SearchWidget/MultiSearchInput.jsx @@ -0,0 +1,136 @@ +import React, { useState, useEffect, useCallback } from 'react'; +import { Dropdown } from 'semantic-ui-react'; + +import CompetitionItem from './CompetitionItem'; +import IncidentItem from './IncidentItem'; +import RegulationItem from './RegulationItem'; +import UserItem from './UserItem'; +import TextItem from './TextItem'; +import useDebounce from '../../lib/hooks/useDebounce'; +import I18n from '../../lib/i18n'; +import { fetchJsonOrError } from '../../lib/requests/fetchWithAuthenticityToken'; + +const classToComponent = { + user: UserItem, + person: UserItem, + competition: CompetitionItem, + regulation: RegulationItem, + text: TextItem, + incident: IncidentItem, +}; + +function ItemFor({ item }) { + const Component = classToComponent[item.class]; + return ( +
+ +
+ ); +} + +const renderLabel = ({ item }) => ({ + color: 'blue', + content: , + className: 'omnisearch-item', + as: 'div', +}); + +export const itemToOption = (item) => ({ + item, + id: item.id, + key: item.id, + value: item.id, + // 'text' is used by the search method from the component, we need to put + // the text with a potential match here! + text: [item.id, item.name, item.title, item.content_html, item.search, item.public_summary].join(' '), + content: , +}); + +const createSearchItem = (search) => itemToOption({ + class: 'text', + id: 'search', + url: `/search?q=${encodeURIComponent(search)}`, + search, +}); + +const DEBOUNCE_MS = 300; + +function MultiSearchInput({ + url, + goToItemOnSelect, + placeholder, + removeNoResultsMessage, + selectedItems, + disabled = false, + multiple = true, + onChange, +}) { + const [search, setSearch] = useState(''); + const [results, setResults] = useState([]); + const [loading, setLoading] = useState(false); + + const debouncedSearch = useDebounce(search, DEBOUNCE_MS); + + // wrap the 'onChange' handler because we want to reset the search string + const onChangeInternal = useCallback((_, { value }) => { + setSearch(''); + onChange(value); + }, [onChange, setSearch]); + + useEffect(() => { + // Do nothing if search string is empty: we're just loading the page + // or we just selected an item. + // Either way, we want to keep the existing results. + if (debouncedSearch.length === 0) return; + if (debouncedSearch.length < 3) { + setResults([]); + } else { + setLoading(true); + // Note: we don't need to do any filtering on the results here, + // FUI's dropdown will automatically remove selected items from the + // options left for selection. + fetchJsonOrError(url(debouncedSearch)) + .then(({ data }) => setResults(data.result.map(itemToOption))) + .finally(() => setLoading(false)); + } + }, [debouncedSearch, url]); + + const options = [...selectedItems, ...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)); + } + + // 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 ( + values.slice(0, 5)} + clearable={!multiple} + icon="search" + className="omnisearch-dropdown" + disabled={disabled} + value={multiple ? selectedItems.map((item) => item.id) : selectedItems[0]?.id} + searchQuery={search} + options={options} + onChange={onChangeInternal} + onSearchChange={(e, { searchQuery }) => setSearch(searchQuery)} + loading={loading} + noResultsMessage={removeNoResultsMessage ? null : I18n.t('search_results.index.not_found.generic')} + placeholder={placeholder || I18n.t('common.search_site')} + renderLabel={renderLabel} + /> + ); +} + +export default MultiSearchInput; diff --git a/WcaOnRails/app/webpacker/components/SearchWidget/UserSearch.jsx b/WcaOnRails/app/webpacker/components/SearchWidget/UserSearch.jsx new file mode 100644 index 0000000000..f43bec9c65 --- /dev/null +++ b/WcaOnRails/app/webpacker/components/SearchWidget/UserSearch.jsx @@ -0,0 +1,52 @@ +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/lib/hooks/useLoadedData.js b/WcaOnRails/app/webpacker/lib/hooks/useLoadedData.js index d0057bd17b..9ba0ff2f8a 100644 --- a/WcaOnRails/app/webpacker/lib/hooks/useLoadedData.js +++ b/WcaOnRails/app/webpacker/lib/hooks/useLoadedData.js @@ -1,4 +1,6 @@ -import { useState, useEffect, useCallback } from 'react'; +import { + useState, useEffect, useCallback, useMemo, +} from 'react'; import { fetchJsonOrError } from '../requests/fetchWithAuthenticityToken'; // This is a hook that can be used to get a data from the website (as json) @@ -33,4 +35,50 @@ 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 c696dd310d..d809f71613 100644 --- a/WcaOnRails/app/webpacker/lib/requests/routes.js.erb +++ b/WcaOnRails/app/webpacker/lib/requests/routes.js.erb @@ -74,6 +74,10 @@ 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 userSearchApiUrl = (query) => `<%= Rails.application.routes.url_helpers.api_v0_search_users_path %>?q=${query}`; + export const geocodingApiUrl = `<%= CGI.unescape(Rails.application.routes.url_helpers.api_v0_geocoding_search_path) %>`; export const adminPostingCompetitionsUrl = `<%= CGI.unescape(Rails.application.routes.url_helpers.results_posting_dashboard_path(format: "json")) %>`; diff --git a/WcaOnRails/config/routes.rb b/WcaOnRails/config/routes.rb index 3eccb52f48..4c14fa9eea 100644 --- a/WcaOnRails/config/routes.rb +++ b/WcaOnRails/config/routes.rb @@ -295,11 +295,11 @@ get '/search' => 'api#omni_search' get '/search/posts' => 'api#posts_search' get '/search/competitions' => 'api#competitions_search' - get '/search/users' => 'api#users_search' + get '/search/users' => 'api#users_search', as: :search_users get '/search/regulations' => 'api#regulations_search' get '/search/incidents' => 'api#incidents_search' get '/users/:id' => 'api#show_user_by_id', constraints: { id: /\d+/ } - get '/users/:wca_id' => 'api#show_user_by_wca_id' + get '/users/:wca_id' => 'api#show_user_by_wca_id', as: :user get '/delegates' => 'api#delegates' get '/persons' => "persons#index" get '/persons/:wca_id' => "persons#show", as: :person diff --git a/WcaOnRails/spec/controllers/delegates_controller_spec.rb b/WcaOnRails/spec/controllers/delegates_controller_spec.rb index dde4b464aa..7a872a1857 100644 --- a/WcaOnRails/spec/controllers/delegates_controller_spec.rb +++ b/WcaOnRails/spec/controllers/delegates_controller_spec.rb @@ -48,7 +48,7 @@ it 'senior delegates can start the probation role' do sign_in FactoryBot.create :senior_delegate - post :start_delegate_probation, params: { wcaId: users[0].wca_id }, format: :json + post :start_delegate_probation, params: { userId: users[0].id }, format: :json parsed_body = JSON.parse(response.body) expect(parsed_body["success"]).to eq true end @@ -64,7 +64,7 @@ it 'WFC leader can start the probation role' do sign_in FactoryBot.create :user, :wfc_member, team_leader: true - post :start_delegate_probation, params: { wcaId: users[0].wca_id }, format: :json + post :start_delegate_probation, params: { userId: users[0].id }, format: :json parsed_body = JSON.parse(response.body) expect(parsed_body["success"]).to eq true end @@ -80,7 +80,7 @@ it 'WFC senior members can start the probation role' do sign_in FactoryBot.create :user, :wfc_member, team_senior_member: true - post :start_delegate_probation, params: { wcaId: users[0].wca_id }, format: :json + post :start_delegate_probation, params: { userId: users[0].id }, format: :json parsed_body = JSON.parse(response.body) expect(parsed_body["success"]).to eq true end @@ -96,7 +96,7 @@ it 'normal user cannot start the probation role' do sign_in FactoryBot.create :user - post :start_delegate_probation, params: { wcaId: users[0].wca_id }, format: :json + post :start_delegate_probation, params: { userId: users[0].id }, format: :json expect(response.status).to eq 401 end