AndyRussG has uploaded a new change for review.
https://gerrit.wikimedia.org/r/154482
Change subject: Add validators module and use it in cohort_upload
......................................................................
Add validators module and use it in cohort_upload
Change-Id: Ic1c2aeb38d812ab9bdf016d87bc6090d8f3eba78
---
M tests/test_controllers/test_cohorts.py
M wikimetrics/controllers/cohorts.py
M wikimetrics/forms/cohort_upload.py
M wikimetrics/forms/fields.py
A wikimetrics/forms/validators.py
5 files changed, 91 insertions(+), 29 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/analytics/wikimetrics
refs/changes/82/154482/1
diff --git a/tests/test_controllers/test_cohorts.py
b/tests/test_controllers/test_cohorts.py
index 5181de7..739349e 100644
--- a/tests/test_controllers/test_cohorts.py
+++ b/tests/test_controllers/test_cohorts.py
@@ -379,7 +379,7 @@
))
assert_equal(response.status_code, 200)
assert_true(response.data.find('<h3>Create a Cohort') >= 0)
- assert_true(response.data.find('That Cohort name is already taken') >=
0)
+ assert_true(response.data.find('This cohort name is taken.') >= 0)
def test_upload_works(self):
response = self.app.post('/cohorts/upload', data=dict(
diff --git a/wikimetrics/controllers/cohorts.py
b/wikimetrics/controllers/cohorts.py
index f7ff8ac..ca20507 100644
--- a/wikimetrics/controllers/cohorts.py
+++ b/wikimetrics/controllers/cohorts.py
@@ -185,10 +185,6 @@
if not form.validate():
flash('Please fix validation problems.', 'warning')
- # NOTE: The following two lines will be removed in validation
refactor
- elif g.cohort_service.get_cohort_by_name(db.get_session(),
form.name.data):
- flash('That Cohort name is already taken.', 'warning')
-
else:
form.parse_records()
vc = ValidateCohort.from_upload(form, current_user.id)
@@ -210,6 +206,10 @@
@app.route('/cohorts/validate/name')
def validate_cohort_name_allowed():
+ """
+ Returns true if there are no other cohorts with this name. Remote call is
+ set up in static/js/cohortUpload.js.
+ """
name = request.args.get('name')
session = db.get_session()
available = g.cohort_service.get_cohort_by_name(session, name) is None
@@ -218,6 +218,10 @@
@app.route('/cohorts/validate/project')
def validate_cohort_project_allowed():
+ """
+ Returns true if a valid project is provided. Remote call is set up in
+ static/js/cohortUpload.js.
+ """
project = request.args.get('project')
valid = project in db.get_project_host_map()
return json.dumps(valid)
diff --git a/wikimetrics/forms/cohort_upload.py
b/wikimetrics/forms/cohort_upload.py
index 5bffe1f..63ef404 100644
--- a/wikimetrics/forms/cohort_upload.py
+++ b/wikimetrics/forms/cohort_upload.py
@@ -3,8 +3,10 @@
from wtforms.validators import Required
from wikimetrics.utils import parse_username
-from fields import RequiredIfNot
from secure_form import WikimetricsSecureForm
+from validators import (
+ CohortNameUnused, CohortNameLegalCharacters, ProjectExists, RequiredIfNot
+)
class CohortUpload(WikimetricsSecureForm):
@@ -16,10 +18,15 @@
when we read text fields (user names or user ids) from a csv file those
would be
returned as strings via python csv module.
"""
- name = StringField([Required()])
+ name = StringField('Name',
+ [Required(), CohortNameUnused(),
+ CohortNameLegalCharacters()])
+
description = TextAreaField()
- project = StringField('Default Project', [Required()])
- csv = FileField('Upload File',
[RequiredIfNot('paste_usernames')])
+ project = StringField('Default Project',
+ [Required(), ProjectExists()])
+
+ csv = FileField('Upload File',
[RequiredIfNot('paste_username')])
paste_username = TextAreaField('Paste Usernames',
[RequiredIfNot('csv')])
validate_as_user_ids = RadioField('Validate As', [Required()], choices=[
('True', 'User Ids (Numbers found in the user_id column of the user
table)'),
diff --git a/wikimetrics/forms/fields.py b/wikimetrics/forms/fields.py
index bb016e9..7f60466 100644
--- a/wikimetrics/forms/fields.py
+++ b/wikimetrics/forms/fields.py
@@ -1,6 +1,5 @@
from datetime import datetime, date, time
from wtforms import Field, BooleanField, DateField, DateTimeField
-from wtforms.validators import Required, ValidationError
from wtforms.widgets import TextInput
@@ -97,22 +96,3 @@
def report_invalid(self):
self.data = None
raise ValueError(self.gettext('Not a valid datetime value'))
-
-
-class RequiredIfNot(Required):
- """
- A validator which makes a field mutually exclusive with another
- """
-
- def __init__(self, other_field_name, *args, **kwargs):
- self.other_field_name = other_field_name
- super(RequiredIfNot, self).__init__(*args, **kwargs)
-
- def __call__(self, form, field):
- other_field = form._fields.get(self.other_field_name)
- other_field_set = other_field and bool(other_field.data)
- field_set = field and bool(field.data)
- if other_field_set == field_set:
- raise ValidationError('Please use either {0} or {1}'.format(
- other_field.label.text, field.label.text
- ))
diff --git a/wikimetrics/forms/validators.py b/wikimetrics/forms/validators.py
new file mode 100644
index 0000000..1c9adc0
--- /dev/null
+++ b/wikimetrics/forms/validators.py
@@ -0,0 +1,71 @@
+import re
+from wtforms import ValidationError
+from wtforms.validators import Required
+
+from wikimetrics.configurables import db
+from wikimetrics.api import CohortService
+
+
+class CohortNameUnused(object):
+ """
+ Checks that a cohort name is unused.
+
+ Note: the same message appears in static/js/cohortUpload.js, and the same
+ logic is in wikimetrics.controllers.cohorts.validate_cohort_name_allowed().
+ This is because the upload form is validated on both the client and the
server.
+
+ TODO: Maybe add this constraint to the model, too?
+ """
+
+ def __call__(self, form, field):
+ session = db.get_session()
+
+ if CohortService().get_cohort_by_name(session, field.data) is not None:
+ raise ValidationError('This cohort name is taken.')
+
+
+class CohortNameLegalCharacters(object):
+ """
+ Checks that a cohort name only contains allowed characters.
+ Note: the same logic and message also appear in static/js/cohortUpload.js.
+ This is because the upload form is validated on both the client and the
server.
+ """
+
+ def __call__(self, form, field):
+
+ if not re.match(r"^[0-9_\-A-Za-z ]*$", field.data):
+ raise ValidationError('Cohort names should only contain letters, '
+ 'numbers, spaces, dashes, and underscores.')
+
+
+class ProjectExists(object):
+ """
+ Checks that a project exists.
+ Note: the same message appears in static/js/cohortUpload.js, and the same
+ logic is in
wikimetrics.controllers.cohorts.validate_cohort_project_allowed().
+ This is because the upload form is validated on both the client and the
server.
+ """
+
+ def __call__(self, form, field):
+
+ if field.data not in db.get_project_host_map():
+ raise ValidationError('That project does not exist.')
+
+
+class RequiredIfNot(Required):
+ """
+ A validator which makes a field mutually exclusive with another
+ """
+
+ def __init__(self, other_field_name, *args, **kwargs):
+ self.other_field_name = other_field_name
+ super(RequiredIfNot, self).__init__(*args, **kwargs)
+
+ def __call__(self, form, field):
+ other_field = form._fields.get(self.other_field_name)
+ other_field_set = other_field and bool(other_field.data)
+ field_set = field and bool(field.data)
+ if other_field_set == field_set:
+ raise ValidationError('Please use either {0} or {1}'.format(
+ other_field.label.text, field.label.text
+ ))
--
To view, visit https://gerrit.wikimedia.org/r/154482
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic1c2aeb38d812ab9bdf016d87bc6090d8f3eba78
Gerrit-PatchSet: 1
Gerrit-Project: analytics/wikimetrics
Gerrit-Branch: master
Gerrit-Owner: AndyRussG <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
