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

Add updated_at field #234

Open
nachov99 opened this issue May 11, 2023 · 21 comments
Open

Add updated_at field #234

nachov99 opened this issue May 11, 2023 · 21 comments

Comments

@nachov99
Copy link

No description provided.

@xtrinch
Copy link
Owner

xtrinch commented May 12, 2023

Yes, that would be a welcome addition. Pull requests welcome

@mohitCodepy
Copy link
Contributor

@xtrinch , May I work on this ? If yes, please assign this issue to me and provide some description about the issue.

@xtrinch
Copy link
Owner

xtrinch commented May 24, 2023

For sure @mohitCodepy. Essentially, it is a datetime field which updates automatically with a new datetime everytime an FCMDevice entry is updated

@Akay7
Copy link
Contributor

Akay7 commented May 26, 2023

Seems that date_created is enough for tracking expired tokens. Or @nachov99 is there specific case where that field could be helpful to have updated_at?

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

@xtrinch
Copy link
Owner

xtrinch commented Jun 6, 2023

Tracking in #239, let's go with an override-able device model

@xtrinch xtrinch assigned Akay7 and unassigned mohitCodepy Jun 6, 2023
@merwok
Copy link

merwok commented Jun 6, 2023

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 created and updated they will even be the same as TimeStampedModel from django-model-utils!

@Akay7
Copy link
Contributor

Akay7 commented Jun 7, 2023

If you name the fields created and updated they will even be the same as TimeStampedModel from django-model-utils!

But is there's a reason for that? I though:

  1. there's no needs to everyone who would like to use that package to pay extra to store data that not required in their DB.
  2. and shouldn't be a limitation to store extra data if it's required according their business requirements

@Eraldo
Copy link

Eraldo commented May 10, 2024

The official docs even recommend tracking the last activity of that token:

When stale tokens reach 270 days of inactivity, FCM will consider them expired tokens. Once a token expires, FCM marks it as invalid and rejects sends to it. However, FCM issues a new token for the app instance in the rare case that the device connects again and the app is opened.

Using the token prevents that inactivity and thus seems to be a valid use case to update the timestamp imo.

And another passage:

Retrieve registration tokens from FCM and store them on your server. An important role for the server is to keep track of each client's token and keep an updated list of active tokens. We strongly recommend implementing a token timestamp in your code and your servers, and updating this timestamp at regular intervals.

The last words in the last sentence explicitly recommend an update timestamp.

@Akay7
Copy link
Contributor

Akay7 commented May 29, 2024

If updated_at or any other field is required for your use-case it's possible to achieve via overridden Device model. You can check that at docs or example how that made at tests .

First version where it possible is 2.2.0rc0

@xtrinch
Copy link
Owner

xtrinch commented May 29, 2024

Honestly @Akay7 , going from @Eraldo 's last comment - while this is now possible with the overrideable device model, I would still add this into the default one, it sounds like such a common use case

@Akay7
Copy link
Contributor

Akay7 commented May 29, 2024

it sounds like such a common use case

@xtrinch

To me and updating this timestamp at regular intervals sounds like typo. Regular updating timestamps on token doesn't make any sense to me. Maybe author wanna tell that we should keep actual token and update tokens instead of timestamps?
Otherwise if we should regularly update timestamps then in what way and for what purpose? Ie if there will be some task that will update timestamp everyday then what is the purpose of that?

But I'm okay if there will be updated by default. It's just not clear to me why it would be useful.

@xtrinch
Copy link
Owner

xtrinch commented May 29, 2024

@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?

@Akay7
Copy link
Contributor

Akay7 commented May 29, 2024

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?

@xtrinch

  1. How can we match tokens on the backend that they came from the same device?
  2. What does the update date mean?

As of now we could remove tokens with which we unable to send message.
But if we track tokens that have not been used for 30 days(could be different staleness window), so that they are not used later. It seems that it is necessary to track not in the device, but to invent something else where the timestamp of the last message sent for this specific token will be stored.

@xtrinch
Copy link
Owner

xtrinch commented May 29, 2024

@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

@Akay7
Copy link
Contributor

Akay7 commented May 29, 2024

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.

@xtrinch

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 updated timestamp for Device(Token) itself is not useful at all.

@xtrinch
Copy link
Owner

xtrinch commented May 29, 2024

@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?

@Akay7
Copy link
Contributor

Akay7 commented May 29, 2024

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.
And possible it will be useful to keep something like recent FCM token that user had before recent update or something like that. And doesn't store information such as Device.name at all.

@xtrinch
Copy link
Owner

xtrinch commented May 29, 2024

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.

@Eraldo
Copy link

Eraldo commented May 30, 2024

Just to make sure we're on the same page... 😉

  • According to what I read FCM tokens are device bound. Meaning different devices will automatically generate new tokens. I made some tests and "Device" does not refer to physical devices. So, for example: Pwa and native app on the same phone yield different devices and thus different tokens. :)
  • A token only expires if it is not used, so given the device that issued the token registers via their sdk repeatedly (regular intervals) the token will probably not expire (no guarantee). If there is no sign of usage, Firebase will flag the tokens as inactive and in the (according to them) rare case of reactivation, a new token might be issued.
    => Because of this, as a developer I would have auto expired all tokens that have an updated_at date (not created_at older than 30 days.

That's my understanding, however I might be wrong. 🤷‍♂️
I enjoy reading the discussion. 👍

And: Yippie! 🎉 Congrats to swappable devices being released and thanks for making me aware. 🙏

FYI: Django v5x GeneratedField can now also be used to rename fields like registration_id => token while staying backwards compatible by providing a generated (optionally virtual) proxy field. ;)

@xtrinch
Copy link
Owner

xtrinch commented May 31, 2024

Yes that also sounds like a valid use case with this time based auto-expiry instead of expiry-on-error @Eraldo .

@Akay7
Copy link
Contributor

Akay7 commented May 31, 2024

  • I would have auto expired all tokens that have an updated_at date (not created_at older than 30 days.

@Eraldo

There are nuances to this way of doing things, too.

  1. When will happening update of updated_at field? When application getting FCM token and send it to backend? What if there's a lot those updates and table at DB could be also big?
  2. It seems if update only timestamp updated_at on token and no any other updates of any other information then maybe we shouldn't store token and last used timestamp together?
  3. What if some tokens have 30 days and other 60 days of inactivity? Or such a change will have to occur sometime in the future.
  4. What could be the harm in trying to send a message with an expired token? Because if we predict expired tokens, and it's not actually expired tokens, then some users won't get notifications, even though they might otherwise.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants