feat(recommendations): [MC-125] Add id to recommendations#63
Conversation
| */ | ||
| listTopics: Array<Topic>; | ||
| /** ----- In Development (2023-08-16) -----Get a slate of ranked recommendations for the MozSocial. */ | ||
| mozSocialSlate: CorpusSlate; |
There was a problem hiding this comment.
This is unrelated to this change, and no longer needed. I will create a PR in recommendation-api to remove it.
| - Recommendation | ||
| id: | ||
| type: string | ||
| description: String identifier for the Recommendation. This value is expected to be different on each request. |
There was a problem hiding this comment.
@ScottDowne I mainly wanted to get your eyes on the openapi.yml changes. Adding a property is non-breaking, right? Is the naming of id good, or would you prefer recommendationId?
There was a problem hiding this comment.
So nothing is wrong with it from a breaking standpoint.
However the name id vs recommendationId is hard to comment on without some help understanding it.
Is it the idea that we migrate the client from using tileId to this new id? If so, I think id is right.
Right now we convert tileId to id in the client anyway because of backwards comp reasons while the client was supporting both apis. Having it go back to just id is nice and clean.
If it's a new id, is different from tileId, then it would be weird having 2 different ids, both being called at one point id. If that makes sense.
There was a problem hiding this comment.
Thanks, that context is helpful.
In the BugZilla meta ticket I proposed adding both this id and tileId to Glean, but based on what you said, it sounds better only to emit id to Glean. I believe that would work for us. I'll update the BugZilla ticket accordingly.
There was a problem hiding this comment.
@ScottDowne will check that id will is not used by any supported version of Firefox Desktop. I'll add a tag to hold deployment until that's done.
There was a problem hiding this comment.
The browser does not use id for any versions that use this API. You can see it here: https://searchfox.org/mozilla-central/source/browser/components/newtab/lib/DiscoveryStreamFeed.jsm#1465-1473
Older versions use the fallback legacy api we setup, which I imagine is the same, so we should be fine here.
3 participants
Goal
Return the
CorpusRecommendation.idfor recommendations, which is unique for each request.Once Glean pings include this property, it will allow us to better track recommendations through our content pipeline, because we'll be able to join between impressions, clicks, and recommendation responses. We only emit recommendation
idthrough Glean, and stop emitting Activity Stream. This will bring Firefox in line with our other clients in how we identify recommendations. We will have to coordinate with the Analytics team to ensure our data modeling is updated before Firefox 120 is released on November 21.I'd love feedback/perspectives on:
idorrecommendationId? The former seemed more consistent with other properties, but I'm open to either.Implementation Decisions
References
JIRA ticket:
BugZilla ticket:
Slack channel:
Documentation: