diff --git a/contentcuration/contentcuration/forms.py b/contentcuration/contentcuration/forms.py
index d9dc781f61..8e9320d85f 100644
--- a/contentcuration/contentcuration/forms.py
+++ b/contentcuration/contentcuration/forms.py
@@ -1,5 +1,4 @@
import json
-from builtins import object
from django import forms
from django.conf import settings
@@ -7,6 +6,7 @@
from django.contrib.auth.forms import UserChangeForm
from django.contrib.auth.forms import UserCreationForm
from django.core import signing
+from django.core.exceptions import ValidationError
from django.db.models import Q
from django.template.loader import render_to_string
@@ -16,23 +16,16 @@
REGISTRATION_SALT = getattr(settings, 'REGISTRATION_SALT', 'registration')
-class ExtraFormMixin(object):
-
- def check_field(self, field, error):
- if not self.cleaned_data.get(field):
- self.errors[field] = self.error_class()
- self.add_error(field, error)
- return False
- return self.cleaned_data.get(field)
-
-
# LOGIN/REGISTRATION FORMS
#################################################################
-class RegistrationForm(UserCreationForm, ExtraFormMixin):
+class RegistrationForm(UserCreationForm):
+ CODE_ACCOUNT_ACTIVE = 'account_active'
+ CODE_ACCOUNT_INACTIVE = 'account_inactive'
+
first_name = forms.CharField(required=True)
last_name = forms.CharField(required=True)
- email = forms.CharField(required=True)
- password1 = forms.CharField(required=True)
+ email = forms.EmailField(required=True)
+ password1 = forms.CharField(required=True, min_length=8)
password2 = forms.CharField(required=True)
uses = forms.CharField(required=True)
other_use = forms.CharField(required=False)
@@ -45,22 +38,18 @@ class RegistrationForm(UserCreationForm, ExtraFormMixin):
locations = forms.CharField(required=True)
def clean_email(self):
- email = self.cleaned_data['email'].strip().lower()
- if User.objects.filter(Q(is_active=True) | Q(deleted=True), email__iexact=email).exists():
- raise UserWarning
+ # ensure email is lower case
+ email = self.cleaned_data["email"].strip().lower()
+ user_qs = User.objects.filter(email__iexact=email)
+ if user_qs.exists():
+ if user_qs.filter(Q(is_active=True) | Q(deleted=True)).exists():
+ raise ValidationError("Account already active", code=self.CODE_ACCOUNT_ACTIVE)
+ else:
+ raise ValidationError("Already registered.", code=self.CODE_ACCOUNT_INACTIVE)
return email
- def clean(self):
- super(RegistrationForm, self).clean()
-
- # Errors should be caught on the frontend
- # or a warning should be thrown if the account exists
- self.errors.clear()
- return self.cleaned_data
-
def save(self, commit=True):
- user = super(RegistrationForm, self).save(commit=commit)
- user.set_password(self.cleaned_data["password1"])
+ user = super(RegistrationForm, self).save(commit=False)
user.first_name = self.cleaned_data["first_name"]
user.last_name = self.cleaned_data["last_name"]
user.information = {
@@ -165,7 +154,7 @@ def save(self, user):
return user
-class StorageRequestForm(forms.Form, ExtraFormMixin):
+class StorageRequestForm(forms.Form):
# Nature of content
storage = forms.CharField(required=True)
kind = forms.CharField(required=True)
@@ -194,7 +183,7 @@ class Meta:
"audience", "import_count", "location", "uploading_for", "organization_type", "time_constraint", "message")
-class IssueReportForm(forms.Form, ExtraFormMixin):
+class IssueReportForm(forms.Form):
operating_system = forms.CharField(required=True)
browser = forms.CharField(required=True)
channel = forms.CharField(required=False)
@@ -204,7 +193,7 @@ class Meta:
fields = ("operating_system", "browser", "channel", "description")
-class DeleteAccountForm(forms.Form, ExtraFormMixin):
+class DeleteAccountForm(forms.Form):
email = forms.CharField(required=True)
def __init__(self, user, *args, **kwargs):
@@ -214,5 +203,5 @@ def __init__(self, user, *args, **kwargs):
def clean_email(self):
email = self.cleaned_data['email'].strip().lower()
if self.user.is_admin or self.user.email.lower() != self.cleaned_data['email']:
- raise UserWarning
+ raise ValidationError("Not allowed")
return email
diff --git a/contentcuration/contentcuration/frontend/accounts/pages/Create.vue b/contentcuration/contentcuration/frontend/accounts/pages/Create.vue
index ba9f8bf338..6e6562ad5b 100644
--- a/contentcuration/contentcuration/frontend/accounts/pages/Create.vue
+++ b/contentcuration/contentcuration/frontend/accounts/pages/Create.vue
@@ -31,31 +31,40 @@
v-model="form.first_name"
maxlength="100"
counter
- :label="$tr('firstNameLabel')"
autofocus
+ :label="$tr('firstNameLabel')"
+ :error-messages="errors.first_name"
+ @input="resetErrors('first_name')"
/>
@@ -200,6 +209,7 @@
import Checkbox from 'shared/views/form/Checkbox';
import { policies } from 'shared/constants';
import DropdownWrapper from 'shared/views/form/DropdownWrapper';
+ import commonStrings from 'shared/translator';
export default {
name: 'Create',
@@ -219,7 +229,6 @@
return {
valid: true,
registrationFailed: false,
- emailErrors: [],
form: {
first_name: '',
last_name: '',
@@ -237,6 +246,13 @@
accepted_policy: false,
accepted_tos: false,
},
+ errors: {
+ first_name: [],
+ last_name: [],
+ email: [],
+ password1: [],
+ password2: [],
+ },
};
},
computed: {
@@ -247,6 +263,9 @@
passwordConfirmRules() {
return [value => (this.form.password1 === value ? true : this.$tr('passwordMatchMessage'))];
},
+ passwordValidationRules() {
+ return [value => (value.length >= 8 ? true : this.$tr('passwordValidationMessage'))];
+ },
acceptedAgreement: {
get() {
return this.form.accepted_tos && this.form.accepted_policy;
@@ -294,10 +313,12 @@
];
},
usageRules() {
- return [() => (this.form.uses.length ? true : this.$tr('fieldRequiredMessage'))];
+ /* eslint-disable-next-line kolibri/vue-no-undefined-string-uses */
+ return [() => (this.form.uses.length ? true : commonStrings.$tr('fieldRequired'))];
},
locationRules() {
- return [() => (this.form.locations.length ? true : this.$tr('fieldRequiredMessage'))];
+ /* eslint-disable-next-line kolibri/vue-no-undefined-string-uses */
+ return [() => (this.form.locations.length ? true : commonStrings.$tr('fieldRequired'))];
},
sources() {
return sources;
@@ -359,7 +380,8 @@
];
},
sourceRules() {
- return [() => (this.form.source.length ? true : this.$tr('fieldRequiredMessage'))];
+ /* eslint-disable-next-line kolibri/vue-no-undefined-string-uses */
+ return [() => (this.form.source.length ? true : commonStrings.$tr('fieldRequired'))];
},
clean() {
return data => {
@@ -438,8 +460,27 @@
.catch(error => {
if (error.message === 'Network Error') {
this.$refs.top.scrollIntoView({ behavior: 'smooth' });
+ } else if (error.response.status === 400) {
+ for (const field of error.response.data) {
+ if (!Object.prototype.hasOwnProperty.call(this.errors, field)) {
+ continue;
+ }
+ let message = '';
+ switch (field) {
+ case 'password1':
+ message = this.$tr('passwordValidationMessage');
+ break;
+ default:
+ /* eslint-disable-next-line kolibri/vue-no-undefined-string-uses */
+ message = commonStrings.$tr('fieldHasError');
+ break;
+ }
+ this.errors[field] = [message];
+ }
+ this.registrationFailed = true;
+ this.valid = false;
} else if (error.response.status === 403) {
- this.emailErrors = [this.$tr('emailExistsMessage')];
+ this.errors.email = [this.$tr('emailExistsMessage')];
} else if (error.response.status === 405) {
this.$router.push({ name: 'AccountNotActivated' });
} else {
@@ -452,12 +493,14 @@
}
return Promise.resolve();
},
+ resetErrors(field) {
+ this.errors[field] = [];
+ },
},
$trs: {
backToLoginButton: 'Sign in',
createAnAccountTitle: 'Create an account',
- fieldRequiredMessage: 'Field is required',
errorsMessage: 'Please fix the errors below',
registrationFailed: 'There was an error registering your account. Please try again',
registrationFailedOffline:
@@ -470,6 +513,7 @@
passwordLabel: 'Password',
confirmPasswordLabel: 'Confirm password',
passwordMatchMessage: "Passwords don't match",
+ passwordValidationMessage: 'Password should be at least 8 characters long',
// Usage question
usageLabel: 'How do you plan on using Kolibri Studio (check all that apply)',
diff --git a/contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js b/contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js
index f2a201b560..143520329b 100644
--- a/contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js
+++ b/contentcuration/contentcuration/frontend/accounts/pages/__tests__/create.spec.js
@@ -17,8 +17,8 @@ const defaultData = {
first_name: 'Test',
last_name: 'User',
email: 'test@test.com',
- password1: 'pass',
- password2: 'pass',
+ password1: 'tester123',
+ password2: 'tester123',
uses: ['tagging'],
storage: '',
other_use: '',
@@ -126,6 +126,11 @@ describe('create', () => {
expect(register).not.toHaveBeenCalled();
});
});
+ it('should fail if password1 is too short', () => {
+ const wrapper = makeWrapper({ password1: '123' });
+ wrapper.vm.submit();
+ expect(register).not.toHaveBeenCalled();
+ });
it('should fail if password1 and password2 do not match', () => {
const wrapper = makeWrapper({ password1: 'some other password' });
wrapper.vm.submit();
@@ -155,7 +160,7 @@ describe('create', () => {
it('should say account with email already exists if register returns a 403', async () => {
wrapper.setMethods({ register: makeFailedPromise(403) });
await wrapper.vm.submit();
- expect(wrapper.vm.emailErrors).toHaveLength(1);
+ expect(wrapper.vm.errors.email).toHaveLength(1);
});
it('should say account has not been activated if register returns 405', async () => {
wrapper.setMethods({ register: makeFailedPromise(405) });
diff --git a/contentcuration/contentcuration/frontend/shared/translator.js b/contentcuration/contentcuration/frontend/shared/translator.js
index b52eecaf59..7aac180a36 100644
--- a/contentcuration/contentcuration/frontend/shared/translator.js
+++ b/contentcuration/contentcuration/frontend/shared/translator.js
@@ -2,6 +2,7 @@ import { createTranslator } from 'shared/i18n';
const MESSAGES = {
fieldRequired: 'This field is required',
+ fieldHasError: 'This field has an error',
titleRequired: 'Title is required',
licenseRequired: 'License is required',
copyrightHolderRequired: 'Copyright holder is required',
diff --git a/contentcuration/contentcuration/frontend/shared/views/form/EmailField.vue b/contentcuration/contentcuration/frontend/shared/views/form/EmailField.vue
index fe0e641c42..ad28d0b91e 100644
--- a/contentcuration/contentcuration/frontend/shared/views/form/EmailField.vue
+++ b/contentcuration/contentcuration/frontend/shared/views/form/EmailField.vue
@@ -17,6 +17,8 @@