Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENG-2960 Read ssoProvider from config host, not p0-prod #135

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

fabgo
Copy link
Contributor

@fabgo fabgo commented Oct 30, 2024

Reads the ssoProvider for login from the host specified in the config, instead of defaulting to p0-prod.

Reads the ssoProvider for login from the host specified in the config, instead of defaulting to p0-prod.
publicDoc(`orgs/${args.org}`)
);
await saveConfig(args.org);
await initializeFirebase();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't follow why we need to add initializeFirebase here/. Wasn't it already initialized if we were able to read the doc before?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are two Firebase, the bootstrap one (which is configured to point to p0-prod) and the regular one, which uses whatever is specified in the config.

export async function saveConfig(orgId: string) {
const orgDoc = await getDoc<RawOrgData, object>(
bootstrapDoc(`orgs/${orgId}`)
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should there be the null check if (!orgData) throw here as well?

We wouldn't want to write the config if there was no org found I assume

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's there on line 33. We use the bootstrap config if no config was found.

Copy link
Contributor

@gergas3 gergas3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@fabgo fabgo marked this pull request as ready for review October 30, 2024 20:20
@fabgo fabgo merged commit c66b655 into main Oct 30, 2024
3 checks passed
@fabgo fabgo deleted the fabian/eng-2960-read-ssoProvider-from-config-host branch October 30, 2024 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants