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

Separate update event into the update, soft remove and restore events #8398

Closed
spotykatch opened this issue Nov 21, 2021 · 5 comments · Fixed by #8403
Closed

Separate update event into the update, soft remove and restore events #8398

spotykatch opened this issue Nov 21, 2021 · 5 comments · Fixed by #8403

Comments

@spotykatch
Copy link
Contributor

spotykatch commented Nov 21, 2021

Feature Description

Separate update event into the update, soft remove and restore events.

The Problem

Soft remove and recover fire update event, so you have to send some data for subscribers to understand it is not an update. While listeners can't get the difference at all. So you have to do it like so

await repository.softRemove(entity, { data: { action: 'soft-remove' } });
afterUpdate(event: UpdateEvent<Entity>): void {
  const { entity, queryRunner: { data: { action } } } = event;
  if ( action === 'soft-remove') {
    // do stuff
  }
}

course i.e.

if (this.softRemoveSubjects.length)
  this.softRemoveSubjects.forEach(subject => this.queryRunner.broadcaster.broadcastAfterUpdateEvent(result, subject.metadata, subject.entity!, subject.databaseEntity, subject.diffColumns, subject.diffRelations));
if (this.recoverSubjects.length)
  this.recoverSubjects.forEach(subject => this.queryRunner.broadcaster.broadcastAfterUpdateEvent(result, subject.metadata, subject.entity!, subject.databaseEntity, subject.diffColumns, subject.diffRelations));

The Solution

Update events should be separated into the update, soft remove, and restore.

if (this.softRemoveSubjects.length)
  this.softRemoveSubjects.forEach(subject => this.queryRunner.broadcaster.broadcastAfterSoftRemoveEvent(result, subject.metadata, subject.entity!, subject.databaseEntity, subject.diffColumns, subject.diffRelations));
if (this.recoverSubjects.length)
  this.recoverSubjects.forEach(subject => this.queryRunner.broadcaster.broadcastAfterRecoverEvent(result, subject.metadata, subject.entity!, subject.databaseEntity, subject.diffColumns, subject.diffRelations));

Considered Alternatives

OR some data should be sent to listeners and subscribers highlighting it's not an update.

Additional Context

So the problem is back compatibility. There are some repositories and tons of users that rely on the current functionality. I think it should be discussed.
Maybe there should be an option inside ormconfig.json

{
  "separateUpdateEvents": true
}

Are you willing to resolve this issue by submitting a Pull Request?

  • ✖️ Yes, I have the time, and I know how to start.
  • ✅ Yes, I have the time, but I don't know how to start. I would need guidance.
  • ✖️ No, I don’t have the time, but I can support (using donations) development.
  • ✖️ No, I don’t have the time and I’m okay to wait for the community / maintainers to resolve this issue.
@spotykatch
Copy link
Contributor Author

LOL, did think that it will take a month or even more. Please, make a review. I'm not sure in some cases.

@spotykatch
Copy link
Contributor Author

Need some help with Chinese docs.

pleerock pushed a commit that referenced this issue Feb 15, 2022
…8403)

Closes: #8398

BREAKING CHANGE: update listeners and subscriber no longer triggered by soft-remove and recover
@shenX-2021
Copy link
Contributor

@pleerock When the BeforeSoftRemove feature will release?

@spotykatch
Copy link
Contributor Author

It's already released in 0.2.42.

@shenX-2021
Copy link
Contributor

@spotykatch thanks!

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

Successfully merging a pull request may close this issue.

2 participants