-
Notifications
You must be signed in to change notification settings - Fork 9
Add tenant user assignment #4947
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
base: feature/tenants
Are you sure you want to change the base?
Conversation
6306f74 to
6e55782
Compare
6e55782 to
3cfe27b
Compare
| imports: [MikroOrmModule.forFeature([Tenant])], | ||
| providers: [TenantResolver], | ||
| imports: [MikroOrmModule.forFeature([Tenant, TenantUser])], | ||
| providers: [TenantResolver, TenantUserResolver, CustomTenantUserResolver, UserService], |
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.
The TenantsModule shouldn't provide UserService, because it isn't part of this module.
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.
Do you know how I can do that ?
Problem:
The auth.module is configurable with forRoot. So a simple import in the imports array does not work.
Current solutions I can think of:
- making AuthModule global
- Making a UserModule which exports the UserService and imports it in AuthModule and Tenants.module (not sure if this would work with the auth.module)
Or like that, found that in a project:
export class XXXModule {
private static instance: DynamicModule;
static getModule(): DynamicModule {
if (!this.instance) throw new Error(`XXXModule not initialized. Call XXXModule.forRoot(config) first.`);
return this.instance;
}
static forRoot(): DynamicModule {
if (!this.instance) {
this.instance = {
module: XXXModule,
imports: [....],
providers: [...],
exports: [...],
};
}
return this.instance;
}
}and for the import:
@Module({
imports: [
forwardRef(() => XXXModule.getModule()),
],
})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.
You could move it to providers here: https://github.com/vivid-planet/comet/blob/feature/tenants/demo-saas/api/src/auth/auth.module.ts#L12. Then it should also be used for the dynamic module. Could you try that?
| @ObjectType() | ||
| @CrudGenerator({ targetDirectory: `${__dirname}/../generated/`, requiredPermission: ["tenantAdministration"], update: false, create: false }) | ||
| @Unique({ properties: ["tenant", "userId"] }) | ||
| export class TenantUser extends BaseEntity { |
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.
@johnnyomair would you call this table
- UserToTenant?
- TenantToUser?
or is TenantUser fine?
it's only an assignment table (for projects there might be a role as well here)
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.
I'd go with TenantToUser or UserToTenant. At least that's what we did for ProductToTag here:
| export class ProductToTag extends BaseEntity { |
| const assignTenantUserQuery = gql` | ||
| mutation AssignTenantUser($tenant: ID!, $userId: String!) { | ||
| assignTenantUser(tenant: $tenant, userId: $userId) { | ||
| const assignTenantUsersQuery = gql` |
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.
Naming: assignTenantUsersMutation.
Description
Adding simple assignment of users to tenants. This PR only handles assignment, but not the actual handling that users see what they should see.
Acceptance criteria
Screenshots/screencasts
Screen.Recording.2025-12-16.at.14.50.38.mov
Further information