Only show recommendations for predefined result types.

[chuckharmston]

• Create a formal interface/base for a "result type" class, which should serve a similar purpose to the current RecommendationRow class.
• Create Wikipedia and TLD result type classes.
  • Could use Wikipedia W in the first column?
• Attach result type classes to an object in the app namespace.
• Add a method to the RecommendationServer class that negotiates for and returns the appropriate result type class or null.
• Refactor RecommendationServer.search to only publish the recommendation event if that method returns a truthy value.
• Refactor RecommendationServer.search to include the appropriate result type class in the event payload.
• Refactor Recommendation.show to use the result type class from the event payload when rendering.

Blocked on https://github.com/mozilla/universal-search-recommendation/issues/117

[jaredhirsch]

@chuckharmston As far as I can tell, the only thing that varies between the TLD and Wikipedia row types is the name of the key used to get the same information (like enhancements.wikipedia.title vs result.title). Why not just have the Recommendation pass a config object into the RecommendationRow constructor, something like

let rowConfig;
if (type === 'wikipedia') {
  rowConfig = {
    title: result._get('enhancements.wikipedia.title'),
    url: result._get('enhancements.wikipedia.url'),
    type: 'wikipedia'
  };
} else {
  rowConfig = {
    title: result._get('result.title'),
    url: result._get('result.url'),
    type: 'tld'
  };
}
const recommendation = new this.RecommendationRow({
  eTLD: this.eTLD,
  io: this.io,
  win: this.win,
  config: rowConfig
});

Since the two types are visually identical, I think this simpler approach would be better. (Even if we do start styling the two types differently, we can use the type as a classname to toggle styling changes.) We're about a week out from public launch, and I'd like to minimize code changes.

[chuckharmston]

Why not...

Because the immediate post-launch roadmap has us doing some radically different things (i.e. movie recommendations); I tried to build this with that in mind. I've been especially sensitive to introducing technical debt, since it's not terribly clear who will be working on this in the near future. I'd prefer to hand this off with as few shortcuts as possible, even though the entire thing is a hack.

But your point is good; if you feel strongly, feel free to r- and close the pull request (this is the issue) and I'll give it another try tomorrow.

[jaredhirsch]

Because the immediate post-launch roadmap has us doing some radically different things

Yeah, I'd say, let's stash this in a branch for now, and revisit when we are in a sprint planning meeting and different card types come up.

This close to launch, I'd prefer to focus on the bugs that were already filed and labeled launch blockers as part of our planning session. So, I'm going to remove the 'public launch' label from this, but we'll revisit as soon as it makes product sense.

[chuckharmston]

Sorry, I guess I'm confused at what you mean. Are you wontfixing this bug, or #166?

[jaredhirsch]

If the server is only going to send over predefined result types (in that blocking bug, https://github.com/mozilla/universal-search-recommendation/issues/117), then there's nothing we have to do in the add-on before public launch. The add-on can just naively display whatever the server sends over.

[chuckharmston]

That's not really true; right now we do some moderately complex negotiating (example) of data sources that mixes what we would now consider "tld" or "wikipedia" results. The different data sources we're using also have different pieces available—we're able to know a defined size of images coming from Clearbit, but we have to pass the width and height of images coming from Embedly so the UI can do display-related calculations.

That's also not how the server works; remember that we're trying to keep all of the logic on the client side to make UI experimentation easier. We don't just pick "an image" to send as "the image"; we augment the result with as many relevant bits as we can, and let the addon negotiate the best way to display them. Even if we were to change that—which I would not recommend—the addon would need to be changed to accomodate that.

[nchapman]

So, I'm going to remove the 'public launch' label from this, but we'll revisit as soon as it makes product sense.

Sorry again don't know all the technical implementation details here, but it's important for the public launch that we limit the types of results and report on those types in our metrics. It seems like this is the implementation of those limits. Let me know if it would help for us to have a quick triage/chat to sort out the specifics.

[jaredhirsch]

@chuckharmston @nchapman Sounds like a meeting is in order

[jaredhirsch]

@chuckharmston @nchapman Sounds like a meeting is in order

Actually, maybe not.

@nchapman If limiting the types of results to TLD and Wikipedia, and reporting on which was shown/clicked in metrics code, is all that's needed from a product perspective, that's a crisp enough definition for me, provided that we do basically the same thing with clearbit vs embedly images in both cases.

@chuckharmston I'd like to find the simplest solution here that meets the product's needs. Suppose we add the 'tld' or 'wikipedia' classname to the existing recommendation, and change nothing else. What would be broken / missing?

[jaredhirsch]

Chuck and I discussed this at length. The old and new APIs are documented via examples here: https://gist.github.com/chuckharmston/861a1442e7dde98aefe333a2347e0361

Major changes:

• TLD vs wikipedia type: detect based on presence of enhancements.tld

TLD type:

• enhancements.keyimage is gone
• enhancements.logo renamed to enhancements.tld
• enhancements.wikipedia added (though we don't currently use it)

Wikipedia type:

• enhancements.keyimage is gone
• enhancements.wikipedia changed shape, from { abstract, slug, title } to { image, title, url }
• enhancements.logo is not sent down for wikipedia; it uses an embedly image or letterbox fallback

[chuckharmston]

Good test cases:

• f to test TLDs
• space de to test Wikipedia pages
• mozilla to test null results