From 559ad9981a6920079c9e16b00d94be1dc76a9ffe Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 9 Apr 2024 13:53:00 -0700 Subject: [PATCH 01/15] feat: make "refine by tags" menu wider --- src/search-modal/FilterByTags.jsx | 2 +- src/search-modal/SearchModal.scss | 7 ++++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/search-modal/FilterByTags.jsx b/src/search-modal/FilterByTags.jsx index 68bd92ee70..4e577cfab4 100644 --- a/src/search-modal/FilterByTags.jsx +++ b/src/search-modal/FilterByTags.jsx @@ -97,7 +97,7 @@ const FilterByTags = () => { label={} > - + { // Show a message if there are no options at all to avoid the impression that the dropdown isn't working diff --git a/src/search-modal/SearchModal.scss b/src/search-modal/SearchModal.scss index c67c6ba2a9..33c689a6e8 100644 --- a/src/search-modal/SearchModal.scss +++ b/src/search-modal/SearchModal.scss @@ -27,7 +27,7 @@ } // Options for the "filter by tag" menu - .pgn__menu { + .pgn__menu.tags-refinement-menu { $indent-initial: 1.3rem; $indent-each: 1.6rem; @@ -46,6 +46,11 @@ .tag-option-4 { padding-left: $indent-initial + (4 * $indent-each); } + + .pgn__menu-item { + // Make the "filter by tag" menu much wider than normal, because we need the space to display the tags hierarchy + width: 400px; + } } .pgn__menu-item { From 308b5aad6c03ba179e2f1f9addd2897182b71ef1 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 10 Apr 2024 11:43:36 -0700 Subject: [PATCH 02/15] feat: working tags filter with multi-select support --- package-lock.json | 341 +-------------------- package.json | 3 +- src/search-modal/ClearFiltersButton.jsx | 8 +- src/search-modal/EmptyStates.jsx | 9 +- src/search-modal/FilterByBlockType.jsx | 57 ++-- src/search-modal/FilterByTags.jsx | 240 ++++++++++----- src/search-modal/Highlight.jsx | 26 ++ src/search-modal/SearchEndpointLoader.jsx | 41 --- src/search-modal/SearchKeywordsField.jsx | 14 +- src/search-modal/SearchModal.jsx | 4 +- src/search-modal/SearchModal.scss | 21 +- src/search-modal/SearchResult.jsx | 21 +- src/search-modal/SearchResults.jsx | 17 ++ src/search-modal/SearchUI.jsx | 25 +- src/search-modal/Stats.jsx | 12 +- src/search-modal/data/api.js | 346 +++++++++++++++++++++- src/search-modal/data/apiHooks.js | 155 +++++++++- src/search-modal/manager/SearchManager.js | 94 ++++++ src/search-modal/messages.js | 20 ++ 19 files changed, 870 insertions(+), 584 deletions(-) create mode 100644 src/search-modal/Highlight.jsx delete mode 100644 src/search-modal/SearchEndpointLoader.jsx create mode 100644 src/search-modal/SearchResults.jsx create mode 100644 src/search-modal/manager/SearchManager.js diff --git a/package-lock.json b/package-lock.json index ee9da9cd37..adb61ca687 100644 --- a/package-lock.json +++ b/package-lock.json @@ -46,16 +46,15 @@ "email-validator": "2.0.4", "file-saver": "^2.0.5", "formik": "2.2.6", - "instantsearch.css": "^8.1.0", "jszip": "^3.10.1", "lodash": "4.17.21", + "meilisearch": "^0.38.0", "moment": "2.29.4", "prop-types": "15.7.2", "react": "17.0.2", "react-datepicker": "^4.13.0", "react-dom": "17.0.2", "react-helmet": "^6.1.0", - "react-instantsearch": "^7.7.1", "react-redux": "7.2.9", "react-responsive": "9.0.2", "react-router": "6.16.0", @@ -110,159 +109,6 @@ "integrity": "sha512-rE0Pygv0sEZ4vBWHlAgJLGDU7Pm8xoO6p3wsEceb7GYAjScrOHpEo8KK/eVkAcnSM+slAEtXjA2JpdjLp4fJQQ==", "dev": true }, - "node_modules/@algolia/cache-browser-local-storage": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/cache-browser-local-storage/-/cache-browser-local-storage-4.23.2.tgz", - "integrity": "sha512-PvRQdCmtiU22dw9ZcTJkrVKgNBVAxKgD0/cfiqyxhA5+PHzA2WDt6jOmZ9QASkeM2BpyzClJb/Wr1yt2/t78Kw==", - "peer": true, - "dependencies": { - "@algolia/cache-common": "4.23.2" - } - }, - "node_modules/@algolia/cache-common": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/cache-common/-/cache-common-4.23.2.tgz", - "integrity": "sha512-OUK/6mqr6CQWxzl/QY0/mwhlGvS6fMtvEPyn/7AHUx96NjqDA4X4+Ju7aXFQKh+m3jW9VPB0B9xvEQgyAnRPNw==", - "peer": true - }, - "node_modules/@algolia/cache-in-memory": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/cache-in-memory/-/cache-in-memory-4.23.2.tgz", - "integrity": "sha512-rfbi/SnhEa3MmlqQvgYz/9NNJ156NkU6xFxjbxBtLWnHbpj+qnlMoKd+amoiacHRITpajg6zYbLM9dnaD3Bczw==", - "peer": true, - "dependencies": { - "@algolia/cache-common": "4.23.2" - } - }, - "node_modules/@algolia/client-account": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/client-account/-/client-account-4.23.2.tgz", - "integrity": "sha512-VbrOCLIN/5I7iIdskSoSw3uOUPF516k4SjDD4Qz3BFwa3of7D9A0lzBMAvQEJJEPHWdVraBJlGgdJq/ttmquJQ==", - "peer": true, - "dependencies": { - "@algolia/client-common": "4.23.2", - "@algolia/client-search": "4.23.2", - "@algolia/transporter": "4.23.2" - } - }, - "node_modules/@algolia/client-analytics": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/client-analytics/-/client-analytics-4.23.2.tgz", - "integrity": "sha512-lLj7irsAztGhMoEx/SwKd1cwLY6Daf1Q5f2AOsZacpppSvuFvuBrmkzT7pap1OD/OePjLKxicJS8wNA0+zKtuw==", - "peer": true, - "dependencies": { - "@algolia/client-common": "4.23.2", - "@algolia/client-search": "4.23.2", - "@algolia/requester-common": "4.23.2", - "@algolia/transporter": "4.23.2" - } - }, - "node_modules/@algolia/client-common": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/client-common/-/client-common-4.23.2.tgz", - "integrity": "sha512-Q2K1FRJBern8kIfZ0EqPvUr3V29ICxCm/q42zInV+VJRjldAD9oTsMGwqUQ26GFMdFYmqkEfCbY4VGAiQhh22g==", - "peer": true, - "dependencies": { - "@algolia/requester-common": "4.23.2", - "@algolia/transporter": "4.23.2" - } - }, - "node_modules/@algolia/client-personalization": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/client-personalization/-/client-personalization-4.23.2.tgz", - "integrity": "sha512-vwPsgnCGhUcHhhQG5IM27z8q7dWrN9itjdvgA6uKf2e9r7vB+WXt4OocK0CeoYQt3OGEAExryzsB8DWqdMK5wg==", - "peer": true, - "dependencies": { - "@algolia/client-common": "4.23.2", - "@algolia/requester-common": "4.23.2", - "@algolia/transporter": "4.23.2" - } - }, - "node_modules/@algolia/client-search": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/client-search/-/client-search-4.23.2.tgz", - "integrity": "sha512-CxSB29OVGSE7l/iyoHvamMonzq7Ev8lnk/OkzleODZ1iBcCs3JC/XgTIKzN/4RSTrJ9QybsnlrN/bYCGufo7qw==", - "peer": true, - "dependencies": { - "@algolia/client-common": "4.23.2", - "@algolia/requester-common": "4.23.2", - "@algolia/transporter": "4.23.2" - } - }, - "node_modules/@algolia/events": { - "version": "4.0.1", - "resolved": "https://registry.npmjs.org/@algolia/events/-/events-4.0.1.tgz", - "integrity": "sha512-FQzvOCgoFXAbf5Y6mYozw2aj5KCJoA3m4heImceldzPSMbdyS4atVjJzXKMsfX3wnZTFYwkkt8/z8UesLHlSBQ==" - }, - "node_modules/@algolia/logger-common": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/logger-common/-/logger-common-4.23.2.tgz", - "integrity": "sha512-jGM49Q7626cXZ7qRAWXn0jDlzvoA1FvN4rKTi1g0hxKsTTSReyYk0i1ADWjChDPl3Q+nSDhJuosM2bBUAay7xw==", - "peer": true - }, - "node_modules/@algolia/logger-console": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/logger-console/-/logger-console-4.23.2.tgz", - "integrity": "sha512-oo+lnxxEmlhTBTFZ3fGz1O8PJ+G+8FiAoMY2Qo3Q4w23xocQev6KqDTA1JQAGPDxAewNA2VBwWOsVXeXFjrI/Q==", - "peer": true, - "dependencies": { - "@algolia/logger-common": "4.23.2" - } - }, - "node_modules/@algolia/recommend": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/recommend/-/recommend-4.23.2.tgz", - "integrity": "sha512-Q75CjnzRCDzgIlgWfPnkLtrfF4t82JCirhalXkSSwe/c1GH5pWh4xUyDOR3KTMo+YxxX3zTlrL/FjHmUJEWEcg==", - "peer": true, - "dependencies": { - "@algolia/cache-browser-local-storage": "4.23.2", - "@algolia/cache-common": "4.23.2", - "@algolia/cache-in-memory": "4.23.2", - "@algolia/client-common": "4.23.2", - "@algolia/client-search": "4.23.2", - "@algolia/logger-common": "4.23.2", - "@algolia/logger-console": "4.23.2", - "@algolia/requester-browser-xhr": "4.23.2", - "@algolia/requester-common": "4.23.2", - "@algolia/requester-node-http": "4.23.2", - "@algolia/transporter": "4.23.2" - } - }, - "node_modules/@algolia/requester-browser-xhr": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/requester-browser-xhr/-/requester-browser-xhr-4.23.2.tgz", - "integrity": "sha512-TO9wLlp8+rvW9LnIfyHsu8mNAMYrqNdQ0oLF6eTWFxXfxG3k8F/Bh7nFYGk2rFAYty4Fw4XUtrv/YjeNDtM5og==", - "peer": true, - "dependencies": { - "@algolia/requester-common": "4.23.2" - } - }, - "node_modules/@algolia/requester-common": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/requester-common/-/requester-common-4.23.2.tgz", - "integrity": "sha512-3EfpBS0Hri0lGDB5H/BocLt7Vkop0bTTLVUBB844HH6tVycwShmsV6bDR7yXbQvFP1uNpgePRD3cdBCjeHmk6Q==", - "peer": true - }, - "node_modules/@algolia/requester-node-http": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/requester-node-http/-/requester-node-http-4.23.2.tgz", - "integrity": "sha512-SVzgkZM/malo+2SB0NWDXpnT7nO5IZwuDTaaH6SjLeOHcya1o56LSWXk+3F3rNLz2GVH+I/rpYKiqmHhSOjerw==", - "peer": true, - "dependencies": { - "@algolia/requester-common": "4.23.2" - } - }, - "node_modules/@algolia/transporter": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/@algolia/transporter/-/transporter-4.23.2.tgz", - "integrity": "sha512-GY3aGKBy+8AK4vZh8sfkatDciDVKad5rTY2S10Aefyjh7e7UGBP4zigf42qVXwU8VOPwi7l/L7OACGMOFcjB0Q==", - "peer": true, - "dependencies": { - "@algolia/cache-common": "4.23.2", - "@algolia/logger-common": "4.23.2", - "@algolia/requester-common": "4.23.2" - } - }, "node_modules/@ampproject/remapping": { "version": "2.3.0", "resolved": "https://registry.npmjs.org/@ampproject/remapping/-/remapping-2.3.0.tgz", @@ -5694,11 +5540,6 @@ "resolved": "https://registry.npmjs.org/@types/cookie/-/cookie-0.3.3.tgz", "integrity": "sha512-LKVP3cgXBT9RYj+t+9FDKwS5tdI+rPBXaNSkma7hvqy35lc7mAokC2zsqWJH0LaqIt3B962nuYI77hsJoT1gow==" }, - "node_modules/@types/dom-speech-recognition": { - "version": "0.0.1", - "resolved": "https://registry.npmjs.org/@types/dom-speech-recognition/-/dom-speech-recognition-0.0.1.tgz", - "integrity": "sha512-udCxb8DvjcDKfk1WTBzDsxFbLgYxmQGKrE/ricoMqHRNjSlSUCcamVTA5lIQqzY10mY5qCY0QDwBfFEwhfoDPw==" - }, "node_modules/@types/eslint": { "version": "8.56.7", "resolved": "https://registry.npmjs.org/@types/eslint/-/eslint-8.56.7.tgz", @@ -5753,11 +5594,6 @@ "@types/node": "*" } }, - "node_modules/@types/google.maps": { - "version": "3.55.6", - "resolved": "https://registry.npmjs.org/@types/google.maps/-/google.maps-3.55.6.tgz", - "integrity": "sha512-RDtveRsejIi7KRnahz+PE1+Uo+6axr98Susjn/7DxNPPej/T0sMMJfnwm3NcQgvVDWvixWCMOn2Sfukq5UVF2g==" - }, "node_modules/@types/graceful-fs": { "version": "4.1.9", "resolved": "https://registry.npmjs.org/@types/graceful-fs/-/graceful-fs-4.1.9.tgz", @@ -5766,11 +5602,6 @@ "@types/node": "*" } }, - "node_modules/@types/hogan.js": { - "version": "3.0.5", - "resolved": "https://registry.npmjs.org/@types/hogan.js/-/hogan.js-3.0.5.tgz", - "integrity": "sha512-/uRaY3HGPWyLqOyhgvW9Aa43BNnLZrNeQxl2p8wqId4UHMfPKolSB+U7BlZyO1ng7MkLnyEAItsBzCG0SDhqrA==" - }, "node_modules/@types/hoist-non-react-statics": { "version": "3.3.5", "resolved": "https://registry.npmjs.org/@types/hoist-non-react-statics/-/hoist-non-react-statics-3.3.5.tgz", @@ -6547,11 +6378,6 @@ "integrity": "sha512-j2afSsaIENvHZN2B8GOpF566vZ5WVk5opAiMTvWgaQT8DkbOqsTfvNAvHoRGU2zzP8cPoqys+xHTRDWW8L+/BA==", "deprecated": "Use your platform's native atob() and btoa() methods instead" }, - "node_modules/abbrev": { - "version": "1.1.1", - "resolved": "https://registry.npmjs.org/abbrev/-/abbrev-1.1.1.tgz", - "integrity": "sha512-nne9/IiQ/hzIhY6pdDnbBtz7DjPTKrY00P/zvPSm5pOFkl6xuGrGnXn/VtTNNfNtAfZ9/1RtehkszU9qcTii0Q==" - }, "node_modules/accepts": { "version": "1.3.8", "resolved": "https://registry.npmjs.org/accepts/-/accepts-1.3.8.tgz", @@ -6720,40 +6546,6 @@ "ajv": "^6.9.1" } }, - "node_modules/algoliasearch": { - "version": "4.23.2", - "resolved": "https://registry.npmjs.org/algoliasearch/-/algoliasearch-4.23.2.tgz", - "integrity": "sha512-8aCl055IsokLuPU8BzLjwzXjb7ty9TPcUFFOk0pYOwsE5DMVhE3kwCMFtsCFKcnoPZK7oObm+H5mbnSO/9ioxQ==", - "peer": true, - "dependencies": { - "@algolia/cache-browser-local-storage": "4.23.2", - "@algolia/cache-common": "4.23.2", - "@algolia/cache-in-memory": "4.23.2", - "@algolia/client-account": "4.23.2", - "@algolia/client-analytics": "4.23.2", - "@algolia/client-common": "4.23.2", - "@algolia/client-personalization": "4.23.2", - "@algolia/client-search": "4.23.2", - "@algolia/logger-common": "4.23.2", - "@algolia/logger-console": "4.23.2", - "@algolia/recommend": "4.23.2", - "@algolia/requester-browser-xhr": "4.23.2", - "@algolia/requester-common": "4.23.2", - "@algolia/requester-node-http": "4.23.2", - "@algolia/transporter": "4.23.2" - } - }, - "node_modules/algoliasearch-helper": { - "version": "3.17.0", - "resolved": "https://registry.npmjs.org/algoliasearch-helper/-/algoliasearch-helper-3.17.0.tgz", - "integrity": "sha512-R5422OiQjvjlK3VdpNQ/Qk7KsTIGeM5ACm8civGifOVWdRRV/3SgXuKmeNxe94Dz6fwj/IgpVmXbHutU4mHubg==", - "dependencies": { - "@algolia/events": "^4.0.1" - }, - "peerDependencies": { - "algoliasearch": ">= 3.1 < 6" - } - }, "node_modules/ansi-escapes": { "version": "4.3.2", "resolved": "https://registry.npmjs.org/ansi-escapes/-/ansi-escapes-4.3.2.tgz", @@ -12001,18 +11793,6 @@ "value-equal": "^1.0.1" } }, - "node_modules/hogan.js": { - "version": "3.0.2", - "resolved": "https://registry.npmjs.org/hogan.js/-/hogan.js-3.0.2.tgz", - "integrity": "sha512-RqGs4wavGYJWE07t35JQccByczmNUXQT0E12ZYV1VKYu5UiAU9lsos/yBAcf840+zrUQQxgVduCR5/B8nNtibg==", - "dependencies": { - "mkdirp": "0.3.0", - "nopt": "1.0.10" - }, - "bin": { - "hulk": "bin/hulk" - } - }, "node_modules/hoist-non-react-statics": { "version": "3.3.2", "resolved": "https://registry.npmjs.org/hoist-non-react-statics/-/hoist-non-react-statics-3.3.2.tgz", @@ -12037,11 +11817,6 @@ "wbuf": "^1.1.0" } }, - "node_modules/htm": { - "version": "3.1.1", - "resolved": "https://registry.npmjs.org/htm/-/htm-3.1.1.tgz", - "integrity": "sha512-983Vyg8NwUE7JkZ6NmOqpCZ+sh1bKv2iYTlUkzlWmA5JD2acKoxd4KVxbMmxX/85mtfdnDmTFoNKcg5DGAvxNQ==" - }, "node_modules/html-encoding-sniffer": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/html-encoding-sniffer/-/html-encoding-sniffer-2.0.1.tgz", @@ -12661,52 +12436,6 @@ "node": ">=12.0.0" } }, - "node_modules/instantsearch-ui-components": { - "version": "0.4.0", - "resolved": "https://registry.npmjs.org/instantsearch-ui-components/-/instantsearch-ui-components-0.4.0.tgz", - "integrity": "sha512-Isa9Ankm89e9PUXsUto6TxYzcQpXKlWZMsKLXc//dO4i9q5JS8s0Es+c+U65jRLK2j1DiVlNx/Z6HshRIZwA8w==", - "dependencies": { - "@babel/runtime": "^7.1.2" - } - }, - "node_modules/instantsearch.css": { - "version": "8.1.0", - "resolved": "https://registry.npmjs.org/instantsearch.css/-/instantsearch.css-8.1.0.tgz", - "integrity": "sha512-rPhcAZ02bLwUn3iOXbldZW/yl+17guWoH3qWYZ8nQEwNBx5+wZ6Bv8mFqqK448+R2aU4nbFKIhmoTIPXI5Zobg==" - }, - "node_modules/instantsearch.js": { - "version": "4.66.1", - "resolved": "https://registry.npmjs.org/instantsearch.js/-/instantsearch.js-4.66.1.tgz", - "integrity": "sha512-RXFLrDSVHTBXeaGrS9Gqb6Vo1a6U0iCoDzNsJDn2kzIGjzP/SaFVLMdFW5ewAgCn9EUPmP2yImQv7mqgzmxe/g==", - "dependencies": { - "@algolia/events": "^4.0.1", - "@types/dom-speech-recognition": "^0.0.1", - "@types/google.maps": "^3.45.3", - "@types/hogan.js": "^3.0.0", - "@types/qs": "^6.5.3", - "algoliasearch-helper": "3.17.0", - "hogan.js": "^3.0.2", - "htm": "^3.0.0", - "instantsearch-ui-components": "0.4.0", - "preact": "^10.10.0", - "qs": "^6.5.1 < 6.10", - "search-insights": "^2.13.0" - }, - "peerDependencies": { - "algoliasearch": ">= 3.1 < 6" - } - }, - "node_modules/instantsearch.js/node_modules/qs": { - "version": "6.9.7", - "resolved": "https://registry.npmjs.org/qs/-/qs-6.9.7.tgz", - "integrity": "sha512-IhMFgUmuNpyRfxA90umL7ByLlgRXu6tIfKPpF5TmcfRLlLCckfP/g3IQmju6jjpu+Hh8rA+2p6A27ZSPOOHdKw==", - "engines": { - "node": ">=0.6" - }, - "funding": { - "url": "https://github.com/sponsors/ljharb" - } - }, "node_modules/internal-slot": { "version": "1.0.7", "resolved": "https://registry.npmjs.org/internal-slot/-/internal-slot-1.0.7.tgz", @@ -15734,15 +15463,6 @@ "node": ">=0.10.0" } }, - "node_modules/mkdirp": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.3.0.tgz", - "integrity": "sha512-OHsdUcVAQ6pOtg5JYWpCBo9W/GySVuwvP9hueRMW7UqshC0tbfzLv8wjySTPm3tfUZ/21CE9E1pJagOA91Pxew==", - "deprecated": "Legacy versions of mkdirp are no longer supported. Please update to mkdirp 1.x. (Note that the API surface has changed to use Promises in 1.x.)", - "engines": { - "node": "*" - } - }, "node_modules/mkdirp-classic": { "version": "0.5.3", "resolved": "https://registry.npmjs.org/mkdirp-classic/-/mkdirp-classic-0.5.3.tgz", @@ -16070,20 +15790,6 @@ "resolved": "https://registry.npmjs.org/node-releases/-/node-releases-2.0.14.tgz", "integrity": "sha512-y10wOWt8yZpqXmOgRo77WaHEmhYQYGNA6y421PKsKYWEK8aW+cqAphborZDhqfyKrbZEN92CN1X2KbafY2s7Yw==" }, - "node_modules/nopt": { - "version": "1.0.10", - "resolved": "https://registry.npmjs.org/nopt/-/nopt-1.0.10.tgz", - "integrity": "sha512-NWmpvLSqUrgrAC9HCuxEvb+PSloHpqVu+FqcO4eeF2h5qYRhA7ev6KvelyQAKtegUbC6RypJnlEOhd8vloNKYg==", - "dependencies": { - "abbrev": "1" - }, - "bin": { - "nopt": "bin/nopt.js" - }, - "engines": { - "node": "*" - } - }, "node_modules/normalize-package-data": { "version": "2.5.0", "resolved": "https://registry.npmjs.org/normalize-package-data/-/normalize-package-data-2.5.0.tgz", @@ -17577,15 +17283,6 @@ "resolved": "https://registry.npmjs.org/postcss-value-parser/-/postcss-value-parser-4.2.0.tgz", "integrity": "sha512-1NNCs6uurfkVbeXG4S8JFT9t19m45ICnif8zWLd5oPSZ50QnwMfK+H3jv408d4jw/7Bttv5axS5IiHoLaVNHeQ==" }, - "node_modules/preact": { - "version": "10.20.1", - "resolved": "https://registry.npmjs.org/preact/-/preact-10.20.1.tgz", - "integrity": "sha512-JIFjgFg9B2qnOoGiYMVBtrcFxHqn+dNXbq76bVmcaHYJFYR4lW67AOcXgAYQQTDYXDOg/kTZrKPNCdRgJ2UJmw==", - "funding": { - "type": "opencollective", - "url": "https://opencollective.com/preact" - } - }, "node_modules/prebuild-install": { "version": "7.1.2", "resolved": "https://registry.npmjs.org/prebuild-install/-/prebuild-install-7.1.2.tgz", @@ -18309,37 +18006,6 @@ "react-is": "^16.13.1" } }, - "node_modules/react-instantsearch": { - "version": "7.7.1", - "resolved": "https://registry.npmjs.org/react-instantsearch/-/react-instantsearch-7.7.1.tgz", - "integrity": "sha512-o6nLY4IZWql6m0LYFSKpPKlAZ8zV3fwnwgswGs1okdw2skb3TXB535/mQCQZF39YjrUqBc3thl/YMnEDnKtVaQ==", - "dependencies": { - "@babel/runtime": "^7.1.2", - "instantsearch-ui-components": "0.4.0", - "instantsearch.js": "4.66.1", - "react-instantsearch-core": "7.7.1" - }, - "peerDependencies": { - "algoliasearch": ">= 3.1 < 5", - "react": ">= 16.8.0 < 19", - "react-dom": ">= 16.8.0 < 19" - } - }, - "node_modules/react-instantsearch-core": { - "version": "7.7.1", - "resolved": "https://registry.npmjs.org/react-instantsearch-core/-/react-instantsearch-core-7.7.1.tgz", - "integrity": "sha512-OTvf/QtJT5zd+EQW+osjPPFNr7Vo9FAzy/zUxeeP+87IS6tiUpQQEDhgFFYBbvU5+97pYl9YmvGQARakNDHJOw==", - "dependencies": { - "@babel/runtime": "^7.1.2", - "algoliasearch-helper": "3.17.0", - "instantsearch.js": "4.66.1", - "use-sync-external-store": "^1.0.0" - }, - "peerDependencies": { - "algoliasearch": ">= 3.1 < 5", - "react": ">= 16.8.0 < 19" - } - }, "node_modules/react-intl": { "version": "6.6.4", "resolved": "https://registry.npmjs.org/react-intl/-/react-intl-6.6.4.tgz", @@ -19728,11 +19394,6 @@ "url": "https://opencollective.com/webpack" } }, - "node_modules/search-insights": { - "version": "2.13.0", - "resolved": "https://registry.npmjs.org/search-insights/-/search-insights-2.13.0.tgz", - "integrity": "sha512-Orrsjf9trHHxFRuo9/rzm0KIWmgzE8RMlZMzuhZOJ01Rnz3D0YBAe+V6473t6/H6c7irs6Lt48brULAiRWb3Vw==" - }, "node_modules/select-hose": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/select-hose/-/select-hose-2.0.0.tgz", diff --git a/package.json b/package.json index 719d5fae4e..bf198982ac 100644 --- a/package.json +++ b/package.json @@ -73,16 +73,15 @@ "email-validator": "2.0.4", "file-saver": "^2.0.5", "formik": "2.2.6", - "instantsearch.css": "^8.1.0", "jszip": "^3.10.1", "lodash": "4.17.21", + "meilisearch": "^0.38.0", "moment": "2.29.4", "prop-types": "15.7.2", "react": "17.0.2", "react-datepicker": "^4.13.0", "react-dom": "17.0.2", "react-helmet": "^6.1.0", - "react-instantsearch": "^7.7.1", "react-redux": "7.2.9", "react-responsive": "9.0.2", "react-router": "6.16.0", diff --git a/src/search-modal/ClearFiltersButton.jsx b/src/search-modal/ClearFiltersButton.jsx index 2b33c981d2..40e88d4e17 100644 --- a/src/search-modal/ClearFiltersButton.jsx +++ b/src/search-modal/ClearFiltersButton.jsx @@ -1,20 +1,20 @@ /* eslint-disable react/prop-types */ // @ts-check import React from 'react'; -import { useClearRefinements } from 'react-instantsearch'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; import { Button } from '@openedx/paragon'; import messages from './messages'; +import { useSearchContext } from './manager/SearchManager'; /** * A button that appears when at least one filter is active, and will clear the filters when clicked. * @type {React.FC>} */ const ClearFiltersButton = () => { - const { refine, canRefine } = useClearRefinements(); - if (canRefine) { + const { canClearFilters, clearFilters } = useSearchContext(); + if (canClearFilters) { return ( - ); diff --git a/src/search-modal/EmptyStates.jsx b/src/search-modal/EmptyStates.jsx index a90a294df2..14d9173269 100644 --- a/src/search-modal/EmptyStates.jsx +++ b/src/search-modal/EmptyStates.jsx @@ -1,7 +1,7 @@ /* eslint-disable react/prop-types */ // @ts-check import React from 'react'; -import { useStats, useClearRefinements } from 'react-instantsearch'; +import { useSearchContext } from './manager/SearchManager'; /** * If the user hasn't put any keywords/filters yet, display an "empty state". @@ -10,16 +10,15 @@ import { useStats, useClearRefinements } from 'react-instantsearch'; * @type {React.FC<{children: React.ReactElement}>} */ const EmptyStates = ({ children }) => { - const { nbHits, query } = useStats(); - const { canRefine: hasFiltersApplied } = useClearRefinements(); - const hasQuery = !!query; + const { canClearFilters: hasFiltersApplied, totalHits, searchKeywords } = useSearchContext(); + const hasQuery = !!searchKeywords; if (!hasQuery && !hasFiltersApplied) { // We haven't started the search yet. Display the "start your search" empty state // Note this isn't localized because it's going to be replaced in a fast-follow PR. return

Enter a keyword or select a filter to begin searching.

; } - if (nbHits === 0) { + if (totalHits === 0) { // Note this isn't localized because it's going to be replaced in a fast-follow PR. return

No results found. Change your search and try again.

; } diff --git a/src/search-modal/FilterByBlockType.jsx b/src/search-modal/FilterByBlockType.jsx index 8c7f590259..bbeda61abc 100644 --- a/src/search-modal/FilterByBlockType.jsx +++ b/src/search-modal/FilterByBlockType.jsx @@ -3,19 +3,15 @@ import React from 'react'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; import { - Button, Badge, Form, Menu, MenuItem, } from '@openedx/paragon'; -import { - useCurrentRefinements, - useRefinementList, -} from 'react-instantsearch'; import SearchFilterWidget from './SearchFilterWidget'; import messages from './messages'; import BlockTypeLabel from './BlockTypeLabel'; +import { useSearchContext } from './manager/SearchManager'; /** * A button with a dropdown that allows filtering the current search by component type (XBlock type) @@ -25,64 +21,55 @@ import BlockTypeLabel from './BlockTypeLabel'; */ const FilterByBlockType = () => { const { - items, - refine, - canToggleShowMore, - isShowingMore, - toggleShowMore, - } = useRefinementList({ attribute: 'block_type', sortBy: ['count:desc', 'name'] }); - - // Get the list of applied 'items' (selected block types to filter) in the original order that the user clicked them. - // The first choice will be shown on the button, and we don't want it to change as the user selects more options. - // (But for the dropdown menu, we always want them sorted by 'count:desc' and 'name'; not in order of selection.) - const refinementsData = useCurrentRefinements({ includedAttributes: ['block_type'] }); - const appliedItems = refinementsData.items[0]?.refinements ?? []; - // If we didn't need to preserve the order the user clicked on, the above two lines could be simplified to: - // const appliedItems = items.filter(item => item.isRefined); + blockTypes, + blockTypesFilter, + setBlockTypesFilter, + } = useSearchContext(); + // TODO: sort blockTypes first by count, then by name const handleCheckboxChange = React.useCallback((e) => { - refine(e.target.value); - }, [refine]); + setBlockTypesFilter(currentFilters => { + if (currentFilters.includes(e.target.value)) { + return currentFilters.filter(x => x !== e.target.value); + } + return [...currentFilters, e.target.value]; + }); + }, [setBlockTypesFilter]); return ( ({ label: }))} + appliedFilters={blockTypesFilter.map(blockType => ({ label: }))} label={} > item.value)} + defaultValue={blockTypesFilter} > { - items.map((item) => ( + Object.entries(blockTypes).map(([blockType, count]) => ( - {' '} - {item.count} + {' '} + {count} )) } { // Show a message if there are no options at all to avoid the impression that the dropdown isn't working - items.length === 0 ? ( + blockTypes.length === 0 ? ( ) : null } - { - canToggleShowMore && !isShowingMore - ? - : null - } ); }; diff --git a/src/search-modal/FilterByTags.jsx b/src/search-modal/FilterByTags.jsx index 4e577cfab4..4be5f62595 100644 --- a/src/search-modal/FilterByTags.jsx +++ b/src/search-modal/FilterByTags.jsx @@ -1,117 +1,199 @@ /* eslint-disable react/prop-types */ // @ts-check import React from 'react'; -import { FormattedMessage } from '@edx/frontend-platform/i18n'; +import { FormattedMessage, useIntl } from '@edx/frontend-platform/i18n'; import { - Button, Badge, Form, + Icon, + IconButton, Menu, MenuItem, + SearchField, } from '@openedx/paragon'; -import { useHierarchicalMenu } from 'react-instantsearch'; +import { ArrowDropDown, ArrowDropUp } from '@openedx/paragon/icons'; import SearchFilterWidget from './SearchFilterWidget'; import messages from './messages'; +import { useSearchContext } from './manager/SearchManager'; +import { useTagFilterOptions } from './data/apiHooks'; +import { LoadingSpinner } from '../generic/Loading'; +import { TAG_SEP } from './data/api'; -// eslint-disable-next-line max-len -/** @typedef {import('instantsearch.js/es/connectors/hierarchical-menu/connectHierarchicalMenu').HierarchicalMenuItem} HierarchicalMenuItem */ +/** + * A menu item with a checkbox and an optional ▼ button (to show/hide children) + * @type {React.FC<{ + * label: string; + * tagPath: string; + * isChecked: boolean; + * onClickCheckbox: () => void; + * tagCount: number; + * hasChildren?: boolean; + * isExpanded?: boolean; + * onToggleChildren?: (tagPath: string) => void; + * }>} + */ +const TagMenuItem = ({ + label, + tagPath, + tagCount, + isChecked, + onClickCheckbox, + hasChildren, + isExpanded, + onToggleChildren, +}) => { + const intl = useIntl(); + const randomNumber = React.useMemo(() => Math.floor(Math.random() * 1000), []); + const checkboxId = tagPath.replace(/[\W]/g, '_') + randomNumber; + + return ( +
+ + + { + hasChildren + ? ( + onToggleChildren?.(tagPath)} + variant="primary" + size="sm" + /> + ) : null + } +
+ ); +}; /** - * A button with a dropdown menu to allow filtering the search using tags. - * This version is based on Instantsearch's component, so it only allows selecting one tag at a - * time. We will replace it with a custom version that allows multi-select. + * A list of menu items with all of the options for tags at one level of the hierarchy. * @type {React.FC<{ - * items: HierarchicalMenuItem[], - * refine: (value: string) => void, - * depth?: number, + * tagSearchKeywords: string; + * parentTagPath?: string; + * toggleTagChildren?: (tagPath: string) => void; + * expandedTags: string[], * }>} */ -const FilterOptions = ({ items, refine, depth = 0 }) => { - const handleCheckboxChange = React.useCallback((e) => { - refine(e.target.value); - }, [refine]); +const TagOptions = ({ + parentTagPath = '', + tagSearchKeywords, + expandedTags, + toggleTagChildren, +}) => { + const searchContext = useSearchContext(); + const { data: tagOptions, isLoading, isError } = useTagFilterOptions({ + ...searchContext, + parentTagPath, + tagSearchKeywords, + }); + + if (isError) { + return ; + } + if (isLoading || tagOptions === undefined) { + return ; + } + + // Show a message if there are no options at all to avoid the impression that the dropdown isn't working + if (Object.keys(tagOptions).length === 0 && !parentTagPath) { + return ; + } return ( - <> +
{ - items.map((item) => ( - - - {item.label}{' '} - {item.count} - - {item.data && } - - )) + tagOptions.map(({ tagName, tagPath, ...t }) => { + const isExpanded = expandedTags.includes(tagPath); + return ( + + { + searchContext.setTagsFilter((tf) => ( + tf.includes(tagPath) ? tf.filter(tp => tp !== tagPath) : [...tf, tagPath] + )); + }} + hasChildren={t.hasChildren} + isExpanded={isExpanded} + onToggleChildren={toggleTagChildren} + /> + {isExpanded ? ( +
+ +
+ ) : null} + + ); + }) } - +
); }; /** @type {React.FC} */ const FilterByTags = () => { - const { - items, - refine, - canToggleShowMore, - isShowingMore, - toggleShowMore, - } = useHierarchicalMenu({ - attributes: [ - 'tags.taxonomy', - 'tags.level0', - 'tags.level1', - 'tags.level2', - 'tags.level3', - ], - }); + const intl = useIntl(); + const { tagsFilter } = useSearchContext(); + const [tagSearchKeywords, setTagSearchKeywords] = React.useState(''); - // Recurse over the 'items' tree and find all the selected leaf tags - (with no children that are checked/"refined") - const appliedItems = React.useMemo(() => { - /** @type {{label: string}[]} */ - const result = []; - /** @type {(itemSet: HierarchicalMenuItem[]) => void} */ - const findSelectedLeaves = (itemSet) => { - itemSet.forEach(item => { - if (item.isRefined && item.data?.find(child => child.isRefined) === undefined) { - result.push({ label: item.label }); - } - if (item.data) { - findSelectedLeaves(item.data); - } - }); - }; - findSelectedLeaves(items); - return result; - }, [items]); + // e.g. {"Location", "Location > North America"} if those two paths of the tag tree are expanded + const [expandedTags, setExpandedTags] = React.useState(/** @type {string[]} */([])); + const toggleTagChildren = React.useCallback(tagWithLineage => { + setExpandedTags(currentList => { + if (currentList.includes(tagWithLineage)) { + return currentList.filter(x => x !== tagWithLineage); + } + return [...currentList, tagWithLineage]; + }); + }, [setExpandedTags]); return ( ({ label: tf.split(TAG_SEP).pop() }))} label={} > - + + setTagSearchKeywords('')} + value={tagSearchKeywords} + placeholder={intl.formatMessage(messages.searchTagsByKeywordPlaceholder)} + className="mx-3 mb-1" + /> - - { - // Show a message if there are no options at all to avoid the impression that the dropdown isn't working - items.length === 0 ? ( - - ) : null - } + - { - canToggleShowMore && !isShowingMore - ? - : null - } ); }; diff --git a/src/search-modal/Highlight.jsx b/src/search-modal/Highlight.jsx new file mode 100644 index 0000000000..07ea1debb8 --- /dev/null +++ b/src/search-modal/Highlight.jsx @@ -0,0 +1,26 @@ +/* eslint-disable react/no-array-index-key */ +/* eslint-disable react/prop-types */ +// @ts-check +import React from 'react'; + +/** + * Render some text that contains ... highlights + * @type {React.FC<{text: string}>} + */ +const Highlight = ({ text }) => { + const parts = text.split(''); + return ( + + {parts.map((part, idx) => { + if (idx === 0) { return {part}; } + const endIdx = part.indexOf(''); + if (endIdx === -1) { return {part}; } + const highLightPart = part.substring(0, endIdx); + const otherPart = part.substring(endIdx + 7); + return {highLightPart}{otherPart}; + })} + + ); +}; + +export default Highlight; diff --git a/src/search-modal/SearchEndpointLoader.jsx b/src/search-modal/SearchEndpointLoader.jsx deleted file mode 100644 index 664a3b5e03..0000000000 --- a/src/search-modal/SearchEndpointLoader.jsx +++ /dev/null @@ -1,41 +0,0 @@ -/* eslint-disable react/prop-types */ -// @ts-check -import React from 'react'; -import { ModalDialog } from '@openedx/paragon'; -import { ErrorAlert } from '@edx/frontend-lib-content-components'; -import { useIntl } from '@edx/frontend-platform/i18n'; - -import { LoadingSpinner } from '../generic/Loading'; -import { useContentSearch } from './data/apiHooks'; -import SearchUI from './SearchUI'; -import messages from './messages'; - -/** @type {React.FC<{courseId: string}>} */ -const SearchEndpointLoader = ({ courseId }) => { - const intl = useIntl(); - - // Load the Meilisearch connection details from the LMS: the URL to use, the index name, and an API key specific - // to us (to the current user) that allows us to search all content we have permission to view. - const { - data: searchEndpointData, - isLoading, - error, - } = useContentSearch(); - - const title = intl.formatMessage(messages.title); - - if (searchEndpointData) { - return ; - } - return ( - <> - {title} - - {/* @ts-ignore */} - {isLoading ? : {error?.message ?? String(error)}} - - - ); -}; - -export default SearchEndpointLoader; diff --git a/src/search-modal/SearchKeywordsField.jsx b/src/search-modal/SearchKeywordsField.jsx index 15614d8c8b..809dd7b430 100644 --- a/src/search-modal/SearchKeywordsField.jsx +++ b/src/search-modal/SearchKeywordsField.jsx @@ -1,25 +1,25 @@ /* eslint-disable react/prop-types */ // @ts-check import React from 'react'; -import { useSearchBox } from 'react-instantsearch'; import { useIntl } from '@edx/frontend-platform/i18n'; import { SearchField } from '@openedx/paragon'; import messages from './messages'; +import { useSearchContext } from './manager/SearchManager'; /** * The "main" input field where users type in search keywords. The search happens as they type (no need to press enter). - * @type {React.FC} + * @type {React.FC<{className?: string}>} */ const SearchKeywordsField = (props) => { const intl = useIntl(); - const { query, refine } = useSearchBox(props); + const { searchKeywords, setSearchKeywords } = useSearchContext(); return ( refine('')} - value={query} + onSubmit={setSearchKeywords} + onChange={setSearchKeywords} + onClear={() => setSearchKeywords('')} + value={searchKeywords} className={props.className} placeholder={intl.formatMessage(messages.inputPlaceholder)} /> diff --git a/src/search-modal/SearchModal.jsx b/src/search-modal/SearchModal.jsx index 93fce12720..e8a8151534 100644 --- a/src/search-modal/SearchModal.jsx +++ b/src/search-modal/SearchModal.jsx @@ -4,8 +4,8 @@ import React from 'react'; import { ModalDialog } from '@openedx/paragon'; import { useIntl } from '@edx/frontend-platform/i18n'; -import SearchEndpointLoader from './SearchEndpointLoader'; import messages from './messages'; +import SearchUI from './SearchUI'; /** @type {React.FC<{courseId: string, isOpen: boolean, onClose: () => void}>} */ const SearchModal = ({ courseId, ...props }) => { @@ -24,7 +24,7 @@ const SearchModal = ({ courseId, ...props }) => { isFullscreenOnMobile className="courseware-search-modal" > - + ); }; diff --git a/src/search-modal/SearchModal.scss b/src/search-modal/SearchModal.scss index 33c689a6e8..a8b01ce713 100644 --- a/src/search-modal/SearchModal.scss +++ b/src/search-modal/SearchModal.scss @@ -28,28 +28,9 @@ // Options for the "filter by tag" menu .pgn__menu.tags-refinement-menu { - $indent-initial: 1.3rem; - $indent-each: 1.6rem; - - .tag-option-1 { - padding-left: $indent-initial + (1 * $indent-each); - } - - .tag-option-2 { - padding-left: $indent-initial + (2 * $indent-each); - } - - .tag-option-3 { - padding-left: $indent-initial + (3 * $indent-each); - } - - .tag-option-4 { - padding-left: $indent-initial + (4 * $indent-each); - } - .pgn__menu-item { // Make the "filter by tag" menu much wider than normal, because we need the space to display the tags hierarchy - width: 400px; + width: 100%; } } diff --git a/src/search-modal/SearchResult.jsx b/src/search-modal/SearchResult.jsx index cb28172eac..d2b572f76f 100644 --- a/src/search-modal/SearchResult.jsx +++ b/src/search-modal/SearchResult.jsx @@ -1,34 +1,27 @@ /* eslint-disable react/prop-types */ // @ts-check import React from 'react'; -import { Highlight } from 'react-instantsearch'; import BlockTypeLabel from './BlockTypeLabel'; +import Highlight from './Highlight'; /** * A single search result (row), usually represents an XBlock/Component - * @type {React.FC<{hit: import('instantsearch.js').Hit<{ - * id: string, - * display_name: string, - * block_type: string, - * 'content.html_content'?: string, - * 'content.capa_content'?: string, - * breadcrumbs: {display_name: string}[]}>, - * }>} + * @type {React.FC<{hit: import('./data/api').ContentHit}>} */ const SearchResult = ({ hit }) => (
- {' '} - () + {' '} + ()
- - + +
{hit.breadcrumbs.map((bc, i) => ( // eslint-disable-next-line react/no-array-index-key - {bc.display_name} {i !== hit.breadcrumbs.length - 1 ? '/' : ''} + {bc.displayName} {i !== hit.breadcrumbs.length - 1 ? '/' : ''} ))}
diff --git a/src/search-modal/SearchResults.jsx b/src/search-modal/SearchResults.jsx new file mode 100644 index 0000000000..8129241519 --- /dev/null +++ b/src/search-modal/SearchResults.jsx @@ -0,0 +1,17 @@ +/* eslint-disable react/prop-types */ +// @ts-check +import React from 'react'; +import { useSearchContext } from './manager/SearchManager'; +import SearchResult from './SearchResult'; + +/** + * A single search result (row), usually represents an XBlock/Component + * @type {React.FC>} + */ +const SearchResults = () => { + const { hits } = useSearchContext(); + // TODO: pagination + return <>{hits.map((hit) => )}; +}; + +export default SearchResults; diff --git a/src/search-modal/SearchUI.jsx b/src/search-modal/SearchUI.jsx index 8becfbe64d..e1258e9794 100644 --- a/src/search-modal/SearchUI.jsx +++ b/src/search-modal/SearchUI.jsx @@ -8,25 +8,19 @@ import { } from '@openedx/paragon'; import { Check } from '@openedx/paragon/icons'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; -import { Configure, InfiniteHits, InstantSearch } from 'react-instantsearch'; -import { instantMeiliSearch } from '@meilisearch/instant-meilisearch'; import ClearFiltersButton from './ClearFiltersButton'; import EmptyStates from './EmptyStates'; -import SearchResult from './SearchResult'; +import SearchResults from './SearchResults'; import SearchKeywordsField from './SearchKeywordsField'; import FilterByBlockType from './FilterByBlockType'; import FilterByTags from './FilterByTags'; import Stats from './Stats'; +import { SearchContextProvider } from './manager/SearchManager'; import messages from './messages'; -/** @type {React.FC<{courseId: string, url: string, apiKey: string, indexName: string}>} */ +/** @type {React.FC<{courseId: string}>} */ const SearchUI = (props) => { - const { searchClient } = React.useMemo( - () => instantMeiliSearch(props.url, props.apiKey, { primaryKey: 'id' }), - [props.url, props.apiKey], - ); - const hasCourseId = Boolean(props.courseId); const [_searchThisCourseEnabled, setSearchThisCourse] = React.useState(hasCourseId); const switchToThisCourse = React.useCallback(() => setSearchThisCourse(true), []); @@ -34,14 +28,7 @@ const SearchUI = (props) => { const searchThisCourse = hasCourseId && _searchThisCourseEnabled; return ( - - {/* Add in a filter for the current course, if relevant */} - + {/* We need to override z-index here or the appears behind the * But it can't be more then 9 because the close button has z-index 10. */} @@ -77,10 +64,10 @@ const SearchUI = (props) => { {/* If there are no results (yet), EmptyStates displays a friendly messages. Otherwise we see the results. */} - + - + ); }; diff --git a/src/search-modal/Stats.jsx b/src/search-modal/Stats.jsx index fabfe76a4f..63bcd8f4f5 100644 --- a/src/search-modal/Stats.jsx +++ b/src/search-modal/Stats.jsx @@ -1,26 +1,24 @@ /* eslint-disable react/prop-types */ // @ts-check import React from 'react'; -import { useStats, useClearRefinements } from 'react-instantsearch'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; import messages from './messages'; +import { useSearchContext } from './manager/SearchManager'; /** * Simple component that displays the # of matching results * @type {React.FC>} */ -const Stats = (props) => { - const { nbHits, query } = useStats(props); - const { canRefine: hasFiltersApplied } = useClearRefinements(); - const hasQuery = !!query; +const Stats = () => { + const { totalHits, searchKeywords, canClearFilters } = useSearchContext(); - if (!hasQuery && !hasFiltersApplied) { + if (!searchKeywords && !canClearFilters) { // We haven't started the search yet. return null; } return ( - + ); }; diff --git a/src/search-modal/data/api.js b/src/search-modal/data/api.js index ce306ee49e..0613a7c1cb 100644 --- a/src/search-modal/data/api.js +++ b/src/search-modal/data/api.js @@ -1,5 +1,5 @@ // @ts-check -import { getConfig } from '@edx/frontend-platform'; +import { camelCaseObject, getConfig } from '@edx/frontend-platform'; import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; export const getContentSearchConfigUrl = () => new URL( @@ -7,6 +7,9 @@ export const getContentSearchConfigUrl = () => new URL( getConfig().STUDIO_BASE_URL, ).href; +/** The separator used for hierarchical tags in the search index, e.g. tags.level1 = "Subject > Math > Calculus" */ +export const TAG_SEP = ' > '; + /** * Get the content search configuration from the CMS. * @@ -21,3 +24,344 @@ export const getContentSearchConfig = async () => { apiKey: response.data.api_key, }; }; + +/** + * Detailed "content" of an XBlock/component, from the block's index_dictionary function. Contents depends on the type. + * @typedef {{htmlContent?: string, capaContent?: string, [k: string]: any}} ContentDetails + */ + +/** + * Meilisearch filters can be expressed as strings or arrays. + * This helper method converts from any supported input format to an array, for consistency. + * @param {import('meilisearch').Filter} [filter] A filter expression, e.g. 'foo = bar' or [['a = b', 'a = c'], 'd = e'] + * @returns {(string | string[])[]} + */ +function forceArray(filter) { + if (typeof filter === 'string') { + return [filter]; + } + if (filter === undefined) { + return []; + } + return filter; +} + +/** + * Given tag paths like ["Difficulty > Hard", "Subject > Math"], convert them to an array of Meilisearch + * filter conditions. The tag filters are all AND conditions (not OR). + * @param {string[]} [tagsFilter] e.g. ["Difficulty > Hard", "Subject > Math"] + * @returns {string[]} + */ +function formatTagsFilter(tagsFilter) { + /** @type {string[]} */ + const filters = []; + + tagsFilter?.forEach((tagPath) => { + const parts = tagPath.split(TAG_SEP); + if (parts.length === 1) { + filters.push(`tags.taxonomy = "${tagPath}"`); + } else { + filters.push(`tags.level${parts.length - 2} = "${tagPath}"`); + } + }); + + return filters; +} + +/** + * Information about a single XBlock returned in the search results + * Defined in edx-platform/openedx/core/djangoapps/content/search/documents.py + * @typedef {Object} ContentHit + * @property {string} id + * @property {string} usageKey + * @property {"course_block"|"library_block"} type + * @property {string} block_id + * @property {string} displayName + * @property {string} blockType The block_type part of the usage key. What type of XBlock this is. + * @property {string} contextKey The course or library ID + * @property {string} org + * @property {{displayName: string}[]} breadcrumbs First one is the name of the course/library itself. + * After that is the name of any parent Section/Subsection/Unit/etc. + * @property {Record<'taxonomy'|'level0'|'level1'|'level2'|'level3', string[]>} tags + * @property {ContentDetails} [content] + * @property {{displayName: string, content: ContentDetails}} formatted Same fields with ... highlights + */ + +/** + * Convert search hits to camelCase + * @param {Record} hit A search result directly from Meilisearch + * @returns {ContentHit} + */ +function formatSearchHit(hit) { + const { _formatted, ...newHit } = hit; + newHit.formatted = { + displayName: _formatted.display_name, + content: _formatted.content ?? {}, + }; + return camelCaseObject(newHit); +} + +/** + * @param {{ + * client: import('meilisearch').MeiliSearch, + * indexName: string, + * searchKeywords: string, + * blockTypesFilter?: string[], + * tagsFilter?: string[], + * extraFilter?: import('meilisearch').Filter, + * }} context + * @returns {Promise<{ + * hits: ContentHit[], + * totalHits: number, + * blockTypes: Record, + * }>} + */ +export async function fetchSearchResults({ + client, + indexName, + searchKeywords, + blockTypesFilter, + /** The full path of tags that each result MUST have, e.g. ["Difficulty > Hard", "Subject > Math"] */ + tagsFilter, + extraFilter, +}) { + /** @type {import('meilisearch').MultiSearchQuery[]} */ + const queries = []; + + // Convert 'extraFilter' into an array + const extraFilterFormatted = forceArray(extraFilter); + + const blockTypesFilterFormatted = blockTypesFilter?.length ? [blockTypesFilter.map(bt => `block_type = ${bt}`)] : []; + + const tagsFilterFormatted = formatTagsFilter(tagsFilter); + + // First query is always to get the hits, with all the filters applied. + queries.push({ + indexUid: indexName, + q: searchKeywords, + filter: [ + // top-level entries in the array are AND conditions and must all match + // Inner arrays are OR conditions, where only one needs to match. + ...extraFilterFormatted, + ...blockTypesFilterFormatted, + ...tagsFilterFormatted, + ], + attributesToHighlight: ['display_name', 'content'], + highlightPreTag: '', + highlightPostTag: '', + }); + + // The second query is to get the possible values for the "block types" filter + queries.push({ + indexUid: indexName, + q: searchKeywords, + facets: ['block_type'], + filter: [ + ...extraFilterFormatted, + // We exclude the block type filter here so we get all the other available options for it. + ...tagsFilterFormatted, + ], + limit: 0, // We don't need any "hits" for this - just the facetDistribution + }); + + const { results } = await client.multiSearch(({ queries })); + return { + hits: results[0].hits.map(formatSearchHit), + totalHits: results[0].totalHits ?? results[0].estimatedTotalHits ?? results[0].hits.length, + blockTypes: results[1].facetDistribution?.block_type ?? {}, + }; +} + +/** + * In the context of a particular search (which may already be filtered to a specific course, specific block types, + * and/or have a keyword search applied), get the tree of tags that can be used to further filter/refine the search. + * + * @param {object} context + * @param {import('meilisearch').MeiliSearch} context.client The Meilisearch client instance + * @param {string} context.indexName Which index to search + * @param {string} context.searchKeywords Overall query string for the search; may be empty + * @param {string[]} [context.blockTypesFilter] Filter to only include these block types e.g. ["problem", "html"] + * @param {import('meilisearch').Filter} [context.extraFilter] Any other filters to apply, e.g. course ID. + * @param {string} [context.parentTagPath] Only fetch tags below this parent tag/taxonomy e.g. "Places > North America" + * @returns {Promise<{tagName: string, tagPath: string, tagCount: number, hasChildren: boolean}[]>} + */ +export async function fetchAvailableTagOptions({ + client, + indexName, + searchKeywords, + blockTypesFilter, + extraFilter, + parentTagPath, + // Ideally this would include 'tagSearchKeywords' to filter the tag tree by keyword search but that's not possible yet +}) { + // Convert 'extraFilter' into an array + const extraFilterFormatted = forceArray(extraFilter); + + const blockTypesFilterFormatted = blockTypesFilter?.length ? [blockTypesFilter.map(bt => `block_type = ${bt}`)] : []; + + // Figure out which "facet" (attribute of the documents in the search index) holds the tags at the level we want. + // e.g. "tags.taxonomy" is the facet/attribute that holds the root tags, and "tags.level0" has its child tags. + let facetName; + let depth; + /** @type {string[]} */ + let parentFilter = []; + if (!parentTagPath) { + facetName = 'tags.taxonomy'; + depth = 0; + } else { + const parentParts = parentTagPath.split(TAG_SEP); + depth = parentParts.length; + facetName = `tags.level${depth - 1}`; + const parentFacetName = parentParts.length === 1 ? 'tags.taxonomy' : `tags.level${parentParts.length - 2}`; + parentFilter = [`${parentFacetName} = "${parentTagPath}"`]; + } + + const { facetDistribution } = await client.index(indexName).search(searchKeywords, { + facets: [facetName], + filter: [...extraFilterFormatted, ...blockTypesFilterFormatted, ...parentFilter], + limit: 0, + }); + if (facetDistribution === undefined) { + throw new Error('Unexpectedly missing facetDistribution in Meilisearch result'); + } + + /** @type {{tagName: string, tagPath: string, tagCount: number, hasChildren: boolean}[]} */ + const tagData = []; + const { facetHits } = await client.index(indexName).searchForFacetValues({ + facetName, + // It's not super clear in the documentation, but facetQuery is basically a "startsWith" query, which is what we + // need here to return just the tags below the selected parent tag. However, it's a fuzzy query that may match + // more tags than we want it to, so we have to explicitly post-process and reduce the set of results using an + // exact match. + facetQuery: parentTagPath, + q: searchKeywords, + filter: [...extraFilterFormatted, ...blockTypesFilterFormatted, ...parentFilter], + }); + facetHits.forEach(({ value: tagPath, count: tagCount }) => { + if (!parentTagPath) { + tagData.push({ + tagName: tagPath, + tagPath, + tagCount, + hasChildren: true, // You can't tag something with just a taxonomy, so this definitely has child tags. + }); + } else { + const parts = tagPath.split(TAG_SEP); + const tagName = parts[parts.length - 1]; + if (tagPath === `${parentTagPath}${TAG_SEP}${tagName}`) { + tagData.push({ + tagName, + tagPath, + tagCount, + hasChildren: false, // We'll set this later + }); + } // Else this is a tag from another taxonomy/parent that was included because this search is "fuzzy". Ignore it. + } + }); + + // Figure out if [some of] the tags at this level have children: + if (depth > 0 && depth < 4) { + const nextLevelFacet = `tags.level${depth}`; // This will give the children of the current tags. + // Retrieve the children of the current tags: + const { facetHits: childFacetHits } = await client.index(indexName).searchForFacetValues({ + facetName: nextLevelFacet, + facetQuery: parentTagPath, + q: searchKeywords, + filter: [...extraFilterFormatted, ...blockTypesFilterFormatted, ...parentFilter], + }); + const meilisearchFacetLimit = 100; // The 'maxValuesPerFacet' on the index. For Open edX we leave the default, 100. + if (childFacetHits.length >= meilisearchFacetLimit) { + // Assume they all have child tags; we can't retrieve more than 100 facet values (per Meilisearch docs) so + // we can't say for sure on a tag-by-tag basis, but we know that at least some of them have children, so + // it's a safe bet that most/all of them have children. And it's not a huge problem if we say they have children + // but they don't. + // eslint-disable-next-line no-param-reassign + tagData.forEach((t) => { t.hasChildren = true; }); + } else if (childFacetHits.length > 0) { + // Some (or maybe all) of these tags have child tags. Let's figure out which ones exactly. + /** @type {Set} */ + const tagsWithChildren = new Set(); + childFacetHits.forEach(({ value }) => { + // Trim the child tag off: 'Places > North America > New York' becomes 'Places > North America' + const tagPath = value.split(TAG_SEP).slice(0, -1).join(TAG_SEP); + tagsWithChildren.add(tagPath); + }); + // eslint-disable-next-line no-param-reassign + tagData.forEach((t) => { t.hasChildren = tagsWithChildren.has(t.tagPath); }); + } + } + + return tagData; +} + +/** + * Best-effort search for *all* tags among the search results (with filters applied) that contain the given keyword. + * + * Unfortunately there is no good Meilisearch API for this, so we just have to do the best we can. If more than 1,000 + * objects are tagged with matching tags, this will be an incomplete result. For example, if 1,000 XBlocks/components + * are tagged with "Tag Alpha 1" and 10 XBlocks are tagged with "Tag Alpha 2", a search for "Alpha" may only return + * ["Tag Alpha 1"] instead of the correct result ["Tag Alpha 1", "Tag Alpha 2"] because we are limited to 1,000 matches, + * which may all have the same tags. + * + * @param {object} context + * @param {import('meilisearch').MeiliSearch} context.client The Meilisearch client instance + * @param {string} context.indexName Which index to search + * @param {string[]} [context.blockTypesFilter] Filter to only include these block types e.g. ["problem", "html"] + * @param {import('meilisearch').Filter} [context.extraFilter] Any other filters to apply to the overall search. + * @param {string} [context.tagSearchKeywords] Only show taxonomies/tags that match these keywords + * @returns {Promise<{ mayBeMissingResults: boolean; matches: {tagPath: string}[] }>} + */ +export async function fetchTagsThatMatchKeyword({ + client, + indexName, + blockTypesFilter, + extraFilter, + tagSearchKeywords, +}) { + if (!tagSearchKeywords || tagSearchKeywords.trim() === '') { + // This data isn't needed if there is no tag keyword search. Don't bother making a search query. + return { matches: [], mayBeMissingResults: false }; + } + // Convert 'extraFilter' into an array + const extraFilterFormatted = forceArray(extraFilter); + + const blockTypesFilterFormatted = blockTypesFilter?.length ? [blockTypesFilter.map(bt => `block_type = ${bt}`)] : []; + + const limit = 1000; // This is the most results we can retrieve in a single query. + + // We search for any matches of the keyword in the "tags" field, respecting the current filters like block type filter + // or current course filter. (Unfortunately we cannot also include the overall `searchKeywords` so this will match + // against more content than it should.) + const { hits } = await client.index(indexName).search(tagSearchKeywords, { + filter: [...extraFilterFormatted, ...blockTypesFilterFormatted], + attributesToSearchOn: ['tags.taxonomy', 'tags.level0', 'tags.level1', 'tags.level2', 'tags.level3'], + attributesToRetrieve: ['tags'], + limit, + // We'd like to use 'showMatchesPosition: true' to know exaclty which tags match, but it doesn't provide the + // detail we need; it's impossible to tell whihc tag at a given level matched based on the returned _matchesPosition + // data - https://github.com/orgs/meilisearch/discussions/550 + }); + + const tagSearchKeywordsLower = tagSearchKeywords.toLocaleLowerCase(); + + /** @type {Set} */ + const matches = new Set(); + + // We have data like this: + // hits: [ + // { + // tags: { taxonomy: "Competency", "level0": "Competency > Abilities", "level1": "Competency > Abilities > ..." }, + // }, ... + // ] + hits.forEach((hit) => { + Object.values(hit.tags).forEach((tagPathList) => { + tagPathList.forEach((tagPath) => { + if (tagPath.toLocaleLowerCase().includes(tagSearchKeywordsLower)) { + matches.add(tagPath); + } + }); + }); + }); + + return { matches: Array.from(matches).map((tagPath) => ({ tagPath })), mayBeMissingResults: hits.length === limit }; +} diff --git a/src/search-modal/data/apiHooks.js b/src/search-modal/data/apiHooks.js index 36e5c2e12f..a4322cf78c 100644 --- a/src/search-modal/data/apiHooks.js +++ b/src/search-modal/data/apiHooks.js @@ -1,16 +1,21 @@ // @ts-check - +import React from 'react'; import { useQuery } from '@tanstack/react-query'; -import { getContentSearchConfig } from './api'; +import { + TAG_SEP, + fetchAvailableTagOptions, + fetchSearchResults, + fetchTagsThatMatchKeyword, + getContentSearchConfig, +} from './api'; /** - * Load the Meilisearch connection details from the CMS: the URL to use, the index name, and an API key specific - * to the current user that allows it to search all content he have permission to view. - * - */ -/* eslint-disable import/prefer-default-export */ -export const useContentSearch = () => ( + * Load the Meilisearch connection details from the CMS: the URL to use, the index name, and an API key specific + * to the current user that allows it to search all content he have permission to view. + * + */ +export const useContentSearchConnection = () => ( useQuery({ queryKey: ['content_search'], queryFn: getContentSearchConfig, @@ -21,3 +26,137 @@ export const useContentSearch = () => ( refetchOnMount: false, }) ); + +/** + * Get the results of a search + * @param {object} context + * @param {import('meilisearch').MeiliSearch} [context.client] The Meilisearch API client + * @param {string} [context.indexName] Which search index contains the content data + * @param {import('meilisearch').Filter} [context.extraFilter] Other filters to apply to the search, e.g. course ID + * @param {string} context.searchKeywords The keywords that the user is searching for, if any + * @param {string[]} context.blockTypesFilter Only search for these block types (e.g. ["html", "problem"]) + * @param {string[]} context.tagsFilter Required tags (all must match), e.g. ["Difficulty > Hard", "Subject > Math"] + */ +export const useContentSearchResults = ({ + client, + indexName, + extraFilter, + searchKeywords, + blockTypesFilter, + tagsFilter, +}) => ( + useQuery({ + enabled: client !== undefined && indexName !== undefined, + queryKey: [ + 'content_search', + 'results', + client?.config.apiKey, + client?.config.host, + indexName, + extraFilter, + searchKeywords, + blockTypesFilter, + tagsFilter, + ], + queryFn: () => { + if (client === undefined || indexName === undefined) { + throw new Error('Required data unexpectedly undefined. Check "enable" condition of useQuery.'); + } + return fetchSearchResults({ + client, + extraFilter, + indexName, + searchKeywords, + blockTypesFilter, + tagsFilter, + }); + }, + // Avoid flickering results when user is typing... keep old results until new is available. + keepPreviousData: true, + }) +); + +/** + * Get the available tags that can be used to refine a search, based on the search filters applied so far. + * Also the user can use a keyword search to find specific tags. + * @param {object} args + * @param {import('meilisearch').MeiliSearch} [args.client] The Meilisearch client instance + * @param {string} [args.indexName] Which index to search + * @param {string} args.searchKeywords Overall query string for the search; may be empty + * @param {string[]} [args.blockTypesFilter] Filter to only include these block types e.g. ["problem", "html"] + * @param {import('meilisearch').Filter} [args.extraFilter] Any other filters to apply to the overall search. + * @param {string} [args.tagSearchKeywords] Only show taxonomies/tags that match these keywords + * @param {string} [args.parentTagPath] Only fetch tags below this parent tag/taxonomy e.g. "Places > North America" + */ +export const useTagFilterOptions = (args) => { + const mainQuery = useQuery({ + enabled: args.client !== undefined && args.indexName !== undefined, + queryKey: [ + 'content_search', + 'tag_filter_options', + args.client?.config.apiKey, + args.client?.config.host, + args.indexName, + args.extraFilter, + args.searchKeywords, + args.blockTypesFilter, + args.parentTagPath, + args.tagSearchKeywords, + ], + queryFn: () => { + const { client, indexName } = args; + if (client === undefined || indexName === undefined) { + throw new Error('Required data unexpectedly undefined. Check "enable" condition of useQuery.'); + } + return fetchAvailableTagOptions({ ...args, client, indexName }); + }, + // Avoid flickering results when user is typing... keep old results until new is available. + keepPreviousData: true, + }); + + const tagKeywordSearchData = useQuery({ + enabled: args.client !== undefined && args.indexName !== undefined, + queryKey: [ + 'content_search', + 'tags_keyword_search_dat', + args.client?.config.apiKey, + args.client?.config.host, + args.indexName, + args.extraFilter, + args.blockTypesFilter, + args.tagSearchKeywords, + ], + queryFn: () => { + const { client, indexName } = args; + if (client === undefined || indexName === undefined) { + throw new Error('Required data unexpectedly undefined. Check "enable" condition of useQuery.'); + } + return fetchTagsThatMatchKeyword({ ...args, client, indexName }); + }, + // Avoid flickering results when user is typing... keep old results until new is available. + keepPreviousData: true, + }); + + const data = React.useMemo(() => { + if (!args.tagSearchKeywords || !tagKeywordSearchData.data) { + // If there's no keyword search being used to filter the list of available tags, just use the results of the + // main query. + return mainQuery.data; + } + if (mainQuery.data === undefined) { + return undefined; + } + // Combine these two queries to filter the list of tags based on the keyword search. + return mainQuery.data.filter(({ tagPath }) => { + // eslint-disable-next-line no-restricted-syntax + for (const matchingTag of tagKeywordSearchData.data.matches) { + if (matchingTag.tagPath === tagPath || matchingTag.tagPath.startsWith(tagPath + TAG_SEP)) { + return true; + } + } + return false; + }); + }, [mainQuery.data, tagKeywordSearchData.data]); + + return { ...mainQuery, data }; +}; diff --git a/src/search-modal/manager/SearchManager.js b/src/search-modal/manager/SearchManager.js new file mode 100644 index 0000000000..7fb88910d7 --- /dev/null +++ b/src/search-modal/manager/SearchManager.js @@ -0,0 +1,94 @@ +/* eslint-disable react/prop-types */ +// @ts-check +/** + * This is a search manager that provides search functionality similar to the + * Instantsearch library. We use it because Instantsearch doesn't support + * multiple selections of hierarchical tags. + * https://github.com/algolia/instantsearch/issues/1658 + */ +import React from 'react'; +import { MeiliSearch } from 'meilisearch'; + +import { useContentSearchConnection, useContentSearchResults } from '../data/apiHooks'; + +/** + * @type {React.Context>, + * blockTypesFilter: string[], + * setBlockTypesFilter: React.Dispatch>, + * tagsFilter: string[], + * setTagsFilter: React.Dispatch>, + * blockTypes: Record, + * extraFilter?: import('meilisearch').Filter, + * canClearFilters: boolean, + * clearFilters: () => void, + * hits: import('../data/api').ContentHit[], + * totalHits: number, + * }>} + */ +const SearchContext = /** @type {any} */(React.createContext(undefined)); + +/** @type {React.FC<{ extraFilter?: import('meilisearch').Filter, children: React.ReactNode }>} */ +export const SearchContextProvider = ({ extraFilter, children }) => { + const [searchKeywords, setSearchKeywords] = React.useState(''); + const [blockTypesFilter, setBlockTypesFilter] = React.useState(/** type {string[]} */([])); + const [tagsFilter, setTagsFilter] = React.useState(/** type {string[]} */([])); + + const canClearFilters = blockTypesFilter.length > 0 || tagsFilter.length > 0; + const clearFilters = React.useCallback(() => { + setBlockTypesFilter([]); + setTagsFilter([]); + }, []); + + // Initialize a connection to Meilisearch: + const { data: connectionDetails } = useContentSearchConnection(); + const indexName = connectionDetails?.indexName; + const client = React.useMemo(() => { + if (connectionDetails?.apiKey === undefined || connectionDetails?.url === undefined) { + return undefined; + } + return new MeiliSearch({ host: connectionDetails.url, apiKey: connectionDetails.apiKey }); + }, [connectionDetails?.apiKey, connectionDetails?.url]); + + // Run the search + const result = useContentSearchResults({ + client, + indexName, + extraFilter, + searchKeywords, + blockTypesFilter, + tagsFilter, + }); + + return React.createElement(SearchContext.Provider, { + value: { + client, + indexName, + searchKeywords, + setSearchKeywords, + blockTypesFilter, + setBlockTypesFilter, + tagsFilter, + setTagsFilter, + extraFilter, + // This is the list of _available_ block types to filter, e.g. {"problem": 3, "html": 2} + blockTypes: result.data?.blockTypes ?? {}, + canClearFilters, + clearFilters, + // The results: + hits: result.data?.hits ?? [], + totalHits: result.data?.totalHits ?? 0, + }, + }, children); +}; + +export const useSearchContext = () => { + const ctx = React.useContext(SearchContext); + if (ctx === undefined) { + throw new Error('Cannot use search components outside of '); + } + return ctx; +}; diff --git a/src/search-modal/messages.js b/src/search-modal/messages.js index 4f85c472e7..a4fbc40ad7 100644 --- a/src/search-modal/messages.js +++ b/src/search-modal/messages.js @@ -25,6 +25,11 @@ const messages = defineMessages({ defaultMessage: 'No tags in current results', description: 'Label shown when there are no options available to filter by tags', }, + 'blockTagsFilter.error': { + id: 'course-authoring.course-search.blockTagsFilter.error', + defaultMessage: 'Error loading tags', + description: 'Label shown when the tags could not be loaded', + }, 'blockType.annotatable': { id: 'course-authoring.course-search.blockType.annotatable', defaultMessage: 'Annotation', @@ -80,6 +85,16 @@ const messages = defineMessages({ defaultMessage: 'Video', description: 'Name of the "Video" component type in Studio', }, + childTagsExpand: { + id: 'course-authoring.course-search.child-tags-expand', + defaultMessage: 'Expand to show child tags of "{tagName}"', + description: 'This text describes the ▼ expand toggle button to non-visual users.', + }, + childTagsCollapse: { + id: 'course-authoring.course-search.child-tags-collapse', + defaultMessage: 'Collapse to hige child tags of "{tagName}"', + description: 'This text describes the ▲ collapse toggle button to non-visual users.', + }, clearFilters: { id: 'course-authoring.course-search.clearFilters', defaultMessage: 'Clear Filters', @@ -110,6 +125,11 @@ const messages = defineMessages({ defaultMessage: 'Search', description: 'Placeholder text shown in the keyword input field when the user has not yet entered a keyword', }, + searchTagsByKeywordPlaceholder: { + id: 'course-authoring.course-search.searchTagsByKeywordPlaceholder', + defaultMessage: 'Search tags', + description: 'Placeholder text shown in the input field that allows searching through the available tags', + }, showMore: { id: 'course-authoring.course-search.showMore', defaultMessage: 'Show more', From 9d09eded86d30bb7732e579959b2cc26c0d9fa2c Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 16 Apr 2024 13:58:45 -0700 Subject: [PATCH 03/15] feat: Infinite loading of additional pages of results --- src/search-modal/SearchResults.jsx | 34 ++++++++++++++++++-- src/search-modal/data/api.js | 9 ++++++ src/search-modal/data/apiHooks.js | 38 +++++++++++++++++++---- src/search-modal/manager/SearchManager.js | 10 +++--- src/search-modal/messages.js | 10 ++++++ 5 files changed, 87 insertions(+), 14 deletions(-) diff --git a/src/search-modal/SearchResults.jsx b/src/search-modal/SearchResults.jsx index 8129241519..31dac5c130 100644 --- a/src/search-modal/SearchResults.jsx +++ b/src/search-modal/SearchResults.jsx @@ -1,17 +1,45 @@ /* eslint-disable react/prop-types */ // @ts-check import React from 'react'; +import { StatefulButton } from '@openedx/paragon'; +import { useIntl } from '@edx/frontend-platform/i18n'; + import { useSearchContext } from './manager/SearchManager'; import SearchResult from './SearchResult'; +import messages from './messages'; /** * A single search result (row), usually represents an XBlock/Component * @type {React.FC>} */ const SearchResults = () => { - const { hits } = useSearchContext(); - // TODO: pagination - return <>{hits.map((hit) => )}; + const intl = useIntl(); + const { + hits, + hasNextPage, + isFetchingNextPage, + fetchNextPage, + } = useSearchContext(); + + const labels = { + default: intl.formatMessage(messages.showMoreResults), + pending: intl.formatMessage(messages.loadingMoreResults), + }; + + return ( + <> + {hits.map((hit) => )} + {hasNextPage + ? ( + + ) : null} + + ); }; export default SearchResults; diff --git a/src/search-modal/data/api.js b/src/search-modal/data/api.js index 0613a7c1cb..d84c4786e4 100644 --- a/src/search-modal/data/api.js +++ b/src/search-modal/data/api.js @@ -109,9 +109,11 @@ function formatSearchHit(hit) { * blockTypesFilter?: string[], * tagsFilter?: string[], * extraFilter?: import('meilisearch').Filter, + * offset?: number, * }} context * @returns {Promise<{ * hits: ContentHit[], + * nextOffset: number|undefined, * totalHits: number, * blockTypes: Record, * }>} @@ -124,6 +126,8 @@ export async function fetchSearchResults({ /** The full path of tags that each result MUST have, e.g. ["Difficulty > Hard", "Subject > Math"] */ tagsFilter, extraFilter, + /** How many results to skip, e.g. if limit=20 then passing offset=20 gets the second page. */ + offset = 0, }) { /** @type {import('meilisearch').MultiSearchQuery[]} */ const queries = []; @@ -135,6 +139,8 @@ export async function fetchSearchResults({ const tagsFilterFormatted = formatTagsFilter(tagsFilter); + const limit = 20; // How many results to retrieve per page. + // First query is always to get the hits, with all the filters applied. queries.push({ indexUid: indexName, @@ -149,6 +155,8 @@ export async function fetchSearchResults({ attributesToHighlight: ['display_name', 'content'], highlightPreTag: '', highlightPostTag: '', + offset, + limit, }); // The second query is to get the possible values for the "block types" filter @@ -169,6 +177,7 @@ export async function fetchSearchResults({ hits: results[0].hits.map(formatSearchHit), totalHits: results[0].totalHits ?? results[0].estimatedTotalHits ?? results[0].hits.length, blockTypes: results[1].facetDistribution?.block_type ?? {}, + nextOffset: results[0].hits.length === limit ? offset + limit : undefined, }; } diff --git a/src/search-modal/data/apiHooks.js b/src/search-modal/data/apiHooks.js index a4322cf78c..bd6a335b0f 100644 --- a/src/search-modal/data/apiHooks.js +++ b/src/search-modal/data/apiHooks.js @@ -1,6 +1,6 @@ // @ts-check import React from 'react'; -import { useQuery } from '@tanstack/react-query'; +import { useInfiniteQuery, useQuery } from '@tanstack/react-query'; import { TAG_SEP, @@ -44,8 +44,8 @@ export const useContentSearchResults = ({ searchKeywords, blockTypesFilter, tagsFilter, -}) => ( - useQuery({ +}) => { + const query = useInfiniteQuery({ enabled: client !== undefined && indexName !== undefined, queryKey: [ 'content_search', @@ -58,7 +58,7 @@ export const useContentSearchResults = ({ blockTypesFilter, tagsFilter, ], - queryFn: () => { + queryFn: ({ pageParam = 0 }) => { if (client === undefined || indexName === undefined) { throw new Error('Required data unexpectedly undefined. Check "enable" condition of useQuery.'); } @@ -69,12 +69,38 @@ export const useContentSearchResults = ({ searchKeywords, blockTypesFilter, tagsFilter, + // For infinite pagination of results, we can retrieve additional pages if requested. + // Note that if there are 20 results per page, the "second page" has offset=20, not 2. + offset: pageParam, }); }, + getNextPageParam: (lastPage) => lastPage.nextOffset, // Avoid flickering results when user is typing... keep old results until new is available. keepPreviousData: true, - }) -); + }); + + const pages = query.data?.pages; + const hits = React.useMemo( + () => pages?.reduce((alllHits, page) => [...alllHits, ...page.hits], []) ?? [], + [pages], + ); + + return { + hits, + // The distribution of block type filter options + blockTypes: pages?.[0]?.blockTypes ?? {}, + status: query.status, + isFetching: query.isFetching, + isFetchingNextPage: query.isFetchingNextPage, + // Call this to load more pages. We include some "safety" features recommended by the docs: this should never be + // called while already fetching a page, and parameters (like 'event') should not be passed into fetchNextPage(). + // See https://tanstack.com/query/v4/docs/framework/react/guides/infinite-queries + fetchNextPage: () => { if (!query.isFetching && !query.isFetchingNextPage) { query.fetchNextPage(); } }, + hasNextPage: query.hasNextPage, + // The last page has the most accurate count of total hits + totalHits: pages?.[pages.length - 1]?.totalHits ?? 0, + }; +}; /** * Get the available tags that can be used to refine a search, based on the search filters applied so far. diff --git a/src/search-modal/manager/SearchManager.js b/src/search-modal/manager/SearchManager.js index 7fb88910d7..5634a8e59d 100644 --- a/src/search-modal/manager/SearchManager.js +++ b/src/search-modal/manager/SearchManager.js @@ -27,6 +27,10 @@ import { useContentSearchConnection, useContentSearchResults } from '../data/api * clearFilters: () => void, * hits: import('../data/api').ContentHit[], * totalHits: number, + * isFetching: boolean, + * hasNextPage: boolean | undefined, + * isFetchingNextPage: boolean, + * fetchNextPage: () => void, * }>} */ const SearchContext = /** @type {any} */(React.createContext(undefined)); @@ -74,13 +78,9 @@ export const SearchContextProvider = ({ extraFilter, children }) => { tagsFilter, setTagsFilter, extraFilter, - // This is the list of _available_ block types to filter, e.g. {"problem": 3, "html": 2} - blockTypes: result.data?.blockTypes ?? {}, canClearFilters, clearFilters, - // The results: - hits: result.data?.hits ?? [], - totalHits: result.data?.totalHits ?? 0, + ...result, }, }, children); }; diff --git a/src/search-modal/messages.js b/src/search-modal/messages.js index a4fbc40ad7..35bd4cfe25 100644 --- a/src/search-modal/messages.js +++ b/src/search-modal/messages.js @@ -135,6 +135,16 @@ const messages = defineMessages({ defaultMessage: 'Show more', description: 'Show more tags / filter options', }, + showMoreResults: { + id: 'course-authoring.course-search.showMoreResults', + defaultMessage: 'Show more results', + description: 'Show more results - a button to add to the list of results by loading more from the server', + }, + loadingMoreResults: { + id: 'course-authoring.course-search.loadingMoreResults', + defaultMessage: 'Loading more results', + description: 'Loading more results - the button displays this message while more results are loading', + }, }); export default messages; From 2bc510a0a533b96b883a0f8b44edb8209f67d6ff Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 16 Apr 2024 14:35:29 -0700 Subject: [PATCH 04/15] feat: display a warning if we're not able to load all the tags --- src/search-modal/FilterByTags.jsx | 19 ++++++++++++----- src/search-modal/data/api.js | 34 +++++++++++++++---------------- src/search-modal/data/apiHooks.js | 10 ++++++--- src/search-modal/messages.js | 5 +++++ 4 files changed, 43 insertions(+), 25 deletions(-) diff --git a/src/search-modal/FilterByTags.jsx b/src/search-modal/FilterByTags.jsx index 4be5f62595..5fae92a9db 100644 --- a/src/search-modal/FilterByTags.jsx +++ b/src/search-modal/FilterByTags.jsx @@ -11,7 +11,7 @@ import { MenuItem, SearchField, } from '@openedx/paragon'; -import { ArrowDropDown, ArrowDropUp } from '@openedx/paragon/icons'; +import { ArrowDropDown, ArrowDropUp, Warning } from '@openedx/paragon/icons'; import SearchFilterWidget from './SearchFilterWidget'; import messages from './messages'; import { useSearchContext } from './manager/SearchManager'; @@ -97,7 +97,7 @@ const TagOptions = ({ toggleTagChildren, }) => { const searchContext = useSearchContext(); - const { data: tagOptions, isLoading, isError } = useTagFilterOptions({ + const { data, isLoading, isError } = useTagFilterOptions({ ...searchContext, parentTagPath, tagSearchKeywords, @@ -106,19 +106,19 @@ const TagOptions = ({ if (isError) { return ; } - if (isLoading || tagOptions === undefined) { + if (isLoading || data.tags === undefined) { return ; } // Show a message if there are no options at all to avoid the impression that the dropdown isn't working - if (Object.keys(tagOptions).length === 0 && !parentTagPath) { + if (data.tags.length === 0 && !parentTagPath) { return ; } return (
{ - tagOptions.map(({ tagName, tagPath, ...t }) => { + data.tags.map(({ tagName, tagPath, ...t }) => { const isExpanded = expandedTags.includes(tagPath); return ( @@ -151,6 +151,15 @@ const TagOptions = ({ ); }) } + { + // Sometimes, due to limitations of how the search index/API works, we aren't able to retrieve all the options: + data.mayBeMissingResults + ? ( + + + + ) : null + }
); }; diff --git a/src/search-modal/data/api.js b/src/search-modal/data/api.js index d84c4786e4..a023bfb3dc 100644 --- a/src/search-modal/data/api.js +++ b/src/search-modal/data/api.js @@ -192,7 +192,10 @@ export async function fetchSearchResults({ * @param {string[]} [context.blockTypesFilter] Filter to only include these block types e.g. ["problem", "html"] * @param {import('meilisearch').Filter} [context.extraFilter] Any other filters to apply, e.g. course ID. * @param {string} [context.parentTagPath] Only fetch tags below this parent tag/taxonomy e.g. "Places > North America" - * @returns {Promise<{tagName: string, tagPath: string, tagCount: number, hasChildren: boolean}[]>} + * @returns {Promise<{ + * tags: {tagName: string, tagPath: string, tagCount: number, hasChildren: boolean}[]; + * mayBeMissingResults: boolean; + * }>} */ export async function fetchAvailableTagOptions({ client, @@ -203,6 +206,8 @@ export async function fetchAvailableTagOptions({ parentTagPath, // Ideally this would include 'tagSearchKeywords' to filter the tag tree by keyword search but that's not possible yet }) { + const meilisearchFacetLimit = 100; // The 'maxValuesPerFacet' on the index. For Open edX we leave the default, 100. + // Convert 'extraFilter' into an array const extraFilterFormatted = forceArray(extraFilter); @@ -225,17 +230,11 @@ export async function fetchAvailableTagOptions({ parentFilter = [`${parentFacetName} = "${parentTagPath}"`]; } - const { facetDistribution } = await client.index(indexName).search(searchKeywords, { - facets: [facetName], - filter: [...extraFilterFormatted, ...blockTypesFilterFormatted, ...parentFilter], - limit: 0, - }); - if (facetDistribution === undefined) { - throw new Error('Unexpectedly missing facetDistribution in Meilisearch result'); - } - + // Now load the facet values. Doing it with this API gives us much more flexibility in loading than if we just + // requested the facets by passing { facets: ["tags"] } into the main search request; that works fine for loading the + // root tags but can't load specific child tags like we can using this approach. /** @type {{tagName: string, tagPath: string, tagCount: number, hasChildren: boolean}[]} */ - const tagData = []; + const tags = []; const { facetHits } = await client.index(indexName).searchForFacetValues({ facetName, // It's not super clear in the documentation, but facetQuery is basically a "startsWith" query, which is what we @@ -248,7 +247,7 @@ export async function fetchAvailableTagOptions({ }); facetHits.forEach(({ value: tagPath, count: tagCount }) => { if (!parentTagPath) { - tagData.push({ + tags.push({ tagName: tagPath, tagPath, tagCount, @@ -258,7 +257,7 @@ export async function fetchAvailableTagOptions({ const parts = tagPath.split(TAG_SEP); const tagName = parts[parts.length - 1]; if (tagPath === `${parentTagPath}${TAG_SEP}${tagName}`) { - tagData.push({ + tags.push({ tagName, tagPath, tagCount, @@ -278,14 +277,13 @@ export async function fetchAvailableTagOptions({ q: searchKeywords, filter: [...extraFilterFormatted, ...blockTypesFilterFormatted, ...parentFilter], }); - const meilisearchFacetLimit = 100; // The 'maxValuesPerFacet' on the index. For Open edX we leave the default, 100. if (childFacetHits.length >= meilisearchFacetLimit) { // Assume they all have child tags; we can't retrieve more than 100 facet values (per Meilisearch docs) so // we can't say for sure on a tag-by-tag basis, but we know that at least some of them have children, so // it's a safe bet that most/all of them have children. And it's not a huge problem if we say they have children // but they don't. // eslint-disable-next-line no-param-reassign - tagData.forEach((t) => { t.hasChildren = true; }); + tags.forEach((t) => { t.hasChildren = true; }); } else if (childFacetHits.length > 0) { // Some (or maybe all) of these tags have child tags. Let's figure out which ones exactly. /** @type {Set} */ @@ -296,11 +294,13 @@ export async function fetchAvailableTagOptions({ tagsWithChildren.add(tagPath); }); // eslint-disable-next-line no-param-reassign - tagData.forEach((t) => { t.hasChildren = tagsWithChildren.has(t.tagPath); }); + tags.forEach((t) => { t.hasChildren = tagsWithChildren.has(t.tagPath); }); } } - return tagData; + // If we hit the limit of facetHits, there are probably even more tags, but there is no API to retrieve + // them (no pagination etc.), so just tell the user that not all tags could be displayed. This should be pretty rare. + return { tags, mayBeMissingResults: facetHits.length >= meilisearchFacetLimit }; } /** diff --git a/src/search-modal/data/apiHooks.js b/src/search-modal/data/apiHooks.js index bd6a335b0f..eae5ded8af 100644 --- a/src/search-modal/data/apiHooks.js +++ b/src/search-modal/data/apiHooks.js @@ -167,13 +167,13 @@ export const useTagFilterOptions = (args) => { if (!args.tagSearchKeywords || !tagKeywordSearchData.data) { // If there's no keyword search being used to filter the list of available tags, just use the results of the // main query. - return mainQuery.data; + return { tags: mainQuery.data?.tags, mayBeMissingResults: mainQuery.data?.mayBeMissingResults ?? false }; } if (mainQuery.data === undefined) { - return undefined; + return { tags: undefined, mayBeMissingResults: false }; } // Combine these two queries to filter the list of tags based on the keyword search. - return mainQuery.data.filter(({ tagPath }) => { + const tags = mainQuery.data.tags.filter(({ tagPath }) => { // eslint-disable-next-line no-restricted-syntax for (const matchingTag of tagKeywordSearchData.data.matches) { if (matchingTag.tagPath === tagPath || matchingTag.tagPath.startsWith(tagPath + TAG_SEP)) { @@ -182,6 +182,10 @@ export const useTagFilterOptions = (args) => { } return false; }); + return { + tags, + mayBeMissingResults: mainQuery.data.mayBeMissingResults || tagKeywordSearchData.data.mayBeMissingResults, + }; }, [mainQuery.data, tagKeywordSearchData.data]); return { ...mainQuery, data }; diff --git a/src/search-modal/messages.js b/src/search-modal/messages.js index 35bd4cfe25..f1651c4b22 100644 --- a/src/search-modal/messages.js +++ b/src/search-modal/messages.js @@ -30,6 +30,11 @@ const messages = defineMessages({ defaultMessage: 'Error loading tags', description: 'Label shown when the tags could not be loaded', }, + 'blockTagsFilter.incomplete': { + id: 'course-authoring.course-search.blockTagsFilter.incomplete', + defaultMessage: 'Sorry, not all tags could be loaded', + description: 'Label shown when the system is not able to display all of the available tag options.', + }, 'blockType.annotatable': { id: 'course-authoring.course-search.blockType.annotatable', defaultMessage: 'Annotation', From 2e2cdf793dd6140b69683efd8be79f44894ad2c7 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 17 Apr 2024 11:52:19 -0700 Subject: [PATCH 05/15] feat: make icons consistent with the rest of Course-Authoring --- src/search-modal/SearchResult.jsx | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/search-modal/SearchResult.jsx b/src/search-modal/SearchResult.jsx index e647d93813..a22aed4581 100644 --- a/src/search-modal/SearchResult.jsx +++ b/src/search-modal/SearchResult.jsx @@ -12,27 +12,27 @@ import { Article, Folder, OpenInNew, - Question, - TextFields, - Videocam, } from '@openedx/paragon/icons'; import { useSelector } from 'react-redux'; import { useNavigate } from 'react-router-dom'; -import { getStudioHomeData } from '../studio-home/data/selectors'; -import messages from './messages'; -import Highlight from './Highlight'; +import { COMPONENT_TYPE_ICON_MAP, TYPE_ICONS_MAP } from '../course-unit/constants'; +import { getStudioHomeData } from '../studio-home/data/selectors'; import { useSearchContext } from './manager/SearchManager'; +import Highlight from './Highlight'; +import messages from './messages'; -const ItemIcon = { - vertical: Folder, +const STRUCTURAL_TYPE_ICONS = { + vertical: TYPE_ICONS_MAP.vertical, sequential: Folder, chapter: Folder, - problem: Question, - video: Videocam, - html: TextFields, }; +/** @param {string} blockType */ +function getItemIcon(blockType) { + return STRUCTURAL_TYPE_ICONS[blockType] ?? COMPONENT_TYPE_ICON_MAP[blockType] ?? Article; +} + /** * A single search result (row), usually represents an XBlock/Component * @type {React.FC<{hit: import('./data/api').ContentHit}>} @@ -124,7 +124,7 @@ const SearchResult = ({ hit }) => { tabIndex={redirectUrl ? 0 : undefined} role="button" > - +
From 2cdbcd8f370bdce553f61a0a1920513602c8ba02 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 17 Apr 2024 14:42:01 -0700 Subject: [PATCH 06/15] test: update tests --- src/search-modal/SearchUI.test.jsx | 68 ++++++++++++------- .../__mocks__/empty-search-result.json | 16 +++-- src/search-modal/__mocks__/facet-search.json | 12 ++++ src/search-modal/__mocks__/search-result.json | 19 +++--- src/search-modal/messages.js | 2 +- 5 files changed, 78 insertions(+), 39 deletions(-) create mode 100644 src/search-modal/__mocks__/facet-search.json diff --git a/src/search-modal/SearchUI.test.jsx b/src/search-modal/SearchUI.test.jsx index 3e217c4a7d..97f8b51ad6 100644 --- a/src/search-modal/SearchUI.test.jsx +++ b/src/search-modal/SearchUI.test.jsx @@ -3,8 +3,10 @@ import React from 'react'; import { IntlProvider } from '@edx/frontend-platform/i18n'; import { initializeMockApp } from '@edx/frontend-platform'; +import { getAuthenticatedHttpClient } from '@edx/frontend-platform/auth'; import { AppProvider } from '@edx/frontend-platform/react'; import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import MockAdapter from 'axios-mock-adapter'; import { fireEvent, render, @@ -19,7 +21,10 @@ import initializeStore from '../store'; import mockResult from './__mocks__/search-result.json'; // @ts-ignore import mockEmptyResult from './__mocks__/empty-search-result.json'; +// @ts-ignore +import mockTagsFacetResult from './__mocks__/facet-search.json'; import SearchUI from './SearchUI'; +import { getContentSearchConfigUrl } from './data/api'; // mockResult contains only a single result - this one: const mockResultDisplayName = 'Test HTML Block'; @@ -29,12 +34,10 @@ const queryClient = new QueryClient(); // Default props for const defaults = { - url: 'http://mock.meilisearch.local/', - apiKey: 'test-key', - indexName: 'studio', courseId: 'course-v1:org+test+123', }; const searchEndpoint = 'http://mock.meilisearch.local/multi-search'; +const facetSearchEndpoint = 'http://mock.meilisearch.local/indexes/studio/facet-search'; const mockNavigate = jest.fn(); @@ -53,15 +56,15 @@ const Wrap = ({ children }) => ( ); +let axiosMock; const returnEmptyResult = (_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); const query = requestData?.queries[0]?.q ?? ''; // We have to replace the query (search keywords) in the mock results with the actual query, - // because otherwise Instantsearch will update the UI and change the query, - // leading to unexpected results in the test cases. + // because otherwise we may have an inconsistent state that causes more queries and unexpected results. mockEmptyResult.results[0].query = query; - // And create the required '_formatted' field; not sure why it's there - seems very redundant. But it's required. + // And fake the required '_formatted' fields; it contains the highlighting ... around matched words // eslint-disable-next-line no-underscore-dangle, no-param-reassign mockEmptyResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); return mockEmptyResult; @@ -78,6 +81,14 @@ describe('', () => { }, }); store = initializeStore(); + // The API method to get the Meilisearch connection details uses Axios: + axiosMock = new MockAdapter(getAuthenticatedHttpClient()); + axiosMock.onGet(getContentSearchConfigUrl()).reply(200, { + url: 'http://mock.meilisearch.local', + index_name: 'studio', + api_key: 'test-key', + }); + // The Meilisearch client-side API uses fetch, not Axios. fetchMock.post(searchEndpoint, (_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); const query = requestData?.queries[0]?.q ?? ''; @@ -85,7 +96,7 @@ describe('', () => { // because otherwise Instantsearch will update the UI and change the query, // leading to unexpected results in the test cases. mockResult.results[0].query = query; - // And create the required '_formatted' field; not sure why it's there - seems very redundant. But it's required. + // And fake the required '_formatted' fields; it contains the highlighting ... around matched words // eslint-disable-next-line no-underscore-dangle, no-param-reassign mockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); return mockResult; @@ -100,8 +111,8 @@ describe('', () => { const { getByText } = render(); // Before the results have even loaded, we see this message: expect(getByText('Start searching to find content')).toBeInTheDocument(); - // When this UI loads, Instantsearch makes two queries. I think one to load the facets and one "blank" search. - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + // When this UI loads, we do a "placeholder" search to load the filter options + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); // And that message is still displayed even after the initial results/filters have loaded: expect(getByText('Start searching to find content')).toBeInTheDocument(); }); @@ -112,14 +123,14 @@ describe('', () => { // Return an empty result set: // Before the results have even loaded, we see this message: expect(getByText('Start searching to find content')).toBeInTheDocument(); - // When this UI loads, Instantsearch makes two queries. I think one to load the facets and one "blank" search. - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + // When this UI loads, the UI makes a search, to get the available "block type" facet values. + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); // And that message is still displayed even after the initial results/filters have loaded: expect(getByText('Start searching to find content')).toBeInTheDocument(); // Enter a keyword - search for 'noresults': fireEvent.change(getByRole('searchbox'), { target: { value: 'noresults' } }); // Wait for the new search request to load all the results: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); expect(getByText('We didn\'t find anything matching your search')).toBeInTheDocument(); }); @@ -129,11 +140,11 @@ describe('', () => { expect(getByText('All courses')).toBeInTheDocument(); expect(queryByText('This course')).toBeNull(); // Wait for the initial search request that loads all the filter options: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); // Enter a keyword - search for 'giraffe': fireEvent.change(getByRole('searchbox'), { target: { value: 'giraffe' } }); // Wait for the new search request to load all the results: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); // Now we should see the results: expect(queryByText('Enter a keyword')).toBeNull(); // The result: @@ -165,11 +176,11 @@ describe('', () => { expect(getByText('This course')).toBeInTheDocument(); expect(queryByText('All courses')).toBeNull(); // Wait for the initial search request that loads all the filter options: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); // Enter a keyword - search for 'giraffe': fireEvent.change(getByRole('searchbox'), { target: { value: 'giraffe' } }); // Wait for the new search request to load all the results: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); // And make sure the request was limited to this course: expect(fetchMock).toHaveLastFetched((_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); @@ -189,14 +200,16 @@ describe('', () => { /** @type {import('@testing-library/react').RenderResult} */ let rendered; beforeEach(async () => { + fetchMock.post(facetSearchEndpoint, mockTagsFacetResult); + rendered = render(); const { getByRole, getByText } = rendered; // Wait for the initial search request that loads all the filter options: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(1, searchEndpoint, 'post'); }); // Enter a keyword - search for 'giraffe': fireEvent.change(getByRole('searchbox'), { target: { value: 'giraffe' } }); // Wait for the new search request to load all the results and the filter options, based on the search so far: - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(2, searchEndpoint, 'post'); }); // And make sure the request was limited to this course: expect(fetchMock).toHaveLastFetched((_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); @@ -217,8 +230,9 @@ describe('', () => { const popupMenu = getByRole('group'); const problemFilterCheckbox = getByLabelTextIn(popupMenu, /Problem/i); fireEvent.click(problemFilterCheckbox, {}); + await waitFor(() => { expect(rendered.getByText('Type: Problem')).toBeInTheDocument(); }); // Now wait for the filter to be applied and the new results to be fetched. - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(4, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); // Because we're mocking the results, there's no actual changes to the mock results, // but we can verify that the filter was sent in the request expect(fetchMock).toHaveLastFetched((_url, req) => { @@ -226,7 +240,7 @@ describe('', () => { const requestedFilter = requestData?.queries[0].filter; return JSON.stringify(requestedFilter) === JSON.stringify([ 'context_key = "course-v1:org+test+123"', - ['"block_type"="problem"'], // <-- the newly added filter, sent with the request + ['block_type = problem'], // <-- the newly added filter, sent with the request ]); }); }); @@ -236,18 +250,22 @@ describe('', () => { // Now open the filters menu: fireEvent.click(getByRole('button', { name: 'Tags' }), {}); // The dropdown menu in this case doesn't have a role; let's just assume it's displayed. - const competentciesCheckbox = getByLabelText(/ESDC Skills and Competencies/i); + const checkboxLabel = /^ESDC Skills and Competencies/i; + await waitFor(() => { expect(getByLabelText(checkboxLabel)).toBeInTheDocument(); }); + // In addition to the checkbox, there is another button to show the child tags: + expect(getByLabelText(/Expand to show child tags of "ESDC Skills and Competencies"/i)).toBeInTheDocument(); + const competentciesCheckbox = getByLabelText(checkboxLabel); fireEvent.click(competentciesCheckbox, {}); // Now wait for the filter to be applied and the new results to be fetched. - await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(4, searchEndpoint, 'post'); }); + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); // Because we're mocking the results, there's no actual changes to the mock results, // but we can verify that the filter was sent in the request - expect(fetchMock).toHaveLastFetched((_url, req) => { + expect(fetchMock).toBeDone((_url, req) => { const requestData = JSON.parse(req.body?.toString() ?? ''); - const requestedFilter = requestData?.queries[0].filter; + const requestedFilter = requestData?.queries?.[0]?.filter; return JSON.stringify(requestedFilter) === JSON.stringify([ 'context_key = "course-v1:org+test+123"', - ['"tags.taxonomy"="ESDC Skills and Competencies"'], // <-- the newly added filter, sent with the request + 'tags.taxonomy = "ESDC Skills and Competencies"', // <-- the newly added filter, sent with the request ]); }); }); diff --git a/src/search-modal/__mocks__/empty-search-result.json b/src/search-modal/__mocks__/empty-search-result.json index 70d3e8f2fa..a0ba5d6db9 100644 --- a/src/search-modal/__mocks__/empty-search-result.json +++ b/src/search-modal/__mocks__/empty-search-result.json @@ -2,16 +2,24 @@ "comment": "This is a mock of the empty response from Meilisearch, based on an actual search in Studio.", "results": [ { - "indexUid": "tutor_studio_content", + "indexUid": "studio", "hits": [], "query": "noresult", "processingTimeMs": 0, - "limit": 21, + "limit": 20, + "offset": 0, + "estimatedTotalHits": 0 + }, + { + "indexUid": "studio", + "hits": [], + "query": "noresult", + "processingTimeMs": 0, + "limit": 0, "offset": 0, "estimatedTotalHits": 0, "facetDistribution": { - "block_type": {}, - "tags.taxonomy": {} + "block_type": {} }, "facetStats": {} } diff --git a/src/search-modal/__mocks__/facet-search.json b/src/search-modal/__mocks__/facet-search.json new file mode 100644 index 0000000000..71db55107e --- /dev/null +++ b/src/search-modal/__mocks__/facet-search.json @@ -0,0 +1,12 @@ +{ + "facetHits": [ + { "value": "ESDC Skills and Competencies", "count": 7 }, + { "value": "FlatTaxonomy", "count": 7 }, + { "value": "HierarchicalTaxonomy", "count": 6 }, + { "value": "Lightcast Open Skills Taxonomy", "count": 6 }, + { "value": "MultiOrgTaxonomy", "count": 7 }, + { "value": "TwoLevelTaxonomy", "count": 7 } + ], + "facetQuery": "", + "processingTimeMs": 0 +} diff --git a/src/search-modal/__mocks__/search-result.json b/src/search-modal/__mocks__/search-result.json index 234bb10c4e..ff4397a406 100644 --- a/src/search-modal/__mocks__/search-result.json +++ b/src/search-modal/__mocks__/search-result.json @@ -74,21 +74,22 @@ "processingTimeMs": 1, "limit": 2, "offset": 0, - "estimatedTotalHits": 2, + "estimatedTotalHits": 2 + }, + { + "indexUid": "studio", + "hits": [], + "query": "learn", + "processingTimeMs": 1, + "limit": 0, + "offset": 0, + "estimatedTotalHits": 0, "facetDistribution": { "block_type": { "html": 2, "problem": 16, "vertical": 2, "video": 1 - }, - "tags.taxonomy": { - "ESDC Skills and Competencies": 1, - "FlatTaxonomy": 2, - "HierarchicalTaxonomy": 1, - "Lightcast Open Skills Taxonomy": 1, - "MultiOrgTaxonomy": 1, - "TwoLevelTaxonomy": 2 } }, "facetStats": {} diff --git a/src/search-modal/messages.js b/src/search-modal/messages.js index 1fa4238e43..8f50fe255d 100644 --- a/src/search-modal/messages.js +++ b/src/search-modal/messages.js @@ -97,7 +97,7 @@ const messages = defineMessages({ }, childTagsCollapse: { id: 'course-authoring.course-search.child-tags-collapse', - defaultMessage: 'Collapse to hige child tags of "{tagName}"', + defaultMessage: 'Collapse to hide child tags of "{tagName}"', description: 'This text describes the ▲ collapse toggle button to non-visual users.', }, clearFilters: { From bc7943ccdb608793dc580ef64903970c38857a1d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 17 Apr 2024 20:58:18 -0700 Subject: [PATCH 07/15] fix: show an error when the search connection isn't working --- src/search-modal/EmptyStates.jsx | 16 ++++++++++++++-- src/search-modal/SearchModal.test.jsx | 10 +--------- src/search-modal/data/apiHooks.js | 1 + src/search-modal/manager/SearchManager.js | 4 +++- src/search-modal/messages.js | 5 +++++ 5 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/search-modal/EmptyStates.jsx b/src/search-modal/EmptyStates.jsx index eeb61365a2..fa5c77b227 100644 --- a/src/search-modal/EmptyStates.jsx +++ b/src/search-modal/EmptyStates.jsx @@ -2,7 +2,7 @@ // @ts-check import React from 'react'; import { FormattedMessage } from '@edx/frontend-platform/i18n'; -import { Stack } from '@openedx/paragon'; +import { Alert, Stack } from '@openedx/paragon'; import { useSearchContext } from './manager/SearchManager'; import EmptySearchImage from './images/empty-search.svg'; @@ -24,9 +24,21 @@ const InfoMessage = ({ title, subtitle, image }) => ( * @type {React.FC<{children: React.ReactElement}>} */ const EmptyStates = ({ children }) => { - const { canClearFilters: hasFiltersApplied, totalHits, searchKeywords } = useSearchContext(); + const { + canClearFilters: hasFiltersApplied, + totalHits, + searchKeywords, + hasError, + } = useSearchContext(); const hasQuery = !!searchKeywords; + if (hasError) { + return ( + + + + ); + } if (!hasQuery && !hasFiltersApplied) { // We haven't started the search yet. Display the "start your search" empty state return ( diff --git a/src/search-modal/SearchModal.test.jsx b/src/search-modal/SearchModal.test.jsx index 6c77043958..a14bb0fc2d 100644 --- a/src/search-modal/SearchModal.test.jsx +++ b/src/search-modal/SearchModal.test.jsx @@ -67,17 +67,9 @@ describe('', () => { expect(await findByText('Start searching to find content')).toBeInTheDocument(); }); - it('should render the spinner while the config is loading', () => { - axiosMock.onGet(getContentSearchConfigUrl()).replyOnce(200, new Promise(() => {})); // never resolves - const { getByRole } = render(); - - const spinner = getByRole('status'); - expect(spinner.textContent).toEqual('Loading...'); - }); - it('should render the error message if the api call throws', async () => { axiosMock.onGet(getContentSearchConfigUrl()).networkError(); const { findByText } = render(); - expect(await findByText('Network Error')).toBeInTheDocument(); + expect(await findByText('An error occurred. Unable to load search results.')).toBeInTheDocument(); }); }); diff --git a/src/search-modal/data/apiHooks.js b/src/search-modal/data/apiHooks.js index eae5ded8af..06867e0ebe 100644 --- a/src/search-modal/data/apiHooks.js +++ b/src/search-modal/data/apiHooks.js @@ -91,6 +91,7 @@ export const useContentSearchResults = ({ blockTypes: pages?.[0]?.blockTypes ?? {}, status: query.status, isFetching: query.isFetching, + isError: query.isError, isFetchingNextPage: query.isFetchingNextPage, // Call this to load more pages. We include some "safety" features recommended by the docs: this should never be // called while already fetching a page, and parameters (like 'event') should not be passed into fetchNextPage(). diff --git a/src/search-modal/manager/SearchManager.js b/src/search-modal/manager/SearchManager.js index 6cbf17864d..bbed2e08e5 100644 --- a/src/search-modal/manager/SearchManager.js +++ b/src/search-modal/manager/SearchManager.js @@ -32,6 +32,7 @@ import { useContentSearchConnection, useContentSearchResults } from '../data/api * isFetchingNextPage: boolean, * fetchNextPage: () => void, * closeSearchModal: () => void, + * hasError: boolean, * }>} */ const SearchContext = /** @type {any} */(React.createContext(undefined)); @@ -55,7 +56,7 @@ export const SearchContextProvider = ({ extraFilter, children, closeSearchModal }, []); // Initialize a connection to Meilisearch: - const { data: connectionDetails } = useContentSearchConnection(); + const { data: connectionDetails, isError: hasConnectionError } = useContentSearchConnection(); const indexName = connectionDetails?.indexName; const client = React.useMemo(() => { if (connectionDetails?.apiKey === undefined || connectionDetails?.url === undefined) { @@ -88,6 +89,7 @@ export const SearchContextProvider = ({ extraFilter, children, closeSearchModal canClearFilters, clearFilters, closeSearchModal: closeSearchModal ?? (() => {}), + hasError: hasConnectionError || result.isError, ...result, }, }, children); diff --git a/src/search-modal/messages.js b/src/search-modal/messages.js index 8f50fe255d..13a23138c4 100644 --- a/src/search-modal/messages.js +++ b/src/search-modal/messages.js @@ -175,6 +175,11 @@ const messages = defineMessages({ defaultMessage: 'Open in new window', description: 'Alt text for the button that opens the search result in a new window', }, + searchError: { + id: 'course-authoring.course-search.searchError', + defaultMessage: 'An error occurred. Unable to load search results.', + description: 'Error message shown when search is not working.', + }, }); export default messages; From 2a218321f6e9a961ac89485f0bd337e5a8e8ebdd Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Wed, 17 Apr 2024 21:06:20 -0700 Subject: [PATCH 08/15] docs: fix docstring --- src/search-modal/SearchResults.jsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/search-modal/SearchResults.jsx b/src/search-modal/SearchResults.jsx index 049c3abf23..518aea36d0 100644 --- a/src/search-modal/SearchResults.jsx +++ b/src/search-modal/SearchResults.jsx @@ -9,7 +9,11 @@ import SearchResult from './SearchResult'; import messages from './messages'; /** - * A single search result (row), usually represents an XBlock/Component + * All of the single results ("hits"), based on the user's search. + * + * Uses "infinite pagination" to load more pages as needed (if users click the + * "Show more results" button). + * * @type {React.FC>} */ const SearchResults = () => { From 173e9bf81b700d882700b12eb390af031a267cd4 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 18 Apr 2024 09:32:06 -0700 Subject: [PATCH 09/15] fix: apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Rômulo Penido --- src/search-modal/SearchResult.jsx | 5 +---- src/search-modal/data/api.js | 5 +++-- src/search-modal/data/apiHooks.js | 16 ++++++---------- 3 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/search-modal/SearchResult.jsx b/src/search-modal/SearchResult.jsx index a22aed4581..c15371d81b 100644 --- a/src/search-modal/SearchResult.jsx +++ b/src/search-modal/SearchResult.jsx @@ -134,10 +134,7 @@ const SearchResult = ({ hit }) => {
- {hit.breadcrumbs.map((bc, i) => ( - // eslint-disable-next-line react/no-array-index-key - {bc.displayName} {i !== hit.breadcrumbs.length - 1 ? '/' : ''} - ))} + {hit.breadcrumbs.map(bc => bc.displayName).join(' / ')}
', highlightPostTag: '', attributesToCrop: ['content'], + cropLength: 20, offset, limit, }); @@ -348,7 +349,7 @@ export async function fetchTagsThatMatchKeyword({ attributesToRetrieve: ['tags'], limit, // We'd like to use 'showMatchesPosition: true' to know exaclty which tags match, but it doesn't provide the - // detail we need; it's impossible to tell whihc tag at a given level matched based on the returned _matchesPosition + // detail we need; it's impossible to tell which tag at a given level matched based on the returned _matchesPosition // data - https://github.com/orgs/meilisearch/discussions/550 }); diff --git a/src/search-modal/data/apiHooks.js b/src/search-modal/data/apiHooks.js index 06867e0ebe..936e12c167 100644 --- a/src/search-modal/data/apiHooks.js +++ b/src/search-modal/data/apiHooks.js @@ -81,7 +81,7 @@ export const useContentSearchResults = ({ const pages = query.data?.pages; const hits = React.useMemo( - () => pages?.reduce((alllHits, page) => [...alllHits, ...page.hits], []) ?? [], + () => pages?.reduce((allHits, page) => [...allHits, ...page.hits], []) ?? [], [pages], ); @@ -174,15 +174,11 @@ export const useTagFilterOptions = (args) => { return { tags: undefined, mayBeMissingResults: false }; } // Combine these two queries to filter the list of tags based on the keyword search. - const tags = mainQuery.data.tags.filter(({ tagPath }) => { - // eslint-disable-next-line no-restricted-syntax - for (const matchingTag of tagKeywordSearchData.data.matches) { - if (matchingTag.tagPath === tagPath || matchingTag.tagPath.startsWith(tagPath + TAG_SEP)) { - return true; - } - } - return false; - }); + const tags = mainQuery.data.tags.filter( + ({ tagPath }) => tagKeywordSearchData.data.matches.some( + (matchingTag) => matchingTag.tagPath === tagPath || matchingTag.tagPath.startsWith(tagPath + TAG_SEP), + ), + ); return { tags, mayBeMissingResults: mainQuery.data.mayBeMissingResults || tagKeywordSearchData.data.mayBeMissingResults, From a0f9bf280800f310ecf53e2af3539601204866b5 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 18 Apr 2024 09:37:50 -0700 Subject: [PATCH 10/15] chore: better handling of highlighting markers --- src/search-modal/Highlight.jsx | 10 ++++++---- src/search-modal/data/api.js | 7 +++++-- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/src/search-modal/Highlight.jsx b/src/search-modal/Highlight.jsx index 07ea1debb8..46ddef9f8f 100644 --- a/src/search-modal/Highlight.jsx +++ b/src/search-modal/Highlight.jsx @@ -3,20 +3,22 @@ // @ts-check import React from 'react'; +import { highlightPostTag, highlightPreTag } from './data/api'; + /** - * Render some text that contains ... highlights + * Render some text that contains matching words which should be highlighted * @type {React.FC<{text: string}>} */ const Highlight = ({ text }) => { - const parts = text.split(''); + const parts = text.split(highlightPreTag); return ( {parts.map((part, idx) => { if (idx === 0) { return {part}; } - const endIdx = part.indexOf(''); + const endIdx = part.indexOf(highlightPostTag); if (endIdx === -1) { return {part}; } const highLightPart = part.substring(0, endIdx); - const otherPart = part.substring(endIdx + 7); + const otherPart = part.substring(endIdx + highlightPostTag.length); return {highLightPart}{otherPart}; })} diff --git a/src/search-modal/data/api.js b/src/search-modal/data/api.js index 41497f71bd..cf9cd6b620 100644 --- a/src/search-modal/data/api.js +++ b/src/search-modal/data/api.js @@ -10,6 +10,9 @@ export const getContentSearchConfigUrl = () => new URL( /** The separator used for hierarchical tags in the search index, e.g. tags.level1 = "Subject > Math > Calculus" */ export const TAG_SEP = ' > '; +export const highlightPreTag = '__meili-highlight__'; // Indicate the start of a highlighted (matching) term +export const highlightPostTag = '__/meili-highlight__'; // Indicate the end of a highlighted (matching) term + /** * Get the content search configuration from the CMS. * @@ -153,8 +156,8 @@ export async function fetchSearchResults({ ...tagsFilterFormatted, ], attributesToHighlight: ['display_name', 'content'], - highlightPreTag: '', - highlightPostTag: '', + highlightPreTag, + highlightPostTag, attributesToCrop: ['content'], cropLength: 20, offset, From ceea96aabb9c0e6eebfaeb3d36a5c3be058d908f Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 18 Apr 2024 09:50:05 -0700 Subject: [PATCH 11/15] test: update tests --- src/search-modal/SearchUI.test.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/search-modal/SearchUI.test.jsx b/src/search-modal/SearchUI.test.jsx index 97f8b51ad6..858c8ee17f 100644 --- a/src/search-modal/SearchUI.test.jsx +++ b/src/search-modal/SearchUI.test.jsx @@ -151,7 +151,7 @@ describe('', () => { expect(getByText('2 results found')).toBeInTheDocument(); expect(getByText(mockResultDisplayName)).toBeInTheDocument(); // Breadcrumbs showing where the result came from: - expect(getByText('The Little Unit That Could')).toBeInTheDocument(); + expect(getByText('TheCourse / Section 2 / Subsection 3 / The Little Unit That Could')).toBeInTheDocument(); const resultItem = getByRole('button', { name: /The Little Unit That Could/ }); @@ -193,7 +193,7 @@ describe('', () => { expect(getByText('2 results found')).toBeInTheDocument(); expect(getByText(mockResultDisplayName)).toBeInTheDocument(); // Breadcrumbs showing where the result came from: - expect(getByText('The Little Unit That Could')).toBeInTheDocument(); + expect(getByText('TheCourse / Section 2 / Subsection 3 / The Little Unit That Could')).toBeInTheDocument(); }); describe('filters', () => { From 9602c8479c20428cfe1888a87290a8e072645f32 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Thu, 18 Apr 2024 14:57:42 -0700 Subject: [PATCH 12/15] test: add test for retrieving child tags --- src/search-modal/SearchUI.test.jsx | 47 ++++++++++++++++++- .../__mocks__/facet-search-level0.json | 13 +++++ .../__mocks__/facet-search-level1.json | 8 ++++ 3 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 src/search-modal/__mocks__/facet-search-level0.json create mode 100644 src/search-modal/__mocks__/facet-search-level1.json diff --git a/src/search-modal/SearchUI.test.jsx b/src/search-modal/SearchUI.test.jsx index 858c8ee17f..ce1c9170c0 100644 --- a/src/search-modal/SearchUI.test.jsx +++ b/src/search-modal/SearchUI.test.jsx @@ -23,6 +23,10 @@ import mockResult from './__mocks__/search-result.json'; import mockEmptyResult from './__mocks__/empty-search-result.json'; // @ts-ignore import mockTagsFacetResult from './__mocks__/facet-search.json'; +// @ts-ignore +import mockTagsFacetResultLevel0 from './__mocks__/facet-search-level0.json'; +// @ts-ignore +import mockTagsFacetResultLevel1 from './__mocks__/facet-search-level1.json'; import SearchUI from './SearchUI'; import { getContentSearchConfigUrl } from './data/api'; @@ -200,7 +204,15 @@ describe('', () => { /** @type {import('@testing-library/react').RenderResult} */ let rendered; beforeEach(async () => { - fetchMock.post(facetSearchEndpoint, mockTagsFacetResult); + fetchMock.post(facetSearchEndpoint, (_path, req) => { + const requestData = JSON.parse(req.body?.toString() ?? ''); + switch (requestData.facetName) { + case 'tags.taxonomy': return mockTagsFacetResult; + case 'tags.level0': return mockTagsFacetResultLevel0; + case 'tags.level1': return mockTagsFacetResultLevel1; + default: throw new Error(`Facet ${requestData.facetName} not mocked for testing`); + } + }); rendered = render(); const { getByRole, getByText } = rendered; @@ -269,5 +281,38 @@ describe('', () => { ]); }); }); + + it('can filter results by a child tag', async () => { + const { getByRole, getByLabelText, queryByLabelText } = rendered; + // Now open the filters menu: + fireEvent.click(getByRole('button', { name: 'Tags' }), {}); + // The dropdown menu in this case doesn't have a role; let's just assume it's displayed. + const expandButtonLabel = /Expand to show child tags of "ESDC Skills and Competencies"/i; + await waitFor(() => { expect(getByLabelText(expandButtonLabel)).toBeInTheDocument(); }); + + // First, the child tag is not shown: + const childTagLabel = /^Abilities/i; + expect(queryByLabelText(childTagLabel)).toBeNull(); + // Click on the button to show children + const expandButton = getByLabelText(expandButtonLabel); + fireEvent.click(expandButton, {}); + // Now the child tag is visible: + await waitFor(() => { expect(queryByLabelText(childTagLabel)).toBeInTheDocument(); }); + // Click on it: + const abilitiesTagFilterCheckbox = getByLabelText(childTagLabel); + fireEvent.click(abilitiesTagFilterCheckbox); + // Now wait for the filter to be applied and the new results to be fetched. + await waitFor(() => { expect(fetchMock).toHaveFetchedTimes(3, searchEndpoint, 'post'); }); + // Because we're mocking the results, there's no actual changes to the mock results, + // but we can verify that the filter was sent in the request + expect(fetchMock).toBeDone((_url, req) => { + const requestData = JSON.parse(req.body?.toString() ?? ''); + const requestedFilter = requestData?.queries?.[0]?.filter; + return JSON.stringify(requestedFilter) === JSON.stringify([ + 'context_key = "course-v1:org+test+123"', + 'tags.level0 = "ESDC Skills and Competencies > Abilities"', + ]); + }); + }); }); }); diff --git a/src/search-modal/__mocks__/facet-search-level0.json b/src/search-modal/__mocks__/facet-search-level0.json new file mode 100644 index 0000000000..b4201c6d88 --- /dev/null +++ b/src/search-modal/__mocks__/facet-search-level0.json @@ -0,0 +1,13 @@ +{ + "facetHits": [ + { "value": "ESDC Skills and Competencies > Abilities", "count": 5 }, + { "value": "ESDC Skills and Competencies > Interests", "count": 1 }, + { "value": "ESDC Skills and Competencies > Knowledge", "count": 7 }, + { "value": "ESDC Skills and Competencies > Personal Attributes", "count": 3 }, + { "value": "ESDC Skills and Competencies > Skills", "count": 8 }, + { "value": "ESDC Skills and Competencies > Work Activities", "count": 5 }, + { "value": "ESDC Skills and Competencies > Work Context", "count": 10 } + ], + "facetQuery": "", + "processingTimeMs": 0 +} diff --git a/src/search-modal/__mocks__/facet-search-level1.json b/src/search-modal/__mocks__/facet-search-level1.json new file mode 100644 index 0000000000..f6a68e3336 --- /dev/null +++ b/src/search-modal/__mocks__/facet-search-level1.json @@ -0,0 +1,8 @@ +{ + "facetHits": [ + { "value": "ESDC Skills and Competencies > Abilities > Cognitive Abilities", "count": 3 }, + { "value": "ESDC Skills and Competencies > Abilities > Physical Abilities", "count": 2 } + ], + "facetQuery": "", + "processingTimeMs": 0 +} From 5e9110b7ccc80283635071d01261248ccf9f29c9 Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 19 Apr 2024 10:52:09 -0700 Subject: [PATCH 13/15] test: add a test for searching by keyword tag --- src/search-modal/FilterByTags.jsx | 4 ++ src/search-modal/SearchUI.test.jsx | 19 ++++++++ .../__mocks__/tags-keyword-search.json | 48 +++++++++++++++++++ src/search-modal/messages.js | 5 ++ 4 files changed, 76 insertions(+) create mode 100644 src/search-modal/__mocks__/tags-keyword-search.json diff --git a/src/search-modal/FilterByTags.jsx b/src/search-modal/FilterByTags.jsx index 5fae92a9db..d36c3e8cfc 100644 --- a/src/search-modal/FilterByTags.jsx +++ b/src/search-modal/FilterByTags.jsx @@ -192,6 +192,10 @@ const FilterByTags = () => { onChange={setTagSearchKeywords} onClear={() => setTagSearchKeywords('')} value={tagSearchKeywords} + screenReaderText={{ + label: intl.formatMessage(messages.searchTagsByKeywordPlaceholder), + submitButton: intl.formatMessage(messages.submitSearchTagsByKeyword), + }} placeholder={intl.formatMessage(messages.searchTagsByKeywordPlaceholder)} className="mx-3 mb-1" /> diff --git a/src/search-modal/SearchUI.test.jsx b/src/search-modal/SearchUI.test.jsx index ce1c9170c0..752bd7c584 100644 --- a/src/search-modal/SearchUI.test.jsx +++ b/src/search-modal/SearchUI.test.jsx @@ -27,6 +27,8 @@ import mockTagsFacetResult from './__mocks__/facet-search.json'; import mockTagsFacetResultLevel0 from './__mocks__/facet-search-level0.json'; // @ts-ignore import mockTagsFacetResultLevel1 from './__mocks__/facet-search-level1.json'; +// @ts-ignore +import mockTagsKeywordSearchResult from './__mocks__/tags-keyword-search.json'; import SearchUI from './SearchUI'; import { getContentSearchConfigUrl } from './data/api'; @@ -42,6 +44,7 @@ const defaults = { }; const searchEndpoint = 'http://mock.meilisearch.local/multi-search'; const facetSearchEndpoint = 'http://mock.meilisearch.local/indexes/studio/facet-search'; +const tagsKeywordSearchEndpoint = 'http://mock.meilisearch.local/indexes/studio/search'; const mockNavigate = jest.fn(); @@ -105,6 +108,7 @@ describe('', () => { mockResult.results[0]?.hits.forEach((hit) => { hit._formatted = { ...hit }; }); return mockResult; }); + fetchMock.post(tagsKeywordSearchEndpoint, mockTagsKeywordSearchResult); }); afterEach(async () => { @@ -314,5 +318,20 @@ describe('', () => { ]); }); }); + + it('can do a keyword search of the tag options', async () => { + const { getByRole, getByLabelText, queryByLabelText } = rendered; + // Now open the filters menu: + fireEvent.click(getByRole('button', { name: 'Tags' }), {}); + // The dropdown menu in this case doesn't have a role; let's just assume it's displayed. + const expandButtonLabel = /Expand to show child tags of "ESDC Skills and Competencies"/i; + await waitFor(() => { expect(getByLabelText(expandButtonLabel)).toBeInTheDocument(); }); + + const input = getByLabelText('Search tags'); + fireEvent.change(input, { target: { value: 'Lightcast' } }); + + await waitFor(() => { expect(queryByLabelText(/^ESDC Skills and Competencies/i)).toBeNull(); }); + expect(queryByLabelText(/^Lightcast/i)).toBeInTheDocument(); + }); }); }); diff --git a/src/search-modal/__mocks__/tags-keyword-search.json b/src/search-modal/__mocks__/tags-keyword-search.json new file mode 100644 index 0000000000..51be970963 --- /dev/null +++ b/src/search-modal/__mocks__/tags-keyword-search.json @@ -0,0 +1,48 @@ +{ + "comment": "Because this document has at least one tag that matches the search 'lightcast', all of its tags get returned.", + "hits": [ + { + "tags": { + "taxonomy": [ + "ESDC Skills and Competencies", + "FlatTaxonomy", + "HierarchicalTaxonomy", + "Lightcast Open Skills Taxonomy" + ], + "level0": [ + "ESDC Skills and Competencies > Interests", + "FlatTaxonomy > flat taxonomy tag 1420", + "FlatTaxonomy > flat taxonomy tag 1683", + "FlatTaxonomy > flat taxonomy tag 2633", + "HierarchicalTaxonomy > hierarchical taxonomy tag 1", + "HierarchicalTaxonomy > hierarchical taxonomy tag 2", + "HierarchicalTaxonomy > hierarchical taxonomy tag 4", + "Lightcast Open Skills Taxonomy > Information Technology Category" + ], + "level1": [ + "ESDC Skills and Competencies > Interests > Holland Codes", + "HierarchicalTaxonomy > hierarchical taxonomy tag 1 > hierarchical taxonomy tag 1.3", + "HierarchicalTaxonomy > hierarchical taxonomy tag 2 > hierarchical taxonomy tag 2.16", + "HierarchicalTaxonomy > hierarchical taxonomy tag 4 > hierarchical taxonomy tag 4.8", + "Lightcast Open Skills Taxonomy > Information Technology Category > Web Content" + ], + "level2": [ + "ESDC Skills and Competencies > Interests > Holland Codes > Interests - Holland Codes", + "HierarchicalTaxonomy > hierarchical taxonomy tag 1 > hierarchical taxonomy tag 1.3 > hierarchical taxonomy tag 1.3.7", + "HierarchicalTaxonomy > hierarchical taxonomy tag 2 > hierarchical taxonomy tag 2.16 > hierarchical taxonomy tag 2.16.31", + "HierarchicalTaxonomy > hierarchical taxonomy tag 4 > hierarchical taxonomy tag 4.8 > hierarchical taxonomy tag 4.8.25", + "Lightcast Open Skills Taxonomy > Information Technology Category > Web Content > Web Resource" + ], + "level3": [ + "ESDC Skills and Competencies > Interests > Holland Codes > Interests - Holland Codes > Artistic", + "ESDC Skills and Competencies > Interests > Holland Codes > Interests - Holland Codes > Investigative" + ] + } + } + ], + "query": "lightcast", + "processingTimeMs": 3, + "limit": 1000, + "offset": 0, + "estimatedTotalHits": 23 +} diff --git a/src/search-modal/messages.js b/src/search-modal/messages.js index 13a23138c4..d72a5b6b78 100644 --- a/src/search-modal/messages.js +++ b/src/search-modal/messages.js @@ -135,6 +135,11 @@ const messages = defineMessages({ defaultMessage: 'Search tags', description: 'Placeholder text shown in the input field that allows searching through the available tags', }, + submitSearchTagsByKeyword: { + id: 'course-authoring.course-search.submitSearchTagsByKeyword', + defaultMessage: 'Submit tag keyword search', + description: 'Text shown to screen reader users for the search button on the tags keyword search', + }, showMore: { id: 'course-authoring.course-search.showMore', defaultMessage: 'Show more', From 32c78ddc04b7afcbf6cf6593e3c368342034a67d Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 19 Apr 2024 10:52:22 -0700 Subject: [PATCH 14/15] perf: remove some unnecessary queries --- src/search-modal/data/apiHooks.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/search-modal/data/apiHooks.js b/src/search-modal/data/apiHooks.js index 936e12c167..02488635da 100644 --- a/src/search-modal/data/apiHooks.js +++ b/src/search-modal/data/apiHooks.js @@ -77,6 +77,7 @@ export const useContentSearchResults = ({ getNextPageParam: (lastPage) => lastPage.nextOffset, // Avoid flickering results when user is typing... keep old results until new is available. keepPreviousData: true, + refetchOnWindowFocus: false, // This doesn't need to be refreshed when the user switches back to this tab. }); const pages = query.data?.pages; @@ -128,7 +129,6 @@ export const useTagFilterOptions = (args) => { args.searchKeywords, args.blockTypesFilter, args.parentTagPath, - args.tagSearchKeywords, ], queryFn: () => { const { client, indexName } = args; @@ -139,13 +139,14 @@ export const useTagFilterOptions = (args) => { }, // Avoid flickering results when user is typing... keep old results until new is available. keepPreviousData: true, + refetchOnWindowFocus: false, // This doesn't need to be refreshed when the user switches back to this tab. }); const tagKeywordSearchData = useQuery({ enabled: args.client !== undefined && args.indexName !== undefined, queryKey: [ 'content_search', - 'tags_keyword_search_dat', + 'tags_keyword_search_data', args.client?.config.apiKey, args.client?.config.host, args.indexName, @@ -162,6 +163,7 @@ export const useTagFilterOptions = (args) => { }, // Avoid flickering results when user is typing... keep old results until new is available. keepPreviousData: true, + refetchOnWindowFocus: false, // This doesn't need to be refreshed when the user switches back to this tab. }); const data = React.useMemo(() => { From 2fab8925dd0da557f4d7635821bec25bb531babe Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Fri, 19 Apr 2024 11:03:35 -0700 Subject: [PATCH 15/15] perf: load tags filter data in parallel when possible --- src/search-modal/data/api.js | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/search-modal/data/api.js b/src/search-modal/data/api.js index cf9cd6b620..126df5248f 100644 --- a/src/search-modal/data/api.js +++ b/src/search-modal/data/api.js @@ -235,6 +235,17 @@ export async function fetchAvailableTagOptions({ parentFilter = [`${parentFacetName} = "${parentTagPath}"`]; } + // As an optimization, start pre-loading the data about "has child tags", if we will need it later. + // Notice we don't 'await' the result of this request, so it can happen in parallel with the main request that follows + const maybeHasChildren = depth > 0 && depth < 4; // If depth=0, it definitely has children; we don't support depth > 4 + const nextLevelFacet = `tags.level${depth}`; // This will give the children of the current tags. + const preloadChildTagsData = maybeHasChildren ? client.index(indexName).searchForFacetValues({ + facetName: nextLevelFacet, + facetQuery: parentTagPath, + q: searchKeywords, + filter: [...extraFilterFormatted, ...blockTypesFilterFormatted, ...parentFilter], + }) : undefined; + // Now load the facet values. Doing it with this API gives us much more flexibility in loading than if we just // requested the facets by passing { facets: ["tags"] } into the main search request; that works fine for loading the // root tags but can't load specific child tags like we can using this approach. @@ -273,15 +284,10 @@ export async function fetchAvailableTagOptions({ }); // Figure out if [some of] the tags at this level have children: - if (depth > 0 && depth < 4) { - const nextLevelFacet = `tags.level${depth}`; // This will give the children of the current tags. + if (maybeHasChildren) { + if (preloadChildTagsData === undefined) { throw new Error('Child tags data unexpectedly not pre-loaded'); } // Retrieve the children of the current tags: - const { facetHits: childFacetHits } = await client.index(indexName).searchForFacetValues({ - facetName: nextLevelFacet, - facetQuery: parentTagPath, - q: searchKeywords, - filter: [...extraFilterFormatted, ...blockTypesFilterFormatted, ...parentFilter], - }); + const { facetHits: childFacetHits } = await preloadChildTagsData; if (childFacetHits.length >= meilisearchFacetLimit) { // Assume they all have child tags; we can't retrieve more than 100 facet values (per Meilisearch docs) so // we can't say for sure on a tag-by-tag basis, but we know that at least some of them have children, so