From 3a2c0fc893e5db24a5a6c83f54b920731cc510dd Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Wed, 19 Jul 2023 20:30:29 +0300 Subject: [PATCH 1/7] Additional search views implementation plan --- ...ementation_plan_additional_search_views.md | 263 ++++++++++++++++++ 1 file changed, 263 insertions(+) create mode 100644 documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md diff --git a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md new file mode 100644 index 00000000000..9e15c40fbb0 --- /dev/null +++ b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md @@ -0,0 +1,263 @@ +2023-07-20 Implementation Plan: Additional Search Views + +**Author**: @obulat + + + + +## Reviewers + + + +- [ ] TBD +- [ ] TBD + +## Project links + + + +- [Project Thread](https://github.com/WordPress/openverse/issues/410) +- [Project Proposal](https://docs.openverse.org/projects/proposals/additional_search_views/20230424-project_proposal_additional_search_views.html) + +## Overview + + + +This plan describes the changes that need to be added on the frontend: the new +components, pages and store. + +## Expected Outcomes + + + +Three collection pages are added to Openverse: + +- items with the selected tag +- items from the selected source (provider) +- items by the selected creator. The pages re-use the existing image grid and + audio collection views and can load more images or audio items on the Load + more button click. The single result pages are updated to add the links to + these pages, as seen in the Figma mockups: [link to Figma mockup] + +## Outlined Steps + + + +### Page changes + +#### Create a page for `tag` / `creator` /`source` collections. + +Nuxt allows creating nested dynamic routes like +`/pages/_collection/_mediaType/_term`. + +Here, the collection can be `tag`, `creator` or `source`. `mediaType` can be one +of the supported media types (currently, `image` and `audio`). `term` refers to +the actual value of the tag, creator or source name. + +This page should use the +[`validate` method](https://v2.nuxt.com/docs/components-glossary/validate/) to +make sure that the `collection`, `searchType` and `term` `params` are valid. + +``` +`validate({ params, $pinia }): { +const { collection, searchType, term } = params +``` + +This page should also update the state (`searchType`, `searchTerm` and +`searchBy` filter) in the `search` store and handle fetching using +`mediaStore`'s `fetchMedia` method in the `useFetch` hook. Since it is not +possible to change the path or query parameters from this page client-side, +fetching can be much simpler than on the current search page (that has to watch +for changes in the route and fetch if necessary). + +This page should have the common header (title, button - link to the source +external URL or the creator URL, and the summary of item counts in the +collection), and a nested component: the image grid or the audio collection. + +### New and updated components + +#### Extract the `VAudioCollection` component + +Currently, it is not possible to reuse the audio collection from the audio +search result page because it is a part of the `audio.vue` page. We should +extract the part that shows the loading skeleton, the column of `VAudioTrack` +rows and the Load more section. This component will be reused in the audio +search page and on the Additional search views. One thing that I'm not sure +about here is whether the analytics events should stay the same and be sent from +all pages - or if we should somehow differentiate the events coming from +different search views. + +#### Add a `VCollectionHeader` component + +Figma links: +[creator desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56323&mode=design&t=pN0PPAzlKQEKT9tJ-4), +[creator mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56362&mode=design&t=suLIyJHNmZrM0mPH-4), +[source desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56381&mode=design&t=suLIyJHNmZrM0mPH-4), +[source mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56421&mode=design&t=suLIyJHNmZrM0mPH-4), +[tag desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1216-58402&mode=design&t=suLIyJHNmZrM0mPH-4), +[tag mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56457&mode=design&t=suLIyJHNmZrM0mPH-4) + +The header should have an icon (tag, creator or source), the name of the +tag/creator/source. For source and the creator, there should be an external link +button. + +The header should also display the number of results, "251 audio files with the +selected tag", "604 images provided by this source", "37 images by this creator +in Hirshhorn Museum and Sculpture Garden". + +#### Add VCreatorLink component + +The "by creator" line under the main item on the single result page should be +replaced with a `VCreatorLink` component - a wrapper over `VButton` with a link +to the localized "creator" collection page. + +#### Add VSourceLink component + +Both the source and the provider should be replaced with `VSourceLink` component +that links to the `source` page. This component is also a wrapper over `VButton` +component. + +#### Update VMediaTag component to be a `VButton` wrapping a `VLink` + +Each `VMediaTag` should be a `VButton` with `as="VLink"` and should link to the +localized page for the tag collection. Figma link: +https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56515&mode=design&t=nCX20BtJYqMOFAQm-4 +[VMediaTag component](https://github.com/WordPress/openverse/blob/c7b76139d5a001ce43bde27805be5394e5732d1a/frontend/src/components/VMediaTag/VMediaTag.vue) +`{{tag}}`. +It should accept `href` as a prop. + +### Other interface changes + +#### Update the "by" area of the single result page + +The "by creator" line should be replaced with a section that is +horizontally-scrollable on mobile, and contains the `VCreatorLink` and +`VSourceLink` components. + +#### Update links in the "information" section + +Add external link icon to the provider link; add creator link with external link +icon The links to creator in the image and audio single result pages should have +an "external link" icon. +[image creator link](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/pages/image/_id/index.vue#L62-L73) +and +[audio creator link](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioTrack/layouts/VFullLayout.vue#L32-L41). +Audio creator link should also be updated to match the image creator link. It +should be a conditional component: `VLink` if the `creator_url` is truthy and +`span` if the `creator_url` is falsy. + +Currently, the `foreign_landing_url` is linked to the "source" in the image page +and "provider" in the audio page. The audio page should be updated to match the +image page: the `foreign_landing_url` link should be added to the "source", not +provider. +[Audio provider link](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioDetails/VAudioDetails.vue#L61C1-L63), +[image source link](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VImageDetails/VImageDetails.vue#L26-L28) + +### Store changes + +#### Update the `searchBy` filter in the `search` store + +Currently, this filter is shaped as other filters: it is a list of objects with +`code`, `name` and `checked` properties. This is a possible value now: `{ +searchBy: { code: "creator", name: "filters.searchBy.creator", checked: true }}. + +Since this filter is mutually exclusive (you cannot search by both the creator +and the tag, for example), this shape is very difficult to update. + +It is better to set the `searchBy` filter to have one of the possible values: +`{ searchBy: }`. + +We should also create a new method, `setSearchBy`, in the `search` store, that +would allow directly set the `searchBy` filter value. + +### Analytics changes + +I am not sure what events we want to add, if any. Should we add `_CLICKED` +events for the new views, or would page views be sufficient? + +Should we add a new `SELECT_COLLECTION_ITEM` event similar to +`SELECT_SEARCH_RESULT`? + +The clicks on external creator link in the `VCollectionHeader` should be tracked +as `VISIT_CREATOR_LINK` events. We don't have a special event for visiting +source, should we add one, or should they be tracked as generic +`VISIT_EXTERNAL_LINK`? + +### Tests + +We should add visual-regression tests for the new views. To minimize flakiness +due to slow loading of the images, we should probably use the +`{ filter: brightness(0%); }` trick for the images on the page. + +The search store tests should be updated to reflect the changes to the filters. + +## Dependencies + +### Infrastructure + + + +These views potentially might cause more load on our infrastructure due to +increase in scraping activity. + +### Tools & packages + + + +No new tools or packages are necessary. + +### Other projects or work + + + +## Design + + + +Figma designs in the dev mode: +https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?node-id=22%3A13656&mode=dev + +## Parallelizable streams + + + +The work on components (`VCreatorLink`, `VSourceLink`, `VMediaTag`), and on the +search store changes can be parallelized. + +We should create a separate branch for this work because adding the links to the +collection pages on the single result pages (`VCreatorLink`, `VSourceLink`, +`VMediaTag`) before the collection pages are ready is not possible. + +## Blockers + + + +The main blocker could be the maintainer capacity. If the "creator" API call is +not easy to construct (since we need to combine both the creator and the +provider), this might also be a blocker. + +## Accessibility + + + +We should make sure that the search titles are accessible, and the pages clearly +indicate the change of context. + +## Rollback + + + +If we work on a separate branch, it would be easy to rollback by reverting the +branch merge commit. + +## Risks + + + +The biggest risk I see is that this project might be seen as an "invitation" to +scraping. Hopefully, the work on providing the dataset would minimize such +risks. + +## Prior art + + From 2e1ad5aa6e51d67c5f6374ba6daee2b692689e6b Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Fri, 21 Jul 2023 10:13:41 +0300 Subject: [PATCH 2/7] Add changes from the review --- ...ementation_plan_additional_search_views.md | 265 ++++++++++++------ 1 file changed, 185 insertions(+), 80 deletions(-) diff --git a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md index 9e15c40fbb0..5d7c5c25514 100644 --- a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md +++ b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md @@ -1,4 +1,4 @@ -2023-07-20 Implementation Plan: Additional Search Views +# 2023-07-20 Implementation Plan: Additional Search Views **Author**: @obulat @@ -34,15 +34,123 @@ Three collection pages are added to Openverse: - items with the selected tag - items from the selected source (provider) -- items by the selected creator. The pages re-use the existing image grid and - audio collection views and can load more images or audio items on the Load - more button click. The single result pages are updated to add the links to - these pages, as seen in the Figma mockups: [link to Figma mockup] +- items by the selected creator. + +The pages re-use the existing image grid and audio collection views and can load +more images or audio items on the Load more button click. The single result +pages are updated to add the links to these pages, as seen in the Figma mockups: + +- [Image single result](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-63284&mode=dev) +- [Audio single result](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-63285&mode=dev) + +For the creator pages, a separate API media view is added to get the items that +match the creator and source pair exactly. ## Outlined Steps +### API changes + +Currently, when filtering the search results, the API matches some query +parameters in a fuzzy way: an item matches the query if the field value contains +the query string as a separate word. When indexing the items, we "analyze" them, +which means that we split the field values by whitespace and stem them. We also +do the same to the query string. This means that the query string "bike" will +match the field value "bikes", "biking", or "bike shop". + +For most of these pages, we need an exact match instead. This section will +describe each page and the necessary changes. + +#### Source page + +To get the items from the selected source, we can use the `source` query +parameter: https://api.openverse.engineering/v1/images/?source=met + +There is a limited number of sources, and we validate the `source` parameter +against the full list. This means that the `source` parameter will be matched +exactly. + +#### Creator page + +We could update the Elasticsearch index and the search controller to allow for +exact matching by the `creator` field. However, I think there is an easier way +that does not require any changes to the indexes: use the Django ORM to filter +the items by the creator name and the provider name. + +We can add a media view at +`/api/v1//creator/?q=&provider=`. It would +use `self.get_queryset().filter(creator=creator, provider=provider)` to get all +the items by the creator from the provider. This view would also need to filter +out dead links, and return a paginated response. + +#### Tag page + +To get the items matching a tag, we can use the `tags` query parameter with a +quoted value. +https://api.openverse.engineering/v1/images/?tags=%22black%20cat%22 This query +returns all images with tags that contain the phrase "black cat". + +This would not match the tags exactly, but tags page is supposed to show all +items that are associated with the same topic or category. + +We could also add a media view at `/api/v1//tags/?q=`, but it +is difficult to implement because the tags are stored as a list JSONField model. + +### Nuxt store changes + +We can re-use the search store as is for these pages. Currently, the store uses +the `searchBy` filter to determine the shape of the API query. If `searchBy` +value is set, then the `q` parameter is replaced with the +`=` query parameter. +https://github.com/WordPress/openverse/blob/b4b46b903731870015c475d2c08eebef7ec6b25b/frontend/src/utils/prepare-search-query-params.ts#L22-L25 + +We can update the `searchBy` filter to have one of the possible values: +`{ searchBy: }`. Then, this value would be used to +construct the API query. + +#### Update the `searchBy` filter + +Currently, this filter is shaped as other filters: it is a list of objects with +`code`, `name` and `checked` properties. This is a possible value now: `{ +searchBy: { code: "creator", name: "filters.searchBy.creator", checked: true }}. + +Since this filter is mutually exclusive (you cannot search by both the creator +and the tag, for example), this shape is very difficult to update. + +It is better to set the `searchBy` filter to have one of the possible values: +`{ searchBy: }`. + +We should create a new method, `setSearchBy`, in the `search` store, that would +allow directly set the `searchBy` filter value. + +This change should also update `prepare-search-query-params` to use the +`searchBy` filter value to construct the API query: + +```typescript +const prepareSearchQuery = ( + searchParams: Record, + mediaType: SupportedMediaType +) => { + if (searchParams.searchBy) { + if (searchBy === "creator") { + // the name of the source is stored in the `Provider` filter + return { + creator: searchTerm, + source: searchParams[`${mediaType}Provider`], + } + } else if (searchBy === "source") { + return { source: searchParams[`${mediaType}Provider`] } + } else if (searchBy === "tag") { + // The parameter is plural in the API + return { tags: searchTerm } + } + } else { + // Current search query transform + } +} +``` + ### Page changes #### Create a page for `tag` / `creator` /`source` collections. @@ -56,16 +164,21 @@ the actual value of the tag, creator or source name. This page should use the [`validate` method](https://v2.nuxt.com/docs/components-glossary/validate/) to -make sure that the `collection`, `searchType` and `term` `params` are valid. +make sure that the `collection`, `mediaType` and `term` `params` are valid. ``` -`validate({ params, $pinia }): { -const { collection, searchType, term } = params +validate({ params, $pinia }): { + const { collection, mediaType, term } = params + // Check that collection is one of ["tag", "creator" or "source"], + // and mediaType is one of `supportedMediaTypes`. + // Check that `term` is correctly escaped. + // If the params are not valid, return `false` to show the error page. + return isValid ? true : false ``` This page should also update the state (`searchType`, `searchTerm` and -`searchBy` filter) in the `search` store and handle fetching using -`mediaStore`'s `fetchMedia` method in the `useFetch` hook. Since it is not +`searchBy` and `provider` filters) in the `search` store and handle fetching +using `mediaStore`'s `fetchMedia` method in the `useFetch` hook. Since it is not possible to change the path or query parameters from this page client-side, fetching can be much simpler than on the current search page (that has to watch for changes in the route and fetch if necessary). @@ -90,12 +203,19 @@ different search views. #### Add a `VCollectionHeader` component Figma links: -[creator desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56323&mode=design&t=pN0PPAzlKQEKT9tJ-4), -[creator mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56362&mode=design&t=suLIyJHNmZrM0mPH-4), -[source desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56381&mode=design&t=suLIyJHNmZrM0mPH-4), -[source mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56421&mode=design&t=suLIyJHNmZrM0mPH-4), -[tag desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1216-58402&mode=design&t=suLIyJHNmZrM0mPH-4), -[tag mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56457&mode=design&t=suLIyJHNmZrM0mPH-4) + +- creator + [desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56323&mode=design&t=pN0PPAzlKQEKT9tJ-4) + and + [mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56362&mode=design&t=suLIyJHNmZrM0mPH-4) +- source + [desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56381&mode=design&t=suLIyJHNmZrM0mPH-4) + and + [mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56421&mode=design&t=suLIyJHNmZrM0mPH-4) +- tag + [desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1216-58402&mode=design&t=suLIyJHNmZrM0mPH-4) + and + [mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56457&mode=design&t=suLIyJHNmZrM0mPH-4) The header should have an icon (tag, creator or source), the name of the tag/creator/source. For source and the creator, there should be an external link @@ -105,19 +225,21 @@ The header should also display the number of results, "251 audio files with the selected tag", "604 images provided by this source", "37 images by this creator in Hirshhorn Museum and Sculpture Garden". -#### Add VCreatorLink component +Note: There are sources that only have works by one creator. In this case, we +should probably still have two separate pages for the source and the creator, +but we might want to add a note that the creator is the only one associated with +this source. -The "by creator" line under the main item on the single result page should be -replaced with a `VCreatorLink` component - a wrapper over `VButton` with a link -to the localized "creator" collection page. +#### Add `VCollectionLink` component and update the title and links container -#### Add VSourceLink component - -Both the source and the provider should be replaced with `VSourceLink` component -that links to the `source` page. This component is also a wrapper over `VButton` -component. +The "by creator" line under the main item on the single result page should be +replaced with a section that has two buttons: one for a creator link and a +source link. These links should be `VButtons` with `as="VLink"` and should link +to the localized page for the creator or the source pages. This section should +be horizontally scrollable on mobile. It should implement a scroll-snap +(example: https://play.tailwindcss.com/AbfA33Za50) -#### Update VMediaTag component to be a `VButton` wrapping a `VLink` +#### Update `VMediaTag` component to be a `VButton` wrapping a `VLink` Each `VMediaTag` should be a `VButton` with `as="VLink"` and should link to the localized page for the tag collection. Figma link: @@ -128,66 +250,49 @@ It should accept `href` as a prop. ### Other interface changes -#### Update the "by" area of the single result page - -The "by creator" line should be replaced with a section that is -horizontally-scrollable on mobile, and contains the `VCreatorLink` and -`VSourceLink` components. - #### Update links in the "information" section -Add external link icon to the provider link; add creator link with external link -icon The links to creator in the image and audio single result pages should have -an "external link" icon. -[image creator link](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/pages/image/_id/index.vue#L62-L73) +The links to creator in the +[image](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/pages/image/_id/index.vue#L62-L73) and -[audio creator link](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioTrack/layouts/VFullLayout.vue#L32-L41). +[audio](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioTrack/layouts/VFullLayout.vue#L32-L41) +single result pages Information section should have an "external link" icon. + Audio creator link should also be updated to match the image creator link. It should be a conditional component: `VLink` if the `creator_url` is truthy and `span` if the `creator_url` is falsy. -Currently, the `foreign_landing_url` is linked to the "source" in the image page -and "provider" in the audio page. The audio page should be updated to match the -image page: the `foreign_landing_url` link should be added to the "source", not -provider. -[Audio provider link](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioDetails/VAudioDetails.vue#L61C1-L63), -[image source link](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VImageDetails/VImageDetails.vue#L26-L28) - -### Store changes +Currently, the `foreign_landing_url` is linked to the "source" in the +[image](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VImageDetails/VImageDetails.vue#L26-L28) +page and "provider" in the +[audio page](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioDetails/VAudioDetails.vue#L61C1-L63). +The audio page should be updated to match the image page: the +`foreign_landing_url` link should be added to the "source", not provider. -#### Update the `searchBy` filter in the `search` store - -Currently, this filter is shaped as other filters: it is a list of objects with -`code`, `name` and `checked` properties. This is a possible value now: `{ -searchBy: { code: "creator", name: "filters.searchBy.creator", checked: true }}. - -Since this filter is mutually exclusive (you cannot search by both the creator -and the tag, for example), this shape is very difficult to update. - -It is better to set the `searchBy` filter to have one of the possible values: -`{ searchBy: }`. +### Analytics changes -We should also create a new method, `setSearchBy`, in the `search` store, that -would allow directly set the `searchBy` filter value. +The views can be tracked as page views, so no separate event is necessary. The +only way to access the pages is directly or via links on the single results, +which will all be captured by standard page visits. -### Analytics changes +Clicking on the items will be tracked as `SELECT_SEARCH_RESULT` events. These +events can be narrowed by pathname (`/search` or `/tag\*`, for example) to +determine where the event occurred. -I am not sure what events we want to add, if any. Should we add `_CLICKED` -events for the new views, or would page views be sufficient? +Two analytics events should be added: -Should we add a new `SELECT_COLLECTION_ITEM` event similar to -`SELECT_SEARCH_RESULT`? +- The clicks on external creator link in the `VCollectionHeader` should be + tracked as `VISIT_CREATOR_LINK` events. -The clicks on external creator link in the `VCollectionHeader` should be tracked -as `VISIT_CREATOR_LINK` events. We don't have a special event for visiting -source, should we add one, or should they be tracked as generic -`VISIT_EXTERNAL_LINK`? +- We should also a special event for visiting source `VISIT_SOURCE_LINK`, + similar to `VISIT_CREATOR_LINK`. ### Tests We should add visual-regression tests for the new views. To minimize flakiness due to slow loading of the images, we should probably use the -`{ filter: brightness(0%); }` trick for the images on the page. +`{ filter: brightness(0%); }` trick for the images on the page: +https://github.com/WordPress/openverse/blob/b4b46b903731870015c475d2c08eebef7ec6b25b/frontend/test/playwright/visual-regression/pages/pages.spec.ts#L49-L55 The search store tests should be updated to reflect the changes to the filters. @@ -210,6 +315,8 @@ No new tools or packages are necessary. +Not applicable. + ## Design @@ -221,20 +328,19 @@ https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?node-i -The work on components (`VCreatorLink`, `VSourceLink`, `VMediaTag`), and on the -search store changes can be parallelized. +The work on components (`VCollectionLink`, `VMediaTag`), and on the search store +changes can be parallelized. -We should create a separate branch for this work because adding the links to the -collection pages on the single result pages (`VCreatorLink`, `VSourceLink`, -`VMediaTag`) before the collection pages are ready is not possible. +We should create a `additional_search_views` feature flag for this work because +adding the links to the collection pages on the single result pages +(`VCollectionLink`, `VMediaTag`) before the collection pages are ready is not +possible. ## Blockers -The main blocker could be the maintainer capacity. If the "creator" API call is -not easy to construct (since we need to combine both the creator and the -provider), this might also be a blocker. +The main blocker could be the maintainer capacity. ## Accessibility @@ -247,16 +353,15 @@ indicate the change of context. -If we work on a separate branch, it would be easy to rollback by reverting the -branch merge commit. +To rollback the changes, we would need to set the feature flag to `OFF`. ## Risks The biggest risk I see is that this project might be seen as an "invitation" to -scraping. Hopefully, the work on providing the dataset would minimize such -risks. +scraping. Hopefully, frontend rate limiting and the work on providing the +dataset would minimize such risks. ## Prior art From 06e6e6c2ead282c53ea1423b45aabbeb0e548d94 Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Fri, 21 Jul 2023 16:53:55 +0300 Subject: [PATCH 3/7] Formatting changes --- ...ementation_plan_additional_search_views.md | 70 +++++++++---------- 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md index 5d7c5c25514..0f6296caf32 100644 --- a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md +++ b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md @@ -9,8 +9,8 @@ -- [ ] TBD -- [ ] TBD +- [ ] @zackkrida +- [ ] @sarayourfriend ## Project links @@ -65,7 +65,8 @@ describe each page and the necessary changes. #### Source page To get the items from the selected source, we can use the `source` query -parameter: https://api.openverse.engineering/v1/images/?source=met +parameter: +[https://api.openverse.engineering/v1/images/?source=met](https://api.openverse.engineering/v1/images/?source=met) There is a limited number of sources, and we validate the `source` parameter against the full list. This means that the `source` parameter will be matched @@ -88,8 +89,8 @@ out dead links, and return a paginated response. To get the items matching a tag, we can use the `tags` query parameter with a quoted value. -https://api.openverse.engineering/v1/images/?tags=%22black%20cat%22 This query -returns all images with tags that contain the phrase "black cat". +[https://api.openverse.engineering/v1/images/?tags=%22black%20cat%22](https://api.openverse.engineering/v1/images/?source=met) +This query returns all images with tags that contain the phrase "black cat". This would not match the tags exactly, but tags page is supposed to show all items that are associated with the same topic or category. @@ -102,8 +103,9 @@ is difficult to implement because the tags are stored as a list JSONField model. We can re-use the search store as is for these pages. Currently, the store uses the `searchBy` filter to determine the shape of the API query. If `searchBy` value is set, then the `q` parameter is replaced with the -`=` query parameter. -https://github.com/WordPress/openverse/blob/b4b46b903731870015c475d2c08eebef7ec6b25b/frontend/src/utils/prepare-search-query-params.ts#L22-L25 +`=` query parameter. API query parameters are constructed +from the search store state in the +[`prepare-search-query-params` method](https://github.com/WordPress/openverse/blob/b4b46b903731870015c475d2c08eebef7ec6b25b/frontend/src/utils/prepare-search-query-params.ts#L22-L25) We can update the `searchBy` filter to have one of the possible values: `{ searchBy: }`. Then, this value would be used to @@ -111,18 +113,14 @@ construct the API query. #### Update the `searchBy` filter -Currently, this filter is shaped as other filters: it is a list of objects with -`code`, `name` and `checked` properties. This is a possible value now: `{ -searchBy: { code: "creator", name: "filters.searchBy.creator", checked: true }}. +The `searchBy` filter will be used to determine the shape of the API query. +While currently these parameters will be mutually exclusive (we can only search +by one of them), we might want to allow searching by multiple parameters in the +future. -Since this filter is mutually exclusive (you cannot search by both the creator -and the tag, for example), this shape is very difficult to update. - -It is better to set the `searchBy` filter to have one of the possible values: -`{ searchBy: }`. - -We should create a new method, `setSearchBy`, in the `search` store, that would -allow directly set the `searchBy` filter value. +For other filters, we only use `toggle` method to update the value. However, for +`searchBy`, we need to be able to check one of the `searchBy` parameters, and +uncheck the others. To enable that, we should add a new `search` store method. This change should also update `prepare-search-query-params` to use the `searchBy` filter value to construct the API query: @@ -166,14 +164,15 @@ This page should use the [`validate` method](https://v2.nuxt.com/docs/components-glossary/validate/) to make sure that the `collection`, `mediaType` and `term` `params` are valid. -``` -validate({ params, $pinia }): { - const { collection, mediaType, term } = params - // Check that collection is one of ["tag", "creator" or "source"], - // and mediaType is one of `supportedMediaTypes`. - // Check that `term` is correctly escaped. - // If the params are not valid, return `false` to show the error page. - return isValid ? true : false +```typescript +function validate({ params, $pinia }): boolean { + const { collection, mediaType, term } = params + // Check that collection is one of ["tag", "creator" or "source"], + // and mediaType is one of `supportedMediaTypes`. + // Check that `term` is correctly escaped. + // If the params are not valid, return `false` to show the error page. + return isValid ? true : false +} ``` This page should also update the state (`searchType`, `searchTerm` and @@ -241,12 +240,12 @@ be horizontally scrollable on mobile. It should implement a scroll-snap #### Update `VMediaTag` component to be a `VButton` wrapping a `VLink` -Each `VMediaTag` should be a `VButton` with `as="VLink"` and should link to the -localized page for the tag collection. Figma link: -https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56515&mode=design&t=nCX20BtJYqMOFAQm-4 -[VMediaTag component](https://github.com/WordPress/openverse/blob/c7b76139d5a001ce43bde27805be5394e5732d1a/frontend/src/components/VMediaTag/VMediaTag.vue) -`{{tag}}`. -It should accept `href` as a prop. +The +[`VMediaTag` component](https://github.com/WordPress/openverse/blob/c7b76139d5a001ce43bde27805be5394e5732d1a/frontend/src/components/VMediaTag/VMediaTag.vue) +should be updated to be a `VButton` wrapping a `VLink`, and should match the +design in the +[Figma mockups](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56515&mode=design&t=nCX20BtJYqMOFAQm-4). +The component should link to the localized page for the tag collection. ### Other interface changes @@ -291,8 +290,8 @@ Two analytics events should be added: We should add visual-regression tests for the new views. To minimize flakiness due to slow loading of the images, we should probably use the -`{ filter: brightness(0%); }` trick for the images on the page: -https://github.com/WordPress/openverse/blob/b4b46b903731870015c475d2c08eebef7ec6b25b/frontend/test/playwright/visual-regression/pages/pages.spec.ts#L49-L55 +[ `{ filter: brightness(0%); }` trick](https://github.com/WordPress/openverse/blob/b4b46b903731870015c475d2c08eebef7ec6b25b/frontend/test/playwright/visual-regression/pages/pages.spec.ts#L49-L55) +for the images on the page. The search store tests should be updated to reflect the changes to the filters. @@ -321,8 +320,7 @@ Not applicable. -Figma designs in the dev mode: -https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?node-id=22%3A13656&mode=dev +[Figma designs in the dev mode](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?node-id=22%3A13656&mode=dev) ## Parallelizable streams From 89a96a3ecb8af1e1ebf66c7fba799b332a7a0e39 Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Tue, 25 Jul 2023 18:20:50 +0300 Subject: [PATCH 4/7] add local changes --- ...ementation_plan_additional_search_views.md | 47 ++++++++++++++++--- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md index 0f6296caf32..060294a5c8b 100644 --- a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md +++ b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md @@ -46,7 +46,39 @@ pages are updated to add the links to these pages, as seen in the Figma mockups: For the creator pages, a separate API media view is added to get the items that match the creator and source pair exactly. -## Outlined Steps +## Step-by-step plan + +The work on adding the new components can be done independently because it does +not affect the existing pages. This includes the following steps: + +- Extract the `VAudioCollection` component +- Add the `VCollectionHeader` component +- Add the `VCollectionLink` component +- Add the new `VTag` component +- Update links in the "information" section +- Add an analytics event for visiting source `VISIT_SOURCE_LINK` that is similar + to `VISIT_CREATOR_LINK`. + +This work can be done in parallel with the API changes. It would be useful to +have the API changes done before the frontend changes, so that we can test the +new pages with the real data and query parameters. + +Other frontend changes will affect the existing pages, so they will need to be +done under a feature flag. This includes the updates to the single result pages, +additional pages and updating the search query. + +- Add a `additional_search_views` feature flag +- Update the `searchBy` filter +- Create a page for `tag` / `creator` /`source` collections. +- Update the `VCollectionLink` area on the single result page +- Use `VTag` component in the search results + +Cleanup after the feature flag is removed: + +- Remove the `additional_search_views` feature flag +- Remove `VMediaTag` component + +## Step details @@ -229,14 +261,17 @@ should probably still have two separate pages for the source and the creator, but we might want to add a note that the creator is the only one associated with this source. -#### Add `VCollectionLink` component and update the title and links container +#### Add `VCollectionLink` component + +This components should be a `VButton` with `as="VLink"` and should link to the +localized page for the creator or the source pages. + +### Update the `VCollectionLink` area on the single result page The "by creator" line under the main item on the single result page should be replaced with a section that has two buttons: one for a creator link and a -source link. These links should be `VButtons` with `as="VLink"` and should link -to the localized page for the creator or the source pages. This section should -be horizontally scrollable on mobile. It should implement a scroll-snap -(example: https://play.tailwindcss.com/AbfA33Za50) +source link. This section should be horizontally scrollable on mobile. It should +implement a scroll-snap (example: https://play.tailwindcss.com/AbfA33Za50) #### Update `VMediaTag` component to be a `VButton` wrapping a `VLink` From a886fd722e1c8f9ccde851e59604f8da6f49e575 Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Wed, 26 Jul 2023 22:18:10 +0300 Subject: [PATCH 5/7] Add API changes to fix blockers --- ...ementation_plan_additional_search_views.md | 434 ++++++++++-------- 1 file changed, 231 insertions(+), 203 deletions(-) diff --git a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md index 060294a5c8b..d636995da0c 100644 --- a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md +++ b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md @@ -18,134 +18,200 @@ - [Project Thread](https://github.com/WordPress/openverse/issues/410) - [Project Proposal](https://docs.openverse.org/projects/proposals/additional_search_views/20230424-project_proposal_additional_search_views.html) +- [Milestone](https://github.com/WordPress/openverse/milestone/17) -## Overview +## Expected Outcomes - + -This plan describes the changes that need to be added on the frontend: the new -components, pages and store. +API endpoints return all media with the selected tag, from the selected source +or by the selected creator, sorted by date added to Openverse. -## Expected Outcomes +Frontend allows to browse media items by a selected creator, source, or with a +selected tag. - +The single result pages link to these collection views; the external links are +also updated to clearly show that they are external. -Three collection pages are added to Openverse: +## Step-by-step plan -- items with the selected tag -- items from the selected source (provider) -- items by the selected creator. +1. Update the Elasticsearch index to enable exact matching of the `tag`, + `source` and `creator` fields (both the query analyzer and the index + analyzer). This will require reindexing. +2. Add API endpoints for exact matching of the `tag`, `source` and `creator` + fields. +3. Create the new components: `VCollectionHeader`, `VCollectionLink` and `VTag`. +4. Update the store and utils used to construct the API query to allow for + searching by `tag`, `creator` or `source`, in addition to the current search + by title/description/tags combination. +5. Add a switchable "additional_search_views" feature flag. +6. Create a page for `tag` / `creator` /`source` collections. The page should + handle fetching and updating the search store state. +7. Update the single result pages: tags area, the "creator" and "source" area + under the main media item. +8. Add the Analytics event `VISIT_SOURCE_LINK` and update where + `VISIT_CREATOR_LINK` is sent. +9. Cleanup after the feature flag is removed: + - Remove conditional rendering on the single result pages. + - Remove the `additional_search_views` feature flag and `VMediaTag` + component. -The pages re-use the existing image grid and audio collection views and can load -more images or audio items on the Load more button click. The single result -pages are updated to add the links to these pages, as seen in the Figma mockups: +## Step details -- [Image single result](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-63284&mode=dev) -- [Audio single result](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-63285&mode=dev) +### 1. Elasticsearch updates -For the creator pages, a separate API media view is added to get the items that -match the creator and source pair exactly. +Currently, when filtering the search results, the API matches some query +parameters in a fuzzy way: an item matches the query if the field value contains +the query string as a separate word. When indexing the items, we "analyze" them, +which means that we split the field values by whitespace and stem them. We also +do the same to the query string. This means that the query string "bike" will +match the field value "bikes", "biking", or "bike shop". -## Step-by-step plan +For these pages, however, we need an exact match. -The work on adding the new components can be done independently because it does -not affect the existing pages. This includes the following steps: +One alternative implementation considered when writing this plan was to use the +database instead of the Elasticsearch to get the results. This would make it +easy to get the exact matches. However, there are some problems with using the +database rather than ES to access anything: -- Extract the `VAudioCollection` component -- Add the `VCollectionHeader` component -- Add the `VCollectionLink` component -- Add the new `VTag` component -- Update links in the "information" section -- Add an analytics event for visiting source `VISIT_SOURCE_LINK` that is similar - to `VISIT_CREATOR_LINK`. +- The database does not cache queries in the same way that ES does. Repeated + queries will not necessarily be as efficient as from ES. +- The database does not score documents at all, so the order will different + dramatically to the way that ES would order the documents. That's an issue + with respect to popularity data today already, but will become even more of an + issue if we start to score documents based on other metrics as theorised by + our search relevancy discussions. +- `creator` is not indexed in the API database, so a query against it will be + very slow. -This work can be done in parallel with the API changes. It would be useful to -have the API changes done before the frontend changes, so that we can test the -new pages with the real data and query parameters. +This is why we should update the existing Elasticsearch index to enable exact +and add new endpoints for the `tag`, `creator` and `source` matches. -Other frontend changes will affect the existing pages, so they will need to be -done under a feature flag. This includes the updates to the single result pages, -additional pages and updating the search query. +The updates to the Elasticsearch index should: -- Add a `additional_search_views` feature flag -- Update the `searchBy` filter -- Create a page for `tag` / `creator` /`source` collections. -- Update the `VCollectionLink` area on the single result page -- Use `VTag` component in the search results +- use + [`keyword` field type](https://www.elastic.co/guide/en/elasticsearch/reference/current/keyword.html) + for mapping and + [`term` query](https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-terms-query.html) + for `creator`, `source` and `tags` fields. This will allow for exact matching + of the values (e.g. `bike` will not match `bikes` or `biking`), and will + probably make the search more performant since the fields and the query won't + need to be analyzed and/or stemmed. -Cleanup after the feature flag is removed: +We will need to run the data refresh to reindex the data. -- Remove the `additional_search_views` feature flag -- Remove `VMediaTag` component +### 2. New API endpoints -## Step details +The new routes should use path parameters instead of query parameters for the +`tag`, `creator` and `source` values. This will make the URLs more readable, +easier to share, will be easier to cache or perform cache invalidation required +by #1969. The path parameters should be URL encoded to preserve special +characters and spaces. - +Instead of using query strings, we can describe the resource via the path: +`//source//creator/` is very clean, easy to read +and understand, and very easy to manage the cache for because it is a static +path. The source page can use the same route by leaving off the creator. This +removes the need to manage specific query params as well and would allow us to +add querying within these routes more easily in the future behind the regular q +parameter if we wanted. -### API changes +For the tag route, the singular `tag` rather than plural `tags` should be used +for legibility since we are presenting a single tag. -Currently, when filtering the search results, the API matches some query -parameters in a fuzzy way: an item matches the query if the field value contains -the query string as a separate word. When indexing the items, we "analyze" them, -which means that we split the field values by whitespace and stem them. We also -do the same to the query string. This means that the query string "bike" will -match the field value "bikes", "biking", or "bike shop". +The new views should use the same pagination and dead link cleanup as the search +views. -For most of these pages, we need an exact match instead. This section will -describe each page and the necessary changes. +### 3. New and updated components -#### Source page +#### Extract the `VAudioCollection` component + +Currently, it is not possible to reuse the audio collection from the audio +search result page because it is a part of the `audio.vue` page. We should +extract the part that shows the loading skeleton, the column of `VAudioTrack` +rows and the Load more section into `VAudioCollection` component. This component +will be reused in the audio search page and on the Additional search views. + +#### Add a `VCollectionHeader` component -To get the items from the selected source, we can use the `source` query -parameter: -[https://api.openverse.engineering/v1/images/?source=met](https://api.openverse.engineering/v1/images/?source=met) +The header should have an icon (tag, creator or source) and the name of the +tag/creator/source. For source and the creator, there should be an external link +button if it's available (not all creators have urls). -There is a limited number of sources, and we validate the `source` parameter -against the full list. This means that the `source` parameter will be matched -exactly. +The header should also display the number of results, "251 audio files with the +selected tag", "604 images provided by this source", "37 images by this creator +in Hirshhorn Museum and Sculpture Garden". -#### Creator page +_Note_: There are sources that only have works by one creator. In this case, we +should probably still have two separate pages for the source and the creator, +but we might want to add a note that the creator is the only one associated with +this source. -We could update the Elasticsearch index and the search controller to allow for -exact matching by the `creator` field. However, I think there is an easier way -that does not require any changes to the indexes: use the Django ORM to filter -the items by the creator name and the provider name. +**Figma links**: **creator** +[desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56323&mode=design&t=pN0PPAzlKQEKT9tJ-4) +and +[mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56362&mode=design&t=suLIyJHNmZrM0mPH-4), +**source** +[desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56381&mode=design&t=suLIyJHNmZrM0mPH-4) +and +[mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56421&mode=design&t=suLIyJHNmZrM0mPH-4), +**tag** +[desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1216-58402&mode=design&t=suLIyJHNmZrM0mPH-4) +and +[mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56457&mode=design&t=suLIyJHNmZrM0mPH-4). -We can add a media view at -`/api/v1//creator/?q=&provider=`. It would -use `self.get_queryset().filter(creator=creator, provider=provider)` to get all -the items by the creator from the provider. This view would also need to filter -out dead links, and return a paginated response. +#### Add `VCollectionLink` component -#### Tag page +This component should be a `VButton` with `as="VLink"`, should have an icon, and +should accept a localized link to the creator or source page. -To get the items matching a tag, we can use the `tags` query parameter with a -quoted value. -[https://api.openverse.engineering/v1/images/?tags=%22black%20cat%22](https://api.openverse.engineering/v1/images/?source=met) -This query returns all images with tags that contain the phrase "black cat". +**Figma link**: +[creator and source buttons](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56972&mode=dev) -This would not match the tags exactly, but tags page is supposed to show all -items that are associated with the same topic or category. +#### Create a new `VTag` component to be a `VButton` wrapping a `VLink` -We could also add a media view at `/api/v1//tags/?q=`, but it -is difficult to implement because the tags are stored as a list JSONField model. +The +[`VMediaTag` component](https://github.com/WordPress/openverse/blob/c7b76139d5a001ce43bde27805be5394e5732d1a/frontend/src/components/VMediaTag/VMediaTag.vue) +should be updated to be a `VButton` wrapping a `VLink`, and should match the +design in the +[Figma mockups](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56515&mode=design&t=nCX20BtJYqMOFAQm-4). +The component should link to the localized page for the tag collection. -### Nuxt store changes +#### Update links in the "information" section -We can re-use the search store as is for these pages. Currently, the store uses -the `searchBy` filter to determine the shape of the API query. If `searchBy` -value is set, then the `q` parameter is replaced with the -`=` query parameter. API query parameters are constructed -from the search store state in the -[`prepare-search-query-params` method](https://github.com/WordPress/openverse/blob/b4b46b903731870015c475d2c08eebef7ec6b25b/frontend/src/utils/prepare-search-query-params.ts#L22-L25) +The links to creator in the +[image](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/pages/image/_id/index.vue#L62-L73) +and +[audio](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioTrack/layouts/VFullLayout.vue#L32-L41) +single result pages Information section should have an "external link" icon. -We can update the `searchBy` filter to have one of the possible values: -`{ searchBy: }`. Then, this value would be used to -construct the API query. +Audio creator link should also be updated to match the image creator link. It +should be a conditional component: `VLink` if the `creator_url` is truthy and +`span` if the `creator_url` is falsy. + +Currently, the `foreign_landing_url` is linked to the "source" in the +[image](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VImageDetails/VImageDetails.vue#L26-L28) +page and "provider" in the +[audio page](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioDetails/VAudioDetails.vue#L61C1-L63). +The audio page should be updated to match the image page: the +`foreign_landing_url` link should be added to the "source", not provider. + +### 4. Nuxt store and API request changes + +We can re-use the search store as is for these pages. Currently, the frontend +can perform searches by `source` parameter. If `searchBy` value is set, then the +`q` parameter is replaced with the `=` query parameter. + +For this project, we should add new values to the `searchBy` filter. + +The API request URL is constructed from the search store state in the +[`prepare-search-query-params` method](https://github.com/WordPress/openverse/blob/b4b46b903731870015c475d2c08eebef7ec6b25b/frontend/src/utils/prepare-search-query-params.ts#L22-L25). +We will need to update this method to use the `searchBy` filter value to +construct the API request path as described in the "API Changes" section. #### Update the `searchBy` filter -The `searchBy` filter will be used to determine the shape of the API query. +The `searchBy` filter will be used to determine the shape of the API request. While currently these parameters will be mutually exclusive (we can only search by one of them), we might want to allow searching by multiple parameters in the future. @@ -154,47 +220,53 @@ For other filters, we only use `toggle` method to update the value. However, for `searchBy`, we need to be able to check one of the `searchBy` parameters, and uncheck the others. To enable that, we should add a new `search` store method. -This change should also update `prepare-search-query-params` to use the -`searchBy` filter value to construct the API query: +If `searchBy` is set to `tag`, `creator` or `source`, then the media store +should create search path instead of the search query. So, instead of calling +`prepareSearchQuery` to create the query parameters, it should call +`prepareSearchPath` to create the path. ```typescript -const prepareSearchQuery = ( +const searchPathOrQuery = searchBy + ? prepareSearchPath(searchParams, mediaType) + : prepareSearchQuery(searchParams, mediaType) + +const prepareSearchPath = ( searchParams: Record, mediaType: SupportedMediaType ) => { - if (searchParams.searchBy) { + let path + if (searchBy === "tag") { + path = `${mediaType}/tag/${searchTerm}` + } else { + path = `${mediaType}/source/${searchParams[`${mediaType}Provider`]}` if (searchBy === "creator") { - // the name of the source is stored in the `Provider` filter - return { - creator: searchTerm, - source: searchParams[`${mediaType}Provider`], - } - } else if (searchBy === "source") { - return { source: searchParams[`${mediaType}Provider`] } - } else if (searchBy === "tag") { - // The parameter is plural in the API - return { tags: searchTerm } + path += `/creator/${searchTerm}` } - } else { - // Current search query transform } + return path } ``` -### Page changes +### 5. Add the `additional_search_views` feature flag + +The flag should be switchable, and off by default. -#### Create a page for `tag` / `creator` /`source` collections. +### 6. Create a page for `tag` / `creator` /`source` collections. Nuxt allows creating nested dynamic routes like `/pages/_collection/_mediaType/_term`. -Here, the collection can be `tag`, `creator` or `source`. `mediaType` can be one -of the supported media types (currently, `image` and `audio`). `term` refers to -the actual value of the tag, creator or source name. +We should add the following pages: -This page should use the +- `/pages/_mediaType/tag/_tag` +- `/pages/_mediaType/source/_source` +- `/pages/_mediaType/source/_source/creator/_creator` (this page might not be + needed as it might be handled by the source page) + +To make sure that the `mediaType`, `source` and `creator` parameters are valid, +this page should use the [`validate` method](https://v2.nuxt.com/docs/components-glossary/validate/) to -make sure that the `collection`, `mediaType` and `term` `params` are valid. +make sure that and show an error page if necessary. ```typescript function validate({ params, $pinia }): boolean { @@ -209,111 +281,59 @@ function validate({ params, $pinia }): boolean { This page should also update the state (`searchType`, `searchTerm` and `searchBy` and `provider` filters) in the `search` store and handle fetching -using `mediaStore`'s `fetchMedia` method in the `useFetch` hook. Since it is not -possible to change the path or query parameters from this page client-side, -fetching can be much simpler than on the current search page (that has to watch -for changes in the route and fetch if necessary). +using `mediaStore`'s `fetchMedia` method in the `useFetch` hook. -This page should have the common header (title, button - link to the source -external URL or the creator URL, and the summary of item counts in the -collection), and a nested component: the image grid or the audio collection. +Since it is not possible to change the path or query parameters from this page +client-side, fetching can be much simpler than on the current search page (that +has to watch for changes in the route and fetch if necessary). -### New and updated components +This page should use `VCollectionHeader` and the image grid or the audio +collection. -#### Extract the `VAudioCollection` component +### 7. Update the single result pages -Currently, it is not possible to reuse the audio collection from the audio -search result page because it is a part of the `audio.vue` page. We should -extract the part that shows the loading skeleton, the column of `VAudioTrack` -rows and the Load more section. This component will be reused in the audio -search page and on the Additional search views. One thing that I'm not sure -about here is whether the analytics events should stay the same and be sent from -all pages - or if we should somehow differentiate the events coming from -different search views. - -#### Add a `VCollectionHeader` component - -Figma links: - -- creator - [desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56323&mode=design&t=pN0PPAzlKQEKT9tJ-4) - and - [mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56362&mode=design&t=suLIyJHNmZrM0mPH-4) -- source - [desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56381&mode=design&t=suLIyJHNmZrM0mPH-4) - and - [mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56421&mode=design&t=suLIyJHNmZrM0mPH-4) -- tag - [desktop](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1216-58402&mode=design&t=suLIyJHNmZrM0mPH-4) - and - [mobile](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56457&mode=design&t=suLIyJHNmZrM0mPH-4) - -The header should have an icon (tag, creator or source), the name of the -tag/creator/source. For source and the creator, there should be an external link -button. - -The header should also display the number of results, "251 audio files with the -selected tag", "604 images provided by this source", "37 images by this creator -in Hirshhorn Museum and Sculpture Garden". +All of these changes should be conditional on whether the +`additional_search_views` feature flag is enabled. -Note: There are sources that only have works by one creator. In this case, we -should probably still have two separate pages for the source and the creator, -but we might want to add a note that the creator is the only one associated with -this source. +The Figma links for new designs: -#### Add `VCollectionLink` component - -This components should be a `VButton` with `as="VLink"` and should link to the -localized page for the creator or the source pages. +- [Image single result](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-63284&mode=dev) +- [Audio single result](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-63285&mode=dev) -### Update the `VCollectionLink` area on the single result page +#### Update the `VCollectionLink` area on the single result page -The "by creator" line under the main item on the single result page should be +The content info line under the main item on the single result page should be replaced with a section that has two buttons: one for a creator link and a source link. This section should be horizontally scrollable on mobile. It should implement a scroll-snap (example: https://play.tailwindcss.com/AbfA33Za50) -#### Update `VMediaTag` component to be a `VButton` wrapping a `VLink` - -The -[`VMediaTag` component](https://github.com/WordPress/openverse/blob/c7b76139d5a001ce43bde27805be5394e5732d1a/frontend/src/components/VMediaTag/VMediaTag.vue) -should be updated to be a `VButton` wrapping a `VLink`, and should match the -design in the -[Figma mockups](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56515&mode=design&t=nCX20BtJYqMOFAQm-4). -The component should link to the localized page for the tag collection. +#### Use `VTag` with links for the tags -### Other interface changes +The tags should be rendered using the `VTag` component with links to the tag +collection page. -#### Update links in the "information" section +#### Update the tags area on the single result page -The links to creator in the -[image](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/pages/image/_id/index.vue#L62-L73) -and -[audio](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioTrack/layouts/VFullLayout.vue#L32-L41) -single result pages Information section should have an "external link" icon. +The tags area should be collapsible to make long lists of tags collapsible: +https://github.com/WordPress/openverse/issues/2589 -Audio creator link should also be updated to match the image creator link. It -should be a conditional component: `VLink` if the `creator_url` is truthy and -`span` if the `creator_url` is falsy. +#### Add the information popover next to source and provider links -Currently, the `foreign_landing_url` is linked to the "source" in the -[image](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VImageDetails/VImageDetails.vue#L26-L28) -page and "provider" in the -[audio page](https://github.com/WordPress/openverse/blob/35f08e5710d59f6c10f0cf54103fcce151adfefe/frontend/src/components/VAudioDetails/VAudioDetails.vue#L61C1-L63). -The audio page should be updated to match the image page: the -`foreign_landing_url` link should be added to the "source", not provider. +The information popover should be added next to the source and provider links +that explains the difference between the source and provider. -### Analytics changes +[**Figma link**](https://www.figma.com/file/niWnCgB7K0Y4e4mgxMrnRC/Additional-search-views?type=design&node-id=1200-56521&mode=dev) -The views can be tracked as page views, so no separate event is necessary. The -only way to access the pages is directly or via links on the single results, -which will all be captured by standard page visits. +### 8. Additional analytics events -Clicking on the items will be tracked as `SELECT_SEARCH_RESULT` events. These -events can be narrowed by pathname (`/search` or `/tag\*`, for example) to -determine where the event occurred. +Some existing events will already track the new views events. The views can be +tracked as page views, so no separate event is necessary. The only way to access +the pages is directly or via links on the single results, which will all be +captured by standard page visits. Clicking on the items will be tracked as +`SELECT_SEARCH_RESULT` events. These events can be narrowed by pathname +(`/search` or `/tag\*`, for example) to determine where the event occurred. -Two analytics events should be added: +Two analytics events should be added or updated: - The clicks on external creator link in the `VCollectionHeader` should be tracked as `VISIT_CREATOR_LINK` events. @@ -321,6 +341,12 @@ Two analytics events should be added: - We should also a special event for visiting source `VISIT_SOURCE_LINK`, similar to `VISIT_CREATOR_LINK`. +### 9. Cleanup after the feature flag is enabled in production + +After the feature flag is enabled in production, we should remove the +conditional rendering on the single result pages and remove the +`additional_search_views` feature flag and (old) `VMediaTag` component. + ### Tests We should add visual-regression tests for the new views. To minimize flakiness @@ -361,13 +387,15 @@ Not applicable. -The work on components (`VCollectionLink`, `VMediaTag`), and on the search store -changes can be parallelized. +The API changes can be done independently of the frontend changes, although they +should be finished before the final testing of the frontend changes. + +Adding the new components (step 3), Nuxt store update (step 4) and the +`additional_search_views` feature flag (step 5) can be done in parallel, and are +not dependent on anything. -We should create a `additional_search_views` feature flag for this work because -adding the links to the collection pages on the single result pages -(`VCollectionLink`, `VMediaTag`) before the collection pages are ready is not -possible. +The work on the single result pages (step 7) can be done in parallel with the +work on the collection pages (step 6), but should follow the previous steps. ## Blockers @@ -386,7 +414,7 @@ indicate the change of context. -To rollback the changes, we would need to set the feature flag to `OFF`. +To roll back the changes, we would need to set the feature flag to `OFF`. ## Risks From 8ca4d4ebe4f56002bcc16a1ff0dd05f4faf9c17e Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Sat, 29 Jul 2023 18:56:48 +0300 Subject: [PATCH 6/7] Replace Elasticsearch changes with Search controller update --- ...ementation_plan_additional_search_views.md | 49 +++++++++++++------ 1 file changed, 33 insertions(+), 16 deletions(-) diff --git a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md index d636995da0c..1c03aa2cf41 100644 --- a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md +++ b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md @@ -58,7 +58,7 @@ also updated to clearly show that they are external. ## Step details -### 1. Elasticsearch updates +### 1. Search controller updates Currently, when filtering the search results, the API matches some query parameters in a fuzzy way: an item matches the query if the field value contains @@ -84,21 +84,38 @@ database rather than ES to access anything: - `creator` is not indexed in the API database, so a query against it will be very slow. -This is why we should update the existing Elasticsearch index to enable exact -and add new endpoints for the `tag`, `creator` and `source` matches. - -The updates to the Elasticsearch index should: - -- use - [`keyword` field type](https://www.elastic.co/guide/en/elasticsearch/reference/current/keyword.html) - for mapping and - [`term` query](https://www.elastic.co/guide/en/elasticsearch/reference/current/query-dsl-terms-query.html) - for `creator`, `source` and `tags` fields. This will allow for exact matching - of the values (e.g. `bike` will not match `bikes` or `biking`), and will - probably make the search more performant since the fields and the query won't - need to be analyzed and/or stemmed. - -We will need to run the data refresh to reindex the data. +To enable exact matching, we don't need any changes in Elasticsearch index +because we already have the `.keyword` fields for `creator`, `source` and +`tags`. We just need to use them in the query. This will allow for exact +matching of the values (e.g. `bike` will not match `bikes` or `biking`), and +will probably make the search more performant since the fields and the query +won't need to be analyzed and/or stemmed. + +The search controller's `search` method should be refactored to be smaller and +allow for more flexibility when creating the search query. The current +implementation of query building consists of 3 steps. + +We **first** apply the `filters`: if the query string has any parameters other +than `q`, we use them for exact matches that must be in the search results, or +must be excluded from the search results (if the parameter starts with +`exclude_`). + +**Then**, if `q` parameter is present, we apply the `q` parameter, which is a +full-text search within `tags`, `title` and `description` fields. This is a +fuzzy search, which means that the query string is stemmed and analyzed, and the +field values are stemmed and analyzed, and the documents are scored based on the +relevance of the match. If `q` is not present, but one of the +`creator`/`source`/`tags` parameter is present, we search within those fields +for fuzzy matches. + +**Finally**, we apply the ranking and sorting parameters, and "highlight" the +fields that were matched in the results. + +The new search controller should allow for using different filters for the first +step and to not use the full-text search. We should also create a new serializer +for collection search requests. It should include the common parameters for +`list` requests, such as `page` and `page_size`, and the parameters for the +exact matches: `tag`, `creator` and `source`. ### 2. New API endpoints From 36c1b818664718b540839d7c1dc828a2a60dd1ca Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Sat, 29 Jul 2023 18:58:08 +0300 Subject: [PATCH 7/7] Add approval Co-authored-by: Zack Krida --- .../20230719-implementation_plan_additional_search_views.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md index 1c03aa2cf41..d14a3328378 100644 --- a/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md +++ b/documentation/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md @@ -9,7 +9,7 @@ -- [ ] @zackkrida +- [x] @zackkrida - [ ] @sarayourfriend ## Project links