Overriding entity doesn't work about notification-bundle HOT 18 OPEN

Hi @maximilienGilet, so clearly there is an issue right here with the bundle when you are trying to override the default entities.
With the actual code, Doctrine find 2 entities with the same table name notification and throw an exception.

The best way should be to make do a refactoring and provide 3 models classes (Notification, NotifiableEntity and NotifiableNotification).

For Symfony Flex users, it will be perfect to create a recipe who provide the entity classes.
However for the others they will need to create the 3 entity classes manually.

@maximilienGilet What do you think about this idea?

from notification-bundle.

Hi @emulienfou this is a great idea !

However I don't use the bundle anymore and don't have much time to make the changes...

Make me a pull request and I'll try and merge it :)

from notification-bundle.

Hi @lukepass ! Thanks for using the bundle !

At first glance it seems like your configuration has a typo : inside the numeric_functions node after your rand function, you have an orm which looks like a bad copy-paste.

It should work if you remove the empty orm: line

from notification-bundle.

Hello @maximilienGilet and thanks for the fast response!
Unfortunately that was just a typo but the error is still the same:

The table with name 'mysite.notification' already exists.

Now I am using an entity called AppNotification and it seems to work but I had to change the table name:

<?php

namespace AppBundle\Entity;

use Doctrine\ORM\Mapping as ORM;
use Symfony\Component\Validator\Constraints as Assert;
use Mgilet\NotificationBundle\Entity\NotificationInterface;
use Mgilet\NotificationBundle\Model\Notification as NotificationModel;

/**
 * AppNotification
 *
 * @ORM\Entity(repositoryClass="AppBundle\Repository\NotificationRepository")
 * @ORM\Table(name="app_notification")
 */
class AppNotification extends NotificationModel implements NotificationInterface
{
    const TYPE_SYSTEM = 'system';
    const TYPE_ACHIEVEMENT = 'achievement';
    const TYPE_CUSTOM = 'custom';

    /**
     * @var string
     *
     * @ORM\Column(name="`type`", type="string", length=255)
     */
    private $type;

    /**
     * @var string|null
     *
     * @ORM\Column(name="sender", type="string", length=255, nullable=true)
     */
    private $sender;

    /**
     * Set type.
     *
     * @param string $type
     *
     * @return AppNotification
     */
    public function setType($type)
    {
        $this->type = $type;

        return $this;
    }

    /**
     * Get type.
     *
     * @return string
     */
    public function getType()
    {
        return $this->type;
    }

    /**
     * Set sender.
     *
     * @param string|null $sender
     *
     * @return AppNotification
     */
    public function setSender($sender = null)
    {
        $this->sender = $sender;

        return $this;
    }

    /**
     * Get sender.
     *
     * @return string|null
     */
    public function getSender()
    {
        return $this->sender;
    }
}

    orm:
        auto_generate_proxy_classes: '%kernel.debug%'
        naming_strategy: doctrine.orm.naming_strategy.underscore
        auto_mapping: true
        dql:
            numeric_functions:
                rand: DoctrineExtensions\Query\Mysql\Rand
        resolve_target_entities:
            Mgilet\NotificationBundle\Entity\Notification: AppBundle\Entity\AppNotification

Is this how it's supposed to work?
It's still creating the notification table (now I have notification and app_notification tables) but leaving notification empty.

Thanks!

from notification-bundle.

Hi @lukepass didn't test it yet, but maybe the Single table inheritance could fix this issue.

However, it seems this annotations need to be added in the Notification entity class inside the NotificationBundle. If this is the case and it works, you should submit a pull-request.

from notification-bundle.

Unfortunately I don't use Flex or Symfony 4. I tried with the single table inheritance but it didn't work well.
For my application I solved the problem just by making another entity with a different table name (see my previous answer) and it's working quite fine. The only drawback is that there is an additional empty notification table.
I agree with @emulienfou solution.

from notification-bundle.

Hi @lukepass, I'm currently using SF4 so it would be great if you can test it on your Symfony version.

However, you will need to use my repository and add the following lines to your composer.json:

{
    "repositories": [
        { "type": "vcs", "url": "https://github.com/emulienfou/notification-bundle" }
    ]
}

Updated documentation to create entities and add Doctrine resolving mapping is here: https://github.com/emulienfou/notification-bundle/blob/master/Resources/doc/index.rst#add-entity-classes-non-symfony-flex-user

Let me know if there is an issue on your side, I will make some fixes before creating the Pull Request and adding the Symfony Flex recipe

from notification-bundle.

Hi @emulienfou is your version overriding the entities right with SF4?

from notification-bundle.

Hi @danielrestrepo exactly, the updated code do not provide entities anymore, only mapped superclasses.
You will need to add 3 entities in your main project who extends the model classes, like provided by the Symfony Flex Recipe.

Do not forget to add the doctrine configuration too, with the resolve_target_entities entries.

from notification-bundle.

Thanks a lot @emulienfou custom mapping works now but I wanted to use the NotificationManager and I ran into a couple of issues as accessor methods are expecting the abstract class and not the interface like here: https://github.com/emulienfou/notification-bundle/blob/master/Model/NotificationInterface.php#L78

If I use there the NotifiableNotificationInterface error would go away. Am I right? Can I send you a PR with those changes?

from notification-bundle.

Hi @danielrestrepo just made the changes to use the interface instead of the model class.
Let me know if there is more issues.
I will take some times to check if there is other issues too

from notification-bundle.

Thanks @emulienfou I'll update from your last commit and test again. For now the only additional thing I found is that Notification entity didn't persist but NotifiableNotificationand Notifiable did when using this code:

                $notification = $this->notification->createNotification('Proceso asignado');
                $notification->setMessage('EL proceso xxx del 2019-04-02 le ha sido asignado');
                $notification->setDate(new \DateTime());

                $stepEvents = $event->getFlowEvents();

                $user = null;
                foreach ($stepEvents as $stepEvent) {
                    if ($stepEvent instanceof AssignUserEvent) {
                        /** @var UserInterface $user */
                        $user = $this->repositoryRegistry->getUserRepository()->find($stepEvent->getUserId()->toString());
                    }
                }

                $this->notification->addNotification([$user], $notification, true);

Using original bundle without custom mappings was working with the same code. Not sure if it is the NotificationManager. Any ideas?

Update

I just needed to add cascade={"persist"} on the owning side of the relationship so please ignore this question.

Thanks anyway. It's been a pleasure collaborating with you

from notification-bundle.

Hi @danielrestrepo I apply the changes your made using interfaces in all models.
Glad you figured out and made this bundle working on your side

from notification-bundle.

Hi @emulienfou got back to your fork and is working nicely. Thanks a lot. I'll let you know if found something worth mentioning.

from notification-bundle.

Hello, I tried overriding the existing Notifications entity with a new class in my src/Entity as mentioned in the docs. And the table also got updated to show my latest changes.
However when I try to call upon setters and getters for the new Entity in which I added a few fields I get the following error:

Attempted to call an undefined method named "setType" of class "Mgilet\NotificationBundle\Entity\Notification".

1. Here is the overridden entity

<?php

namespace App\Entity;

use Doctrine\ORM\Mapping as ORM;
use Mgilet\NotificationBundle\Entity\NotificationInterface;
use Mgilet\NotificationBundle\Model\Notification as NotificationModel;

/**
 * @ORM\Entity(repositoryClass="Mgilet\NotificationBundle\Entity\Repository\NotificationRepository")
 *
 */
class Notification extends NotificationModel implements NotificationInterface
{
    /**
     * Overrides the existing Notifications entity
     *
     * @ORM\Column(name="type", type="string", length=255, nullable=false)
     */
    protected $type;

    /**
     * Get the Notification type
     *
     * @return string
     */
    public function getType() : string {
        return $this->type;
    }

    /**
     * Set the Notification type
     *
     * @param $type
     */
    public function setType($type){
        $this->type = $type;
    }
}

2. Here is doctrine.yml

orm:
        auto_generate_proxy_classes: "%kernel.debug%"
        auto_mapping: true
        mappings:
            App:
                is_bundle: false
                type: annotation
                dir: '%kernel.project_dir%/src//Entity'
                prefix: 'App\Entity'
                alias: App
        resolve_target_entities:
            Mgilet\NotificationBundle\Entity\Notification: App\Entity\Notification

3. Here is what I try to achieve in my Controller

$manager = $this->get('mgilet.notification');
            $notif = $manager->createNotification('Project Creation');
            $notif->setMessage('Your project '.$p->getDisplayName().' was created!');
            $notif->setLink('Link here');
            $notif->setType('web');

@maximilienGilet @emulienfou
But it doesn't work. Some help here would be very useful as I am running late for my major projects

from notification-bundle.

So, it worked because you have to create an Instance of the new overridden entity rather than creating the other entity. That fixed it for me

from notification-bundle.

Hi guys,
@aquibbaig how did you solve this problem? I have exactly the same.
How to extend NotificationManager and don't lose events and all useful things but work on extended entity?

from notification-bundle.

Hi @marcin-dorosiewicz, you can follow up to Step No 2 here. After that in the controller use dependency injection to get the overridden notification class. Create an instance of the overridden notification class. And pass it into $manager->addNotification().

However, if it is of no use, you can post the exact problem here to be of more help. Good luck!

from notification-bundle.