Only index full names for Users with public profiles
Review Request #9924 — Created May 10, 2018 and submitted
Previously we were indexing the full name, which allowed users to search
by full name. However, if a user has marked their profile as private, we
do not want them to be searchable by their name.Additionally, this patch addresses two further issues:
The
show_profile
attribute was stored as aBooleanField
, but
there is a longstanding Haystack bug that results in boolean fields
always being returned from Whoosh asTrue
. This has been replaced with
a custom BooleanField with the correct behaviour.The
show_profile
as indexing the result ofUser.is_profile_visible()
,
which will always returnFalse
when there is no user argument
supplied. Instead, we now store theis_private
attribute of
Profile
directly on the index.Finally, the search API has been updated to strip the full name out of
results when using the database query backend (i.e., when either search
or on-the-fly indexing is disabled).
Ran unit tests.
Description | From | Last Updated |
---|---|---|
E303 too many blank lines (2) |
reviewbot | |
E303 too many blank lines (2) |
reviewbot | |
E303 too many blank lines (2) |
reviewbot | |
E303 too many blank lines (2) |
reviewbot | |
This should be stripped out for private profiles as well. |
chipx86 | |
This function returns a single unicode string, not a list. |
chipx86 | |
Backticks in summaries can render strangely. We should just show the text as-is. |
chipx86 | |
Missing "Returns". |
chipx86 | |
We don't want to use get_profile() in search indexes, because any users without a profile will end up triggering database … |
chipx86 | |
Same as above regarding backticks. |
chipx86 | |
This is missing a description. |
chipx86 | |
Same comment about backticks. |
chipx86 | |
Same regarding get_profile usage. |
chipx86 | |
Blank line between these. |
chipx86 | |
What license is that under? |
chipx86 | |
This bit would be faster as: try: return self.value_map[value] except KeyError: return bool(value) |
chipx86 | |
How about: if isinstance(instance, Profile): kwargs['sender'] = User instance = instance.user self.handle_save(...) |
chipx86 | |
I don't believe filters are allowed in a blocktrans. This may need more closer testing. Instead, I think you can … |
chipx86 | |
The e-mail address also needs to be wrapped in this. |
chipx86 | |
Can you print the resulting expression and make sure the grouping for the full name query ends up looking like … |
chipx86 | |
Can this be moved into the initial query = above? |
chipx86 | |
Missing the unicode_literals import. |
chipx86 |
- Commit:
-
56cf4940aefda8d12f405f4cf1303e56c0beeb8d8b3086e0ab6b9103fc4dfa7ff0aed2b45f987280
Checks run (2 succeeded)
- Change Summary:
-
Add search/fields.py
- Commit:
-
8b3086e0ab6b9103fc4dfa7ff0aed2b45f987280f04c5287bc45de7b705c6312e986b12d717283e8
Checks run (2 succeeded)
- Change Summary:
-
Fix actual indexed text
- Commit:
-
f04c5287bc45de7b705c6312e986b12d717283e8a1549989c90a625a90c5d0ec5e005e68a41fd02b
- Change Summary:
-
Fix actual indexed text
- Commit:
-
a1549989c90a625a90c5d0ec5e005e68a41fd02b7d8a61dcaa6cd7f183c95330f332d1a3554dc45f
- Diff:
-
Revision 5 (+330 -14)
- Change Summary:
-
Post the correct change
- Diff:
-
Revision 6 (+330 -14)
- Change Summary:
-
Rebasing. Replace more usages.
- Depends On:
-
- Commit:
7d8a61dcaa6cd7f183c95330f332d1a3554dc45f02d3c96185581167324768eedba1b7a99ee0f4c3- Diff:
Revision 7 (+353 -21)
Checks run (2 succeeded)
flake8 passed.JSHint passed.
-
-
-
-
-
-
We don't want to use
get_profile()
in search indexes, because any users without a profile will end up triggering database changes. This will be slow and disruptive. Instead, we need to see if a profile was fetched for the user at all (query cache has a value) and operate based on that.We should probably do that in a private method in this class, or maybe add a
cached_only=
argument toget_profile()
? -
-
-
-
-
-
-
-
How about:
if isinstance(instance, Profile): kwargs['sender'] = User instance = instance.user self.handle_save(...)
-
I don't believe filters are allowed in a
blocktrans
. This may need more closer testing.Instead, I think you can do:
and result.author|default:result.username as author
. -
-
Can you print the resulting expression and make sure the grouping for the full name query ends up looking like
((a | b) & is_private=True)
? I want to be sure the grouping is going to be future-proof. -
- Change Summary:
-
Addressed issues.
- Commit:
-
02d3c96185581167324768eedba1b7a99ee0f4c3542e8a786781cc32e36cb52572154fe40b1effb3
- Diff:
-
Revision 8 (+444 -23)
Checks run (2 succeeded)
- Change Summary:
-
Remove unused import.
- Commit:
-
542e8a786781cc32e36cb52572154fe40b1effb3048c2f2b220b66ffc81c5f11eca9c3981c87f0ae
- Diff:
-
Revision 9 (+444 -24)
Checks run (2 succeeded)
- Change Summary:
-
Addressed all remaining feedback.
- Commit:
-
048c2f2b220b66ffc81c5f11eca9c3981c87f0aecef8f850146c37290c39889af3d142e75f2df157
- Diff:
-
Revision 10 (+441 -24)
Checks run (2 succeeded)
- Change Summary:
-
Fix unit tests that failed as a result of changes to /r/9930
- Commit:
-
cef8f850146c37290c39889af3d142e75f2df157ed0c3ef6676d119029937ec28cdc25f17c083d09
- Diff:
-
Revision 11 (+445 -26)