-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add a new module API to update user presence state. #16544
Conversation
So to check, there are now three ways to alter presence state:
Am I right that (1) persists indefinitely until it is replaced, but (2) and (3) expire thanks to this wheel timer thing? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than spelling out the new sub-option explicitly, I'm happy as long as my understanding in #16544 (comment) is correct.
Yes; there's a 4th way too -- which is a EDU received from a remote server. (This is analogous to (2) though and also times out, but in a slightly different way.) |
This switches enabled to be true/false/"untracked" instead of two different booleans. Most places that used use_presence actually want to only kick in if presence is being internally tracked. The few spots which don't are updated to use the new presence_enabled, which is true if presence enabled = true or "untracked".
# Whether presence is enabled *at all*. | ||
self.presence_enabled = bool(presence_enabled) | ||
# Whether to internally track presence, requires that presence is enabled, | ||
self.track_presence = self.presence_enabled and presence_enabled != "untracked" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think track_presence
is very clear---thank you!
I think the phrase "Whether presence is enabled" is still a bit confusing. Can we call self.presence_enabled
something like self.report_presence
? (I'm guessing that this bool now controls the reporting of presence down /sync and in response to explicit presence GETs).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It ends up getting used a bit internally too, I'm not sure that presence_enabled
is the best name, but I think report_presence
doesn't quite capture it either.
So it gets used for:
- Whether to calculate presence for sync & initial sync.
- Whether to return real presence info from
/presence
. - Whether to persist any presence information into the database.
- Whether the presence handler APIs can be called to update presence state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think report_presence
easily covers 1 & 2, it doesn't feel quite right for 3 & 4 though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blimey, that's a lot to juggle. Let's leave it as-is then. If nothing else, we'll have these comments as signposts for our future selves!
This adds a module API which allows a module to update a user's presence state/status message. This is useful for controlling presence from an external system.
To fully control presence from the module the presence.enabled config parameter gains a new state of "untracked" which disables internal tracking of presence changes via user actions, etc. Only updates from the module will be persisted and sent down sync properly).