-
-
Notifications
You must be signed in to change notification settings - Fork 158
[IMP] convert attrs attribute to attributes during v17 migration #164
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
base: master
Are you sure you want to change the base?
Conversation
legalsylvain
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks !
StefanRijnhart
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
It seems attrs="{'invisible': [('some_field', '=', True)]} is transformed to invisible="some_field == True". Can you make this invisible="some_field"?
43818da to
b5b5841
Compare
|
@StefanRijnhart added that. At first, I deliberately didn't include handling the = True case here because a =/!= False domain leaf will be is (not) null in sql, which is guaranteed to map to falsiness/truthiness, while != True could be any truthy value that's not True. But then I think you're right and nobody uses it that way, so we're better off supporting the much more realistic case where this is used on boolean fields |
|
Thanks, boolean issue is now solved. Next issue I now encounter is attrs with newlines in the XML. Such a snippet leads to |
b5b5841 to
40ee4a2
Compare
|
@StefanRijnhart thank you for battle testing this, fixed |
StefanRijnhart
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, working well now!
this transforms
to
which is quite a pain to do manually