[Feature Request] Simple way to extend/override TypeOrmCrudService

[alexmantaut]

Hi,

I am trying to create a custom TypeOrmCrudService for my application, overriding the behaviour of the different methods...

To do this, I am basing my override methods on the current implementations on TypeOrmCrudService. Thing is, there is a lot of really useful methods on the base class (ex getOneOrFail(), prepareEntityBeforeSave(), etc) , but I cannot use them on my override method, because they have private visibility...

Would it be possible to make those methods protected instead, to be able to call them from derived classes? Or do you feel this would make the interface harder to maintain? If not, what would be a good way to be able to use those methods?

Thanks,

[RDeluxe]

Agreed, that's not a big change but that would make things easier.

[jbjhjm]

Yes please. Private methods are so annoying for those who needs to customize functionality. +1 for protected methods!

[mbaroukh]

Hi. I'm new to nest. I use this module because it allow to quickly bootstrap a project (coupled with react-admin). Thanks for it :). But I also had this problem. For example, all my entities have a modificationDate field and I want to update them before saving. Do I miss a method to actually do it easily ?

For now, I made de TypeORM Repository wrapper which use a method beforeSave called to wrap entity before each save : https://gist.github.com/mbaroukh/841618b97fce7aa29c08a0bd2daff35b

It seems to work for my need. I use it this way :

@Injectable()
export class ClientsService extends TypeOrmCrudService<Client> {
  constructor(
    @InjectRepository(Client) private readonly clientRepository: Repository<Client>,
  ) { 
    super(new RepositoryWrapper(clientRepository, e => {
      return {
        ...e,
        modificationDate: new Date()
      }
    }))
  }
}

But I can't believe there is no other way to do this.

[jbjhjm]

@mbaroukh No need to set this up via a service, typeORM offers this out of the box:

export class MyEntity {
  // ... 
  // date column which will be static after INSERT
  @CreateDateColumn({ name: 'created_at', nullable: false })
  createdAt: Date;

  // date column which will be updated on every UPDATE
  @UpdateDateColumn({ name: 'updated_at', nullable: true })
  updatedAt: Date;

  // if you need versioning, this will be incremented on every UPDATE
  @VersionColumn() version: number;

}

[mbaroukh]

very good point @jbjhjm , thank you ! But I will continue my way until prepareEntityBeforeSave() is protected for now because modification date is not the only column. I have other fields to consolidate on save.

Thanks !

[jbjhjm]

@mbaroukh there's a solution for this too:

export class MyEntity {

  @BeforeUpdate()
  prepareTheEntity() {
    // this code will be executed before running queries to update the db table.
    // There are others like @BeforeInsert(), @AfterUpdate() etcpp.
    // in your case just do the following here to achieve the same:
    this.modificationDate = new Date()
  }

}

[mbaroukh]

Hi @jbjhjm. I already tried this. But the event is not fired. I suppose I reach this issue and maybe it has not been published yet in stable version : https://github.com/nestjsx/crud/issues/51

I tried to upgrade to 4.3.0-alpha.0 (upgrading crud, crud-typeorm and crud-request but I have this typescript error :

node_modules/@nestjsx/crud-request/lib/types/request-query.types.d.ts:62:5 - error TS2411: Property '$and' of type 'undefined' is not assignable to string index type 'string | number | boolean | SFieldOperator | (SFields | SConditionAND)[]'.

62     $and?: never;

But I must says that I'm not fan of this method anyway. I think that every interaction with entity must be centralized in service, dao or repository. This is why being able to override prepareEntityBeforeSave would be a greater option imho.

[jbjhjm]

@mbaroukh in 4.3.0-alpha0 this should work fine, I'm using it in my repo and had no problems at all. What does the code of your entity looks like? I guess you made sure the error's gone when you don't use one of the event decorators.

Best practice depends on what you need to do I think. If it's transforming data etc my opinion is it's good to do this inside the entity as it allows you to use it easily and not need to care about internals just the final entity with transformations etc applied.

If you want to decouple this typeORM offers the same events to be placed in a special class named Subscriber which is essentially like a service allowing the same event listeners to be specified as methods. Be aware though that it's default behavior is to subscribe to all events of a CONNECTION, not an entity.

Currently I'm using such subscribers like a "plug-in-solution" for typeORM to provide custom db functionality based on attached Decorator metadata. I think it's the best / most reusable method that's available right now.

---

Nevertheless, I agree 100% it would be beneficial to use protected methods to allow for customization without headaches :)

[mbaroukh]

@jbjhjm in 4.3.0-alpha.0, I only updated crud-xxx packages and I have this error whenever I use or not the event decorators. I saw this this morning and could not investigate more. I'm surprised it works for you :(. I tried with TS 3.6.3, 3.6.4 and 3.7.1.

You are right that there are multiple solutions and that's what is nice :). My bias is that I'm coming from Java world where I uses to have simple objects for entities (POJO). I'll look at those subscribers, thanks.

[jbjhjm]

@mbaroukh Hmm I'm using an older typescript / nestjs version. Possibly it's broken with most recent versions. Should plan in some time to do updates soon. My setup is:

{
    "@nestjs/common": "^6.5.3",
    "@nestjs/core": "^6.5.3",
    "@nestjs/platform-express": "^6.5.3",
    "@nestjs/swagger": "^3.1.0",
    "@nestjs/typeorm": "^6.1.3",
    "@nestjsx/crud": "^4.3.0-alpha.0",
    "@nestjsx/crud-typeorm": "^4.2.0",
    "typescript": "^3.5.3",
}

[mbaroukh]

well, this error disappear if I use "strict: false" in tsconfig.json. But I don't wan't to do that. Nobody should disable strict mode :(.

[alexmantaut]

Just created a PR for the issue with the visibility. I was conservative and only changed the visibility of 3 methods, as I was unsure whether it was a good idea to change visibility for other methods...

Please let me know if you want me to change the visibility of other methods in the class as well.