Skip to content
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.

Sign up for GitHub

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

Display document byline viewlet only to anonymous users #90

Merged
merged 2 commits into from
Jun 2, 2016
Merged

Display document byline viewlet only to anonymous users #90

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1845be1
Document byline viewlet is now displayed only to anonymous users
hvelarde May 5, 2016
7a603af
Remove code used to show the lock status and history view in the docu…
hvelarde May 5, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ Changelog

Incompatibilities:

- Deprecated ``plone.app.layout.globals.pattern_settings``.
- Deprecated ``plone.app.layout.globals.pattern_settings``.
Moved view to ``Products.CMFPlone.patterns.view``.
Deprecated also pointless interface for this view.
Addresses https://github.com/plone/Products.CMFPlone/issues/1513 and goes together with https://github.com/plone/Products.CMFPlone/issues/1514.
Expand All @@ -18,6 +18,10 @@ New:

Fixes:

- Document byline viewlet is now displayed only to anonymous users if permited by the `Allow anyone to view 'about' information` option in the `Security Settings` of `Site Setup` (closes `CMFPlone#1556`_).
Code used to show the lock status and history view was removed from the document byline as this information was not available to anonymous users anyway.
[hvelarde]

- Fix body class ``pat-markspeciallinks`` not set.
Fixes #84.
[thet]
Expand Down Expand Up @@ -1708,3 +1712,4 @@ Fixes:
.. _`CMFPlone#1037`: https://github.com/plone/Products.CMFPlone/issues/1037
.. _`CMFPlone#1151`: https://github.com/plone/Products.CMFPlone/issues/1151
.. _`CMFPlone#1178`: https://github.com/plone/Products.CMFPlone/issues/1178
.. _`CMFPlone#1556`: https://github.com/plone/Products.CMFPlone/issues/1556
38 changes: 1 addition & 37 deletions plone/app/layout/viewlets/content.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,43 +71,7 @@ def show(self):
ISecuritySchema,
prefix='plone',
)
return not self.anonymous or settings.allow_anon_views_about

def show_history(self):
has_access_preview_versions_permission = _checkPermission(
'CMFEditions: Access previous versions',
self.context
)
if not has_access_preview_versions_permission:
return False
if IViewView.providedBy(self.__parent__):
return True
if IFolderContentsView.providedBy(self.__parent__):
return True
return False

def locked_icon(self):
if not getSecurityManager().checkPermission('Modify portal content',
self.context):
return ""

locked = False
lock_info = queryMultiAdapter((self.context, self.request),
name='plone_lock_info')
if lock_info is not None:
locked = lock_info.is_locked()
else:
context = aq_inner(self.context)
lockable = getattr(
context.aq_explicit, 'wl_isLocked', None) is not None
locked = lockable and context.wl_isLocked()

if not locked:
return ""

portal = self.portal_state.portal()
icon = portal.restrictedTraverse('lock_icon.png')
return icon.tag(title='Locked')
return self.anonymous and settings.allow_anon_views_about
Copy link
Member

Choose a reason for hiding this comment

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

It'd be nice to add a test so we don't get a regression in this important line again. Where do you think it would be the best place?

Copy link
Member

Choose a reason for hiding this comment

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

Why not showing for non-anonymous too?
Only reason I can think of is, that the some of the information is available in the toolbar too. But is the toolbar also visible for simple members without editing rights? Then we need this viewlet also for them.

Copy link
Member

@davilima6 davilima6 May 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, the clock icon is available to Members.

Copy link
Member Author

@hvelarde hvelarde May 5, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

@thet well, another proof that the toolbar design was half baked and the removal of the viewlet was a bad idea in the first place; I would prefer to leave the document byline visible for all, but in the past we had some heated discussions as some people simply wanted to get rid of it without thinking on all the unintended consequences. I was trying to move on but, yes, you are right.

IMO the document byline helps to remove the cluttering of the toolbar and there are many regressions caused by its removal.

@davilima6 the clock icon is not solving all cases; see plone/Products.CMFPlone#1575


def creator(self):
return self.context.Creator()
Expand Down
14 changes: 0 additions & 14 deletions plone/app/layout/viewlets/document_byline.pt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@
id="plone-document-byline"
i18n:domain="plone"
tal:condition="view/show">
<span id="lock-icon"
tal:define="lock_icon view/locked_icon"
tal:condition="python:lock_icon">
<img tal:replace="structure lock_icon" />
</span>

<tal:creator tal:define="creator_short_form view/creator;"
tal:condition="creator_short_form">
<tal:name tal:define="creator_long_form string:?author=${creator_short_form};
Expand Down Expand Up @@ -58,14 +52,6 @@
i18n:translate="time_expired">expired</span>
</tal:expired>

<span class="contentHistory" id="content-history"
tal:condition="view/show_history">
—
<a href="#"
tal:attributes="href string:${here/absolute_url}/@@historyview"
i18n:translate="label_history">History</a>
</span>

<div class="documentContributors"
tal:define="contributors context/Contributors"
tal:condition="contributors"
Expand Down
17 changes: 0 additions & 17 deletions plone/app/layout/viewlets/tests/test_content.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,23 +50,6 @@ def _get_viewlet(self):
viewlet.update()
return viewlet

def test_anonymous_locked_icon(self):
viewlet = self._get_viewlet()
ILockable(self.context).lock()
self.logout()
viewlet = self._get_viewlet()
self.assertEqual(viewlet.locked_icon(), '')

def test_locked_icon(self):
viewlet = self._get_viewlet()
self.assertEqual(viewlet.locked_icon(), "")
ILockable(self.context).lock()
lockIconUrl = (
'<img src="http://nohost/plone/lock_icon.png" alt="" '
'title="Locked" height="16" width="16" />'
)
self.assertEqual(viewlet.locked_icon(), lockIconUrl)

def test_pub_date(self):
# configure our portal to enable publication date on pages globally on
# the site
Expand Down