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

Experimental support to include bundled aggregations in search results (MSC3666) #11837

Merged
merged 7 commits into from
Feb 8, 2022
Merged

Experimental support to include bundled aggregations in search results (MSC3666) #11837

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
bbd10e3
Remove an unused variable.
clokep Jan 21, 2022
4b3aebe
Include aggregations in results.
clokep Jan 21, 2022
1de1e58
Newsfragment
clokep Jan 26, 2022
3bd4e78
Merge remote-tracking branch 'origin/develop' into clokep/search-aggr…
clokep Feb 7, 2022
adfdc64
Only request bundled aggregations for a single event once.
clokep Feb 7, 2022
58ebd31
Merge remote-tracking branch 'origin/develop' into clokep/search-aggr…
clokep Feb 8, 2022
3c2336b
Fix some inefficiencies due to a merge.
clokep Feb 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions synapse/handlers/search.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,6 @@ async def search(

results = search_result["results"]

results_map = {r["event"].event_id: r for r in results}

Comment on lines -241 to -242
Copy link
Member

Choose a reason for hiding this comment

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

ugh. If this function wasn't 50000 lines long, we'd be able to follow this sort of thing much more easily.

Copy link
Member Author

@clokep clokep Feb 1, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, PyCharm noticed it and declared it unused. 😢

I can refactor this method a bit first if you'd like. I think moving the context calculation would be easy enough.

Copy link
Member

Choose a reason for hiding this comment

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

it would certainly be nice to refactor it if you have a few tuits. Suggest doing as a followup though.

Copy link
Member Author

Choose a reason for hiding this comment

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

I took a look at doing this and it quickly spiraled. Will do as a follow-up.

rank_map.update({r["event"].event_id: r["rank"] for r in results})

filtered_events = await search_filter.filter([r["event"] for r in results])
Expand Down