Skip to content

Commit

Permalink
Add an option to exclude the current context.
Browse files Browse the repository at this point in the history 
Conflicts:
	CHANGES.rst
	plone/portlet/collection/collection.py
	plone/portlet/collection/tests/test_portlet_collection.py
  • Loading branch information
@rpatterson
rpatterson committed Jul 14, 2014
1 parent 6edf752 commit a48b7d9
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 4 deletions.
5 changes: 4 additions & 1 deletion CHANGES.rst
View file
Expand Up @@ -4,7 +4,10 @@ Changelog
3.0.1 (unreleased)
------------------

- Nothing changed yet.
- Add an option for excluding the render context from the collection results
since in most cases it's undesirable to include the current context in a
listing on that context's view.
[rpatterson]


3.0 (2014-04-05)
Expand Down
27 changes: 25 additions & 2 deletions plone/portlet/collection/collection.py
View file
Expand Up @@ -74,6 +74,14 @@ class ICollectionPortlet(IPortletDataProvider):
required=True,
default=False)

exclude_context = schema.Bool(
title=_(u"Exclude the Current Context"),
description=_(
u"If enabled, the listing will not include the current item the "
u"portlet is rendered for if it otherwise would be."),
required=True,
default=True)


class Assignment(base.Assignment):
"""
Expand All @@ -94,13 +102,15 @@ class Assignment(base.Assignment):
target_collection = None

def __init__(self, header=u"", uid=None, limit=None,
random=False, show_more=True, show_dates=False):
random=False, show_more=True, show_dates=False,
exclude_context=True):
self.header = header
self.uid = uid
self.limit = limit
self.random = random
self.show_more = show_more
self.show_dates = show_dates
self.exclude_context = exclude_context

@property
def title(self):
Expand Down Expand Up @@ -158,13 +168,20 @@ def _standard_results(self):
results = []
collection = self.collection()
if collection is not None:
context_path = '/'.join(self.context.getPhysicalPath())
exclude_context = getattr(self.data, 'exclude_context', False)
limit = self.data.limit
if limit and limit > 0:
# pass on batching hints to the catalog
results = collection.queryCatalog(batch=True, b_size=limit)
results = collection.queryCatalog(
batch=True, b_size=limit + exclude_context)
results = results._sequence
else:
results = collection.queryCatalog()
if exclude_context:
results = [
brain for brain in results
if brain.getPath() != context_path]
if limit and limit > 0:
results = results[:limit]
return results
Expand All @@ -174,11 +191,17 @@ def _random_results(self):
results = []
collection = self.collection()
if collection is not None:
context_path = '/'.join(self.context.getPhysicalPath())
exclude_context = getattr(self.data, 'exclude_context', False)
results = collection.queryCatalog(sort_on=None)
if results is None:
return []
limit = self.data.limit and min(len(results), self.data.limit) or 1

if exclude_context:
results = [
brain for brain in results
if brain.getPath() != context_path]
if len(results) < limit:
limit = len(results)
results = random.sample(results, limit)
Expand Down
125 changes: 124 additions & 1 deletion plone/portlet/collection/tests/test_portlet_collection.py
View file
Expand Up @@ -198,7 +198,8 @@ def testSimpleQuery(self):
mapping = PortletAssignmentMapping()
mapping['foo'] = collection.Assignment(
header=u"title",
uid=self.folder.collection.UID()
uid=self.folder.collection.UID(),
exclude_context=False,
)
collectionrenderer = self.renderer(
context=None,
Expand Down Expand Up @@ -306,3 +307,125 @@ def testRandomQuery(self):
)
collectionrenderer.data.limit = 10
self.assertTrue(len(collectionrenderer.results()) >= 6)

def test_exclude_context(self):
"""
The exclude context field controls including self in the results.
"""
for idx in range(4):
self._createType(
self.folder, 'News Item',
'foo-news-item-title-{0}'.format(idx))
self.folder.collection.query = [{
'i': 'portal_type',
'o': 'plone.app.querystring.operation.string.is',
'v': 'News Item'}]
self.folder.collection.sort_on = 'created'
context = self.folder['foo-news-item-title-1']
included = [
self.folder['foo-news-item-title-{0}'.format(idx)].absolute_url()
for idx in (0, 2)]
limited = self.folder['foo-news-item-title-3'].absolute_url()

assignment = collection.Assignment(
header=u"title", uid=self.folder.collection.UID(), limit=2)

folder_renderer = self.renderer(
context=self.folder, assignment=assignment)
folder_results = [
brain.getURL() for brain in folder_renderer.results()]
self.assertEqual(
len(folder_results), 2, 'Wrong number of folder rendered results')
self.assertIn(
included[0], folder_results,
'Folder rendered results missing item')
self.assertIn(
context.absolute_url(), folder_results,
'Folder rendered results missing context item')
self.assertNotIn(
included[1], folder_results,
'Folder rendered results included too many items')
self.assertNotIn(
limited, folder_results,
'Folder rendered results included way too many items')

renderer = self.renderer(context=context, assignment=assignment)
results = [
brain.getURL() for brain in renderer.results()]
self.assertEqual(
len(results), 2, 'Wrong number of context rendered results')
self.assertIn(
included[0], results,
'Context rendered results missing item')
self.assertIn(
included[1], results,
'Context rendered results missing item')
self.assertNotIn(
context.absolute_url(), results,
'Context rendered results included context')
self.assertNotIn(
limited, results,
'Context rendered results included too many items')

assignment.exclude_context = False
context_renderer = self.renderer(
context=context, assignment=assignment)
context_results = [
brain.getURL() for brain in context_renderer.results()]
self.assertEqual(
len(context_results), 2,
'Wrong number of context rendered results')
self.assertIn(
included[0], context_results,
'Context rendered results missing item')
self.assertIn(
context.absolute_url(), context_results,
'Context rendered results missing context')
self.assertNotIn(
included[1], context_results,
'Context rendered results included too many items')
self.assertNotIn(
limited, context_results,
'Context rendered results included way too many items')

del assignment.exclude_context
missing_renderer = self.renderer(
context=context, assignment=assignment)
missing_results = [
brain.getURL() for brain in missing_renderer.results()]
self.assertEqual(
len(missing_results), 2,
'Wrong number of context rendered results')
self.assertIn(
included[0], missing_results,
'Context rendered results missing item')
self.assertIn(
context.absolute_url(), missing_results,
'Context rendered results missing context')
self.assertNotIn(
included[1], missing_results,
'Context rendered results included too many items')
self.assertNotIn(
limited, missing_results,
'Context rendered results included way too many items')

assignment.limit = 4
assignment.random = True
assignment.exclude_context = True
random_renderer = self.renderer(context=context, assignment=assignment)
random_results = [
brain.getURL() for brain in random_renderer.results()]
self.assertEqual(
len(random_results), 3, 'Wrong number of random rendered results')
self.assertIn(
included[0], random_results,
'Context rendered results missing item')
self.assertIn(
included[1], random_results,
'Context rendered results missing item')
self.assertIn(
limited, random_results,
'Context rendered results missing item')
self.assertNotIn(
context.absolute_url(), random_results,
'Context rendered results included context')

3 comments on commit a48b7d9

@davisagli
Copy link
Member

Choose a reason for hiding this comment

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

In my opinion we could just add this feature without exposing an option for it. I'm not sure why you would ever want to show a link to the current item, so it seems like it just clutters the portlet settings.

@rpatterson
Copy link
Member Author

@rpatterson rpatterson commented on a48b7d9 Jul 14, 2014 via email

Choose a reason for hiding this comment

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

@davisagli
Copy link
Member

Choose a reason for hiding this comment

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

Anyone else have an opinion here?

Please sign in to comment.