Skip to content

Commit

Permalink
Fix control panel filtering (https://dev.plone.org/ticket/13557)
Browse files Browse the repository at this point in the history
  • Loading branch information
danjacka committed May 1, 2013
1 parent ecb9897 commit 939a852
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 3 deletions.
3 changes: 2 additions & 1 deletion CHANGES.txt
Expand Up @@ -4,7 +4,8 @@ Changelog
1.2.3 (unreleased)
------------------

- Nothing changed yet.
- Fix control panel filtering (https://dev.plone.org/ticket/13557)
[danjacka]


1.2.2 (2013-01-13)
Expand Down
4 changes: 2 additions & 2 deletions plone/app/registry/browser/records.py
Expand Up @@ -27,7 +27,7 @@ def __call__(self):
search = form.get('q')
searchp = form.get('qp')
compare = _is_in
if searchp:
if searchp is not None:
search = searchp
if search is not None and search.startswith('prefix:'):
search = search[len('prefix:'):]
Expand All @@ -52,7 +52,7 @@ def __call__(self):
break
if recordPrefix not in self.prefixes:
self.prefixes[recordPrefix] = prefixValue
if compare(search, record.__name__):
if compare(search, prefixValue):

This comment has been minimized.

Copy link
@vangheem

vangheem May 1, 2013

Member

I'm not sure what has changed but this line seems like it shouldn't be necessary. Perhaps just set a default value of prefixValue = record.name?

This comment has been minimized.

Copy link
@danjacka

danjacka May 1, 2013

Author Member

Sorry @vangheem, I don't understand your comment.

To give you more context, an example when filtering on IQueryField:
search = 'plone.app.querystring.interfaces.IQueryField'
ifaceName = 'plone.app.querystring.interfaces.IQueryField'
prefixValue = 'plone.app.querystring.interfaces.IQueryField'
record.__name__ = 'plone.app.querystring.field.effective.operations'

.. so compare(search, record.__name__) never matches.

This comment has been minimized.

Copy link
@vangheem

vangheem May 1, 2013

Member

Shouldn't it be getting set to the actual prefix on line 51?

This comment has been minimized.

Copy link
@danjacka

danjacka May 1, 2013

Author Member

Line 51 is not called in this case, because the test at line 42 evaluates to True.

This comment has been minimized.

Copy link
@vangheem

vangheem May 1, 2013

Member

Right, but there must be cases where it evaluates to False. Hence my point at setting a default for prefixValue instead of not using it at all.

self.records.append(record)
self.records = Batch(self.records, 10,
int(form.get('b_start', '0')), orphan=1)
Expand Down

2 comments on commit 939a852

@danjacka
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vangheem OK just committed d104154 I'm still not 100% I understand your comments though! Since you wrote the code originally, please feel free to change my fix as you like.

@vangheem
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a43df30 is what you're looking for?

Please sign in to comment.