Validate Project-URL metadata #2667
Conversation
warehouse/forklift/legacy.py
Outdated
| for datum in field.data: | ||
| _validate_project_url(datum) | ||
| if isinstance(field.data, str): | ||
| _validate_project_url(field.data) # Or raise ValidationError? |
There was a problem hiding this comment.
@dstufft i'm not sure if we want to just accept a string or not here.
There was a problem hiding this comment.
Project-URL is a multiple-use field, so this should always be a list. I think it should just fail validation if it isn't.
There was a problem hiding this comment.
|
Wouldn't this be an issue for any |
|
yeah, let's see if I can put a validator on the |
|
I think we might want a failing test like this on @pytest.mark.parametrize(
("value", "expected"),
[
("", []),
("foobar", ["foobar"]),
]
)
def test_coerce_string_into_list(self, value, expected):
class MyForm(Form):
test = legacy.ListField()
form = MyForm(MultiDict({'test': value}))
assert form.test.data == expectedThe issue seems to be that |
|
yep! I'm retooling to give |
|
@di good call! your approach ended up being better and leading me to the simple solution 742c240#diff-dbf9e293f2b07f35422689148e11b039R267 I left a bunch of empty string and validation test cases in place just to keep us from regressing! |
warehouse/forklift/legacy.py
Outdated
| @@ -264,7 +264,7 @@ def _construct_dependencies(form, types): | |||
| class ListField(wtforms.Field): | |||
|
|
|||
| def process_formdata(self, valuelist): | |||
| self.data = [v.strip() for v in valuelist] | |||
| self.data = [v.strip() for v in valuelist if v] | |||
There was a problem hiding this comment.
This still could give us empty strings if one of the values is just whitespace:
>>> [v.strip() for v in ['', ' ', 'a'] if v]
['', 'a']
RE: #2666