Add onUpdate event listener for installing starter blueprints on Playground urls #4249
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,71 +26,122 @@ import reportError from "@/telemetry/reportError"; | |
|
|
||
| const { reducer, actions } = extensionsSlice; | ||
|
|
||
| const PLAYGROUND_URL = "https://www.pixiebrix.com/playground"; | ||
|
|
||
| function installBlueprint( | ||
| state: ExtensionOptionsState, | ||
| blueprint: RecipeDefinition | ||
| ): ExtensionOptionsState { | ||
| return reducer( | ||
| state, | ||
| actions.installRecipe({ | ||
| recipe: blueprint, | ||
| extensionPoints: blueprint.extensionPoints, | ||
| }) | ||
| ); | ||
| } | ||
|
|
||
| async function installBlueprints( | ||
| blueprints: RecipeDefinition[] | ||
| ): Promise<boolean> { | ||
| let installed = false; | ||
| if (blueprints.length === 0) { | ||
| return installed; | ||
| } | ||
|
|
||
| let extensionsState = await loadOptions(); | ||
| for (const blueprint of blueprints) { | ||
| const blueprintAlreadyInstalled = extensionsState.extensions.some( | ||
| (extension) => extension._recipe.id === blueprint.metadata.id | ||
| ); | ||
|
|
||
| if (!blueprintAlreadyInstalled) { | ||
| extensionsState = installBlueprint(extensionsState, blueprint); | ||
| installed = true; | ||
| } | ||
| } | ||
|
|
||
| await saveOptions(extensionsState); | ||
| await forEachTab(queueReactivateTab); | ||
| return installed; | ||
| } | ||
|
|
||
| async function getShouldFirstTimeInstall(): Promise<boolean> { | ||
| const client = await maybeGetLinkedApiClient(); | ||
| if (client == null) { | ||
| console.debug( | ||
| "Skipping starter blueprint installation because the extension is not linked to the PixieBrix service" | ||
| ); | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| const { | ||
| data: { install_starter_blueprints: shouldInstall }, | ||
| } = await client.get("/api/onboarding/starter-blueprints/install/"); | ||
|
|
||
| if (shouldInstall) { | ||
| // If the starter blueprint request fails for some reason, or the user's primary organization | ||
| // gets removed, we'd still like to mark starter blueprints as installed for this user | ||
| // so that they don't see onboarding views/randomly have starter blueprints installed | ||
| // the next time they open the extension | ||
| await client.post("/api/onboarding/starter-blueprints/install/"); | ||
| } | ||
|
|
||
| return shouldInstall; | ||
| } catch (error) { | ||
| reportError(error); | ||
|
There was a problem hiding this comment. What is going to be returned from this function when it catches an error? I think There was a problem hiding this comment. 👍 TypeScript in loose mode misses a lot of such warnings. It should have complained that the signature is There was a problem hiding this comment. Thanks for catching this Ben! This was indeed a mistake. |
||
| } | ||
| } | ||
|
|
||
| async function getStarterBlueprints(): Promise<RecipeDefinition[]> { | ||
| const client = await maybeGetLinkedApiClient(); | ||
| if (client == null) { | ||
| console.debug( | ||
| "Skipping starter blueprint installation because the extension is not linked to the PixieBrix service" | ||
| ); | ||
| return []; | ||
|
|
||
| } | ||
|
|
||
| try { | ||
| const { data: starterBlueprints } = await client.get<RecipeDefinition[]>( | ||
| "/api/onboarding/starter-blueprints/" | ||
| ); | ||
| return starterBlueprints; | ||
| } catch (error) { | ||
| reportError(error); | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| async function installStarterBlueprints(): Promise<boolean> { | ||
| const starterBlueprints = await getStarterBlueprints(); | ||
| return installBlueprints(starterBlueprints); | ||
| } | ||
|
|
||
| export async function firstTimeInstallStarterBlueprints(): Promise<void> { | ||
| const shouldInstall = await getShouldFirstTimeInstall(); | ||
| if (!shouldInstall) { | ||
| return; | ||
| } | ||
|
|
||
| const installed = await installStarterBlueprints(); | ||
|
There was a problem hiding this comment. While unlikely, it could be called concurrently with |
||
|
|
||
| if (installed) { | ||
| void browser.tabs.create({ | ||
| url: PLAYGROUND_URL, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| function initStarterBlueprints(): void { | ||
| browser.tabs.onUpdated.addListener((tabId, changeInfo, tab) => { | ||
| if (tab?.url?.startsWith(PLAYGROUND_URL)) { | ||
| void installStarterBlueprints(); | ||
|
There was a problem hiding this comment. I think my comment got lost: This listener will run How about this pattern: async function initStarterBlueprints() {
if (await isInstalled()) {
return
}
if (await shouldInstall() || await onPlayGroundOpen()) {
install()
}
}
async function onPlayGroundOpen() {
return new Promise(resolve => {
const listener = (id, info, tab) => {
if (tab?.url?.startsWith(PLAYGROUND_URL)) {
browser.tabs.onUpdated.removeListener(listener)
resolve(true)
}
}
browser.tabs.onUpdated.addListener(listener)
})
}To note:
There was a problem hiding this comment. The function will run, but I think the check on line 54 prevents any blueprint from being installed twice, right? What kind of trouble are you imagining this will cause? Can you explain how your code example here avoids that trouble? There was a problem hiding this comment.
At this point line 54 hasn't been reached yet, so any logic there is futile. My example avoids the trouble because it does call There was a problem hiding this comment.
Some possibilities:
There was a problem hiding this comment. @fregante I'm not sure the pattern you suggested fits the needs of the feature, unless I'm misunderstanding your pseudocode. We need users to always have Starter Blueprints installed for them when visiting the Playground page, to catch the cases if they were to uninstall the Blueprints and revisit the Playground, or if an error like a network request failed to install them the first time. In this case, I think I might go with a locking (and/or maybe debounce?) pattern in order to solve the concerns you outlined above. There was a problem hiding this comment.
I didn't think about that, although it would only be hit if they visit the playground, delete the blueprint, and then visit the playground again — all while never quitting the browser. I guess it's fine to debounce it to once per minute, that would work even better. There was a problem hiding this comment. Thanks for the discussion Federico! Yeah, debounce sounds good to me also. There was a problem hiding this comment. Yeah thanks federico! Great considerations here |
||
| } | ||
| }); | ||
|
|
||
| void firstTimeInstallStarterBlueprints(); | ||
| } | ||
|
|
||
| export default initStarterBlueprints; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To ensure that it's not called undebounced (as it currently is on line 150), we can prepend
_to the name.I generally inline the function in the
debouncedefinition (debounce(() => {}, opts)) but for long functions it's less readable.