Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MultipleSelectField preparation bug when PropertyLoader field is in __require_fields__ #4

Closed
moschlar opened this issue Nov 6, 2014 · 5 comments

Comments

@moschlar
Copy link
Contributor

moschlar commented Nov 6, 2014

The current code in sprox/formbase.py#L302-L304, leads to the problem that PropertyLoader fields that are in __require_fields__ will get UnicodeString as their validator.

Now if that MultipleSelectField has options that have one- and more-digit IDs, due to the widget using the validator to cast the list of selected values in self.value to a unicode string, but to get the selected options, an in operator is used.
So, if the option with id=13 would have to be checked, the options with id=1 and id=3 will also be checked.

I'm too tired to put up a demonstrational exampe right now, but maybe it gets clear what the problem is. ;)

@percious
Copy link

percious commented Nov 6, 2014

Please do create an example if you can.

Thanks Moritz.

-chris

@moschlar
Copy link
Contributor Author

Okay, done.

See a standalone minimal working example here:
https://gist.github.com/moschlar/c56c3382292f83b882da

(You could change Child.__repr__ to get even more funny results...)

@moschlar
Copy link
Contributor Author

amol- added a commit that referenced this issue Dec 8, 2014
@amol-
Copy link
Member

amol- commented Dec 8, 2014

Note that there is an error in the example as it doesn't use an EditFormFiller so repr of the entities ends up being the value.

I have been able to reproduce the issue in a test unit, it should have been fixed simply switching from UnicodeString to NotEmpty, have yet to check that no side effects appeared in real software

@amol-
Copy link
Member

amol- commented Dec 9, 2014

@moschlar can you confirm the issue is solved by 5650ec6 for you? I'm thinking about pushing a new release as soon as the last changes are tested on real world apps and I would like to ensure that this is working as expected :)

@amol- amol- closed this as completed Feb 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants