News-Portlet integration #2

Merged
merged 29 commits into from Feb 5, 2013

Conversation

Projects
None yet
5 participants
Contributor

tschanzt commented Jan 31, 2013

@maethu I transfered the news from simplelayout.types.news and made some changes. Could you please take a look at it.

@tschanzt could you please correct this translation.

@jone jone commented on an outdated diff Jan 31, 2013

ftw/contentpage/browser/newslisting.py
@@ -0,0 +1,94 @@
+from zope.publisher.browser import BrowserView
+import DateTime
+from Products.CMFCore.utils import getToolByName
+from zope.app.pagetemplate.viewpagetemplatefile import ViewPageTemplateFile
@jone

jone Jan 31, 2013

Owner

Please use Five's ViewPageTemplateFile. This import will not work on Plone 4.3.

Owner

jone commented Jan 31, 2013

@tschanzt I think upgrade steps are required. It is not released yet but there are already some installations. @maethu what do you think?

Owner

jone commented Jan 31, 2013

There are untranslatable strings, see the Translation Report

@jone jone commented on an outdated diff Jan 31, 2013

ftw/contentpage/tests/test_news_views.py
@@ -0,0 +1,28 @@
+import unittest2 as unittest
+from ftw.contentpage.testing import FTW_CONTENTPAGE_FUNCTIONAL_TESTING
+import transaction
+from plone.testing.z2 import Browser
+from plone.app.testing import TEST_USER_NAME, TEST_USER_PASSWORD
@jone

jone Jan 31, 2013

Owner

unused imports

Owner

jone commented Jan 31, 2013

You've introduced various violations compared to the master - please fix those violations.

@jone jone and 1 other commented on an outdated diff Jan 31, 2013

ftw/contentpage/tests/test_news_portlets_functional.py
+ self.browser.getControl(name="form.widgets.path.widgets.query").value = u"ne"
+ #Click the Searchbutton
+ self.browser.getControl(name="form.widgets.path.buttons.search").click()
+ # select the correct radio button over the Label. Remember to use the Text of the Label and not the id. It won't work.
+ self.browser.getControl("Newsfolder1").selected = True
+
+
+ self.browser.getControl(name="form.buttons.add").click()
+
+ def setUp(self):
+ self.portal = self.layer['portal']
+ self.browser = Browser(self.layer['portal'])
+ self.browser.handleErrors
+ self.newsfolder1 = self.portal.get(self.portal.invokeFactory('NewsFolder', 'newsfolder1', title="Newsfolder1"))
+ self.newsfolder2 = self.portal.get(self.portal.invokeFactory('NewsFolder', 'newsfolder2', title="Newsfolder2"))
+ self.newsfolder1.invokeFactory('News', 'news1', description="This Description must be longer than 50 chars so we are able to test if it will be croped", image=open("%s/dummy.png" % os.path.split(__file__)[0], 'r'))
@jone

jone Jan 31, 2013

Owner

Could you also spend some pep8-❤️ in the tests?
It's not very readable when there are that long lines 😺

@jone jone and 1 other commented on an outdated diff Jan 31, 2013

ftw/contentpage/tests/test_news_portlets_functional.py
+
+ def test_addform_cancel(self):
+ self.browser.addHeader('Authorization', 'Basic %s:%s' % (
+ TEST_USER_NAME, TEST_USER_PASSWORD, ))
+
+ self.browser.open(self.portal.absolute_url()+'/++contextportlets++plone.leftcolumn/+/newsportlet')
+ self.browser.getControl(name="form.buttons.cancel_add").click()
+ self.assertEqual(self.portal.absolute_url()+'/@@manage-portlets', self.browser.url)
+
+ def test_addform_send_error(self):
+ self.browser.addHeader('Authorization', 'Basic %s:%s' % (
+ TEST_USER_NAME, TEST_USER_PASSWORD, ))
+
+ self.browser.open(self.portal.absolute_url()+'/++contextportlets++plone.leftcolumn/+/newsportlet')
+ self.browser.addHeader('Authorization', 'Basic %s:%s' % (
+ TEST_USER_NAME, TEST_USER_PASSWORD, ))
@jone

jone Jan 31, 2013

Owner

Duplicate authorization? Think that is not necessary..
Why not move the authorization to setUp ?

@maethu

maethu Jan 31, 2013

Owner

I prefer a private method called _auth(), which does the authentification.

@maethu maethu commented on an outdated diff Jan 31, 2013

ftw/contentpage/content/newsfolder.py
@@ -0,0 +1,19 @@
+from Products.ATContentTypes.content import folder
+from AccessControl import ClassSecurityInfo
+from ftw.contentpage.config import PROJECTNAME
+from simplelayout.base.interfaces import ISimpleLayoutCapable
+from zope.interface import implements
+from DateTime import DateTime
+from Products.Archetypes.atapi import registerType
+from ftw.contentpage.interfaces import INewsFolder
+
@maethu

maethu Jan 31, 2013

Owner

pep8 violation

@maethu maethu commented on the diff Jan 31, 2013

ftw/contentpage/interfaces.py
@@ -27,3 +27,9 @@ class IOrgUnitMarker(Interface):
class ICategorizable(Interface):
"""Marker interface for categorizable content"""
+
@maethu

maethu Jan 31, 2013

Owner

pep8 violation

@maethu maethu commented on an outdated diff Jan 31, 2013

...contentpage/locales/de/LC_MESSAGES/ftw.contentpage.po
#. Default: "Tel. ${num}"
#: ./ftw/contentpage/browser/address.pt:9
msgid "text_phone"
msgstr "Tel. ${num}"
+#: ./ftw/contentpage/portlets/news_portlet.py:34
+msgid "xx"
+msgstr ""
+
@maethu

maethu Jan 31, 2013

Owner

wtf is xx? better no translation I think.

@maethu maethu commented on the diff Jan 31, 2013

ftw/contentpage/portlets/news_portlet.py
+ '/'.join([portal_path, item.strip('/')]))
+ cs_uids.append(obj.UID())
+ query['cs_uids'] = cs_uids
+
+ if self.data.subjects:
+ query['Subject'] = self.data.subjects
+
+ query['sort_on'] = 'effective'
+ query['sort_order'] = 'descending'
+ results = catalog.searchResults(query)
+ return results[0:self.data.quantity - 1]
+
+ def crop_desc(self, description):
+ ploneview = self.context.restrictedTraverse('@@plone')
+ return ploneview.cropText(description, self.data.desc_length)
+
@maethu

maethu Jan 31, 2013

Owner

pep8 violation

@maethu maethu commented on the diff Jan 31, 2013

ftw/contentpage/tests/test_news_portlets_functional.py
@@ -0,0 +1,212 @@
@maethu

maethu Jan 31, 2013

Owner

Respect pep8/pylint also in tests

Owner

maethu commented Jan 31, 2013

Topic Upgrade step:
imho it's no necessary, there's no release yet. @buchi do you need an upgrade step for your site?

Contributor

tschanzt commented Feb 1, 2013

@jone can you take another look?

Owner

jone commented Feb 1, 2013

@maethu maethu commented on the diff Feb 4, 2013

ftw/contentpage/vocabularies.py
@@ -0,0 +1,22 @@
+from zope.schema import vocabulary
+from zope.schema.interfaces import IVocabularyFactory
+from zope.schema.vocabulary import SimpleTerm
+from zope.interface import directlyProvides
+from Products.CMFCore.utils import getToolByName
+
+
+def SubjectVocabulary(context):
@maethu

maethu Feb 4, 2013

Owner

@tschanzt I think it is better if you use portal_catalog.uniqueValuesFor("Subject"). This already does everything you want.
And pls add a test for this feature.

@maethu maethu commented on the diff Feb 4, 2013

ftw/contentpage/viewlets/byline.py
+
+ def creator(self):
+ userid = self.context.Creator()
+ mt = getToolByName(self.context, 'portal_membership')
+ member = mt.getMemberById(userid)
+ if member:
+ return member.getProperty('fullname') or userid
+ return userid
+
+ def getWorkflowState(self):
+ state = self.context_state.workflow_state()
+ plone_tools = getMultiAdapter(
+ (self.context, self.request),
+ name='plone_tools')
+
+ workflows = plone_tools.workflow().getWorkflowsFor(self.context)
@maethu

maethu Feb 4, 2013

Owner

@tschanzt This part is not well tested

@maethu maethu commented on an outdated diff Feb 4, 2013

ftw/contentpage/portlets/news_portlet.py
+ @button.buttonAndHandler(_(u"label_save", default=u"Save"), name='add')
+ def handleAdd(self, action):
+ data, errors = self.extractData()
+ if errors:
+ self.status = self.formErrorsMessage
+ return
+ obj = self.createAndAdd(data)
+ if obj is not None:
+ # mark only as finished if we get the new object
+ self._finishedAdd = True
+
+ @button.buttonAndHandler(_(u"label_cancel", default=u"Cancel"),
+ name='cancel_add')
+ def handleCancel(self, action):
+ nextURL = self.nextURL()
+ if nextURL:
@maethu

maethu Feb 4, 2013

Owner

@tschanzt according to the nextURL function, nextURL is always True. The condition is not necessary.

@maethu maethu commented on the diff Feb 4, 2013

ftw/contentpage/portlets/news_portlet.py
+ self.path = path or []
+ self.subjects = subjects or []
+ self.show_desc = show_desc
+ self.desc_length = desc_length
+
+ @property
+ def title(self):
+ return u'News Portlet'
+
+
+class Renderer(base.Renderer):
+ render = ViewPageTemplateFile('news_portlet.pt')
+
+ def tag_image(self, brain):
+ if not self.data.show_image:
+ return ''
@maethu

maethu Feb 4, 2013

Owner

@tschanzt This line is not tested. Pls add at testcase, which does not show a image in the portlet

@maethu maethu commented on an outdated diff Feb 4, 2013

ftw/contentpage/content/newsfolder.py
+from AccessControl import ClassSecurityInfo
+from ftw.contentpage.config import PROJECTNAME
+from simplelayout.base.interfaces import ISimpleLayoutCapable
+from zope.interface import implements
+from DateTime import DateTime
+from Products.Archetypes.atapi import registerType
+from ftw.contentpage.interfaces import INewsFolder
+
+
+class NewsFolder(folder.ATFolder):
+
+ implements(INewsFolder, ISimpleLayoutCapable)
+ security = ClassSecurityInfo()
+
+ def getDefaultEffectiveDate(self):
+ return DateTime().Date()
@maethu

maethu Feb 4, 2013

Owner

@tschanzt This function is never called. Still necessary?

@maethu maethu commented on the diff Feb 4, 2013

ftw/contentpage/browser/newslisting.py
+ return self.template_rss()
+ return self.template()
+
+ def get_today(self, plus=0, minus=0):
+ date = DateTime.DateTime() + plus - minus
+ return date.Date()
+
+ def get_creator(self, item):
+ memberid = item.Creator
+ mt = getToolByName(self.context, 'portal_membership')
+ member = mt.getMemberById(memberid)
+ if member:
+ return member
+ return None
+
+ def get_news(self):
@maethu

maethu Feb 4, 2013

Owner

@tschanzt This function is no really tested

tschanzt was assigned Feb 5, 2013

Contributor

tschanzt commented Feb 5, 2013

@maethu could you take another look?

maethu merged commit 8eec64e into master Feb 5, 2013

maethu deleted the tt-news branch Feb 5, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment