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

NotificationList passed to templates should be a list of NotifiableNotification #20

Closed
nreynis opened this issue Jan 12, 2018 · 1 comment
Milestone

Comments

@nreynis
Copy link

nreynis commented Jan 12, 2018

In the template MgiletNotificationBundle::notification_list.html.twig the notificationList is an array of objects with the seen property and a Notification instance.
Fetching whole NotifiableNotification entities would not cost any performance overhead (you are already joining on the table) and provides several advantages:

  • clarity, you know what you fetch
  • consistency, you would have the same list in notification_list.html.twig and notification.html.twig
  • routes (mark as seen, mark as unseen) would be easier to generate as you would have the NotifiableEntity->id at your disposal
  • you are fetching an entity, not a freeform object, this means that entity could then implements JsonSerializable interface; that would make notifications refreshing (or lazy load) much more practical
  • you could get ride of that ugly notificationItem[0] in your twig template and have something like item.notification and item.notifiable.id
@nreynis nreynis changed the title NotificationList passed to templates should be list of NotifiableNotification NotificationList passed to templates should be a list of NotifiableNotification Jan 12, 2018
nreynis pushed a commit to nreynis/notification-bundle that referenced this issue Jan 12, 2018
Consistent twig parameter names, use 'notificationList' in both views
@maximilienGilet
Copy link
Owner

maximilienGilet commented Apr 15, 2018

Closed, implemented in #23

Currently in branch dev. Will be merged with version 3.0

Thanks :D

@maximilienGilet maximilienGilet added this to the 3.0 milestone Apr 15, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants