Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix handling of public rooms filter with a network tuple. #14053

Merged
merged 6 commits into from
Oct 5, 2022

Conversation

clokep
Copy link
Member

@clokep clokep commented Oct 4, 2022

Fixes two related bugs:

  • The handling of [None] for room types was not quite correct (as you'd end up with the wrong number of arguments).
  • The ordering of arguments when providing both a network tuple and room type field was incorrect.

Fixes #14051

@clokep
Copy link
Member Author

clokep commented Oct 4, 2022

CC @SimonBrandner

@clokep clokep marked this pull request as ready for review October 4, 2022 17:53
@clokep clokep requested a review from a team as a code owner October 4, 2022 17:53
Copy link
Contributor

@SimonBrandner SimonBrandner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

Comment on lines +2221 to +2222
if instance_id:
body["third_party_instance_id"] = "test|test"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's unclear to me what this does....

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly it just demonstrates that the conditions fail in some situations, e.g. the parameters don't line up with the SQL query.

Copy link
Contributor

@DMRobertson DMRobertson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many thanks!

Co-authored-by: David Robertson <[email protected]>
@clokep clokep enabled auto-merge (squash) October 5, 2022 12:22
@clokep clokep merged commit 0b037d6 into develop Oct 5, 2022
@clokep clokep deleted the clokep/fix-public-rooms branch October 5, 2022 12:49
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect SQL query construction in _construct_room_type_where_clause
3 participants