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

spec is contradictory about /context's support for filters #2338

Closed
richvdh opened this issue Nov 1, 2019 · 2 comments · Fixed by #2344
Closed

spec is contradictory about /context's support for filters #2338

richvdh opened this issue Nov 1, 2019 · 2 comments · Fixed by #2344
Assignees
@turt2live
Labels
client-server Client-Server API spec-omission implemented but not currently specified
Milestone
r0.6.0 final clar...

Comments

@richvdh
Copy link
Member

richvdh commented Nov 1, 2019

According to the spec, /context supports lazy-loading of members; however, lazy-loading is configured via a filter, and the spec doesn't say anything about /context supporting filters
@turt2live turt2live added client-server Client-Server API spec-bug Something which is in the spec, but is wrong labels Nov 1, 2019
@turt2live
Copy link
Member

What does Synapse do? (and is there a TODO comment to fix it?)

@richvdh
Copy link
Member Author

richvdh commented Nov 1, 2019

synapse apparently supports filters on /context.

@turt2live turt2live added spec-omission implemented but not currently specified and removed spec-bug Something which is in the spec, but is wrong labels Nov 1, 2019
@turt2live turt2live added this to the r0.6.0 final clarifications milestone Nov 4, 2019
@turt2live turt2live self-assigned this Nov 4, 2019
turt2live added a commit that referenced this issue Nov 4, 2019
@turt2live
Add filter query param to /context
021c056 
This was missed as part of lazy-loading.

Fixes #2338
@turt2live turt2live mentioned this issue Nov 4, 2019
Merged
babolivier added a commit to matrix-org/synapse that referenced this issue Nov 5, 2019
@babolivier
Filter events_before and events_after in /context requests
1dffa78 
While the current version of the spec doesn't say much about how this endpoint uses filters (see matrix-org/matrix-spec-proposals#2338), the current implementation is that some fields of an EventFilter apply (the ones that are used when running the SQL query) and others don't (the ones that are used by the filter itself) because we don't call event_filter.filter(...). This seems counter-intuitive and probably not what we want so this commit fixes it.
@babolivier babolivier mentioned this issue Nov 5, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@turt2live turt2live

Labels
client-server Client-Server API spec-omission implemented but not currently specified
Projects
None yet
Milestone
r0.6.0 final clarifications
Development

Successfully merging a pull request may close this issue.

Add filter query param to /context matrix-org/matrix-spec-proposals
2 participants
@turt2live @richvdh