We use full-path imports (see below), let's follow this style here as well, and since it's a pagure module, it should be in the next block of imports with the other imports from pagure

Did you see: http://wtforms.simplecodes.com/docs/0.6.1/validators.html#wtforms.validators.Email ?

This is a select field, so I'm not sure these checks are necessary, did you really have a problem with them? If so that sounds like a bug in our code

We use full-path imports (see below), let's follow this style here as well, and since it's a pagure module, it should be in the next block of imports with the other imports from pagure

done

Did you see: http://wtforms.simplecodes.com/docs/0.6.1/validators.html#wtforms.validators.Email ?

The problem with their email validator is, that they assume it's a string (as coming from a form, not any value type as coming from a json API call).
Also, the error messages were unclear - too general. Are you ok with this custom one, or should I return to the built-in one?

This is a select field, so I'm not sure these checks are necessary, did you really have a problem with them? If so that sounds like a bug in our code

As a namespace, in the api call, I was able to pass whatever: True, 123, [1,2,3].

As a namespace, in the api call, I was able to pass whatever: True, 123, [1,2,3].

Hm, odd, that sounds like a bug in the API code, thanks for finding this

Hm, looks like the API endpoint at https://pagure.io/pagure/blob/master/f/pagure/api/project.py#_758 restricts the namespace available, odd :(

I tried to pass different types of values as namespace in api call and it throws an error as it should. It seems I made a mistake and tested something else...who knows..anyways, it works :)

But I found a bug on wtforms. When I try to pass True as namespace, I think I should get this error: raise ValueError(self.gettext('Invalid Choice: could not coerce')). Instead I get an AttributeError: 'bool' object has no attribute 'decode'

Hm, seems it's expected a string and you're passing a boolean. Is this while calling wtforms via the API or by calling it directly? I'd expect that via the API (or the UI), everything gets passed as a string (since wtforms is what converts these variable to boolean when they should be)

Yes, it is while calling wtforms via the API, using python requests. Do you want me to show you the script I'm using?

I think the problem could be in def convert_value. The method it trying to call decode on boolean when it should instead just do six.string_type(val).

Yes, it is while calling wtforms via the API, using python requests. Do you want me to show you the script I'm using?

If you still have it, it would be most interesting yes, we could/should even turn it into a test for pagure :)

Sure, here it is:
https://pastebin.com/hPRW0X0c

Pingou, does the email validator look good to you?

@lenkaseg Alright, if you have some time this week, let's try to end this PR (by either merging it or not :)).

I ran the script you linked to and it seems to have worked fine, was this not the intent?
Could we add some tests to this PR showing the issues with the current validators?

I think the issue with the email was that in an API call I could pass [1,2,3] or True as an email.

I tried to pass following parameters with values.

avatar_email = 123 (pass)
avatar_email = [1,2,3] (pass as 1)
avatar_email = True (pass as true)
avatar_email = 'fdagre' (pass)
namespace = 123 (doesn't pass, attribute error, forms.py 65)
namespace = [1,2,3] (doesn't pass, attribute error, forms.py, 65)
namespace = True (doesn't pass, attribute error, forms.py, 65)
namespace = 'fdgfs' (doesn't pass, bad request)

I'm going to write the tests.

This are my results:
https://pastebin.com/pTxKatHZ

results with validator:

avatar_email = 123 (does not pass: Email fields should be of text type.) avatar_email = [1,2,3] (does not pass: Email fields should be of text type.)
avatar_email = True (does not pass; Email fields should be of text type.)
avatar_email = 'fdagre' (does not pass: avatar_email must be an email) avatar_email = 'mighty@email.com' (pass)

let's do: if not isinstance(email.data, six.tex_type):, it's the more pythonic way to do this :)

Shouldn't we check the data returned as well?

I incorporated the changes and rebased.

In pagure we keep the import ... above the from ... import... so we should move two lines up :)

One last detail and it's looking good :)

I'll fix it! Now only why Jenkins fails..

it's failing for all PRs atm :(

Ah, ok :)

Done

Thanks! :)