fedora-infra / fas

Fedora Account System
https://admin.fedoraproject.org/accounts
GNU General Public License v2.0
40 stars 50 forks source link

Issue #173: if the irc nick field is empty set it to None instead #175

Closed skrzepto closed 8 years ago

skrzepto commented 8 years ago

173

If the irc nick field is empty set it to None instead

pypingou commented 8 years ago

:+1: for me

pypingou commented 8 years ago

We may want to .strip() while at it, unless it's already done somewhere (and that might be true for more fields)

skrzepto commented 8 years ago

Forgot about that edge case, let me test locally to see if it faults.

pypingou commented 8 years ago

let me test locally

Unit-tests!! Ok I'm out :D

skrzepto commented 8 years ago

let me test locally

Unit-tests!! Ok I'm out :D

Soon :) still trying to fully understand how fas works

skrzepto commented 8 years ago

Looks like the other unique fields have some kind of client verification. I'll look into writing unittests now to this

laxathom commented 8 years ago

Have you looked at defined a default None here https://github.com/fedora-infra/fas/blob/FAS_3.0/fas/forms/people.py#L166 see if this work? This might avoid to add am if statement on the register.

skrzepto commented 8 years ago

@laxathom unfortunately the default None doesn't help at least doesn't pass the unit tests I wrote. But this change does

skrzepto commented 8 years ago

Any comments on this?

skrzepto commented 8 years ago

@bowlofeggs I applied you feedback, thanks :) are we good to merge?