Skip to content
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

eager relation load breaks foreign key update #177

Open
Diluka opened this issue Jul 19, 2019 · 15 comments
Open

eager relation load breaks foreign key update #177

Diluka opened this issue Jul 19, 2019 · 15 comments

Comments

@Diluka
Copy link
Contributor

Diluka commented Jul 19, 2019

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.

public async updateOne(req: CrudRequest, dto: T): Promise<T> {
const found = await this.getOneOrFail(req);
/* istanbul ignore else */
if (
hasLength(req.parsed.paramsFilter) &&
!req.options.routes.updateOneBase.allowParamsOverride
) {
for (const filter of req.parsed.paramsFilter) {
dto[filter.field] = filter.value;
}
}
return this.repo.save<any>({ ...found, ...dto });
}

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 added a commit that referenced this issue Aug 1, 2019
removed redundant `getOneOrFail` in `updateOne`. and introduce `ensureExists` to check record if
exist

fix #177
@Diluka Diluka self-assigned this Aug 1, 2019
@alexmantaut
Copy link

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?

@Diluka
Copy link
Contributor Author

Diluka commented Nov 12, 2019

@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 Diluka closed this as completed Nov 12, 2019
@alexmantaut
Copy link

alexmantaut commented Nov 12, 2019

Just tested with next and it seems to work.

Looking forward for the next release.

@eranshmil
Copy link

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

@kumkao
Copy link

kumkao commented Jul 7, 2020

same issue, It's not fix for now.

@Diluka
Copy link
Contributor Author

Diluka commented Jul 8, 2020

oh, it's back again

? { ...found, ...dto, ...paramsFilters, ...req.parsed.authPersist }
: { ...found, ...dto, ...req.parsed.authPersist };

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 Diluka reopened this Jul 8, 2020
@Diluka Diluka removed their assignment Jul 8, 2020
@hsuanyi-chou
Copy link

hsuanyi-chou commented Jul 16, 2020

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.

@gamingumar
Copy link

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

@hsuanyi-chou
Copy link

hsuanyi-chou commented Sep 11, 2020

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.😨

@dschoeni
Copy link

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
Copy link

dschoeni commented Apr 6, 2021

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.

@Farzad6049
Copy link

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

@cyc1e
Copy link

cyc1e commented Sep 16, 2021

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

@4ant0m
Copy link

4ant0m commented Mar 14, 2023

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);
  }
}

@flchaux
Copy link

flchaux commented Aug 28, 2023

It seems being due to a strange TypeORM behaviour. Issue have been raised: typeorm/typeorm#10136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests