Yeah, I wasn't a fan of that either. I like your suggestion. I've
implemented it and pushed the commit.

Jeff


On Mon, Mar 3, 2014 at 4:59 AM, Giorgos Logiotatidis <
notifications@github.com> wrote:

> In mozillians/phonebook/forms.py:
>
> > @@ -37,7 +37,11 @@ def clean(self):
> >          if type:
> >              validator = ExternalAccount.ACCOUNT_TYPES[type].get('validator')
> >              if validator:
> > -                cleaned_data['identifier'] = validator(identifier)
> > +                if validator.__name__ == 'validate_username_not_url':
>
> While this works perfectly, I don't like doing such special handling of a
> validator. Instead a suggest the following:
>
>    - clean() is actually first cleaning the username.
>    - validate_usename_not_url only validates with the username is a url
>
> if type:
>   url = ExternalAccount.ACCOUNT_TYPES[type].get('url')
>   if url and self.identifier.startswith('http'):
>       # try to cleanup identifier
>
>   if validator:
>       # validate
>
> Ideally we'd do all the validation / clean within the model which would
> make them useful for admin panel too, but our django version does not
> support errors on specific fields when validating in model's clean(), so
> nevermind.
>
> What do you think?
>
> --
> Reply to this email directly or view it on GitHub<https://github.com/mozilla/mozillians/pull/830/files#r10206725>
> .
>