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

lastActivity gets deleted on logout #80

Closed
KristerV opened this issue Sep 9, 2015 · 5 comments
Closed

lastActivity gets deleted on logout #80

KristerV opened this issue Sep 9, 2015 · 5 comments

Comments

@KristerV
Copy link
Contributor

KristerV commented Sep 9, 2015

Hi,

Maybe you remember me, i've contributed PR's before.

the user-status package seems to delete the status.lastActivity when user logs out. This makes sense if lastActivity is used only for idle tracking, but realistically I want to know when the user was last active even when he has logged out - this field is the best source for this information. Honestly I spent a good hour trying to fix this in your package but did not understand what piece of code is responsible.

Temporarily I got my fix by using your API, but please can you fix this? This extra field will not hurt anyone if it stays there and it's valuable information.

@mizzao
Copy link
Collaborator

mizzao commented Sep 9, 2015

It's right here, but you will have to double check that the presence of the lastActivity field doesn't make some other code (either in this package or elsewhere) think that the user is idle when they are actually offline.

https://github.com/mizzao/meteor-user-status/blob/master/status.coffee#L54

Really, one hour? There are only ~200 lines of server-side code here. Generally you seem to be super impatient and edgy when it comes to requesting changes to OSS packages. Why don't you try to calm yourself a little? :)

@KristerV
Copy link
Contributor Author

haha, yeah that's me - impatient. I get really impatient when I see a feature that to me is so obviously crippling the functionality. Thus happened with this case.

Yeah well I'm an idiot. I cloned this packege and was testing in my own package, changes didn't take effect. The line you highlighted is the one I figured had to be the correct one, but because changes didn't take effect I kept searching.. awesome :D

Anyway how do I make sure noone's app is going to break from this? It seems to me that checking if user is idle is what the idle field is for so seems safe.

@mizzao
Copy link
Collaborator

mizzao commented Sep 10, 2015

"Crippling the functionality" seems a bit intense to me. It's all open-source and free to use, so there's always a way to improve and collaborate. I think you might find it easier to work with other people if you don't come off so aggressively!

I don't understand your second paragraph.

The idle monitoring is originally designed to only work for online users. If you try to use lastActivity for every user, you will have to make sure that all connections have idle monitoring turned on, otherwise you may have missing or outdated lastActivity values even for users who are active. That's the main reason I think it might cause an issue. You should give it a try, though.

@KristerV
Copy link
Contributor Author

sorry, not really agressive behind this computer. it's just how I speak.

Ah, I see your point.. okay that makes sense. Okay in that case using the server API seems like a better option in the end of the day - so the functionality of the package is more explicit. I'll add a note about this to the readme at least.

Hey thanks for bearing with me :D And I'll try tone down the vocabulary :)

@mizzao
Copy link
Collaborator

mizzao commented Sep 12, 2015

I actually don't mind the feature you proposed, as long as we make clear that lastActivity is only valid as a measure of activity when the developer turns on idle tracking for all connections.

But if you are using last activity over long periods like that, I think lastLogin is close enough for what you need without having to go through all these extra hoops.

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

2 participants