search

jwplayer / ott-web-app

Reference implementation for JWP-powered apps
Apache License 2.0
69 stars 53 forks source link

feat(project): add new type of lists (recommendations) #556

Closed AntonLantukh closed 1 week ago

AntonLantukh commented 3 weeks ago

Description

  1. Add new "content_list" type support (recommendations).
  2. Support it for Landing Page and Menu.

Steps completed:

According to our definition of done, I have completed the following steps:

github-actions[bot] commented 3 weeks ago

Visit the preview URL for this PR (updated for commit ddeeb74):

https://ottwebapp--pr556-feat-add-content-lis-xm1980a1.web.app

(expires Wed, 24 Jul 2024 07:42:20 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c198f8a3a199ba8747819f7f1e45cf602b777529

AntonLantukh commented 2 weeks ago

App Config for testing: 0i9cflkq

AntonLantukh commented 2 weeks ago

@ChristiaanScheermeijer

Do we need to add e2e testing for this?

We do need. I will add it to the Q3 plan.

Do we really need an abstraction for the content and playlist APIs or could it just be two separate services? This could prevent some breaking changes in the usePlaylist hook for example.

Do you mean having two hooks "usePlaylist" and "usePlaylistContent" which could be called conditionally + removing the controller completely? We still need to know the type to make a decision, so there will be also a "parent" hook receiving it as an argument.

Should we use different names for the service/controller/types?

I agree naming is a problem. My idea was to consume playlists, content lists (recommendations), series using controller in the future. That is why I used "ContentController" name. We also have some plans to deliever an additional "media" endpoint returning data based on the content type (maybe series, stream data inside). Though it can be also overengineering to consider this case in advance and to avoid braking changes we could start with conditional calls.

AntonLantukh commented 2 weeks ago

@ChristiaanScheermeijer @dbudzins simplified approach without abstraction layer.

AntonLantukh commented 2 weeks ago

@ChristiaanScheermeijer @dbudzins

  1. Switched to a separate endpoint for content lists.
  2. Reused PlaylistScreen for content lists.
  3. Changed interface of usePlaylist and added optional params (was needed in one place).
  4. Separated query options into a separate function to reuse in usePlaylists / usePlaylist
AntonLantukh commented 2 weeks ago

@ChristiaanScheermeijer rolled back playlistURL to the previous interface.