stephpy / timeline

Standalone library to make a timeline.
MIT License
89 stars 20 forks source link

Notifications: real "markasread"-flag instead of deleting entries #8

Closed KeKs0r closed 10 years ago

KeKs0r commented 11 years ago

Hi,

I am heavily using the notificationsmanager with an own implementation and in my use case it would be gread to mark notifications as read without deleting them from the database.

I thought i might use an issue, that we can publicly discuss how this could be implemented. Maybe @stephpy has already some initial ideas how this could be done. In general I think we need two things: Extend the Entity with an flag for it. And if reading from database an option to select also "read" entries, but default behaviour should be that only unread statements are selected.

stephpy commented 11 years ago

Hi, there is already an issue at this subject: https://github.com/stephpy/TimelineBundle/issues/82

It can be implemented yes. But this is really different between drivers. On ORM/ODM, we can add a status on timeline table, but on redis, we should have an other one key

At this moment, UnreadNotificationManager has "duplicated" actions from timeline, if we support a isRead property on timeline entity, UnreadNotificationManager will be no more useful, Why not !

It's a good feature. A bit complicated because i would like to support each features for each drivers.

stephpy commented 11 years ago

We would have:

$timelineManager = $this->get('spy.timeline_manager');
$timelineManager->markActionAsRead .........
$timelineManager->markAllActionsAsRead .........
$timelineManager->markAllActionsAsRead .........
$timelineManager->getTimeline(array('isRead' => null))

// if isRead is defined, fetch only read/not read timeline actions, else, get all.

And BTW, delete UnreadNotificationManager.

KeKs0r commented 11 years ago

Another Option would be to use a "softdelete" from the doctrineextensions: https://github.com/l3pp4rd/DoctrineExtensions/blob/master/doc/softdeleteable.md But with that you also have the problem with supporting different database drivers.

stephpy commented 11 years ago

Using softDelete "behavior" is a good solution yes, but if has not to be part of TimelineBundle, you have to use it on your own application.

thomask commented 11 years ago

:+1:

thomask commented 11 years ago

So what is the use-case here? For me it would be to have an overview page of a user's notifications, read and unread. Softdeletable would be a bit hacky for this purpose. What are the other use-cases for having notifications marked as read?

stephpy commented 11 years ago

Yes, using SoftDeletable behavior is may be tricky ...

Using spy_timeline.unread_notifications is not mandatory, this is indeed a basic implementation of notifications.

You can easily create your own notifier and a dedicated model/entity to this.

Notification:
    subject_id: (int)
    action_id: (int)
    is_read: (boolean)
    created_at: (datetime)

In spy_timeline.unread_notifications i chosen to KISS strategy, i'll not cover all usecases. :)