crud icon indicating copy to clipboard operation
crud copied to clipboard

eager relation load breaks foreign key update

Open Diluka opened this issue 5 years ago • 15 comments

When relation set to eager loading, update a foreign key will be failed.

partial entity

export class Book {
  catId:number;

  cat:Cat;
}

When update catId it will load cat to override the update.

https://github.com/nestjsx/crud/blob/d27a1d69238bd35699c590a1eae4a517063c5c59/packages/crud-typeorm/src/typeorm-crud.service.ts#L122-L136

cat in found and only catId in dto The result will be

{
    "catId": "new value",
    "cat": {"id":"old value"}
}

typeorm will use object instead of id

Diluka avatar Jul 19 '19 09:07 Diluka

I can confirm that I have the same issue. I see that there is a separate branch for the fix... what is the status on the fix?

alexmantaut avatar Nov 12 '19 08:11 alexmantaut

@alexmantaut thanks for the attention. you can see after the refactor this issue seems gone. you can try the next version or just waiting for the next release.

btw, @zMotivat0r When is it released?

Diluka avatar Nov 12 '19 09:11 Diluka

Just tested with next and it seems to work.

Looking forward for the next release.

alexmantaut avatar Nov 12 '19 22:11 alexmantaut

I'm having the same issue, anyway to hack this until it will be fixed?

eranshmil avatar Jan 09 '20 16:01 eranshmil

same issue, It's not fix for now.

kumkao avatar Jul 07 '20 15:07 kumkao

oh, it's back again https://github.com/nestjsx/crud/blob/644acfdaa2c986cd63ad368fa7b3a27b6482002d/packages/crud-typeorm/src/typeorm-crud.service.ts#L181-L182

don't need ...found here

https://typeorm.io/#/repository-api

save - Saves a given entity or array of entities. If the entity already exist in the database, it is updated. If the entity does not exist in the database, it is inserted. It saves all given entities in a single transaction (in the case of entity, manager is not transactional). Also supports partial updating since all undefined properties are skipped. Returns the saved entity/entities.

recently, I have a lot of work to do. It would be great if someone could help to fix.

Diluka avatar Jul 08 '20 02:07 Diluka

I got the same issue and solved it by just override it. so it could update foreign key value.

  @Override()
  createOne(
    @ParsedRequest() req: CrudRequest,
    @ParsedBody() dto: Hero,
  ) {
    return this.base.createOneBase(req, dto);
  }

where dto: Hero for your entity. In my case is customersEntity.

hsuanyi-chou avatar Jul 16 '20 14:07 hsuanyi-chou

@hsuanyi-chou isn't createOne for creating a new entity? I have also tried with updateOne, but still having issue.

gamingumar avatar Sep 10 '20 16:09 gamingumar

sorry, my mistake! I copied the example code but forgot to replace createOne to updateOne.

here is my code. backend nestJs:

    @Override()
    updateOne(@ParsedRequest() req: CrudRequest, @ParsedBody() dto: CustomersEntity) {
      return this.base.createOneBase(req, dto); // my mistake! forgot to replace `createOne` to `updateOne`
    }

in frontend, still using patch to update my data with id in params. (in case of this issue being fixed, just remove the backend parts.)

it doesn't insert a new data, but update the data.

frontend angular:

const body = {
      shopId: this.customerForm.get('shopId').value,  // the foreign key
      name: this.customerForm.get('name').value.trim(),
      birth: this.customerForm.get('birth').value,
      phone: this.customerForm.get('phone').value || ''
}

this.http.patch<Customer>(`api/customers/${this.customerForm.get('id').value}`, body).subscribe(res => { ... });

I don't know why.😨

hsuanyi-chou avatar Sep 11 '20 14:09 hsuanyi-chou

I created a PR a couple weeks back that should fix this issue, see #622 - I wanted to ask whether there is anything I could help with to get this PR merged / released?

Its a tough year and time is probably short in favor of more important things (health and keeping your sanity in 2020), so let me know what can be done to help.

dschoeni avatar Dec 16 '20 17:12 dschoeni

I'd like to re-raise this issue @zMotivat0r - Does the PR I submitted look good? I currently need to maintain a fork of the project in our application because this breaks functionality with PATCH.

dschoeni avatar Apr 06 '21 19:04 dschoeni

a temporary solution:

@Override('updateOneBase')
    updateOne(@ParsedRequest() req: CrudRequest, @ParsedBody() dto) {
        const newReq = _.cloneDeep(req)
        newReq.options.query.join = {}
        return this.service.updateOne(newReq, dto)
 }

I use lodash to copy req object with out reference if you directly change req , it affect other request such as getOne or getMany

Farzad6049 avatar Apr 20 '21 06:04 Farzad6049

Hi all, I'm having the same issue, whether to wait for a fix?

cyc1e avatar Sep 16 '21 14:09 cyc1e

import { cloneDeep } from 'lodash'; /**

  • At the moment we have an issue while trying PUT or PATCH foreign keys with ?join=relation.
  • So if we have relationID_1 = 1 and relationID_2 = 2 and will try to replace them to some different one the change wouldn't happen,
  • bc the current flow of TypeOrmCrudFramework:
    1. search an exemplar with current params
    1. it returns the current exemplar with included join relations
    1. save the full entity with overriding params and if we put included child relations
    • then it would override the previous ones, bc child relations were got from point 1)
  • that's why we get this strange behavior decorator */
export function FixUpdateReplaceOne<T extends { new (...args: any[]): any }>(
  target: T,
) {
  const decoratedClass = class extends target {
    constructor(...args: any[]) {
      super(...args);
    }

    async _callFunctionWithOverrideParams(
      args: any[],
      func: string,
    ): Promise<Array<any>> {
      const id = args[0].parsed.paramsFilter.find(
        (params) => params.field === args[0].options.params.id.field,
      );
      const newArgs = cloneDeep(args);
      newArgs[0].parsed.join = [];
      Object.keys(newArgs[0].options.query.join).forEach((key) => {
        if (newArgs[0].options.query.join[key].eager === true) {
          newArgs[0].options.query.join[key].eager = false;
        }
      });
      newArgs[1] = { ...newArgs[1], [id?.field]: id?.value };
      await super[func].call(this, ...newArgs);
      return super.getOne({
        ...args[0],
      });
    }

    async updateOne(...args: any[]) {
      return await this._callFunctionWithOverrideParams.apply(this, [
        args,
        'updateOne',
      ]);
    }

    async replaceOne(...args: any[]) {
      return await this._callFunctionWithOverrideParams.apply(this, [
        args,
        'replaceOne',
      ]);
    }
  };
  Object.defineProperty(decoratedClass, 'name', {
    value: target.name,
  });
  return decoratedClass;
}

Then in your service

@Injectable()
@FixUpdateReplaceOne
export class YourService extends TypeOrmCrudService<YourEntity> {
  constructor(
    @InjectRepository(YourEntity)
    public repo: Repository<YourEntity>,
  ) {
    super(repo);
  }
}

4ant0m avatar Mar 14 '23 14:03 4ant0m

It seems being due to a strange TypeORM behaviour. Issue have been raised: https://github.com/typeorm/typeorm/issues/10136

flchaux avatar Aug 28 '23 13:08 flchaux