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.

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

Improved performance of Sample header table rendering #1427

Merged
merged 27 commits into from
Aug 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
8a9c2ff
Cache permission lookup
ramonski Aug 16, 2019
9ffe881
Use queryAdapter instead of getAdapter
ramonski Aug 16, 2019
20d5657
Whitespace only
ramonski Aug 16, 2019
d8d722a
Reduced code complexity
ramonski Aug 16, 2019
deb9c92
Removed unnecessary field lookup
ramonski Aug 16, 2019
66d0fc9
Lookup the field type only once
ramonski Aug 16, 2019
6e03402
Use security API
ramonski Aug 16, 2019
ee0d663
Comment only
ramonski Aug 16, 2019
6088458
Minor refactoring
ramonski Aug 16, 2019
7b7344d
Refactored import
ramonski Aug 16, 2019
c803aa4
Removed unused variable
ramonski Aug 16, 2019
59b18a8
simplified check
ramonski Aug 17, 2019
5dca55b
Refactored field checker to methods
ramonski Aug 17, 2019
734f7bc
Removed unneeded tal:define variables
ramonski Aug 17, 2019
75a1607
Refactored field accessor
ramonski Aug 17, 2019
3f01e3a
Removed TODO comment
ramonski Aug 17, 2019
54ba395
Cache field type methods
ramonski Aug 17, 2019
14eee05
Use the portal object for cache
ramonski Aug 17, 2019
71989ef
Changelog updated
ramonski Aug 17, 2019
c24fced
Only show save button if edit is allowed on the context
ramonski Aug 17, 2019
8ab799e
Refactored code for lesser complexity and better understanding
ramonski Aug 18, 2019
f614d0f
Added comment
ramonski Aug 18, 2019
af5b182
Added comment
ramonski Aug 18, 2019
a5e4ed4
Fixed broken field adapters
ramonski Aug 18, 2019
bc07306
Fixed invalid adapter name
ramonski Aug 18, 2019
7d93601
Comment only
ramonski Aug 18, 2019
534d11d
Comment only
ramonski Aug 18, 2019
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
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Changelog

**Changed**

- #1427 Improved performance of Sample header table rendering
- #1417 Cache allowed transitions for analyses on the request
- #1413 Improved Email Publication

Expand Down
15 changes: 13 additions & 2 deletions bika/lims/browser/analysisrequest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,12 @@ def __call__(self, request, data):
if not self.include_fields or "Analyses" in self.include_fields:
data['Analyses'] = self.ar_analysis_values()

class mailto_link_from_contacts:

class mailto_link_from_contacts(object):
"""Custom header table field adapter

see: bika.lims.browser.header_table
"""

def __init__(self, context):
self.context = context
Expand All @@ -127,7 +132,12 @@ def __call__(self, field):
return ",".join(ret)


def mailto_link_from_ccemails(ccemails):
class mailto_link_from_ccemails(object):
"""Custom header table field adapter

see: bika.lims.browser.header_table
"""

def __init__(self, context):
self.context = context

Expand All @@ -136,6 +146,7 @@ def __call__(self, field):
addresses = ccemails.split(",")
ret = []
for address in addresses:
address = address.strip()
mailto = "<a href='mailto:%s'>%s</a>" % (
address, address)
ret.append(mailto)
Expand Down
3 changes: 2 additions & 1 deletion bika/lims/browser/analysisrequest/configure.zcml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@
provides="bika.lims.interfaces.IJSONReadExtender"
/>

<!-- Custom field adapters for sample header view -->
<adapter
for="bika.lims.interfaces.IAnalysisRequest"
factory="bika.lims.browser.analysisrequest.mailto_link_from_contacts"
Expand All @@ -67,7 +68,7 @@
for="bika.lims.interfaces.IAnalysisRequest"
factory="bika.lims.browser.analysisrequest.mailto_link_from_contacts"
provides="bika.lims.interfaces.IHeaderTableFieldRenderer"
name="CContact"
name="CCContact"
/>
<adapter
for="bika.lims.interfaces.IAnalysisRequest"
Expand Down
223 changes: 144 additions & 79 deletions bika/lims/browser/header_table.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,36 @@
# Copyright 2018-2019 by it's authors.
# Some rights reserved, see README and LICENSE.

from AccessControl import getSecurityManager
from AccessControl.Permissions import view
from bika.lims import api
from bika.lims import bikaMessageFactory as _
from bika.lims import logger
from bika.lims.api.security import check_permission
from bika.lims.browser import BrowserView
from bika.lims.interfaces import IHeaderTableFieldRenderer
from bika.lims.utils import t
from plone.memoize import view as viewcache
from plone.memoize.volatile import ATTR
from plone.memoize.volatile import CONTAINER_FACTORY
from plone.memoize.volatile import cache
from Products.Archetypes.event import ObjectEditedEvent
from Products.CMFCore.permissions import ModifyPortalContent
from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile
from zope import event
from zope.component import getAdapter
from zope.component.interfaces import ComponentLookupError
from zope.component import queryAdapter


def store_on_portal(method, obj, *args, **kwargs):
"""Volatile cache storage on the portal object
"""
portal = api.get_portal()
return portal.__dict__.setdefault(ATTR, CONTAINER_FACTORY())


def field_type_cache_key(method, self, field):
"""Cache key to distinguish the type evaluation of a field
"""
return field.getName()


class HeaderTableView(BrowserView):
Expand All @@ -39,7 +56,6 @@ class HeaderTableView(BrowserView):
template = ViewPageTemplateFile("templates/header_table.pt")

def __call__(self):
self.errors = {}
if "header_table_submitted" in self.request:
schema = self.context.Schema()
fields = schema.fields()
Expand All @@ -66,6 +82,101 @@ def __call__(self):
self.context.plone_utils.addPortalMessage(message, "info")
return self.template()

@viewcache.memoize
def is_edit_allowed(self):
"""Check permission 'ModifyPortalContent' on the context
"""
return check_permission(ModifyPortalContent, self.context)

@cache(field_type_cache_key, get_cache=store_on_portal)
def is_reference_field(self, field):
"""Check if the field is a reference field
"""
return field.getType().find("Reference") > -1

@cache(field_type_cache_key, get_cache=store_on_portal)
def is_boolean_field(self, field):
"""Check if the field is a boolean
"""
return field.getWidgetName() == "BooleanWidget"

@cache(field_type_cache_key, get_cache=store_on_portal)
def is_date_field(self, field):
"""Check if the field is a date field
"""
return field.getType().lower().find("datetime") > -1

def get_boolean_field_data(self, field):
"""Get boolean field view data for the template
"""
value = field.get(self.context)
fieldname = field.getName()

return {
"fieldName": fieldname,
"mode": "structure",
"html": t(_("Yes")) if value else t(_("No"))
}

def get_reference_field_data(self, field):
"""Get reference field view data for the template
"""
targets = None
fieldname = field.getName()

accessor = getattr(self.context, "get%s" % fieldname, None)
if accessor and callable(accessor):
targets = accessor()
else:
targets = field.get(self.context)

if targets:
if not isinstance(targets, list):
targets = [targets, ]

if all([check_permission(view, target) for target in targets]):
elements = [
"<div id='{id}' class='field reference'>"
" <a class='link' uid='{uid}' href='{url}'>"
" {title}"
" </a>"
"</div>"
.format(id=target.getId(),
uid=target.UID(),
url=target.absolute_url(),
title=target.Title())
for target in targets]

return {
"fieldName": fieldname,
"mode": "structure",
"html": "".join(elements),
}
else:
return {
"fieldName": fieldname,
"mode": "structure",
"html": ", ".join([ta.Title() for ta in targets]),
}

return {
"fieldName": fieldname,
"mode": "structure",
"html": ""
}

def get_date_field_data(self, field):
"""Render date field view data for the template
"""
value = field.get(self.context)
fieldname = field.getName()

return {
"fieldName": fieldname,
"mode": "structure",
"html": self.ulocalized_time(value, long_format=True)
}

def three_column_list(self, input_list):
list_len = len(input_list)

Expand All @@ -84,82 +195,37 @@ def _list_end(num):
final.append(column)
return final

# TODO Revisit this
def render_field_view(self, field):
fieldname = field.getName()
field = self.context.Schema()[fieldname]
ret = {"fieldName": fieldname, "mode": "view"}
try:
adapter = getAdapter(self.context,
interface=IHeaderTableFieldRenderer,
name=fieldname)

except ComponentLookupError:
adapter = None
if adapter:
ret = {'fieldName': fieldname,
'mode': 'structure',
'html': adapter(field)}
else:
if field.getWidgetName() == "BooleanWidget":
value = field.get(self.context)
ret = {
"fieldName": fieldname,
"mode": "structure",
"html": t(_("Yes")) if value else t(_("No"))
}
elif field.getType().find("Reference") > -1:
# Prioritize method retrieval over schema"s field
targets = None
if hasattr(self.context, "get%s" % fieldname):
fieldaccessor = getattr(self.context, "get%s" % fieldname)
if callable(fieldaccessor):
targets = fieldaccessor()
if not targets:
targets = field.get(self.context)

if targets:
if not type(targets) == list:
targets = [targets, ]
sm = getSecurityManager()
if all([sm.checkPermission(view, ta) for ta in targets]):
elements = [
"<div id='{id}' class='field reference'>"
" <a class='link' uid='{uid}' href='{url}'>"
" {title}"
" </a>"
"</div>"
.format(id=target.getId(),
uid=target.UID(),
url=target.absolute_url(),
title=target.Title())
for target in targets]

ret = {
"fieldName": fieldname,
"mode": "structure",
"html": "".join(elements),
}
else:
ret = {
"fieldName": fieldname,
"mode": "structure",
"html": ", ".join([ta.Title() for ta in targets]),
}
else:
ret = {
"fieldName": fieldname,
"mode": "structure",
"html": "",
}
elif field.getType().lower().find("datetime") > -1:
value = field.get(self.context)
ret = {
"fieldName": fieldname,

# lookup custom render adapter
adapter = queryAdapter(self.context,
interface=IHeaderTableFieldRenderer,
name=fieldname)

# Note: Adapters for contact fields:
# bika.lims.browser.analysisrequest.mailto_link_from_contacts
# bika.lims.browser.analysisrequest.mailto_link_from_ccemails
#
# -> TODO Remove?

# return immediately if we have an adapter
if adapter is not None:
return {"fieldName": fieldname,
"mode": "structure",
"html": self.ulocalized_time(value, long_format=True)
}
return ret
"html": adapter(field)}

# field data for *view* mode for the template
data = {"fieldName": fieldname, "mode": "view"}

if self.is_boolean_field(field):
data = self.get_boolean_field_data(field)
elif self.is_reference_field(field):
data = self.get_reference_field_data(field)
elif self.is_date_field(field):
data = self.get_date_field_data(field)

return data

def get_field_visibility_mode(self, field):
"""Returns "view" or "edit" modes, together with the place within where
Expand All @@ -179,8 +245,7 @@ def get_field_visibility_mode(self, field):
# modes) only if the current user has enough privileges.
if field.checkPermission("edit", self.context):
mode = "edit"
sm = getSecurityManager()
if not sm.checkPermission(ModifyPortalContent, self.context):
if not self.is_edit_allowed():
logger.warn("Permission '{}' granted for the edition of '{}', "
"but 'Modify portal content' not granted"
.format(field.write_permission, field.getName()))
Expand Down
13 changes: 5 additions & 8 deletions bika/lims/browser/templates/header_table.pt
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,9 @@
</head>

<body
tal:define="portal context/@@plone_portal_state/portal;
checkPermission python:context.portal_membership.checkPermission;
dummy python:view.get_fields_grouped_by_location();
tal:define="dummy python:view.get_fields_grouped_by_location();
prominent python:dummy[0];
sublists python:dummy[1];
errors view/errors"
sublists python:dummy[1]"
tal:condition="sublists">
<form method="post" name="header_form">
<input type="hidden" name="header_table_submitted" value="1" />
Expand Down Expand Up @@ -86,13 +83,13 @@
</tal:repeat>
</tr>
</table>
<input class="context btn btn-sm btn-primary"
<input tal:condition="view/is_edit_allowed"
class="context btn btn-sm btn-primary"
type="submit"
name="form.button.save"
value="Save"
i18n:domain="plone"
i18n:attributes="value label_save;"
/>
i18n:attributes="value label_save;"/>
</form>
</body>
</html>