feat(recommend): Add trending types and methods #1396

SophieManley03 · 2022-02-25T10:48:17Z

What?

Prepare typing and methods for the new recommend model named trending.

Why?

We want to release trending model in beta on March, 10th. In order to have a successful release we would like to update the recommend library so that use can use trending with it.

How?

Here are the specs for trending. It will give a better idea of what is implemented in the search.

We have 3 types of trending:

trending items: Return trending items for a given index.
trending items for a facet: Return trending items given a facetName and a facetValue.
trending facet: Return trending facetValue given a facetName

We don't want to break the current implementation of recommend for our customers.

SophieManley03 · 2022-02-25T10:53:08Z

packages/recommend/src/types/RecommendationsQuery.ts

@@ -15,7 +15,7 @@ export type RecommendationsQuery = {
  /**
   * The `objectID` of the item to get recommendations for.
   */
-  readonly objectID: string;
+  readonly objectID?: string;


I didn't want to break the existing implementation for related-products and bought-together. So I put it optional but it is still mandatory for these models :/

it should be required, but omitted from the new items then probably, as how facetName is omitted for fbt

It is omitted actually, how come it's still needed?

the problem if I do that is with getRecommendations . getRecommendation is used for every get method of Recommend and the objectID will be mandatory even for trends

Can we still remodel the code so that types are accurate?

So after a third thought, I wanted to try to make it optional here but required for RP and FBT using Required in their own Query type.
I did that to be able to use getRecommendations for all models.

Which bring me to a question after a discussion with @PLNech:
What is the purpose of getRecommendation? Does it make sense to make it retrieve recommendations for all kind of model? @francoischalifour maybe you'll know?

Put differently: was getRecommendation a hack while we defined the actual model names before GA, or is it a feature designed to allow using not-yet-public new models?

If the former, then it might be time we drop this generic method (which brings subpar "common denominator" DX compared to specialized methods) and only implement Trending as a new, fully-typed method

If the latter, we'll need to find a generic implementation that is least disruptive to current users.

getRecommendations() is useful for the situation we're in right now: users can already use the trending model, without it being officially supported with a method in the client.

We didn't have a team to maintain this Recommend client initially so we provided it as an "escape hatch" if we're not reactive to support new models in the client.

getRecommendations() was just the very initial implementation that satisfied us at the time, feel free to change it as you need. And if it's not helpful for the new model, don't feel like you need to use it. (In the future, we can also deprecate it.)

I'll ask the opposite question then: what's the purpose of having a specialized method for each model rather than just using getRecommendations()? It's basically a multi-query endpoint, and I think it has its uses (like making a single call to retrieve both trending items and trending facets for example). While calling getRelatedProducts() as a multi-query is strange IMO.

codesandbox-ci · 2022-02-25T10:58:24Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit cfe8fc5:

Sandbox	Source
algolia/create-instantsearch-app	Configuration

jojva

LGTM 👍

Haroenv · 2022-02-28T11:07:33Z

packages/recommend/src/types/RecommendationsQuery.ts

@@ -15,7 +15,7 @@ export type RecommendationsQuery = {
  /**
   * The `objectID` of the item to get recommendations for.
   */
-  readonly objectID: string;
+  readonly objectID?: string;


it should be required, but omitted from the new items then probably, as how facetName is omitted for fbt

Haroenv

👌

packages/recommend/src/methods/getTrendingFacets.ts

packages/recommend/src/methods/getTrendingItemsForFacet.ts

…earch-client-javascript into feat/recommend-trending

…ng-item

shortcuts

It looks good to me!

Only one non-blocking comment if @francoischalifour have nothing agains it

packages/recommend/src/__tests__/getTrendingItems.test.ts

packages/recommend/src/types/FrequentlyBoughtTogetherQuery.ts

…earch-client-javascript into feat/recommend-trending

PLNech

LGTM! Let's see how it goes once implemented in @algolia/recommend 🙃

Haroenv

looks good!

Haroenv · 2022-03-14T10:26:27Z

packages/recommend/src/types/RecommendModel.ts

+export type RecommendModel = 'related-products' | 'bought-together' | TrendingModel;
+export type TrendingModel = 'trending-items' | 'trending-facets';


Suggested change

export type RecommendModel = 'related-products' | 'bought-together' | TrendingModel;

export type TrendingModel = 'trending-items' | 'trending-facets';

export type TrendingModel = 'trending-items' | 'trending-facets';

export type RecommendModel = 'related-products' | 'bought-together' | TrendingModel;

Sorry D: #1399

SophieManley03 · 2022-03-14T12:17:08Z

Thank you all for the reviews 🙇‍♀️

feat(recommend): Add trending types and methods

36bbcfc

SophieManley03 added the Scope: Recommend label Feb 25, 2022

SophieManley03 requested a review from jojva February 25, 2022 10:48

SophieManley03 self-assigned this Feb 25, 2022

SophieManley03 commented Feb 25, 2022

View reviewed changes

SophieManley03 requested review from tkrugg, francoischalifour and Haroenv February 25, 2022 10:56

shortcuts mentioned this pull request Feb 25, 2022

[recommend]: add new strategies algolia/api-clients-automation#176

Closed

SophieManley03 marked this pull request as ready for review February 25, 2022 12:46

jojva previously approved these changes Feb 25, 2022

View reviewed changes

Haroenv requested a review from shortcuts February 28, 2022 11:05

Haroenv requested changes Feb 28, 2022

View reviewed changes

SophieManley03 dismissed jojva’s stale review via 931ff60 February 28, 2022 15:35

SophieManley03 force-pushed the feat/recommend-trending branch from 931ff60 to c951aae Compare February 28, 2022 15:55

fix(recommend): Add a trendingQuery type

727c787

SophieManley03 force-pushed the feat/recommend-trending branch from c951aae to 727c787 Compare February 28, 2022 16:09

feat(recommend): increase bundle size

b1f7794

SophieManley03 requested a review from Haroenv February 28, 2022 17:16

Haroenv reviewed Mar 1, 2022

View reviewed changes

packages/recommend/src/methods/getTrendingFacets.ts Show resolved Hide resolved

packages/recommend/src/methods/getTrendingItemsForFacet.ts Outdated Show resolved Hide resolved

SophieManley03 added 2 commits March 7, 2022 16:24

fix: remove TrendingType

d64940d

Merge branch 'feat/recommend-trending' of github.com:algolia/algolias…

4014f29

…earch-client-javascript into feat/recommend-trending

SophieManley03 requested a review from Haroenv March 7, 2022 15:52

fix: remove items for facet as in the search and use it within trendi…

0d304e6

…ng-item

shortcuts previously approved these changes Mar 10, 2022

View reviewed changes

packages/recommend/src/__tests__/getTrendingItems.test.ts Show resolved Hide resolved

packages/recommend/src/types/FrequentlyBoughtTogetherQuery.ts Outdated Show resolved Hide resolved

Merge branch 'master' into feat/recommend-trending

3567914

shortcuts enabled auto-merge (squash) March 10, 2022 16:52

fix: reintroduce trendingQuery

a1d7636

SophieManley03 dismissed shortcuts’s stale review via bd20da2 March 11, 2022 09:18

Merge branch 'feat/recommend-trending' of github.com:algolia/algolias…

cfe8fc5

…earch-client-javascript into feat/recommend-trending

SophieManley03 force-pushed the feat/recommend-trending branch from bd20da2 to cfe8fc5 Compare March 11, 2022 09:21

SophieManley03 requested a review from PLNech March 11, 2022 09:36

PLNech approved these changes Mar 11, 2022

View reviewed changes

Haroenv approved these changes Mar 14, 2022

View reviewed changes

shortcuts merged commit 6882ec8 into master Mar 14, 2022

shortcuts deleted the feat/recommend-trending branch March 14, 2022 11:02

shortcuts mentioned this pull request Mar 14, 2022

fix(recommend): declare type before usage #1399

Merged

		export type RecommendModel = 'related-products' \| 'bought-together' \| TrendingModel;
		export type TrendingModel = 'trending-items' \| 'trending-facets';

feat(recommend): Add trending types and methods #1396

feat(recommend): Add trending types and methods #1396

Uh oh!

Conversation

SophieManley03 commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SophieManley03 Feb 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SophieManley03 Mar 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codesandbox-ci bot commented Feb 25, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jojva left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Haroenv left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shortcuts left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

PLNech left a comment

Choose a reason for hiding this comment

Uh oh!

Haroenv left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SophieManley03 commented Mar 14, 2022

Uh oh!

Uh oh!

SophieManley03 commented Feb 25, 2022 •

edited

Loading

SophieManley03 Feb 28, 2022 •

edited

Loading

SophieManley03 Mar 7, 2022 •

edited

Loading

codesandbox-ci bot commented Feb 25, 2022 •

edited

Loading