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
Fix handling of broken objects in ZMI #336
Conversation
src/OFS/zpt/main.zpt
Outdated
rkey python:request.get('rkey','asc'); | ||
rkey_alt python:request.get('rkey','asc')=='asc' and 'desc' or 'asc'; | ||
sort_func python:['nocase','cmp'][skey not in ['getId','meta_type','id','title']]; | ||
obs python:sequence.sort(obs, ((skey,sort_func,rkey),) )" | ||
sort_func python:['strcoll','cmp'][skey not in ['meta_type','id','title']]; |
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.
'cmp' if skey in ['meta_type', 'id', 'title'] else 'strcoll'
would be a bit easier to read in my mind.
src/OFS/zpt/main.zpt
Outdated
(key, getattr(item[1],key)) | ||
for key in ['title', 'meta_type'] | ||
if hasattr(item[1],key) | ||
] |
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.
I think it would be easier to read when using if ... else None
and filtering out None
afterwards.
src/OFS/zpt/main.zpt
Outdated
for item in here.objectItems() | ||
]; | ||
obs python: [dict(item) for item in obs]; | ||
obs python: sequence.sort(obs, ((skey,sort_func,rkey),), mapping=1 ); |
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.
PEP-8, please.
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.
Mostly LGTM I have only some minor comments.
src/OFS/tests/testSorting.py
Outdated
self.assertTrue(one_before_two == one_before_two_found) | ||
|
||
self.browser.open(base_url % ('id', 'asc')) | ||
do_assert(one_before_two = True) |
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.
PEP-8 does not allow spaces around =
in function calls.
src/OFS/tests/testSorting.py
Outdated
one_before_two_found = ( | ||
self.browser.contents.find('File2') > self.browser.contents.find('File1') | ||
) | ||
self.assertTrue(one_before_two == one_before_two_found) |
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.
What about assertEqual
?
Instead of obtaining the list of objects from
objectValues
and assuming that these provide all necessary attributes (likegetId()
), a list of dictionaries is created on the basis ofobjectItems
, which has access to the object's ID as it is known to the parent. This is used for sorting as well as interaction with the specific items, so it allows to show broken objects and also delete them.fixes #311
fixes #213