DoctrineBehaviors icon indicating copy to clipboard operation
DoctrineBehaviors copied to clipboard

`deleted_by_id` field stays `null` with Softdeletable

Open yahyaerturan opened this issue 3 years ago • 12 comments

When I create a User, created_by_id, created_at, updated_by_id, updated_at fills correctly. When I remove, deleted_at fills correctly. But deleted_by_id stays NULL in database. I tried to change order of traits and interfaces, but didn't help. I am not sure if it is a bug or simply I couldn't make it work.. But I decided to ask..

User Entity

# App\Entity\User
namespace App\Entity;

use App\Repository\UserRepository;
use Doctrine\ORM\Mapping as ORM;
use Knp\DoctrineBehaviors\Contract\Entity\BlameableInterface;
use Knp\DoctrineBehaviors\Contract\Entity\SoftDeletableInterface;
use Knp\DoctrineBehaviors\Contract\Entity\TimestampableInterface;
use Knp\DoctrineBehaviors\Model\Blameable\BlameableTrait;
use Knp\DoctrineBehaviors\Model\SoftDeletable\SoftDeletableTrait;
use Knp\DoctrineBehaviors\Model\Timestampable\TimestampableTrait;
use Symfony\Component\Security\Core\User\UserInterface;

/**
 * @ORM\Entity(repositoryClass=UserRepository::class)
 * @ORM\InheritanceType("JOINED")
 */
class User implements UserInterface, TimestampableInterface, BlameableInterface, SoftDeletableInterface
{
    use TimestampableTrait,
        BlameableTrait,
        SoftDeletableTrait;

User Controller

    public function delete(Request $request, User $row): Response
    {
        if ($this->isCsrfTokenValid('delete'.$row->getId(), $request->request->get('_token'))) {
            $entityManager = $this->getDoctrine()->getManager();

            try {
                $entityManager->remove($row);
                $entityManager->flush();
                $this->addFlash('success', 'app.messages.delete_success');
            } catch (\Exception $e) {
                $this->addFlash('error', 'app.messages.an_error_occurred');
            }
        }

        return $this->redirectToRoute('app_'.self::MODULE_ROUTE.'_index');
    }

yahyaerturan avatar Aug 15 '20 13:08 yahyaerturan

Im having the exact same issue, did you find a solution?

wesselvans avatar Oct 20 '20 21:10 wesselvans

I have the same problem with Symfony 5.1.8

zeridos avatar Nov 12 '20 12:11 zeridos

same problem with symfony 5.1.8 and easyadmin 3.1.6 (if that matters)

Adan0s avatar Nov 12 '20 13:11 Adan0s

Will this be fixed soon? 7 months later still same issue.

herbyxxx avatar Mar 04 '21 10:03 herbyxxx

Could be, it's up to. Would you like to contribute?

TomasVotruba avatar Mar 04 '21 14:03 TomasVotruba

Hello

same problem with Symfony 5.4

aasa avatar Jul 01 '21 10:07 aasa

We'll need a pull-request with faling test case for this bug.

TomasVotruba avatar Aug 09 '21 09:08 TomasVotruba

I don't a have a PR with failing test, but I do have a quick patch:


// BlameableEventSubscriber.php
public function preRemove(LifecycleEventArgs $lifecycleEventArgs): void
{
	...
	$this->getUnitOfWork()->scheduleExtraUpdate($entity, [
		self::DELETED_BY => [$oldValue, $user],
	]);
}

Inspired by code from SoftDeletableEventSubscriber.php. Seems to work now.

Gappa avatar Oct 04 '21 06:10 Gappa

I have the same problem and the solution of @Gappa works fine. I'm trying to create a test case that will trigger the BlameableEventSubscriber::preRemove so I can do a PR but haven't been able yet.

May be there are some edge cases that need the solution proposed by @Gappa and that's why not all people have this issue. In my case I'm deleting an entity from EasyAdmin and the deleted_by_id value is not set. But if I do a test with something like this:

$blameableEntity = new BlameableEntity();

$this->entityManager->persist($blameableEntity);
$this->entityManager->flush();

$this->entityManager->remove($blameableEntity);
$this->entityManager->flush();

$this->assertSame('user', $blameableEntity->getDeletedBy());
$this->assertNotNull($blameableEntity->getDeletedBy());

It works fine and I've checked that the code goes into the preRemove function without @Gappa change.

enekochan avatar Nov 25 '21 11:11 enekochan

@enekochan Hi, thanks for looking into this. Have you managed to create failing test case that mimics your use case? I can try to fix it, but I need the failing CI to know what needs to be made green :+1:

TomasVotruba avatar Jan 01 '22 17:01 TomasVotruba

II encountered this problem today. Thanks to your comments, I was able to create this EventSubscriber which solves this problem. If I don't make the 2 modifications at the same time, $unitOfWork->getScheduledEntityDeletions() no longer returns my entity in the second event.

<?php

declare(strict_types=1);

namespace App\EventListener;

use Doctrine\Bundle\DoctrineBundle\EventSubscriber\EventSubscriberInterface;
use Doctrine\ORM\Event\OnFlushEventArgs;
use Doctrine\ORM\Events;
use Knp\DoctrineBehaviors\Contract\Entity\BlameableInterface;
use Knp\DoctrineBehaviors\Contract\Entity\SoftDeletableInterface;
use Knp\DoctrineBehaviors\Contract\Provider\UserProviderInterface;

final class BlameableAndSoftDeletableEventSubscriber implements EventSubscriberInterface
{
    /**
     * @var string
     */
    private const DELETED_BY = 'deletedBy';

    /**
     * @var string
     */
    private const DELETED_AT = 'deletedAt';

    public function __construct(
        private readonly UserProviderInterface $userProvider,
    ) {
    }

    public function onFlush(OnFlushEventArgs $onFlushEventArgs): void
    {
        $entityManager = $onFlushEventArgs->getObjectManager();
        $unitOfWork    = $entityManager->getUnitOfWork();

        $user = $this->userProvider->provideUser();
        if (null === $user) {
            return;
        }

        foreach ($unitOfWork->getScheduledEntityDeletions() as $entity) {
            if (!$entity instanceof SoftDeletableInterface || !$entity instanceof BlameableInterface) {
                continue;
            }

            $oldDeletedBy = $entity->getDeletedBy();
            $oldDeletedAt = $entity->getDeletedAt();

            $entity->setDeletedBy($user);
            $entity->delete();
            $entityManager->persist($entity);

            $unitOfWork->propertyChanged($entity, self::DELETED_BY, $oldDeletedBy, $entity->getDeletedBy());
            $unitOfWork->propertyChanged($entity, self::DELETED_AT, $oldDeletedAt, $entity->getDeletedAt());
            $unitOfWork->scheduleExtraUpdate($entity, [
                self::DELETED_BY => [$oldDeletedBy, $entity->getDeletedBy()],
                self::DELETED_AT => [$oldDeletedAt, $entity->getDeletedAt()],
            ]);
        }
    }

    /**
     * @return string[]
     */
    public function getSubscribedEvents(): array
    {
        return [Events::onFlush];
    }
}

tfavareille avatar Nov 19 '22 20:11 tfavareille

We can fix by creating custom blameable event

<?php
namespace App\EventSubscriber;

use Doctrine\ORM\Events;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Event\LifecycleEventArgs;
use Doctrine\ORM\UnitOfWork;
use Knp\DoctrineBehaviors\Contract\Entity\BlameableInterface;
use Knp\DoctrineBehaviors\Contract\Provider\UserProviderInterface;
use Doctrine\Bundle\DoctrineBundle\EventSubscriber\EventSubscriberInterface;
class CustomBlameableEventSubscriber implements EventSubscriberInterface
{
    private const DELETED_BY = 'deletedBy';
    private $userProvider,$entityManager;
    public function __construct(UserProviderInterface $userProvider,EntityManagerInterface $entityManager) {
        $this->userProvider = $userProvider;
        $this->entityManager = $entityManager;
    }
    public function preRemove(LifecycleEventArgs $lifecycleEventArgs): void
    {
        $entity = $lifecycleEventArgs->getEntity();
        if (! $entity instanceof BlameableInterface) {
            return;
        }

        $user = $this->userProvider->provideUser();
        if ($user === null) {
            return;
        }

        $oldDeletedBy = $entity->getDeletedBy();
        $entity->setDeletedBy($user);

        $this->getUnitOfWork()->scheduleExtraUpdate($entity, [
            self::DELETED_BY => [$oldDeletedBy, $user],
        ]);
    }

    private function getUnitOfWork(): UnitOfWork
    {
        return $this->entityManager->getUnitOfWork();
    }

    public function getSubscribedEvents(): array
    {
        return [Events::preRemove];
    }
}

phoekyawatit avatar Nov 17 '23 06:11 phoekyawatit