-
Notifications
You must be signed in to change notification settings - Fork 18
Conversation
Don't merge this request since this has some breaking changes |
This pull request introduces 4 alerts when merging 37e18fc into 6b1b7a0 - view on LGTM.com new alerts:
|
Create the issue number and tad it with the template. |
This pull request introduces 7 alerts when merging 25f35c0 into 9a5008d - view on LGTM.com new alerts:
|
@Entity('users') | ||
@Unique(['email']) | ||
export default class Users { | ||
export class Users { | ||
@PrimaryGeneratedColumn() |
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.
Class name should be User
cascade: ['update'], | ||
}) | ||
@JoinColumn({ name: 'id' }) | ||
roleUsers: RoleUsers[]; |
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.
This roleUsers property is a single item. So change it ti roleUser
import { Roles } from './index'; | ||
|
||
@Entity('role_users') | ||
export class RoleUsers { |
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.
Class name should be RoleUser
|
||
@ManyToOne((type) => Roles, (roles) => roles.roleUsers) | ||
@JoinColumn({ name: 'roleId' }) | ||
roles: Roles; |
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.
Change property roles to role
import { Roles } from './index'; | ||
|
||
@Entity('role_actions') | ||
export class RoleActions { |
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.
RoleAction instead of RoleActions
import { IsString, IsOptional } from 'class-validator'; | ||
|
||
@Entity('roles') | ||
export class Roles { |
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.
Role instead of Roles
async execute(command: AuthCommand) { | ||
const { user, type } = command; | ||
const role = await this.roleService.getRoleByName( | ||
'SuperAdmin', |
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.
Please remove SuperAdmin
generationStrategy: 'increment', | ||
}, | ||
{ | ||
name: 'name', |
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.
name should be unique
columnNames: ['createdBy'], | ||
referencedColumnNames: ['id'], | ||
referencedTableName: 'users', | ||
onDelete: 'CASCADE', |
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.
onDelete: 'CASCADE', | |
onDelete: 'CASCADE', - better avoid cascade delete, instead use soft delete |
}, | ||
{ | ||
name: 'createdAt', | ||
type: 'timestamp', |
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.
type: 'timestamp', | |
type: 'timestamp', - better use bigint instead of timestamp. Applicable to all timestamp type |
SonarCloud Quality Gate failed.
|
This pull request introduces 7 alerts when merging c89fd2f into 6349432 - view on LGTM.com new alerts:
|
Role-based access control
CQRS implementation