feat: add searchScore field to pages #320
Conversation
dopry
force-pushed
the
feat/searchScore
branch
10 times, most recently
from
614f5b9 to
aaeb34d
Compare
| self.assertEquals(page_data[5]["searchScore"], None) | ||
|
|
||
| @unittest.skipIf( | ||
| connection.vendor != "postgresql", |
There was a problem hiding this comment.
what about MySQL? it should support scoring
There was a problem hiding this comment.
I guess what I am saying is this should be skipIf(connection.vendor == "sqlite") since below we have
if connection.vendor != "sqlite":
qs = qs.annotate_score("search_score")
There was a problem hiding this comment.
I'm not running mysql, so I don't know if the test would pass or fail or what search scores would be generated by it. The same with elastic search. My instinct says each search backend is going to return different scores and ordering. I feel like we should implement tests for each search backend as we implement testing support for that backend. Although setting this up to fail when a new search backend is added to our test matrix, would probably help ensure we don't overlook it when we add those. I was just focusing on the postgres case here. How do you think we should proceed?
There was a problem hiding this comment.
The more I thought about it, I agree with the test selection solely on sqlite. Although at some point in the future the test selection will need to consider both the search backend and database connection.
There was a problem hiding this comment.
@zerolab any other feedback on this, or can we work on getting it merged?
|
🎉 |
Add a searchScore property to the pages types.