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

feat(*): add subdomain management for multiworkpsace #8680

Conversation

AMoreaux
Copy link
Contributor

@AMoreaux AMoreaux commented Nov 22, 2024

No description provided.

Reorganized the sign-in-up code for better readability and maintainability. Introduced validation for workspace invitations and moved some utility functions to dedicated methods. Added a migration for new auth provider columns in the workspace entity.
Removed unused `EnvironmentService` and redundant `code` property in exceptions for clarity. Simplified email validation in `workspace-invitation.service.ts`. Refactored tests to remove unnecessary `targetWorkspaceSubdomain` property.
Removed redundant services from auth and core modules to streamline the code. Added debug log statements in sign-in-up service and skipped certain test cases temporarily for evaluation.
Enhanced Logo component to accept dynamic size and an optional twenty logo display. This allows for more flexible and customizable logo dimensions, and the inclusion of Twenty's logo can now be controlled via props.
Changed StyledMainLogo type to use Pick for logo prop. This ensures only the required 'logo' property is passed, improving type safety and code clarity.
Consolidated the size prop inside the StyledMainLogo component call. This removes redundancy and aligns with component usage patterns in the codebase.
Eliminate debug logging for workspace invite tokens and validation. This cleanup improves readability and reduces console clutter during runtime.
…t-auth-methods-by-workspace

# Conflicts:
#	packages/twenty-front/src/modules/auth/sign-in-up/components/SignInUpForm.tsx
#	packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUpForm.ts
Remove unused imports from SignInUpGlobalScopeForm.tsx to improve code readability and reduce clutter. This change simplifies the module by eliminating unnecessary dependencies.
Suppress eslint warnings for console.error in error handling. This ensures cleaner code and maintains log output for better debugging.
Correct the expected state in useAuth test to reflect the accurate status of authProviders. This includes setting 'google' and 'password' to true, and initializing 'sso' as an empty array.
Updated the Logo component to accept primary and secondary logos, replacing the workspaceLogo prop. Adjusted the getImageAbsoluteURI utility function for type safety and URL handling.
Use the new `Spacing` type to ensure consistent spacing units across the theme and Logo components. This change streamlines unit handling and improves type safety throughout the codebase.
…ninup

# Conflicts:
#	packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts
…lect-auth-methods-by-workspace

# Conflicts:
#	packages/twenty-front/src/generated/graphql.tsx
Updated SSO identity provider type and added types for workspace queries. Removed deprecated mutations and queries related to SSO identity providers and JWT generation. Added new fields and types to support improved workspace querying and switching capabilities.
refactor(auth): thanks greptile
```
…facto/improve-signinup

# Conflicts:
#	packages/twenty-server/src/engine/core-modules/auth/dto/available-workspaces.output.ts
#	packages/twenty-server/src/engine/core-modules/auth/services/auth.service.ts
#	packages/twenty-server/src/engine/core-modules/auth/services/sign-in-up.service.ts
#	packages/twenty-server/src/engine/core-modules/user/services/user.service.ts
#	packages/twenty-server/src/engine/core-modules/workspace/services/workspace.service.ts
Removed the redundant `code` property from WorkspaceInvitationException. Adjusted the import order in WorkspaceResolver to comply with coding standards.
Move import of workspaceValidator to maintain consistent import structure. This enhances readability and aligns with the project's import organization standards.
Removed unused imports from SignInUpGlobalScopeForm and SettingsSecurityOptionsList components. This cleanup improves code readability and reduces potential maintenance overhead.
Replace user object with user ID in saveDefaultWorkspace method call to fix incorrect parameter type. This ensures that the function operates as expected and prevents potential errors in default workspace assignment.
Added `GET_PUBLIC_WORKSPACE_DATA_BY_SUBDOMAIN` query mock handler to return mock workspace data. Also updated the button text check in SignInUp stories for consistency.
Deleted outdated user and workspace validation logic, and removed legacy verify-auth controller. Updated Microsoft and Google auth controllers to handle errors correctly and redirect. Added subdomain handling in various components to support multi-workspace setup.
const currentWorkspace = useRecoilValue(currentWorkspaceState);
const isMultiWorkspaceEnabled = useRecoilValue(isMultiWorkspaceEnabledState);
const { enqueueSnackBar } = useSnackBar();
const { redirectToHome, redirectToWorkspace } = useUrlManager();

const switchWorkspace = async (workspaceId: string) => {
Copy link
Member

Choose a reason for hiding this comment

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

make it a recoilcallback

isDefined(workspacePublicData?.subdomain) &&
workspacePublicData.subdomain !== getWorkspaceSubdomain
) {
redirectToWorkspace(workspacePublicData.subdomain);
Copy link
Member

Choose a reason for hiding this comment

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

double check, I think it's should not happen

@@ -133,7 +137,24 @@ export class WorkspaceInvitationService {
);
}

// QUERY METHODS
async findInvitationByWorkspaceSubdomainAndUserEmail({
subdomain,
Copy link
Member

Choose a reason for hiding this comment

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

Overall, I feel that subdomain / domain logic should stay in DomainManager module and that we should keep passing workspaceId in the rest of the code

private readonly urlManagerService: UrlManagerService,
) {}

async getAuthProvidersByWorkspaceId(workspaceId: string) {
Copy link
Member

Choose a reason for hiding this comment

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

I would create a module in auth/workspace-auth-provider.module.ts that would only have this

Copy link
Contributor Author

@AMoreaux AMoreaux Dec 3, 2024

Choose a reason for hiding this comment

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

I move it into an utils instead of a module. Is it ok?

Refactored the getDomainNameByEmail function into its own file for
better separation of concerns and reusability. The addition of unit
tests ensures that the function behaves correctly, improving code
reliability. Also corrected a typo in the self-hosting documentation.
… manager usage

Remove the unused WorkspaceGettersService to simplify workspace
handling. Replace UrlManagerService with DomainManagerService across
multiple modules for improved domain management. Ensure proper retrieval
of auth providers by workspace encapsulated in a new utility function
for better maintainability and clarity.
@charlesBochet charlesBochet merged commit 844d218 into feat/add-is-multi-workspace-flag Dec 3, 2024
6 of 11 checks passed
@charlesBochet charlesBochet deleted the feat/add-subdomain-management-for-multiworpsace branch December 3, 2024 17:47
Copy link

github-actions bot commented Dec 3, 2024

Thanks @AMoreaux for your contribution!
This marks your 15th PR on the repo. You're top 2% of all our contributors 🎉
See contributor page - Share on LinkedIn - Share on Twitter

Contributions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants