Notification Improvements?

[dwasyl]

Hey there,

I've been running an older fork of django-nyt with a few tweaks. I have to update my fork to the latest build anyway so I thought I'd check in and see if there's any interest in the specific changes made:

Improved notifymail with a number of changes:

• Filter notifications only to active users (somewhat mentioned in Global setting: Don't create notifications when settings__user__is_active==False #136) except handled by notifymail.py
• notifymail checks Setting.interval to be > -1 before sending notifications. -1 treated as disabling notifications (e-mails) for a user
• notifymail checks the new field Settings.last_sent and compares it to the user's interval setting before sending mail. This makes cron jobs work properly with set intervals.
• Support for HTML e-mail notification templates, notifymail tries for the HTML file and falls back to the text file.

Added to the Settings object:

• Setting had a new field added last_sent to store a datetime when notifications were last sent to the user

Added a new setting and option to existing:

• Added NYT_SEND_ONLY_LATEST setting to enable e-mail notifications for most recent notification or all unset notifications for the subscription.
• Notification interval setting of -1 to represent disable e-mail notifications,

Improved the content returned by get_notifications:

• Adding a target_obj method to Subscription to return the target object, if any.
• Added type_lbl field to returned Json data in addition to the NotficationType.key that was already included.
• Added target field to returned Json data, as the string representation of the target object (if any).

@benjaoming would there be any interest in some or all of these things as a PR?

[benjaoming]

Hi @dwasyl - thanks so much for the detailed menu :)

I have to focus on getting a new release out (which I'm doing now) and after that, I can try to list out which of these things are already changed or fixed - and which would be interesting in a PR.

Thanks!

[benjaoming]

@dwasyl hey sorry that I didn't get back - here's the quick list:

Improved notifymail with a number of changes:

All those sound like great bug fixes that don't break any existing functionality 👍 A PR for this would be most welcome.

Setting had a new field added last_sent

I think this would be long on the Subscription object.

But other than that, sounds like a great improvement and necessary for one of the bug fixes mentioned in the previous list.

Added NYT_SEND_ONLY_LATEST setting to enable e-mail notifications for most recent notification or all unset notifications for the subscription.

I'm afraid I'm not quite following this one.

Notification interval setting of -1 to represent disable e-mail notifications,

That will work fine as an explicit alternative to deleting the Settings object (which would otherwise be the behavior) - or I think also setting the interval to None.

Improved the content returned by get_notifications:

I don't quite understand the changes / motivation, but if you put this in a separate PR, that works well 👍