-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Add automatic company logo fetching during workspace creation #9158
base: main
Are you sure you want to change the base?
Conversation
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.
PR Summary
This PR introduces automatic company logo fetching during workspace creation by integrating a new CompanyEnrichmentService that uses twenty-icons.com to fetch logos based on work email domains.
- Added new
CompanyEnrichmentService
in/packages/twenty-server/src/engine/core-modules/company-enrichment/company-enrichment.service.ts
with extensible architecture for future enrichment features - Integrated logo fetching in
SignInUpService.signUpOnNewWorkspace()
to automatically set workspace logos during creation - Service uses existing
isWorkEmail
utility to validate work domains before fetching logos - Added placeholder for future company data enrichment methods in
CompanyEnrichmentService
- Service follows twenty-icons.com URL pattern:
https://twenty-icons.com/{domain}
💡 (2/5) Greptile learns from your feedback when you react with 👍/👎!
4 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile
const domain = getDomainNameByEmail(email); | ||
|
||
return `${this.TWENTY_ICONS_BASE_URL}/${domain}`; |
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.
style: No error handling if domain extraction fails. Should wrap in try/catch to handle potential errors from getDomainNameByEmail.
|
||
const domain = getDomainNameByEmail(email); | ||
|
||
return `${this.TWENTY_ICONS_BASE_URL}/${domain}`; |
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.
logic: URL should be URI encoded to handle special characters in domain names.
const logoUrl = | ||
await this.companyEnrichmentService.getCompanyLogoUrl(email); |
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.
style: consider adding error handling here in case the logo fetch fails to prevent workspace creation from failing
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
Hey, this is a great PR. What you did was perfectly in line with the initial issue, but I kind of changed my mind halfway while reviewing your code. It might be over-engineering to introduce this enrichment service now and therefore I removed the company service to simplify things |
Thanks for the review @FelixMalfait! I understand the decision to simplify the implementation. I've tested your simplified version and confirmed the logo appears correctly during sign-up. Is there anything else needed from my side, or is this ready to be merged? Let me know if there's anything else you'd like me to help with! 👍 |
Thanks for testing it again @samyakpiya Feel free to DM me on Discord, happy to point you to different things we're working on! |
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.
Great work!
Thanks Felix! Really enjoyed working on this feature. I've sent you a DM on Discord to discuss other areas where I can contribute. Looking forward to working on more issues! 🚀 |
Closes #9151
Description
This PR automatically sets a workspace's logo based on the user's work email domain during signup. When a user creates a new workspace using their work email (e.g., @airbnb.com), the system will fetch and set their company logo from twenty-icons.com as the default workspace logo.
Implementation Details
CompanyEnrichmentService
to handle company-related data enrichmentTesting
twenty-9151.mp4
I tested the following scenarios:
Technical Notes
isWorkEmail
utility