fix: resolve issue with find with relations returns soft-deleted entities

[luiseariass]

Description of change

This pull request includes fix for soft delete behavior when including soft deletes relationships.
fix: resolve issue with find with relations returns soft-deleted entities #6265

Pull-Request Checklist

• Code is up-to-date with the master branch
• npm run lint passes with this change
• npm run test passes with this change
• This pull request links relevant issues as Fixes #6265
• There are new or updated unit tests validating the change
• Documentation has been updated to reflect this change
• The new commits follow conventions explained in CONTRIBUTING.md

[nebkat]

Ideally squash the two commits into one now but otherwise looks good to me 🎉

[nebkat]

Actually you should also add tests to verify the functionality, there should already be some examples testing non relation soft deleted items that you can mostly copy paste.

[nebkat]

Tests look reasonable. Strictly this should also be a "feat:" rather than "fix:" but not so important. One thing that will be an issue is that this is essentially a breaking change for current users so not sure how to work around that.

[caiomestres]

Tests look reasonable. Strictly this should also be a "feat:" rather than "fix:" but not so important. One thing that will be an issue is that this is essentially a breaking change for current users so not sure how to work around that.

A bool property for nested soft deletes wouldn't work @nebkat ?

something like
userRepository.find({ relations: ["profile", "photos", "videos"], nestedSoftDelete: true });

default would be false, that way maintaining backward compatibility

[nebkat]

That works but it must be covered for both find and query builder, and it must also match the current naming (withDeleted I think).

[luiseariass]

I propose we add a new flag withDeletedRelations: boolean to the @DeleteDateColumn decorator, with a default value of false. This way we don't affect current integrations.
The option withDeleted: true should overwrite withDeletedRelations: true behavior.

[nebkat]

I don't think it should be on @DeleteDateColumn since that applies to the entity being deleted whether it is a relation or not. Whether deleted relations are included should be down to each query, and in future the default should switch to not including soft deleted relations.

[luiseariass]

I was just looking for a way not to affect the current behavior, but I fervently believe that the behavior of the relationships should be the same as the main entity, we can work on updating the documentation of when the join is used and when it is used the soft delete

[nebkat]

Personally I think there should just be a new major release with the breaking change but that doesn't seem to be planned so we have to work around it unfortunately.

[RAllner]

Hey guys. Are there any news on this PR? Looking forward to use it, since I could get rid of a bigger list of workarounds.

[caiomestres]

Hey guys. Are there any news on this PR? Looking forward to use it, since I could get rid of a bigger list of workarounds.

Waiting for @nebkat to deliberate about the best approach...

[brennedith]

@nebkat, following on @caiomestres' train of thought:

Find

const firstUser = await User.findOne({
  where: {
    id: 1,
  },
  withDeletedRelations: false,
});

QueryBuilder

const firstUser = await connection
  .getRepository(User)
  .createQueryBuilder('user')
  .withDeletedRelations(false)
  .where('user.id = :id', { id: 1 })
  .getOne();

Is this something that can be achieved? If so, can you suggest to us where to start?

Thanks in advance

[nebkat]

Yes that looks like the ideal solution.

[pleerock]

I don't think we need extra flag at least because it can raise questions like "how to do it with deleted relations but without deleted relations of relations".

Also I don't think we need a backward, because without this change current behaviour looks like a bug.

Thank you for contribution, I'm going to merge this one.

[luiseariass]

when is the next release @pleerock?

[pleerock]

Available in the latest 0.2.31 version.

[stovberpv]

Seems to work but withDeleted: true doesn't.

When i want to query deleted rows and use:

builder.withDeleted();

the query builds as follows:

SELECT *
  FROM plan
 INNER JOIN blank ON blank.id = plan.blank_id
                 AND (blank.deleted_at IS NULL)            <-- still here

[luiseariass]

Seems to work but withDeleted: true doesn't.

When i want to query deleted rows and use:

builder.withDeleted();

the query builds as follows:

SELECT *
  FROM plan
 INNER JOIN blank ON blank.id = plan.blank_id
                 AND (blank.deleted_at IS NULL)            <-- still here

What query in typeom you use?

[stovberpv]

@luiseariass i have 2 entities with one-to-one relation

@Entity()
class Plan {
  @OneToOne(() => Blank, blank => blank.id)
  @JoinColumn({ name: 'blank_id' })
  blank: Blank;
}

@Entity()
class Blank {
  @OneToOne(() => Plan, plan => plan.blank)
  plan: Plan;

  @DeleteDateColumn()
  deleted_at: Date;
}

And query builder looks like this

const res = await this.manager
  .getRepository(Plan)
  .createQueryBuilder('plan')
  .innerJoinAndSelect(`plan.blank`, 'blank')
  .withDeleted()
  .getMany();

[stovberpv]

i figure it out, the withDeleted() needs to be added before the innerJoinAndSelect;

[jdnielss]

thank you for fix the bug ////

[br-dr3]

Any suggestion on how to bring relation deleted without flag? Many soft deletes just exist to continue showing some text of a deleted entity..