Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MSC3026: "busy" presence state #3026
base: old_master
Are you sure you want to change the base?
MSC3026: "busy" presence state #3026
Changes from 6 commits
8fcecdc
69c273c
c03419c
2a34e2e
d617090
912de13
cd3c14f
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
While I don't have an objection to adding
busy
in particular, this raises the larger question of what presence states we should have. Is it sufficient to just addbusy
, or do should we be adding more? For example, XMPP defines 6 states (4 states with names, plus a non-specific "online" state when theshow
element is not present, and a state where the client is offline.) Are we eventually going to want equivalents for XMPP's other states that we're missing? How do we decide where to draw the line?FWIW, I think that adding a 4th
busy
state is probably OK. Adding much more than that would be of questionable value.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.
Yeah, this was basically my feeling too. How is
busy
different fromunavailable
? Should we use status messages instead?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.
matrix-org/synapse#12213 asserts that only the
/presence
API can be used to set busy and that subsequent/sync
calls after a user is set tobusy
should not revert a user toonline
automatically. I don't think this makes sense and it would be a lot clearer for a client to sync withset_presence=busy
if it is still busy. This would give/sync?set_presence=?
and/presence
the same "power" and not different behavior.I don't think this MSC could be merged without either documenting the Synapse implementation or a clarification of the MSC.
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.
Yeah, I think this probably does make sense - my basic understanding of it was that the presence API is the 'user level' presence and the sync is the 'device level' presence, so previously the user is busy rather than a device, but this would just be switching to say that a device is busy (which perhaps makes more sense: the user is on a call on a specific device).
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.
Even though it's obvious, we may also want to explicitly state that homeservers shouldn't update a user's presence state to "online" if activity is detected.
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.
Synapse (at least) also transitions users to
offline
in some situations (e.g. if the user stops syncing and enough time has passed). This still seems somewhat valid (e.g. if a device just disappears forever without transition offbusy
then you at some point want to transition the account to something else).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.
This sort of backwards compatibility would go in the unstable prefix section - clients (now) can use spec versions to detect support otherwise.
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.
The spec says that the server aggregates presence data across devices, which I assume means that, e.g. if a user has one device that's
online
and another that'soffline
, then the user is marked asonline
. How doesbusy
fit into this? If a user has a device that'sonline
, and one that'sbusy
, whatm.presence
event should be sent to others? I think that my expectation would be thatbusy
would take precedence overonline
.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'm pretty sure it does in the current implementation. There seems to be one api end point for presence that was overlooked so we disabled it on our forks but when the online state comes through a sync it is ignored by synapse - matrix-org/synapse#12213