senaite · ramonski · Jan 15, 2020 · Dec 20, 2019 · Dec 20, 2019 · Dec 20, 2019
diff --git a/CHANGES.rst b/CHANGES.rst
@@ -16,6 +16,7 @@ Changelog
 
 **Changed**
 
+- #1495 Better Remarks handling and display
 - #1502 Improved DateTime Widget
 - #1490 Support Dexterity Behavior Fields in API
 - #1488 Support Dexterity Contents in Catalog Indexers

diff --git a/bika/lims/browser/analysisrequest/templates/analysisrequest_manage_results.pt b/bika/lims/browser/analysisrequest/templates/analysisrequest_manage_results.pt
@@ -8,21 +8,6 @@
   <head>
     <title></title>
     <metal:block fill-slot="javascript_head_slot">
-      <!-- TODO move to senaite.lims bootstrap css -->
-      <style type="text/css">
-       #remarks-widget {
-         padding-top: 2em;
-       }
-       #archetypes-fieldname-Remarks {
-         padding: 0!important;
-       }
-       #remarks-widget fieldset legend {
-         display: none;
-       }
-       #remarks-widget input.saveRemarks {
-         margin-top: 1em;
-       }
-      </style>
     </metal:block>
   </head>
 
@@ -87,7 +72,7 @@
            https://github.com/senaite/senaite.core/pull/920 -->
       <div class="row">
         <div id="remarks-widget"
-             class="col-sm-12"
+             class="col-sm-12 remarks-widget"
              tal:define="checkPermission nocall: context/portal_membership/checkPermission;
                  mode python:'edit' if checkPermission('senaite.core: Field: Edit Remarks', context) else 'view';
                  field python:context.Schema()['Remarks'];
@@ -96,9 +81,7 @@
             <img i18n:attributes="title" title="Remarks" src="++resource++bika.lims.images/remarks.png"/>
             <span i18n:translate="">Remarks</span>
           </h3>
-          <div class="well">
-            <metal:widget use-macro="python:context.widget('Remarks', mode=mode)"/>
-          </div>
+          <metal:widget use-macro="python:context.widget('Remarks', mode=mode)"/>
         </div>
       </div>
       <!-- /Remarks Widget -->

diff --git a/bika/lims/browser/analysisrequest/templates/analysisrequest_view.pt b/bika/lims/browser/analysisrequest/templates/analysisrequest_view.pt
@@ -8,21 +8,6 @@
     <title></title>
     <metal:block fill-slot="javascript_head_slot"
                  tal:define="portal context/@@plone_portal_state/portal;">
-      <!-- TODO move to senaite.lims bootstrap css -->
-      <style type="text/css">
-       #remarks-widget {
-         padding-top: 2em;
-       }
-       #archetypes-fieldname-Remarks {
-         padding: 0!important;
-       }
-       #remarks-widget fieldset legend {
-         display: none;
-       }
-       #remarks-widget input.saveRemarks {
-         margin-top: 1em;
-       }
-      </style>
     </metal:block>
   </head>
 
@@ -72,7 +57,7 @@
            https://github.com/senaite/senaite.core/pull/920 -->
       <div class="row">
         <div id="remarks-widget"
-             class="col-sm-12"
+             class="col-sm-12 remarks-widget"
              tal:define="checkPermission nocall: context/portal_membership/checkPermission;
                  mode python:'edit' if checkPermission('senaite.core: Field: Edit Remarks', context) else 'view';
                  field python:context.Schema()['Remarks'];
@@ -81,9 +66,7 @@
             <img i18n:attributes="title" title="Remarks" src="++resource++bika.lims.images/remarks.png"/>
             <span i18n:translate="">Remarks</span>
           </h3>
-          <div class="well">
-            <metal:widget use-macro="python:context.widget('Remarks', mode=mode)"/>
-          </div>
+          <metal:widget use-macro="python:context.widget('Remarks', mode=mode)"/>
         </div>
       </div>
       <!-- /Remarks Widget -->

diff --git a/bika/lims/browser/fields/remarksfield.py b/bika/lims/browser/fields/remarksfield.py
@@ -18,18 +18,102 @@
 # Copyright 2018-2019 by it's authors.
 # Some rights reserved, see README and LICENSE.
 
+import re
+
+import six
+
 from AccessControl import ClassSecurityInfo
-from AccessControl import getSecurityManager
+from bika.lims import api
 from bika.lims.browser.widgets import RemarksWidget
+from bika.lims.events import RemarksAddedEvent
 from bika.lims.interfaces import IRemarksField
+from bika.lims.utils import tmpID
 from DateTime import DateTime
 from Products.Archetypes.event import ObjectEditedEvent
 from Products.Archetypes.Field import ObjectField
 from Products.Archetypes.Registry import registerField
+from Products.CMFPlone.i18nl10n import ulocalized_time
 from zope import event
 from zope.interface import implements
 
 
+class RemarksHistory(list):
+    """A list containing a remarks history, but __str__ returns the legacy
+    format from instances prior v1.3.3
+    """
+
+    def html(self):
+        return api.text_to_html(str(self))
+
+    def __str__(self):
+        """Returns the remarks in legacy format
+        """
+        remarks = map(lambda rec: str(rec), self)
+        remarks = filter(None, remarks)
+        return "\n".join(remarks)
+
+    def __eq__(self, y):
+        if isinstance(y, six.string_types):
+            return str(self) == y
+        return super(RemarksHistory, self).__eq__(y)
+
+
+class RemarksHistoryRecord(dict):
+    """A dict implementation that represents a record/entry of Remarks History
+    """
+
+    def __init__(self, *arg, **kw):
+        super(RemarksHistoryRecord, self).__init__(*arg, **kw)
+        self["id"] = self.id or tmpID()
+        self["user_id"] = self.user_id
+        self["user_name"] = self.user_name
+        self["created"] = self.created or DateTime().ISO()
+        self["content"] = self.content
+
+    @property
+    def id(self):
+        return self.get("id", "")
+
+    @property
+    def user_id(self):
+        return self.get("user_id", "")
+
+    @property
+    def user_name(self):
+        return self.get("user_name", "")
+
+    @property
+    def created(self):
+        return self.get("created", "")
+
+    @property
+    def created_ulocalized(self):
+        return ulocalized_time(self.created,
+                               long_format=True,
+                               context=api.get_portal(),
+                               request=api.get_request(),
+                               domain="senaite.core")
+
+    @property
+    def content(self):
+        return self.get("content", "")
+
+    @property
+    def html_content(self):
+        return api.text_to_html(self.content)
+
+    def __str__(self):
+        """Returns a legacy string format of the Remarks record
+        """
+        if not self.content:
+            return ""
+        if self.created and self.user_id:
+            # Build the legacy format
+            return "=== {} ({})\n{}".format(self.created, self.user_id,
+                                            self.content)
+        return self.content
+
+
 class RemarksField(ObjectField):
     """A field that stores remarks.  The value submitted to the setter
     will always be appended to the actual value of the field.
@@ -44,40 +128,65 @@ class RemarksField(ObjectField):
     })
 
     implements(IRemarksField)
-
     security = ClassSecurityInfo()
 
-    security.declarePrivate('set')
+    @property
+    def searchable(self):
+        """Returns False, preventing this field to be searchable by AT's
+        SearcheableText
+        """
+        return False
 
+    @security.private
     def set(self, instance, value, **kwargs):
         """Adds the value to the existing text stored in the field,
         along with a small divider showing username and date of this entry.
         """
+
         if not value:
             return
-        value = value.strip()
-        date = DateTime().rfc822()
-        user = getSecurityManager().getUser()
-        username = user.getUserName()
-        divider = "=== {} ({})".format(date, username)
-        existing_remarks = instance.getRawRemarks()
-        remarks = '\n'.join([divider, value, existing_remarks])
-        ObjectField.set(self, instance, remarks)
-        # reindex the object after save to update all catalog metadata
+
+        if isinstance(value, RemarksHistory):
+            # Override the whole history here
+            history = value
+
+        elif isinstance(value, (list, tuple)):
+            # This is a list, convert to RemarksHistory
+            remarks = map(lambda item: RemarksHistoryRecord(item), value)
+            history = RemarksHistory(remarks)
+
+        elif isinstance(value, RemarksHistoryRecord):
+            # This is a record, append to the history
+            history = self.get_history(instance)
+            history.insert(0, value)
+
+        elif isinstance(value, six.string_types):
+            # Create a new history record
+            record = self.to_history_record(value)
+
+            # Append the new record to the history
+            history = self.get_history(instance)
+            history.insert(0, record)
+
+        else:
+            raise ValueError("Type not supported: {}".format(type(value)))
+
+        # Store the data
+        ObjectField.set(self, instance, history)
+
+        # N.B. ensure updated catalog metadata for the snapshot
         instance.reindexObject()
+
         # notify object edited event
         event.notify(ObjectEditedEvent(instance))
 
-    def get_cooked_remarks(self, instance):
-        text = self.get(instance)
-        if not text:
-            return ""
-        return text.replace('\n', '<br/>')
+        # notify new remarks for e.g. later email notification etc.
+        event.notify(RemarksAddedEvent(instance, history))
 
     def get(self, instance, **kwargs):
-        """Returns raw field value.
+        """Returns a RemarksHistory object
         """
-        return self.getRaw(instance, **kwargs)
+        return self.get_history(instance)
 
     def getRaw(self, instance, **kwargs):
         """Returns raw field value (possible wrapped in BaseUnit)
@@ -88,5 +197,92 @@ def getRaw(self, instance, **kwargs):
             value = value()
         return value
 
+    def to_history_record(self, value):
+        """Transforms the value to an history record
+        """
+        user = api.get_current_user()
+        contact = api.get_user_contact(user)
+        fullname = contact and contact.getFullname() or ""
+        if not contact:
+            # get the fullname from the user properties
+            props = api.get_user_properties(user)
+            fullname = props.get("fullname", "")
+        return RemarksHistoryRecord(user_id=user.id,
+                                    user_name=fullname,
+                                    content=value.strip())
+
+    def get_history(self, instance):
+        """Returns a RemarksHistory object with the remarks entries
+        """
+        remarks = instance.getRawRemarks()
+        if not remarks:
+            return RemarksHistory()
+
+        # Backwards compatibility with legacy from < v1.3.3
+        if isinstance(remarks, six.string_types):
+            parsed_remarks = self._parse_legacy_remarks(remarks)
+            if parsed_remarks is None:
+                remark = RemarksHistoryRecord(content=remarks.strip())
+                remarks = RemarksHistory([remark, ])
+            else:
+                remarks = RemarksHistory(
+                    map(lambda r: RemarksHistoryRecord(r), parsed_remarks))
+
+        return remarks
+
+    def _parse_legacy_remarks(self, remarks):
+        """Parse legacy remarks
+        """
+        records = []
+        # split legacy remarks on the "===" delimiter into lines
+        lines = remarks.split("===")
+        for line in lines:
+            # skip empty lines
+            if line == "":
+                continue
+
+            # strip leading and trailing whitespaces
+            line = line.strip()
+
+            # split the line into date, user and content
+            groups = re.findall(r"(.*) \((.*)\)\n(.*)", line, re.DOTALL)
+
+            # we should have one tuple in the list
+            if len(groups) != 1:
+                continue
+
+            group = groups[0]
+
+            # cancel the whole parsing
+            if len(group) != 3:
+                return None
+
+            created, userid, content = group
+
+            # try to get the full name of the user id
+            fullname = self._get_fullname_from_user_id(userid)
+
+            # append the record
+            records.append({
+                "created": created,
+                "user_id": userid,
+                "user_name": fullname,
+                "content": content,
+            })
+
+        return records
+
+    def _get_fullname_from_user_id(self, userid, default=""):
+        """Try the fullname of the user
+        """
+        fullname = default
+        user = api.get_user(userid)
+        if user:
+            props = api.get_user_properties(user)
+            fullname = props.get("fullname", fullname)
+            contact = api.get_user_contact(user)
+            fullname = contact and contact.getFullname() or fullname
+        return fullname
+
 
-registerField(RemarksField, title='Remarks', description='')
+registerField(RemarksField, title="Remarks", description="")
diff --git a/bika/lims/browser/js/bika.lims.worksheet.js b/bika/lims/browser/js/bika.lims.worksheet.js
@@ -881,13 +881,17 @@
       el = event.currentTarget;
       $(el).prepOverlay({
         subtype: "ajax",
-        filter: "h1,span.remarks_history",
+        filter: "h1,div.remarks-widget",
         config: {
           closeOnClick: true,
           closeOnEsc: true,
           onBeforeLoad: function(event) {
             var overlay;
             overlay = this.getOverlay();
+            $("div.pb-ajax>div", overlay).addClass("container");
+            $("h3", overlay).remove();
+            $("textarea", overlay).remove();
+            $("input", overlay).remove();
             return overlay.draggable();
           },
           onLoad: function(event) {