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.

Sign up for GitHub

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

Jump to bottom

/messages filter param should reflect that it takes a filter component, not a filter collection (SPEC-451) #199

Open
matrixbot opened this issue Sep 7, 2016 · 6 comments
Open

/messages filter param should reflect that it takes a filter component, not a filter collection (SPEC-451) #199

matrixbot opened this issue Sep 7, 2016 · 6 comments
Labels
A-Client-Server Issues affecting the CS API enhancement A suggestion for a relatively simple improvement to the protocol

Comments

@matrixbot
Copy link
Member

matrixbot commented Sep 7, 2016 •
edited by richvdh
Loading

Submitted by @​matthew:matrix.org
also, it needs to be specced.

(Imported from https://matrix.org/jira/browse/SPEC-451)
@matrixbot
Copy link
Member Author

Jira watchers: @ara4n

@matrixbot
Copy link
Member Author

Superficially /messages and /sync both take filter params. Except /sync is a 'filter collection' (which may be either an ID or a blob of JSON, and is also known as a 'filter'), whereas /messages takes a 'filter component' as a blob of JSON (sometimes called a 'filter'). At the very least, the params should be named differently, and the whole naming convention should be untangled.

-- @ara4n

@matrixbot matrixbot added the p4 label Oct 28, 2016
@matrixbot matrixbot changed the title /messages filter param should reflect that it takes a filter component, not a filter collection /messages filter param should reflect that it takes a filter component, not a filter collection (SPEC-451) Oct 31, 2016
@matrixbot matrixbot added the spec-bug Something which is in the spec, but is wrong label Nov 7, 2016
@richvdh richvdh mentioned this issue Nov 7, 2016
Closed
@matrixbot matrixbot added spec-omission implemented but not currently specified enhancement A suggestion for a relatively simple improvement to the protocol and removed spec-bug Something which is in the spec, but is wrong spec-omission implemented but not currently specified labels Nov 7, 2016
@richvdh
Copy link
Member

richvdh commented Nov 7, 2016

It got specced by matrix-org/matrix-spec-proposals#390, but the naming convention is still mixed up

@ara4n ara4n removed the p4 label Sep 3, 2018
@ara4n
Copy link
Member

ara4n commented Sep 3, 2018

this continues to bite me constantly (esp with LL)

@turt2live turt2live added the A-Client-Server Issues affecting the CS API label Sep 6, 2018
@bmarty
Copy link

bmarty commented Apr 12, 2019

Also /context has the same filter parameter that /messages.

Documentation of filter param is still missing in r4.0 for /context, that what leads me to this issue (/messages has the filter parameter documented)

Ref:

alphapapa referenced this issue in alphapapa/ement.el Jul 25, 2021
@alphapapa
Change/Fix: Displaynames when calling /messages
d2122f4 
The ultimate fix for this was to send /messages the proper kind of
filter, a RoomEventFilter, rather than sending it the kind of filter
that /sync expects.  This is legitimately confusing, and there's even
an issue about it:
<https://github.com/matrix-org/matrix-doc/issues/706>.

Thanks to Michael (@t3chguy) in #matrix-dev:matrix.org, who helped me
identify the problem, this commit fixes the problem by using the right
kind of filter for /messages.

Also thanks to him for clarifying that membership events may be in
both state and timeline events in a room, so calculating a displayname
requires searching both.  This commit tries to be more...comprehensive
in doing this (perhaps more than necessary, but tidying that up can be
done later if optimization is needed).
alphapapa referenced this issue in alphapapa/ement.el Jul 25, 2021
@alphapapa
Change/Fix: Displaynames when calling /messages
e98fb62 
The ultimate fix for this was to send /messages the proper kind of
filter, a RoomEventFilter, rather than sending it the kind of filter
that /sync expects.  This is legitimately confusing, and there's even
an issue about it:
<https://github.com/matrix-org/matrix-doc/issues/706>.

This commit fixes the problem by using the right kind of filter for
/messages.

Thanks to Michael (@t3chguy) in #matrix-dev:matrix.org, who helped me
identify the problem.

Also thanks to him for clarifying that membership events may be in
both state and timeline events in a room, so calculating a displayname
requires searching both.  This commit tries to be more...comprehensive
in doing this (perhaps more than necessary, but tidying that up can be
done later if optimization is needed).
@alphapapa
Copy link

@ara4n As you requested, I'm "complaining" to nudge this issue along. :) Thanks.

@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Client-Server Issues affecting the CS API enhancement A suggestion for a relatively simple improvement to the protocol
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@alphapapa @turt2live @ara4n @richvdh @bmarty @matrixbot