maximilienGilet / notification-bundle

A simple Symfony bundle to notify user
https://packagist.org/packages/mgilet/notification-bundle
MIT License
107 stars 58 forks source link

Custom notification added configuration and return of the custom class in manager creation method #46

Closed LeandroLuccerini closed 3 years ago

LeandroLuccerini commented 6 years ago

This PR is about issue #39

I've found that if the default Notification entity is overridden, the NotificationManager::createNotification method still returns a Mgilet\NotificationBundle\Entity\Notification object and it throws an exception when flushing to the db due to resolve_target_entities directive configured in doctrine orm.

What i have done

  1. Added a configuration to the bundle where to specify the customized notification class;
  2. Added a compiler pass named ResolveTargetEntitiesPass that adds the resolve_target_entities without explicitly changing doctrine configuration;
  3. Changed the manager that now returns the customized notification class;
  4. Changed the ordering default field from id to date.

Point no. 4 was added due to the possibility to customize the notification entity. To me it was necessary to change the notification ID from int autoincrement to UUID so the sort didn't work.

BC breaks

Accepting this PR means that who already uses a customized notification class must remove the resolve_target_entities directive from the doctrine configuration and add a configuration file.

Example

After adding the customized notification class as in #39 just add a configuration file

config/packages/mgilet_notification.yaml

mgilet_notification:
  notification_class: App\Entity\MyCustomNotification

Can it be useful?

maximilienGilet commented 6 years ago

Hi ! Thanks for the fix.

However the break change will need a new major version of the bundle.

I will merge this in the dev branch. This will be included in the version 4, along other new features.

Feel free to suggest features :+1:

LeandroLuccerini commented 6 years ago

Ok, thank you. This bundle is great! Nice job :+1: