search

NYPL / staff-picks

1 stars 3 forks source link

Dynamic filters by season and audience #175

Closed holingpoon closed 6 years ago

holingpoon commented 6 years ago

When the user selects another season, the filters should reload according to the season.

When the user selects another audience, the filters should load the sub-genres.

This issue cross-references Jira Ticket WWW-214

holingpoon commented 6 years ago

Technical Notes:

  1. utils.getSelectableTags() should already return tags associated with books by audience
  2. BookFilters.getFilterArray(selectableFilters, filters) is returning a list of selectableFilters that are in the filters list, but the filters list do not contain all values of selectableFilters, hence filter array only returns the union of selectableFilters and filters, thereby missing some of the selectableFilters
holingpoon commented 6 years ago

Suggested fixes:

  1. BookFilters.getFilterArray(selectableFilters, filters) needs to return all the elements in selectableFilters

Logic rework

  1. We need to rethink the code logic to accomodate both best-books and staff-picks since there used to be a default baseline data set. Currently, we should be returning sets with distinct values to render BookFilters to reflect (audience, season) pair, but the code is not doing that now.
EdwinGuzman commented 6 years ago

  1. This is not exactly true. utils.getSelectableTags() will return all the tags that are available to select based on the set of picks that was passed to it. So the set of picks should already be filtered by audience when it is passed to that function. But, the function itself does not return tags associated with books by audience. This is because if you want all tags in a set of picks that contain "young adult" and "children" but NOT "adult", then it's possible to do so.

  2. What do you mean that "the filters list do not contain all values of selectableFilters"? getFilterArray does not return a union of selectableFilters and filters. From the filter and _contains functions, it is creating a list of filters of each filter in filters array that is in the selectableFilters array. It is okay to not have some filters from selectableFilters in the final array. This is because a pick can have ["funny", "adventure"] as tags but the set of official filters maybe doesn't have "funny". If that's the case, then we don't want "funny".

The problem is that the official set of filters has changed many times and it is not pulled in from one source but instead created from the tags in each pick. So yes, you could check that getFilterArray does return all the selectable filters, but also check if we need it at all.

holingpoon commented 6 years ago

Thanks @EdwinGuzman! I kept searching and I found out why the filters are not rendering correctly. The API calls are not set to the new convention yet so it is returning a fixed set of data from utils.getAllTags(). I will pair up with @gkallenberg on fixing the API routes. Thanks!

EdwinGuzman commented 6 years ago

Wait, that's correct. You do want to get all the filters from the JSON response back in getAllTags. Those are passed to the client-side and that's where they get filtered down based on aged. You could filter on the server side and that would be helpful too, but const filters = utils.getAllTags(data.picks) should be correct because we do want to store all the filters for, say, Winter 2018.

holingpoon commented 6 years ago

Closing this ticket since #186 is now merged into development