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

Added custom domain functions to admin panel #8227

Merged
merged 7 commits into from
Sep 5, 2024

Conversation

kurtisassad
Copy link
Contributor

@kurtisassad kurtisassad commented Jun 24, 2024

Link to Issue

Closes: #8226
Closes: #7683

Description of Changes

  • Adds ability to add and update new custom domains on admin panel

Test Plan

  • Add HEROKU_APP_NAME, HEROKU_API_TOKEN, MAGIC_API_KEY, and MAGIC_CLIENT_ID to .env
  • Go to http://localhost:8080/admin panel and make sure you are logged in as a super admin.
  • Refresh existing custom domain and ensure it returns data
  • Try refreshing a non-existing custom domain and ensure it returns error
  • Try adding a new custom domain to a community. Then try refreshing it to ensure it returns data.
  • Try adding the same custom domain twice, make sure it throws an error

Deployment plan

  • We need to add MAGIC_CLIENT_ID to the environment variables as it is not there currently

@timolegros
Copy link
Collaborator

timolegros commented Jun 24, 2024

@kurtisassad as part of this PR can we add a Knowledgebase entry that lists all the places where we need to whitelist custom domains (e.g. Alchemy, Magic, etc)?

Never mind it seems to be there already. That said, we need to somehow keep track of when growth is linking a new custom domain because Alchemy and Magic (not sure about Magic) don't have APIs to update their whitelists programmatically.

@kurtisassad kurtisassad added the deployment plan (PRs only) requires manual infrastructure changes on release label Aug 13, 2024
@kurtisassad kurtisassad marked this pull request as ready for review August 13, 2024 14:16
@timolegros
Copy link
Collaborator

Per my previous comment, how do we deal with Alchemy whitelisting? Since they don't have an API to update the whitelist we need to manually add domains to the whitelist. Most chain interactions from a custom domain for an EVM community will fail unless we add that community to the whitelist. This should be very clearly outlined to Growth and it does mean that Growth still needs to create a ticket when adding a Custom domain for EVM communities.

@kurtisassad
Copy link
Contributor Author

Discussed with Tim, we will get this in first and then figure out the alchemy whitelist flow, there seems to be some docs on generating a JWT, so we can go with that approach when he comes back from vacation

@jnaviask
Copy link
Collaborator

@timolegros my question is what direct onchain requests do we actually make on custom domains / how large is the breakage surface area in practice?

@kurtisassad kurtisassad force-pushed the ka.herokuCustomDomains branch 3 times, most recently from e1817ba to a56be87 Compare August 16, 2024 15:51
@timolegros
Copy link
Collaborator

@timolegros my question is what direct onchain requests do we actually make on custom domains / how large is the breakage surface area in practice?

  1. Cannot load the community if it has stake or contests enabled
    image
  2. Cannot create stake/contests

@jnaviask
Copy link
Collaborator

@timolegros @kurtisassad IMO it is acceptable to get custom domain automation out where growth needs to make an eng request if stake/contest support is needed. Better than the current flow.

Copy link
Collaborator

@timolegros timolegros left a comment

Choose a reason for hiding this comment

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

LGTM just 2 quick suggestions.

# Conflicts:
#	libs/model/src/middleware/authorization.ts
#	libs/schemas/src/commands/community.schemas.ts
#	packages/commonwealth/client/scripts/views/pages/AdminPanel/UpdateCustomDomainTask.tsx
#	packages/commonwealth/server/api/community.ts
Copy link
Contributor

@masvelio masvelio left a comment

Choose a reason for hiding this comment

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

small comments but looks good to me in general, didn't test it manually though

@kurtisassad kurtisassad merged commit 299cac5 into master Sep 5, 2024
9 checks passed
@kurtisassad kurtisassad deleted the ka.herokuCustomDomains branch September 5, 2024 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployment plan (PRs only) requires manual infrastructure changes on release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add custom domain functionality to admin panel Fully automate custom domain configuration
4 participants