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

[AntonLantukh]

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:

• Acceptance criteria met
• Unit tests added
• Docs updated (including config and env variables)
• Translations added
• UX tested
• Browsers / platforms tested
• Rebased & ready to merge without conflicts
• Reviewed own code

[github-actions]

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]

App Config for testing: 0i9cflkq

[ChristiaanScheermeijer]

The PR overall looks good, but I do have some questions:

• Do you have content list IDs to test with? You just added it 😄
• Do we need to add e2e testing for this?
• 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
• Should we use different names for the service/controller/types?
  • Content list can be confused with contentTypes
  • RecommendationsService sounds like it's used for different purposes
  • I suggest renaming it to PlaylistController, PlaylistService, PlaylistContentService and PLAYLIST_TYPE removing the content analogy from most most places

[AntonLantukh]

@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.

[budzworthy]

I'm still going through some parts, but I think we can keep this change simpler without adding the content service abstractions. If we add the new content list endpoint to the existing Api Service, we can do switching at the hook or controller level to get playlists or content lists as needed.

[AntonLantukh]

@ChristiaanScheermeijer @dbudzins simplified approach without abstraction layer.

[AntonLantukh]

@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

[ChristiaanScheermeijer]

Perfect @AntonLantukh! This looks good to me.

Only the playlistURL method has changed, but that's only used once in our platforms 😄

[AntonLantukh]

@ChristiaanScheermeijer rolled back playlistURL to the previous interface.

[budzworthy]

You've gotta add usePlacehoderData and params inputs to usePlaylists, but then you avoid the getQueryOptions construct, which is a bit non-standard I think.

(We can do this later. I know you want to get the changes out, and the double playlist hooks wasn't added from this PR)

Suggested change

	const queryClient = useQueryClient();
	const siteId = useConfigStore((state) => state.config.siteId);
	const queryOptions = getPlaylistQueryOptions({ type, contentId, siteId, params, queryClient, enabled, usePlaceholderData });
	return useQuery<Playlist | undefined, ApiError>(queryOptions);
	return usePlaylists([{contentId, type}], enabled ? 1 : 0, usePlaceholderData, params)[0];

[AntonLantukh]

I didn't want to do it because of the different use case we have here. usePlaylists is intended to work with a list of shelves on the Landing Page. So it also covers the cases of Favorites and Continue Watching, while usePlaylist only supports content_list and playlist types which makes the purpose a bit different.

[budzworthy]

2 small comments. The one about the Routes and PlaylistScreenRouter component, I'd strongly suggest doing, but I'll approve as-is, because I know you're pressed for time.

[budzworthy]

lgtm