uninstall experiment, and event wrap router

[meandavejustice]

fixes #8 #9 #17

[meandavejustice]

r? @lmorchard @pdehaan @6a68

[meandavejustice]

I plan to do something different with the updates/state management once I know a bit more about the server

[jaredhirsch]

I"ll take a look after this morning's meeting

[jaredhirsch]

looking now

[jaredhirsch]

It seems like the uninstall-related code could be simplified a bit, two things that come to mind:

First, each addon has a unique id that you can use to check against lists of addons, and so forth. That id is the <em:id> field in install.rdf / update.rdf, which translates to the id property in sdk addons' package.json. It's guaranteed to be globally unique (every addon on AMO has a distinct id). By using that id, you can get rid of all the downcase-and-trim code sprinkled throughout.

Second, rather than iterate all the user's addons and compare with a list in package.json, as you do in the uninstall callback, it seems simpler to just track if an experiment is installed using the simple-storage high-level sdk api. Just maintain an array of installed experiment ids. Then, when you need to uninstall everything, you can fetch that list, call AddonManager.getAddonByID() for each id, and ask each experiment to uninstall itself:

  experiments.forEach(function(id) {
    let addon = AddonManager.getAddonByID(id);
    addon.uninstall();
  })

(Unfortunately, Addon.uninstall doesn't take a callback or return a promise, so we can't be sure it actually completed. For now, this is fine, although we might later want to wire up an AddonListener and only mark addons as deleted when AddonListener.onUninstalled() gets called with that Addon as the argument.)

One last thing to note: if the user uninstalls Idea Town via the Addons screen, we need to detect it and uninstall the child experiments before we get uninstalled. It looks like the way to do this with SDK is to use system/unload. Here's the MDN docs and a quite helpful SO answer.

So, summarizing, could you: use ids to track experiments; store a list of installed experiments on the client; and listen for idea town getting uninstalled via the addons screen?

I have some other notes on the high-level code architecture; I'm still writing them up, will post in a bit.

Great work, man! This thing is really coming together :-)

[jaredhirsch]

Actually, I think my comments above are enough to cover in one review. I was thinking about how we might simplify the postmessage events API, but we'll have a new set of wireframes to examine tomorrow; we can file followup bugs to make sure the API just covers the spec'd cases.

[jaredhirsch]

@meandavejustice Feel free to reassign back to me when this is ready for a final look

[jaredhirsch]