-
Notifications
You must be signed in to change notification settings - Fork 195
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
Add updated_at field #234
Comments
Yes, that would be a welcome addition. Pull requests welcome |
@xtrinch , May I work on this ? If yes, please assign this issue to me and provide some description about the issue. |
For sure @mohitCodepy. Essentially, it is a datetime field which updates automatically with a new datetime everytime an FCMDevice entry is updated |
Seems that Maybe better instead of bloating models for all users that use that app, we could allow override models to users who needs that? It could be done with swapper. In the same way as it was done for django-cities |
Tracking in #239, let's go with an override-able device model |
On the other hand, it takes one param to DatetimeField to save the model creation date or modification date automatically: https://docs.djangoproject.com/en/4.2/ref/models/fields/#django.db.models.DateField If you name the fields |
But is there's a reason for that? I though:
|
The official docs even recommend tracking the last activity of that token:
Using the token prevents that inactivity and thus seems to be a valid use case to update the timestamp imo. And another passage:
The last words in the last sentence explicitly recommend an update timestamp. |
To me But I'm okay if there will be |
@Akay7 If FCM issues new tokens for same app instance, and one would update the token on the same device model instance, then update date would be the one to look at for expiry, not create date? |
As of now we could remove tokens with which we unable to send message. |
@akay I think that kind of depends on the update strategy that the developer chooses.. Let's for example say we'd just update the device instance on a new token - always using the same device instance for all of user's tokens, then update date would be the one to use to track expiry. If one just creates new devices on new tokens and lets old ones be deactivated then create date is perfectly sufficient As for removing tokens which we are unable to send messages to - that's what the deactivate feature already does and there's also a setting to auto remove these devices |
I don't think that is possible to recognize the device on all platforms. Even if that is possible than bad actor will be able to rewrite identifiers of valid devices with invalid token. Since getting device identifier is possible only on client side code. So in my opinion is best user case that use multiple Devices where each device will have specific FCM Token as it's independent devices in that case |
@Akay7 I'm not sure what you mean by recognize. You'd basically just maintain one device instance per user - it shouldn't be that hard to securely upsert the device instance of the user? |
@xtrinch Oh okay. I missed point that there will be a single device per user. In that case everything will be slightly different. |
Yeah, well I'm guessing most use cases will involve one user having multiple devices but you never know. I'd still add the update date to the base model, it doesn't really hurt anything and it might just help someone. |
Just to make sure we're on the same page... 😉
That's my understanding, however I might be wrong. 🤷♂️ And: Yippie! 🎉 Congrats to swappable devices being released and thanks for making me aware. 🙏 FYI: Django v5x |
Yes that also sounds like a valid use case with this time based auto-expiry instead of expiry-on-error @Eraldo . |
There are nuances to this way of doing things, too.
I think disabling tokens that failed to send a message with is a more pythonic way(EAFP) to go. But since it is a package that can be used in different ways, I think it is possible to add some functionality that will allow to work with messages in some special ways. For example, check token expiration by time. |
No description provided.
The text was updated successfully, but these errors were encountered: