Skip to content

Commit

Permalink
Merge pull request #1630 from AdrianGaudebert/930086-not-operator
Browse files Browse the repository at this point in the history 
Fixes bug 930086 - Not operator is accepted everywhere.
  • Loading branch information
@adngdb
adngdb committed Nov 4, 2013
2 parents 3cce06f + bdc71bd commit 12729f9
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 77 deletions.
15 changes: 14 additions & 1 deletion webapp-django/crashstats/supersearch/form_fields.py
View file
Expand Up @@ -57,6 +57,19 @@ def value_to_string(self, value):
return unicode(value)


class MultipleValueField(forms.MultipleChoiceField):
"""This is the same as a MultipleChoiceField except choices don't matter
as no validation will be done. The advantage is that it will take a list
as input, and output a list as well, allowing several values to be passed.
In the end, it's like a CharField that can take a list of values. It is
used as the default field for supersearch.
"""

def validate(self, value):
pass


class MultiplePrefixedValueField(PrefixedField):
"""Special field that uses a SelectMultiple widget to deal with multiple
values. """
Expand Down Expand Up @@ -101,7 +114,7 @@ def value_to_string(self, value):
return value


class StringField(forms.CharField):
class StringField(MultipleValueField):
"""A CharField with a different name, to be considered as a string
by the dynamic_form.js library. This basically enables string operators
on that field ("contains", "starts with"... ).
Expand Down
45 changes: 22 additions & 23 deletions webapp-django/crashstats/supersearch/forms.py
View file
@@ -1,6 +1,5 @@
from django import forms

from crashstats.crashstats.form_fields import SignatureField
from crashstats.supersearch import form_fields


Expand All @@ -13,35 +12,35 @@
class SearchForm(forms.Form):
'''Handle the data populating the search form. '''

address = forms.CharField(required=False)
app_notes = forms.CharField(required=False)
address = form_fields.MultipleValueField(required=False)
app_notes = form_fields.MultipleValueField(required=False)
build_id = form_fields.IntegerField(required=False)
cpu_info = form_fields.StringField(required=False)
cpu_name = forms.CharField(required=False)
cpu_name = form_fields.MultipleValueField(required=False)
date = form_fields.DateTimeField(required=False)
distributor = forms.CharField(required=False)
distributor_version = forms.CharField(required=False)
distributor = form_fields.MultipleValueField(required=False)
distributor_version = form_fields.MultipleValueField(required=False)
dump = form_fields.StringField(required=False)
flash_version = forms.CharField(required=False)
flash_version = form_fields.MultipleValueField(required=False)
install_age = form_fields.IntegerField(required=False)
java_stack_trace = forms.CharField(required=False)
java_stack_trace = form_fields.MultipleValueField(required=False)
last_crash = form_fields.IntegerField(required=False)
platform = forms.MultipleChoiceField(required=False)
platform_version = forms.CharField(required=False)
plugin_name = forms.CharField(required=False)
plugin_filename = forms.CharField(required=False)
plugin_version = forms.CharField(required=False)
processor_notes = forms.CharField(required=False)
product = forms.MultipleChoiceField(required=False)
productid = forms.CharField(required=False)
platform = form_fields.MultipleValueField(required=False)
platform_version = form_fields.MultipleValueField(required=False)
plugin_name = form_fields.MultipleValueField(required=False)
plugin_filename = form_fields.MultipleValueField(required=False)
plugin_version = form_fields.MultipleValueField(required=False)
processor_notes = form_fields.MultipleValueField(required=False)
product = form_fields.MultipleValueField(required=False)
productid = form_fields.MultipleValueField(required=False)
reason = form_fields.StringField(required=False)
release_channel = forms.CharField(required=False)
signature = SignatureField(required=False) # CharField
topmost_filenames = forms.CharField(required=False)
release_channel = form_fields.MultipleValueField(required=False)
signature = form_fields.StringField(required=False)
topmost_filenames = form_fields.MultipleValueField(required=False)
uptime = form_fields.IntegerField(required=False)
user_comments = forms.CharField(required=False)
version = forms.MultipleChoiceField(required=False)
winsock_lsp = forms.CharField(required=False)
user_comments = form_fields.MultipleValueField(required=False)
version = form_fields.MultipleValueField(required=False)
winsock_lsp = form_fields.MultipleValueField(required=False)

# TODO: This doesn't work and needs to be fixed.
# Pending on https://bugzilla.mozilla.org/show_bug.cgi?id=919559
Expand Down Expand Up @@ -104,7 +103,7 @@ def get_fields_list(self):
value_type = 'number'
elif field_type == 'date':
value_type = 'date'
elif isinstance(field, (SignatureField, form_fields.StringField)):
elif isinstance(field, form_fields.StringField):
value_type = 'string'
else:
value_type = 'enum'
Expand Down
36 changes: 19 additions & 17 deletions webapp-django/crashstats/supersearch/static/supersearch/js/lib/dynamic_form.js
View file
Expand Up @@ -2,6 +2,9 @@
(function ($) {
'use strict';

// String used to separate values in select2 fields.
var VALUES_SEPARATOR = '|||';

/**
* Create a new dynamic form or run an action on an existing form.
*
Expand All @@ -12,13 +15,13 @@
*
* If `action` is none of those values, create a new dynamic form where
* the first argument is the URL of the JSON file describing the fields
* and the optiional second argument is an object containing the initial
* and the optional second argument is an object containing the initial
* form values.
*/
function dynamicForm(action, initialParams, container_id) {
function dynamicForm(action, initialParams, containerId) {
var form = this;
initialParams = initialParams || null;
container_id = container_id || null;
containerId = containerId || null;

if (action === 'newLine' || action === 'getParams' || action === 'setParams') {
var dynamic = form.data('dynamic');
Expand Down Expand Up @@ -51,8 +54,8 @@
var lastFieldLineId = 0;
var container = form;

if (container_id) {
container = $(container_id, form);
if (containerId) {
container = $(containerId, form);
}

// first display a loader while the fields data is being downloaded
Expand All @@ -74,7 +77,7 @@

var OPERATORS = {
'has': 'has terms',
'!has': 'does not have terms',
'!': 'does not have terms',
'=': 'is',
'!=': 'is not',
'~': 'contains',
Expand All @@ -93,7 +96,7 @@

// Order matters here, the first operator will be used as the default
// value when no operator is passed for a field.
var OPERATORS_BASE = ['has', '!has'];
var OPERATORS_BASE = ['has', '!'];
var OPERATORS_RANGE = ['>', '>=', '<', '<='];
var OPERATORS_REGEX = ['~', '=', '$', '^', '!=', '!~', '!$', '!^'];
var OPERATORS_EXISTENCE = ['__null__', '!__null__'];
Expand Down Expand Up @@ -139,18 +142,15 @@
var filter = filters[f];
var value = null;

if (filter.operator === 'has') {
value = filter.value.split(',');
if (filter.operator === OPERATORS_BASE[0]) {
value = filter.value.split(VALUES_SEPARATOR);
}
else if (filter.operator === '!has') {
value = filter.value.split(',');
else {
value = filter.value.split(VALUES_SEPARATOR);
for (var i = value.length - 1; i >= 0; i--) {
value[i] = '!' + value[i];
value[i] = filter.operator + value[i];
}
}
else {
value = filter.operator + filter.value;
}

if (params[filter.field] !== undefined) {
if (!Array.isArray(params[filter.field])) {
Expand Down Expand Up @@ -245,7 +245,7 @@
*/
function getOperatorFromValue(value) {
// These operators need to be sorted by decreasing size.
var operators = ['__null__', '<=', '>=', '~', '$', '^', '=', '<', '>'];
var operators = ['__null__', '<=', '>=', '~', '$', '^', '=', '<', '>', '!'];
var prefix = '!';

for (var i = 0, l = operators.length; i < l; i++) {
Expand Down Expand Up @@ -445,7 +445,9 @@
}
this.line.append(this.valueInput);

var selectParams = {};
var selectParams = {
'separator': VALUES_SEPARATOR
};
if (field.extendable !== false) {
selectParams.tags = values;
}
Expand Down
42 changes: 6 additions & 36 deletions webapp-django/crashstats/supersearch/tests/test_forms.py
View file
Expand Up @@ -67,34 +67,19 @@ def get_new_form(data):
})
ok_(not form.is_valid()) # expect values as lists

form = get_new_form({
'product': ['SomeUnkownProduct']
})
ok_(not form.is_valid()) # invalid product

form = get_new_form({
'version': ['invalidVersion']
})
ok_(not form.is_valid()) # invalid version

form = get_new_form({
'platform': ['winux']
})
ok_(not form.is_valid()) # invalid platform

form = get_new_form({
'date': '2012-01-16 12:23:34324234'
})
ok_(not form.is_valid()) # invalid datetime

# Test all valid data
form = get_new_form({
'signature': '~sig',
'signature': ['~sig'],
'product': ['WaterWolf', 'SeaMonkey', 'NightTrain'],
'version': ['20.0'],
'platform': ['Linux', 'Mac OS X'],
'date': ['>2012-01-16 12:23:34', '<=2013-01-16 12:23:34'],
'reason': 'some reason',
'reason': ['some reason'],
'build_id': '<20200101344556',
})
ok_(form.is_valid(), form.errors)
Expand Down Expand Up @@ -122,37 +107,22 @@ def get_new_form(data):
})
ok_(not form.is_valid()) # expect values as lists

form = get_new_form({
'product': ['SomeUnkownProduct']
})
ok_(not form.is_valid()) # invalid product

form = get_new_form({
'version': ['invalidVersion']
})
ok_(not form.is_valid()) # invalid version

form = get_new_form({
'platform': ['winux']
})
ok_(not form.is_valid()) # invalid platform

form = get_new_form({
'date': '2012-01-16 12:23:34324234'
})
ok_(not form.is_valid()) # invalid datetime

# Test all valid data
form = get_new_form({
'signature': '~sig',
'signature': ['~sig'],
'product': ['WaterWolf', 'SeaMonkey', 'NightTrain'],
'version': ['20.0'],
'platform': ['Linux', 'Mac OS X'],
'date': ['>2012-01-16 12:23:34', '<=2013-01-16 12:23:34'],
'reason': 'some reason',
'reason': ['some reason'],
'build_id': '<20200101344556',
'email': '^mail.com',
'url': '$http://',
'email': ['^mail.com'],
'url': ['$http://'],
})
ok_(form.is_valid(), form.errors)

Expand Down
41 changes: 41 additions & 0 deletions webapp-django/crashstats/supersearch/tests/test_views.py
View file
Expand Up @@ -398,6 +398,47 @@ def mocked_get(url, **options):
ok_('Version' in response.content)
ok_('1.0' in response.content)

@mock.patch('requests.post')
@mock.patch('requests.get')
def test_search_results_parameters(self, rget, rpost):
def mocked_post(**options):
assert 'bugs' in options['url'], options['url']
return Response("""
{"hits": [],
"total": 0
}
""")

def mocked_get(url, **options):
assert 'supersearch' in url

# Verify that all expected parameters are in the URL.
ok_('product/WaterWolf%2BNightTrain' in url)
ok_('address/0x0%2B0xa' in url)
ok_('reason/%5Ehello%2B%24thanks' in url)
ok_('java_stack_trace/Exception' in url)

return Response("""{
"hits": [],
"facets": "",
"total": 0
}""")

rpost.side_effect = mocked_post
rget.side_effect = mocked_get

url = reverse('supersearch.search_results')

response = self.client.get(
url, {
'product': ['WaterWolf', 'NightTrain'],
'address': ['0x0', '0xa'],
'reason': ['^hello', '$thanks'],
'java_stack_trace': 'Exception',
}
)
eq_(response.status_code, 200)

@mock.patch('requests.post')
@mock.patch('requests.get')
def test_search_results_pagination(self, rget, rpost):
Expand Down

0 comments on commit 12729f9

Please sign in to comment.