Skip to content

Commit

Permalink
Improve App Hosting Web App selection UX (#7100)
Browse files Browse the repository at this point in the history 
---------

Co-authored-by: Mathusan Selvarajah <[email protected]>
  • Loading branch information
@mathu97
mathu97 and mathu97 authored May 8, 2024
1 parent fc6b77d commit 8748b80
Show file tree
Hide file tree
Showing 3 changed files with 73 additions and 124 deletions.
128 changes: 42 additions & 86 deletions src/apphosting/app.ts
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import * as fuzzy from "fuzzy";
import * as inquirer from "inquirer";
import { AppPlatform, WebAppMetadata, createWebApp, listFirebaseApps } from "../management/apps";
import { promptOnce } from "../prompt";
import { AppMetadata, AppPlatform, createWebApp, listFirebaseApps } from "../management/apps";
import { FirebaseError } from "../error";
import { logWarning } from "../utils";

const CREATE_NEW_FIREBASE_WEB_APP = "CREATE_NEW_WEB_APP";
const CONTINUE_WITHOUT_SELECTING_FIREBASE_WEB_APP = "CONTINUE_WITHOUT_SELECTING_FIREBASE_WEB_APP";
Expand All @@ -11,7 +9,7 @@ export const webApps = {
CREATE_NEW_FIREBASE_WEB_APP,
CONTINUE_WITHOUT_SELECTING_FIREBASE_WEB_APP,
getOrCreateWebApp,
promptFirebaseWebApp,
generateWebAppName,
};

type FirebaseWebApp = { name: string; id: string };
Expand All @@ -34,22 +32,7 @@ async function getOrCreateWebApp(
backendId: string,
): Promise<FirebaseWebApp | undefined> {
const webAppsInProject = await listFirebaseApps(projectId, AppPlatform.WEB);

if (webAppsInProject.length === 0) {
// create a web app using backend id
const { displayName, appId } = await createFirebaseWebApp(projectId, {
displayName: backendId,
});
return { name: displayName, id: appId };
}

const existingUserProjectWebApps = new Map(
webAppsInProject.map((obj) => [
// displayName can be null, use app id instead if so. Example - displayName: "mathusan-web-app", appId: "1:461896338144:web:426291191cccce65fede85"
obj.displayName ?? obj.appId,
obj.appId,
]),
);
const existingUserProjectWebApps = firebaseAppsToMap(webAppsInProject);

if (firebaseWebAppName) {
if (existingUserProjectWebApps.get(firebaseWebAppName) === undefined) {
Expand All @@ -58,79 +41,24 @@ async function getOrCreateWebApp(
);
}

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return { name: firebaseWebAppName, id: existingUserProjectWebApps.get(firebaseWebAppName)! };
}

return await webApps.promptFirebaseWebApp(projectId, backendId, existingUserProjectWebApps);
}

/**
* Prompts the user for the web app that they would like to associate their backend with
* @param projectId user's projectId
* @param backendId user's backendId
* @param existingUserProjectWebApps a map of a user's firebase web apps to their ids
* @return the name and ID of a web app
*/
async function promptFirebaseWebApp(
projectId: string,
backendId: string,
existingUserProjectWebApps: Map<string, string>,
): Promise<FirebaseWebApp | undefined> {
const existingWebAppKeys = Array.from(existingUserProjectWebApps.keys());

const firebaseWebAppName = await promptOnce({
type: "autocomplete",
name: "app",
message:
"Which of the following Firebase web apps would you like to associate your backend with?",
source: (_: any, input = ""): Promise<(inquirer.DistinctChoice | inquirer.Separator)[]> => {
return new Promise((resolve) =>
resolve([
new inquirer.Separator(),
{
name: "Create a new Firebase web app.",
value: CREATE_NEW_FIREBASE_WEB_APP,
},
{
name: "Continue without a Firebase web app.",
value: CONTINUE_WITHOUT_SELECTING_FIREBASE_WEB_APP,
},
new inquirer.Separator(),
...fuzzy.filter(input, existingWebAppKeys).map((result) => {
return result.original;
}),
]),
);
},
});

if (firebaseWebAppName === CREATE_NEW_FIREBASE_WEB_APP) {
const newFirebaseWebApp = await createFirebaseWebApp(projectId, { displayName: backendId });
return { name: newFirebaseWebApp.displayName, id: newFirebaseWebApp.appId };
} else if (firebaseWebAppName === CONTINUE_WITHOUT_SELECTING_FIREBASE_WEB_APP) {
return;
return {
name: firebaseWebAppName,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
id: existingUserProjectWebApps.get(firebaseWebAppName)!,
};
}

// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
return { name: firebaseWebAppName, id: existingUserProjectWebApps.get(firebaseWebAppName)! };
}
const webAppName = await generateWebAppName(projectId, backendId);

/**
* A wrapper for createWebApp to catch and log quota errors
*/
async function createFirebaseWebApp(
projectId: string,
options: { displayName?: string },
): Promise<WebAppMetadata> {
try {
return await createWebApp(projectId, options);
const app = await createWebApp(projectId, { displayName: webAppName });
return { name: app.displayName, id: app.appId };
} catch (e) {
if (isQuotaError(e)) {
throw new FirebaseError(
logWarning(
"Unable to create a new web app, the project has reached the quota for Firebase apps. Navigate to your Firebase console to manage or delete a Firebase app to continue. ",
{ original: e instanceof Error ? e : undefined },
);
return;
}

throw new FirebaseError("Unable to create a Firebase web app", {
Expand All @@ -139,6 +67,34 @@ async function createFirebaseWebApp(
}
}

async function generateWebAppName(projectId: string, backendId: string): Promise<string> {
const webAppsInProject = await listFirebaseApps(projectId, AppPlatform.WEB);
const appsMap = firebaseAppsToMap(webAppsInProject);
if (!appsMap.get(backendId)) {
return backendId;
}

let uniqueId = 1;
let webAppName = `${backendId}_${uniqueId}`;

while (appsMap.get(webAppName)) {
uniqueId += 1;
webAppName = `${backendId}_${uniqueId}`;
}

return webAppName;
}

function firebaseAppsToMap(apps: AppMetadata[]): Map<string, string> {
return new Map(
apps.map((obj) => [
// displayName can be null, use app id instead if so. Example - displayName: "mathusan-web-app", appId: "1:461896338144:web:426291191cccce65fede85"
obj.displayName ?? obj.appId,
obj.appId,
]),
);
}

/**
* TODO: Make this generic to be re-used in other parts of the CLI
*/
Expand Down
2 changes: 1 addition & 1 deletion src/apphosting/index.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ export async function doSetup(

const webApp = await webApps.getOrCreateWebApp(projectId, webAppName, backendId);
if (webApp) {
logSuccess(`Firebase web app set to ${webApp.name}.\n`);
logSuccess(`Created a new Firebase web app named "${webApp.name}"`);
} else {
logWarning(`Firebase web app not set`);
}
Expand Down
67 changes: 30 additions & 37 deletions src/test/apphosting/app.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { webApps } from "../../apphosting/app";
import * as apps from "../../../src/management/apps";
import * as prompts from "../../prompt";
import * as sinon from "sinon";
import { expect } from "chai";
import { FirebaseError } from "../../error";
Expand All @@ -9,21 +8,19 @@ describe("app", () => {
const projectId = "projectId";
const backendId = "backendId";

let listFirebaseApps: sinon.SinonStub;

describe("getOrCreateWebApp", () => {
let createWebApp: sinon.SinonStub;
let listFirebaseApps: sinon.SinonStub;
let promptFirebaseWebApp: sinon.SinonStub;

beforeEach(() => {
createWebApp = sinon.stub(apps, "createWebApp");
listFirebaseApps = sinon.stub(apps, "listFirebaseApps");
promptFirebaseWebApp = sinon.stub(webApps, "promptFirebaseWebApp");
});

afterEach(() => {
createWebApp.restore();
listFirebaseApps.restore();
promptFirebaseWebApp.restore();
});

it("should create an app with backendId if no web apps exist yet", async () => {
Expand All @@ -49,53 +46,49 @@ describe("app", () => {
);
});

it("prompts user for a web app if none is provided", async () => {
listFirebaseApps.returns(
Promise.resolve([
{ displayName: "testWebApp1", appId: "testWebAppId1", platform: apps.AppPlatform.WEB },
]),
);

const userSelection = { name: "testWebApp2", id: "testWebAppId2" };
promptFirebaseWebApp.returns(Promise.resolve(userSelection));
it("returns undefined if user has reached the app limit for their project", async () => {
listFirebaseApps.returns(Promise.resolve([]));
createWebApp.throws({ original: { status: 429 } });

await expect(webApps.getOrCreateWebApp(projectId, null, backendId)).to.eventually.deep.equal(
userSelection,
);
expect(promptFirebaseWebApp).to.be.called;
const app = await webApps.getOrCreateWebApp(projectId, null, backendId);
expect(app).equal(undefined);
});
});

describe("promptFirebaseWebApp", () => {
let promptOnce: sinon.SinonStub;
let createWebApp: sinon.SinonStub;

describe("generateWebAppName", () => {
beforeEach(() => {
promptOnce = sinon.stub(prompts, "promptOnce");
createWebApp = sinon.stub(apps, "createWebApp");
listFirebaseApps = sinon.stub(apps, "listFirebaseApps");
});

afterEach(() => {
promptOnce.restore();
createWebApp.restore();
listFirebaseApps.restore();
});

it("creates a new web app if user selects that option", async () => {
promptOnce.returns(webApps.CREATE_NEW_FIREBASE_WEB_APP);
createWebApp.returns({ displayName: backendId, appId: "appId" });
it("returns backendId if no such web app already exists", async () => {
listFirebaseApps.returns(Promise.resolve([]));

await webApps.promptFirebaseWebApp(projectId, backendId, new Map([["app", "appId"]]));
expect(createWebApp).to.be.calledWith(projectId, { displayName: backendId });
const appName = await webApps.generateWebAppName(projectId, backendId);
expect(appName).equal(backendId);
});

it("skips a web selection if user selects that option", async () => {
promptOnce.returns(webApps.CONTINUE_WITHOUT_SELECTING_FIREBASE_WEB_APP);
it("returns backendId as appName with a unique id if app with backendId already exists", async () => {
listFirebaseApps.returns(Promise.resolve([{ displayName: backendId, appId: "1234" }]));

await expect(
webApps.promptFirebaseWebApp(projectId, backendId, new Map([["app", "appId"]])),
).to.eventually.equal(undefined);
const appName = await webApps.generateWebAppName(projectId, backendId);
expect(appName).equal(`${backendId}_1`);
});

it("returns appropriate unique id if app with backendId already exists", async () => {
listFirebaseApps.returns(
Promise.resolve([
{ displayName: backendId, appId: "1234" },
{ displayName: `${backendId}_1`, appId: "1234" },
{ displayName: `${backendId}_2`, appId: "1234" },
]),
);

expect(createWebApp).to.not.be.called;
const appName = await webApps.generateWebAppName(projectId, backendId);
expect(appName).equal(`${backendId}_3`);
});
});
});

0 comments on commit 8748b80

Please sign in to comment.