search

stephenmuss / django-ios-notifications

Django iOS Notifications makes it easy to send push notifications to iOS devices
Other
230 stars 72 forks source link

support slient notification #54

Closed yongbo closed 10 years ago

yongbo commented 10 years ago

Add a field named slient to Notification, if silent is True set aps['content-available'] = 1

Because of the file ios_notifications/migrations/0003_auto__add_field_notification_loc_payload.py use users.user , I use django.contrib.autu.models.User. I doesn't commit my schema migration file.

stephenmuss commented 10 years ago

Shouldn't the field be silent not slient?

yongbo commented 10 years ago

I am sorry,

I had changed it.

please review the pull request. thank you .

2014-09-14 4:37 GMT+08:00 Stephen Muss notifications@github.com:

Shouldn't the field be silent not slient?

— Reply to this email directly or view it on GitHub https://github.com/stephenmuss/django-ios-notifications/pull/54#issuecomment-55505984 .

stephenmuss commented 10 years ago

Thanks @yongbo

mbargiel commented 10 years ago

Uh, no, if we modify the Model and there should be a field backing the new property, we must add that to a South migration, otherwise we just introduced desync'ed the models and database of everyone using the lastest commit to of DIN.

Also, if "users.user" we should fix ios_notifications/migrations/0003_auto__add_field_notification_loc_payload.py to use the same approach as the other migrations (I remember there was a discussion about it).

On Sun, Sep 14, 2014 at 7:56 AM, Stephen Muss notifications@github.com wrote:

Thanks @yongbo https://github.com/yongbo

— Reply to this email directly or view it on GitHub https://github.com/stephenmuss/django-ios-notifications/pull/54#issuecomment-55523299 .

mbargiel commented 10 years ago

By the way, I'll fix the previous migrations (0002 and 0003) that should support custom user models. I'll commit something to that effect soon. Because it requires changing the migration manually each time, we might consider creating a migration template, or perhaps a sed script or something to run on the generated migration. It's not too hard to do manually for now but it's easy to overlook (as we did with 0002 and 0003).

EDIT: See https://github.com/stephenmuss/django-ios-notifications/tree/fix_migrations. Untested at this time (I did this on a PC where Python isn't even installed :P I'll test later this week before merging in master.)

On Mon, Sep 15, 2014 at 9:23 AM, Maxime Bargiel maxime.bargiel@gmail.com wrote:

Uh, no, if we modify the Model and there should be a field backing the new property, we must add that to a South migration, otherwise we just introduced desync'ed the models and database of everyone using the lastest commit to of DIN.

Also, if "users.user" we should fix ios_notifications/migrations/0003_auto__add_field_notification_loc_payload.py to use the same approach as the other migrations (I remember there was a discussion about it).

On Sun, Sep 14, 2014 at 7:56 AM, Stephen Muss notifications@github.com wrote:

Thanks @yongbo https://github.com/yongbo

— Reply to this email directly or view it on GitHub https://github.com/stephenmuss/django-ios-notifications/pull/54#issuecomment-55523299 .

stephenmuss commented 10 years ago

@mbargiel Yep, I was planning to add the migration.