Skip to content

Commit

Permalink
Add changes from code review
Browse files Browse the repository at this point in the history 
Signed-off-by: Olga Bulat <[email protected]>
  • Loading branch information
@obulat
obulat committed Mar 7, 2024
1 parent 7ecbcd5 commit db675b4
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 23 deletions.
2 changes: 1 addition & 1 deletion ...ls/additional_search_views/20230424-project_proposal_additional_search_views.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
## Reviewers

- [x] @obulat
- [ ] @krysal
- [x] @krysal

## Project summary

Expand Down
55 changes: 34 additions & 21 deletions ...additional_search_views/20230719-implementation_plan_additional_search_views.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,11 @@ API returns all media with the selected tag, from the selected source or by the
selected creator, sorted by date added to Openverse.

Frontend allows to browse media items by a selected creator, source, or with a
selected tag. These pages are indexed by search engines, have relevant SEO
properties and can be shared with an attractive thumbnail and relevant data.
selected tag using URLs like `/<mediaType>/collection?tag=tagName`,
`/<mediaType>/collection?source=flickr` and
`/<mediaType>/collection?source=flickr&creator=Photographer`. These pages are
indexed by search engines, have relevant SEO properties and can be shared with
an attractive thumbnail and relevant data.

The single result pages are updated to add the links to source, creator and tag
collections.
Expand Down Expand Up @@ -118,13 +121,13 @@ for fuzzy matches.
**Finally**, we apply the ranking and sorting parameters, and "highlight" the
fields that were matched in the results.

The search controller needs to be updated to extract the first 2 steps into
The search controller needs to be updated to extract the first 2 steps into a
`build_search_query` method. A new `build_collection_query` should be added and
used when the `collection` parameter is present. This method should create a
filter query for the relevant field and value.

The pagination and dead link cleanup should be the same for additional search
views as for the default search views.
queries as for the default search queries.

#### Search request serializer updates

Expand All @@ -142,20 +145,25 @@ launched. In the text below, for brevity, the prefix is omitted.
```

The collections will use the `search` endpoint with `collection` query parameter
set to `tag`, `creator` or `source`, and the relevant query parameters. The
`collection` parameter validator will check that the request contains the
necessary additional parameters: `source` for `collection=source`, `creator` and
`source` for `collection=creator`, and `tag` for `collection=tag`. The value of
the `source` parameter will also be checked to be an existing source using the
provider store.
set to `tag`, `creator` or `source`, and the following additional query
parameters. The `collection` parameter validator will check that the request
contains the necessary additional parameters: `source` for `collection=source`,
`creator` and `source` for `collection=creator`, and `tag` for `collection=tag`.

The middleware validates that the values of parameters is not empty, and that
the `source` parameter is an existing source in the provider store. If the
values are invalid, the 404 yellow error page is shown.

For the tag route, the new singular `tag` parameter should be used, rather than
the existing plural `tags` should be used for legibility since we are presenting
a single tag.
the existing plural `tags`, since we are only presenting a single tag.

The existing `source` and `creator` parameters will be reused, but will be
parsed differently when `collection` parameter is present: they will only allow
a single value instead of being split by `,` as it is for the default search.
a single value instead of being split by `,` as it is for the default search. If
the `source` contains invalid values, such as `source=flickr,europeana`, when
the `collection` parameter is present, a 400 error with `detail` of "Invalid
source parameter for source collection: `flickr,europeana`" is returned. If
possible, a list of valid sources should be returned in the error message.

Their documentation should be updated to reflect that. The additional
documentation for the search parameters should be added as a draft so that it's
Expand All @@ -164,7 +172,11 @@ remove the `unstable__` prefix.

### 2. Add the `additional_search_views` feature flag

The flag should be switchable, and off by default.
The flag should be switchable, and off by default. This will mean that the
changes are visible on [staging](https://staging.openverse.org) when the flag is
switched on using the preferences page. When the features are stable, we will
turn the flag on to test the new views in production. After we conclude that the
project is successful, we will remove the flag and the conditional rendering.

### 3. Nuxt store and API request changes

Expand All @@ -184,7 +196,7 @@ the current approach. If it is set to `"collection"`, the query will be
constructed using the new method (`buildCollectionQuery`) to set `collection`
and other relevant parameters using the new `collectionParams` object in the
store. It will _not_ be setting the filter parameters such as license or
category.
category, and will ignore such unsupported query parameters.

### 4. Create collection pages

Expand Down Expand Up @@ -222,10 +234,15 @@ search engines.

The following titles should be used for the pages:

- "Images by Olga in Flickr" for the creator page
- "Images by Olga at Flickr" for the creator page
- "Audio from Wikimedia" for the source page
- "Images with the cat tag" for the tag page

There are i18n consideration for these titles that we will work on during the
implementation. It is important to make the titles translatable, which is
difficult if the non-translatable dynamic names are used inside the sentence due
to different sentence structures.

The generic Openverse thumbnail will be used. We could also generate a thumbnail
for the collection pages in the future, but this is not in scope for this
project.
Expand Down Expand Up @@ -359,7 +376,7 @@ Analytics events should be added or updated:

- We should also a special event for visiting source `VISIT_SOURCE_LINK`,
similar to `VISIT_CREATOR_LINK`.
-

- The `REACH_RESULT_END`, `LOAD_MORE` and `SELECT_SEARCH_RESULT` events should
add strategy (`search`/`tag`/`creator`/`source`) and set `query` to tag name,
source name or source/creator pair: `cat`, `flickr` or `flickr/Olga`.
Expand Down Expand Up @@ -455,10 +472,6 @@ The biggest risk I see is that this project might be seen as an "invitation" to
scraping. Hopefully, frontend rate limiting and the work on providing the
dataset would minimize such risks.

## Prior art

<!-- Include links to documents and resources that you used when coming up with your solution. Credit people who have contributed to the solution that you wish to acknowledge. -->

## Plan revisions

This plan was significantly revised on 2024-03-01 due to problems discovered
Expand Down
2 changes: 1 addition & 1 deletion ...search_views/SUPERSEDED-20230719-implementation_plan_additional_search_views.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
- [x] @sarayourfriend

```{warning}
This is the original version. For the current version, see [Current plan](/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md) for the original version.
This is the original version. For the current version, see [the updated plan](/projects/proposals/additional_search_views/20230719-implementation_plan_additional_search_views.md).
```

## Project links
Expand Down

0 comments on commit db675b4

Please sign in to comment.