woowabros / nestjs-library-crud

Automatically generate CRUD Rest API based on NestJS and TypeOrm
https://www.npmjs.com/package/@nestjs-library/crud
MIT License
187 stars 34 forks source link

[Regression]: 0.11.0 , nested entity update support broke readMany #463

Closed chihabhajji closed 6 months ago

chihabhajji commented 7 months ago

Hello, as always, thank you for the amazing lib,

Last week's feature #389 to be able to support table inheritence has rendered readMany and readOne to break, the lib wasnt able to read the metadata of the entity thus making it break them, reverted to 0.10.2 and everything works...

I can provider a minimal reproduction repo if necessary

jiho-kr commented 7 months ago

hi @chihabhajji

I'll check it first when i have some time. Can you give me more information so I can check?

chihabhajji commented 7 months ago

yes, ill start working on a minimal reproduction after i finish work

chihabhajji commented 6 months ago

https://github.com/chihabhajji/woowabros-reproduction

image

swagger available at /docs

chihabhajji commented 6 months ago

side note, the same works if i down grade to 0.10 but then i lose the ability to fetch nested entities edit: this repo can also serve as a test for all the new features since it also has table inheritance

jiho-kr commented 6 months ago

Okay, I`ll look into the information you gave.

jiho-kr commented 6 months ago

I'm trying to look at this, but I got covid-19. i waiting for condition to recover.

chihabhajji commented 6 months ago

no worries, get well soon, your health is the priority

chihabhajji commented 6 months ago

closing, fixed by using class-validator on my entities instead of @nestjs/class-validator, kindly add a PSA in the readme for new users

chihabhajji commented 6 months ago

my bad, i down graded to 0.10.2 when i did that so it worked

jiho-kr commented 6 months ago

i fixed it (https://github.com/woowabros/nestjs-library-crud/pull/470)

chihabhajji commented 6 months ago

you're the best, also i hope your health is better because i also caught the rona 🤦

chihabhajji commented 6 months ago

i tried the branch, the regression seems to be fixed, but extending a base entity seems to break (the base entity has created_at deleted_at and updated_at timestamps) image

jiho-kr commented 6 months ago

It seems like failure for softDelete condition.

Would you like to check if you have defined @DeleteDateColumn on entity?

chihabhajji commented 6 months ago

`ts export abstract class BaseFinvoEntity extends BaseEntity { @IsDateString(undefined, { groups: [GROUP.READ_MANY, GROUP.READ_ONE], }) @IsOptional({ groups: [GROUP.READ_MANY, GROUP.READ_ONE] }) @Type(() => Date) @CreateDateColumn() @ApiProperty({ type: Date, format: 'date-time' }) declare readonly createdAt?: Date;

@IsDateString(undefined, {
    groups: [GROUP.READ_MANY, GROUP.READ_ONE],
})
@IsOptional({ groups: [GROUP.READ_MANY, GROUP.READ_ONE] })
@UpdateDateColumn()
@Type(() => Date)
@ApiProperty({ type: Date, format: 'date-time' })
declare readonly updatedAt?: Date;

@Exclude()
@IsOptional({ always: true })
@DeleteDateColumn({
    type: 'timestamp',
    nullable: true,
})
@IsDateString(undefined, {
    groups: [GROUP.READ_MANY, GROUP.READ_ONE, GROUP.SEARCH, GROUP.PARAMS],
})
@Type(() => Date)
@ApiHideProperty()
declare readonly deletedAt?: Date;

} ` yes, it is defined, also, i dont know if this was the behavior before, but update seems to be inserting a new document

jiho-kr commented 6 months ago

WHERE "ClientEntity"."deleted_at" IS NULL is generated by typeorm (withDeleted option).

Would you like to check it without CRUD library?

this.repository.findOne({ withDeleted: true });
jiho-kr commented 6 months ago

https://github.com/woowabros/nestjs-library-crud/releases/tag/0.11.1

chihabhajji commented 6 months ago

🙌🙌