Simplify Nuxt query parameter computation (#3136)

* Simplify query building and remove searchBy filter

* Simplify search query building

* Remove unused function

* Improve `getSearchPath` function

* Add and update unit tests

* Add missing tapes

* Remove tapes with searchBy parameter

* Remove searchBy from tests

* Remove sensitive ff

* Add changes from the code review

frontend/src/composables/use-external-sources.ts

@@ -31,7 +31,7 @@ export const useExternalSources = () => {
     return IMAGE
   })
   const externalSources = computed(() => {
-    const query = searchStore.searchQueryParams
+    const query = searchStore.apiSearchQueryParams
     const type = externalSourcesType.value
     return getAdditionalSources(type, query)
   })

frontend/src/constants/filters.ts

@@ -22,7 +22,6 @@ export interface Filters {
   sizes: FilterItem[]
   audioProviders: FilterItem[]
   imageProviders: FilterItem[]
-  searchBy: FilterItem[]
 }
 export type FilterCategory = keyof Filters
 
@@ -40,7 +39,6 @@ export const mediaFilterKeys = deepFreeze<Record<SearchType, FilterCategory[]>>(
       "aspectRatios",
       "sizes",
       "imageProviders",
-      "searchBy",
     ],
     [AUDIO]: [
       "licenseTypes",
@@ -49,11 +47,10 @@ export const mediaFilterKeys = deepFreeze<Record<SearchType, FilterCategory[]>>(
       "audioExtensions",
       "lengths",
       "audioProviders",
-      "searchBy",
     ],
     [VIDEO]: [],
     [MODEL_3D]: [],
-    [ALL_MEDIA]: ["licenseTypes", "licenses", "searchBy"],
+    [ALL_MEDIA]: ["licenseTypes", "licenses"],
   }
 )
 
@@ -96,7 +93,6 @@ const filterCodesPerCategory = deepFreeze<Record<FilterCategory, string[]>>({
   sizes: ["small", "medium", "large"],
   audioProviders: [],
   imageProviders: [],
-  searchBy: ["creator"],
 })
 /**
  * Converts the filterCodesPerCategory object into the format that's used by the filter store.

frontend/src/data/media-service.ts

@@ -1,5 +1,5 @@
 import { decodeMediaData } from "~/utils/decode-media-data"
-import type { ApiQueryParams } from "~/utils/search-query-transform"
+import type { PaginatedSearchQuery } from "~/types/search"
 import type { ApiService } from "~/data/api-service"
 import type { DetailFromMediaType, Media } from "~/types/media"
 import { AUDIO, type SupportedMediaType } from "~/constants/media"
@@ -47,7 +47,7 @@ class MediaService<T extends Media> {
    * @param params - API search query parameters
    */
   async search(
-    params: ApiQueryParams
+    params: PaginatedSearchQuery
   ): Promise<MediaResult<Record<string, Media>>> {
     // Add the `peaks` param to all audio searches automatically
     if (this.mediaType === AUDIO) {
@@ -88,14 +88,11 @@ class MediaService<T extends Media> {
         `MediaService.getRelatedMedia() id parameter required to retrieve related media.`
       )
     }
-    const params: ApiQueryParams = {}
-    if (this.mediaType === AUDIO) {
-      params.peaks = "true"
-    }
+    const params = this.mediaType === AUDIO ? { peaks: "true" } : undefined
     const res = (await this.apiService.get(
       this.mediaType,
       `${id}/related`,
-      params as unknown as Record<string, string>
+      params
     )) as AxiosResponse<MediaResult<DetailFromMediaType<T>[]>>
     return {
       ...res.data,

frontend/src/pages/search.vue

@@ -101,7 +101,7 @@ export default defineComponent({
     const {
       searchTerm,
       searchType,
-      searchQueryParams: query,
+      apiSearchQueryParams: query,
       searchTypeIsSupported: supported,
     } = storeToRefs(searchStore)
 

frontend/src/stores/media/index.ts

@@ -3,7 +3,6 @@ import { defineStore } from "pinia"
 import { warn } from "~/utils/console"
 import { hash, rand as prng } from "~/utils/prng"
 import { isRetriable, parseFetchingError } from "~/utils/errors"
-import prepareSearchQueryParams from "~/utils/prepare-search-query-params"
 import type {
   AudioDetail,
   DetailFromMediaType,
@@ -25,6 +24,8 @@ import { isSearchTypeSupported, useSearchStore } from "~/stores/search"
 import { useRelatedMediaStore } from "~/stores/media/related-media"
 import { deepFreeze } from "~/utils/deep-freeze"
 
+import { PaginatedSearchQuery } from "~/types/search"
+
 export type MediaStoreResult = {
   count: number
   pageCount: number
@@ -442,19 +443,13 @@ export const useMediaStore = defineStore("media", {
       mediaType: SupportedMediaType
       shouldPersistMedia: boolean
     }) {
-      const queryParams = prepareSearchQueryParams({
-        ...useSearchStore().searchQueryParams,
-      })
-      let page = 1
-      if (shouldPersistMedia) {
-        /**
-         * If `shouldPersistMedia` is true, then we increment the page that was set by a previous
-         * fetch. Normally, if `shouldPersistMedia` is true, `page` should have been set to 1 by the
-         * previous fetch.
-         */
-        page = this.results[mediaType].page + 1
-        queryParams.page = `${page}`
+      let page = this.results[mediaType].page + 1
+      const queryParams: PaginatedSearchQuery = {
+        ...useSearchStore().apiSearchQueryParams,
+        // Don't need to set `page` parameter for the first page.
+        page: shouldPersistMedia ? `${page}` : undefined,
       }
+
       this._updateFetchState(mediaType, "start")
       try {
         const accessToken = this.$nuxt.$openverseApiToken
@@ -467,7 +462,7 @@ export const useMediaStore = defineStore("media", {
          * In such cases, we show the "No results" client error page.
          */
         if (!mediaCount) {
-          page = 0
+          page = 1
           errorData = {
             message: `No results found for ${queryParams.q}`,
             code: NO_RESULT,

frontend/src/stores/search.ts

@@ -19,10 +19,9 @@ import {
   searchPath,
 } from "~/constants/media"
 import {
-  ApiQueryParams,
   filtersToQueryData,
   queryDictionaryToQueryParams,
-  queryStringToSearchType,
+  pathToSearchType,
   queryToFilterData,
 } from "~/utils/search-query-transform"
 import {
@@ -39,6 +38,8 @@ import { useProviderStore } from "~/stores/provider"
 import { useFeatureFlagStore } from "~/stores/feature-flag"
 import { useMediaStore } from "~/stores/media"
 
+import { SearchQuery, PaginatedSearchQuery } from "~/types/search"
+
 import type { Ref } from "vue"
 
 import type { Dictionary } from "vue-router/types/router"
@@ -62,45 +63,39 @@ export interface SearchState {
 /**
  * Builds the search query parameters for the given search type, filters, and search term.
  * `q` parameter is always included as the first query parameter.
+ * If the search type is not supported, only the `q` parameter is included.
+ * This is used, for instance, for content switcher links for `video`/`model_3d` search pages.
  * Only the filters that are relevant for the search type and have a value are included.
  *
- * Some parameters are included in the query depending on the mode:
- * - `INCLUDE_SENSITIVE_QUERY_PARAM` is added to the API search query if the setting is `on`
- * in the featureFlagStore.
+ * `INCLUDE_SENSITIVE_QUERY_PARAM` is never added to the frontend search query. It is added
+ * to the API search query if the setting is `on` in the featureFlagStore.
  */
-function computeQueryParams(
+export function computeQueryParams(
   searchType: SearchType,
   filters: Filters,
   searchTerm: string,
   mode: "frontend" | "API"
 ) {
-  // The filters object is converted to a Record<string, string> object.
-  // e.g., { licenseTypes: [{ code: "commercial", checked: true }] }
-  // => { license_type: "commercial" }
-  const query = { ...filtersToQueryData(filters, searchType) }
-
+  const q = searchTerm.trim()
+  if (!isSearchTypeSupported(searchType)) {
+    return { q }
+  }
   // Ensure that `q` always comes first in the frontend URL.
-  const search_query: ApiQueryParams = { q: searchTerm.trim() }
-
-  // Parameters that are included in the query "as is".
-  const param_names = Object.keys(query).filter(
-    (key): key is Exclude<keyof ApiQueryParams, "q"> =>
-      !["q", INCLUDE_SENSITIVE_QUERY_PARAM].includes(key)
-  )
-
-  for (const api_param_name of param_names) {
-    if (query[api_param_name]?.length) {
-      search_query[api_param_name] = query[api_param_name]
-    }
+  const searchQuery: SearchQuery = {
+    q,
+    // The filters object is converted to a Record<string, string> object.
+    // e.g., { licenseTypes: [{ code: "commercial", checked: true }] }
+    // => { license_type: "commercial" }
+    ...filtersToQueryData(filters, searchType),
   }
 
   // `INCLUDE_SENSITIVE_QUERY_PARAM` is used in the API params, but not shown on the frontend.
   const ffStore = useFeatureFlagStore()
   if (mode === "API" && ffStore.isOn("fetch_sensitive")) {
-    search_query[INCLUDE_SENSITIVE_QUERY_PARAM] = "true"
+    searchQuery[INCLUDE_SENSITIVE_QUERY_PARAM] = "true"
   }
 
-  return search_query
+  return searchQuery
 }
 
 export const useSearchStore = defineStore("search", {
@@ -122,37 +117,17 @@ export const useSearchStore = defineStore("search", {
     },
 
     /**
-     * Returns the search query parameters for API request:
-     * drops all parameters with blank values.
+     * Returns the search query parameters for API request.
+     * The main difference between api and frontend query parameters is that
+     * the API query parameters include the `include_sensitive_results` parameter.
      */
-    searchQueryParams(state) {
-      if (isSearchTypeSupported(state.searchType)) {
-        return computeQueryParams(
-          state.searchType,
-          state.filters,
-          state.searchTerm,
-          "API"
-        )
-      } else {
-        return { q: state.searchTerm }
-      }
-    },
-
-    /**
-     * Returns the search query parameters for API request:
-     * drops all parameters with blank values.
-     */
-    frontendSearchUrlParams(state) {
-      if (isSearchTypeSupported(state.searchType)) {
-        return computeQueryParams(
-          state.searchType,
-          state.filters,
-          state.searchTerm,
-          "frontend"
-        )
-      } else {
-        return { q: state.searchTerm }
-      }
+    apiSearchQueryParams(state) {
+      return computeQueryParams(
+        state.searchType,
+        state.filters,
+        state.searchTerm,
+        "API"
+      )
     },
 
     /**
@@ -170,16 +145,12 @@ export const useSearchStore = defineStore("search", {
     /**
      * Returns the object with filters for selected search type,
      * with codes, names for i18n labels, and checked status.
-     *
-     * Excludes `searchBy` filters that we don't display.
      */
     searchFilters(state) {
-      return mediaFilterKeys[state.searchType]
-        .filter((filterKey) => filterKey !== "searchBy")
-        .reduce((obj, filterKey) => {
-          obj[filterKey] = this.filters[filterKey]
-          return obj
-        }, {} as Filters)
+      return mediaFilterKeys[state.searchType].reduce((obj, filterKey) => {
+        obj[filterKey] = this.filters[filterKey]
+        return obj
+      }, {} as Filters)
     },
 
     /**
@@ -223,37 +194,32 @@ export const useSearchStore = defineStore("search", {
       return this.getSearchPath()
     },
     /**
-     * Returns localized search path for the given search type.
+     * Returns localized frontend search path for the given search type.
      *
      * If search type is not provided, returns the path for the current search type.
      * If query is not provided, returns current query parameters.
+     * If only the search type is provided, the query is computed for this search type.
      */
     getSearchPath({
       type,
       query,
-    }: { type?: SearchType; query?: ApiQueryParams } = {}): string {
-      const searchType = type || this.searchType
-      let queryParams
-      if (!query) {
-        if (type && isSearchTypeSupported(type)) {
-          queryParams = computeQueryParams(
-            type,
-            this.filters,
-            this.searchTerm,
-            "frontend"
-          )
-        } else {
-          queryParams = this.frontendSearchUrlParams
-        }
-      } else {
-        queryParams = query
-      }
+    }: { type?: SearchType; query?: PaginatedSearchQuery } = {}): string {
+      const searchType = type ?? this.searchType
+      const queryParams =
+        query ??
+        computeQueryParams(
+          searchType,
+          this.filters,
+          this.searchTerm,
+          "frontend"
+        )
 
       return this.$nuxt.localePath({
         path: searchPath(searchType),
-        query: queryParams as Dictionary<string>,
+        query: queryParams as unknown as Dictionary<string>,
       })
     },
+
     setSearchType(type: SearchType) {
       const featureFlagStore = useFeatureFlagStore()
       if (
@@ -486,7 +452,7 @@ export const useSearchStore = defineStore("search", {
       })
 
       this.setSearchTerm(query.q)
-      this.searchType = queryStringToSearchType(path)
+      this.searchType = pathToSearchType(path)
       if (!isSearchTypeSupported(this.searchType)) return
 
       const newFilterData = queryToFilterData({
@@ -506,9 +472,9 @@ export const useSearchStore = defineStore("search", {
     isFilterDisabled(
       item: FilterItem,
       filterCategory: FilterCategory
-    ): boolean | undefined {
+    ): boolean {
       if (!["licenseTypes", "licenses"].includes(filterCategory)) {
-        return
+        return false
       }
       if (item.code === "commercial" || item.code === "modification") {
         const targetCode = {
@@ -532,11 +498,5 @@ export const useSearchStore = defineStore("search", {
         )
       }
     },
-
-    isFilterChecked(filterCategory: FilterCategory, code: string): boolean {
-      const filterItems = this.filters[filterCategory]
-      const idx = filterItems.findIndex((f) => f.code === code)
-      return idx >= 0 && filterItems[idx].checked
-    },
   },
 })