-
Notifications
You must be signed in to change notification settings - Fork 223
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.
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
bug-1928934: fix super search fields in signature report forms #6786
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,7 +92,7 @@ <h2 id="param-_results_offset">_results_offset</h2> | |
|
||
<p class="description"> | ||
Use this parameter to get a large number of results instead of setting | ||
an arbitraty big <code>_results_number</code> value. Run queries in a | ||
an arbitrary big <code>_results_number</code> value. Run queries in a | ||
loop, incrementing this by the value of <code>_results_number</code>, | ||
and concatenate the content of the <code>hits</code> key. | ||
</p> | ||
|
@@ -146,7 +146,7 @@ <h2 id="param-_aggs.*">_aggs.*</h2> | |
run an aggregation on <code>field_1</code>, then for each bucket of | ||
that aggregation, it will aggregate on <code>field_2</code>, and then | ||
for each bucket of that sub-aggregation, it will aggregate on | ||
<code>field_3</code>. Theoratically, we could have any number of | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Theoradically should be a word. It's not misspelled that way here, but it made me think a bit about it. |
||
<code>field_3</code>. Theoretically, we could have any number of | ||
levels of aggregations, but in practice we only allow all "level 1" | ||
aggregations, and a few "level 2" and "level 3". | ||
</p> | ||
|
@@ -300,6 +300,9 @@ <h2 id="param-{{ filter.name }}">{{ filter.name }}</h2> | |
{% endfor %} | ||
</p> | ||
|
||
{# NOTE(willkg): we use permissions_needed so it matches the | ||
protected crash schema rather than the webapp permissions which | ||
users won't understand #} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this note here so someone doesn't think this is a mistake. This will cause the data dictionary to show "public" and "protected" (processed crash schema permission terms) rather than "" and "crashstats.view_pii" (webapp-centric terms). The latter set of permissions are an internal implementation detail using terms we don't use in our documentation. |
||
{% if filter.permissions_needed %} | ||
<p class="permissions">{{ filter.permissions_needed | join(', ') }}</p> | ||
{% endif %} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,10 +9,12 @@ | |
|
||
import pyquery | ||
|
||
from django.contrib.auth.models import AnonymousUser | ||
from django.urls import reverse | ||
from django.utils.encoding import smart_str | ||
|
||
from crashstats.crashstats import models | ||
from crashstats.signature.views import get_fields | ||
from socorro.lib.libdatetime import date_to_string, utc_now | ||
from socorro.lib.libooid import create_new_ooid, date_from_ooid | ||
|
||
|
@@ -661,3 +663,16 @@ def test_signature_bugzilla(self, client, db, es_helper): | |
< content.find("Related Crash Signatures") | ||
< content.find("Bugs for <code>OOM | small</code>") | ||
) | ||
|
||
def test_get_fields(self, db, user_helper): | ||
# Create anonymous user and get fields | ||
user = AnonymousUser() | ||
fields = get_fields(user) | ||
len_public_fields = len(fields) | ||
assert len(fields) > 0 | ||
|
||
# Create user with protected data access, get fields, and make sure there are | ||
# more of them than if the user didn't have protected data access | ||
user = user_helper.create_protected_user() | ||
fields = get_fields(user) | ||
assert len(fields) > len_public_fields | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Super search fields change over time, so I didn't want to create a test we had to update periodically. Instead, this asserts there are fields in the case where the user doesn't have protected data access and asserts that if the user does have protected data access, the user sees more fields than if the user didn't. That feels like a good enough set of assertions that aren't fragile. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty arbitraty sometimes.