From 8b7a98e8d4d80b54037964b5f68cc3b4a613c81a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= <jpuiggene@naralabs.com>
Date: Wed, 21 Feb 2018 23:53:35 +0100
Subject: [PATCH 1/6] Render analysis remarks toggling baloon in template +
 view and redux js

---
 bika/lims/browser/analyses.py                 | 53 ++++++++++++-------
 bika/lims/browser/js/bika.lims.worksheet.js   | 11 +---
 .../templates/bika_listing_table_items.pt     |  6 ++-
 3 files changed, 40 insertions(+), 30 deletions(-)

diff --git a/bika/lims/browser/analyses.py b/bika/lims/browser/analyses.py
index 41304ea37d..2d318c6ad3 100644
--- a/bika/lims/browser/analyses.py
+++ b/bika/lims/browser/analyses.py
@@ -537,7 +537,6 @@ def folderitem(self, obj, item, index):
         item['service_uid'] = obj.getServiceUID
         item['Keyword'] = obj.getKeyword
         item['Unit'] = format_supsub(obj.getUnit) if obj.getUnit else ''
-        item['Remarks'] = obj.getRemarks
         item['retested'] = obj.getRetested
         item['class']['retested'] = 'center'
 
@@ -584,6 +583,8 @@ def folderitem(self, obj, item, index):
         self._folder_item_fieldicons(obj)
         # Renders DryMatter if necessary
         self._folder_item_dry_matter(obj, item)
+        # Renders remarks toggle button
+        self._folder_item_remarks(obj, item)
 
         return item
 
@@ -1041,7 +1042,7 @@ def _folder_item_verify_icons(self, analysis_brain, item):
             # display a warning icon
             msg = t(_("Submitted and verified by the same user: {}"))
             icon = get_image('warning.png', title=msg.format(submitter))
-            self._append_after_element(item, 'state_title', icon)
+            self._append_html_element(item, 'state_title', icon)
 
         num_verifications = analysis_brain.getNumberOfRequiredVerifications
         if num_verifications > 1:
@@ -1060,7 +1061,7 @@ def _folder_item_verify_icons(self, analysis_brain, item):
                                    str(scale),
                                    str(done),
                                    str(num_verifications))
-            self._append_after_element(item, 'state_title', anchor)
+            self._append_html_element(item, 'state_title', anchor)
 
         if analysis_brain.review_state != 'to_be_verified':
             # The verification of analysis has already been done or first
@@ -1084,13 +1085,13 @@ def _folder_item_verify_icons(self, analysis_brain, item):
                 # Same user who submitted can verify
                 title = t(_("Can verify, but submitted by current user"))
                 html = get_image('warning.png', title=title)
-                self._append_after_element(item, 'state_title', html)
+                self._append_html_element(item, 'state_title', html)
                 return
 
             # User who submitted cannot verify
             title = t(_("Cannot verify, submitted by current user"))
             html = get_image('submitted-by-current-user.png', title=title)
-            self._append_after_element(item, 'state_title', html)
+            self._append_html_element(item, 'state_title', html)
             return
 
         # This user verified this analysis before
@@ -1099,7 +1100,7 @@ def _folder_item_verify_icons(self, analysis_brain, item):
             # Multi verification by same user is not allowed
             title = t(_("Cannot verify, was verified by current user"))
             html = get_image('submitted-by-current-user.png', title=title)
-            self._append_after_element(item, 'state_title', html)
+            self._append_html_element(item, 'state_title', html)
             return
 
         # Multi-verification by same user, but non-consecutively, is allowed
@@ -1107,13 +1108,13 @@ def _folder_item_verify_icons(self, analysis_brain, item):
             # Current user was not the last user to verify
             title = t(_("Can verify, but was already verified by current user"))
             html = get_image('warning.png', title=title)
-            self._append_after_element(item, 'state_title', html)
+            self._append_html_element(item, 'state_title', html)
             return
 
         # Last user who verified is the same as current user
         title = t(_("Cannot verify, last verified by current user"))
         html = get_image('submitted-by-current-user.png', title=title)
-        self._append_after_element(item, 'state_title', html)
+        self._append_html_element(item, 'state_title', html)
         return
 
     def _folder_item_assigned_worksheet(self, analysis_brain, item):
@@ -1142,7 +1143,7 @@ def _folder_item_assigned_worksheet(self, analysis_brain, item):
                     mapping={'worksheet_id': safe_unicode(worksheet.id)}))
         img = get_image('worksheet.png', title=title)
         anchor = get_link(worksheet.absolute_url(), img)
-        self._append_after_element(item, 'state_title', anchor)
+        self._append_html_element(item, 'state_title', anchor)
 
     def _folder_item_reflex_icons(self, analysis_brain, item):
         """Adds an icon to the item dictionary if the analysis has been
@@ -1156,7 +1157,7 @@ def _folder_item_reflex_icons(self, analysis_brain, item):
             return
         img = get_image('reflexrule.png',
                         title=t(_('It comes form a reflex rule')))
-        self._append_after_element(item, 'Service', img)
+        self._append_html_element(item, 'Service', img)
 
     def _folder_item_report_visibility(self, analysis_brain, item):
         """Set if the hidden field can be edited (enabled/disabled)
@@ -1216,19 +1217,33 @@ def _folder_item_dry_matter(self, analysis_brain, item):
             item['after']['ResultDM'] = "<em class='discreet'>%</em>"
         item['ResultDM'] = ''
 
-    def _append_after_element(self, item, element, html, glue="&nbsp;"):
-        """Appends html value after element in the item dict
+    def _folder_item_remarks(self, analysis_brain, item):
+        item['Remarks'] = analysis_brain.getRemarks
+        if not self.is_analysis_edition_allowed(analysis_brain):
+            return
+
+        # Analysis can be edited. Add the remarks toggle button
+        title = t(_("Add Remark"))
+        img = get_image('comment_ico.png', title=title)
+        kwargs = {'class': "add-remark"}
+        anchor = get_link('#', img, **kwargs)
+        self._append_html_element(item, 'Service', anchor, after=False)
+
+    def _append_html_element(self, item, element, html, glue="&nbsp;", after=True):
+        """Appends an html value after or before the element in the item dict
 
         :param item: dictionary that represents an analysis row
-        :element: id of the element the html must be added thereafter
-        :html: element to append
-        :glue: glue to use for appending"""
-        item['after'] = item.get('after', {})
-        original = item['after'].get(element, '')
+        :param element: id of the element the html must be added thereafter
+        :param html: element to append
+        :param glue: glue to use for appending
+        :param after: if the html content must be added after or before"""
+        position = after and 'after' or 'before'
+        item[position] = item.get(position, {})
+        original = item[position].get(element, '')
         if not original:
-            item['after'][element] = html
+            item[position][element] = html
             return
-        item['after'][element] = glue.join([original, html])
+        item[position][element] = glue.join([original, html])
 
 
 class QCAnalysesView(AnalysesView):
diff --git a/bika/lims/browser/js/bika.lims.worksheet.js b/bika/lims/browser/js/bika.lims.worksheet.js
index f67bb9b9ca..d2fcd552e3 100644
--- a/bika/lims/browser/js/bika.lims.worksheet.js
+++ b/bika/lims/browser/js/bika.lims.worksheet.js
@@ -259,19 +259,12 @@ function WorksheetManageResultsView() {
     }
 
     function loadRemarksEventHandlers() {
-        // Add a baloon icon before Analyses' name when you'd add a remark. If you click on, it'll display remarks textarea.
-        var txt1 = '<a href="#" class="add-remark"><img src="'+window.portal_url+'/++resource++bika.lims.images/comment_ico.png" title="'+_('Add Remark')+'")"></a>';
-        var pointer = $(".listing_remarks:contains('')").closest('tr').prev().find('td.service_title span.before');
-        $(pointer).append(txt1);
-
+        // On click, toggle the remarks field
         $("a.add-remark").click(function(e){
             e.preventDefault();
             var rmks = $(this).closest('tr').next('tr').find('td.remarks');
-            if (rmks.length > 0) {
-                rmks.toggle();
-            }
+            $(rmks).find('div.remarks-placeholder').toggle();
         });
-        $("a.add-remark").click();
     }
 
     function loadDetectionLimitsEventHandlers() {
diff --git a/bika/lims/browser/templates/bika_listing_table_items.pt b/bika/lims/browser/templates/bika_listing_table_items.pt
index 1ce41714f8..aada8e4d46 100644
--- a/bika/lims/browser/templates/bika_listing_table_items.pt
+++ b/bika/lims/browser/templates/bika_listing_table_items.pt
@@ -452,8 +452,7 @@
                   isanalysis python:'obj' in item and item['obj'].meta_type in ['Analysis', 'DuplicateAnalysis', 'ReferenceAnalysis'];
                   hasremarks python:True if item.get('Remarks','') else False;
                   remarksedit python: remarksenabled and 'Remarks' in item.get('allow_edit',[]);
-                  isforaggregatedlist python: True if review_state_id=='to_be_verified' else False;
-                  showremarks python:isanalysis and (hasremarks or remarksedit or isforaggregatedlist)">
+                  showremarks python:isanalysis and (hasremarks or remarksedit)">
 
       <tr tal:condition="showremarks"
           tal:define="keyword python:item.has_key('Keyword') and item['Keyword'] or '';"
@@ -467,6 +466,8 @@
         <td tal:attributes="colspan python:int(nr_cols)-1;
                             class string:${item/state_class} result remarks"
             style="padding-right:10px;padding-bottom:5px;">
+          <div class="remarks-placeholder"
+               tal:attributes="style python:'' if hasremarks else 'display:none'">
           <span i18n:translate="">Remarks:</span>&nbsp;
           <textarea style="width:100%;display:block;"
                     autocomplete="off"
@@ -485,6 +486,7 @@
                 tal:attributes="class python:'listing_remarks';
                                 name string:Remarks.${item/uid}:records;"
                 tal:content="python:item.get('Remarks', '') if hasremarks else 'No Remarks.'"/>
+          </div>
         </td>
       </tr>
     </tal:remarks_condition>

From f200a8f60743af81dd9a86a4bb8d4c8a988ef243 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= <jpuiggene@naralabs.com>
Date: Wed, 21 Feb 2018 23:56:42 +0100
Subject: [PATCH 2/6] _folder_item_remarks function documented

---
 bika/lims/browser/analyses.py | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/bika/lims/browser/analyses.py b/bika/lims/browser/analyses.py
index 2d318c6ad3..0641d4781e 100644
--- a/bika/lims/browser/analyses.py
+++ b/bika/lims/browser/analyses.py
@@ -1218,8 +1218,17 @@ def _folder_item_dry_matter(self, analysis_brain, item):
         item['ResultDM'] = ''
 
     def _folder_item_remarks(self, analysis_brain, item):
+        """Renders the Remarks field for the passed in analysis and if the
+        edition of the analysis is permitted, adds a button to toggle the
+        visibility of remarks field
+
+        :param analysis_brain: Brain that represents an analysis
+        :param item: analysis' dictionary counterpart that represents a row"""
         item['Remarks'] = analysis_brain.getRemarks
+
         if not self.is_analysis_edition_allowed(analysis_brain):
+            # Edition not allowed, do not add the remarks toggle button, the
+            # remarks field will be displayed without the option to hide it
             return
 
         # Analysis can be edited. Add the remarks toggle button

From b8028f99bbfaa6a34c7468f8a92fdb33b9b6b78e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= <jpuiggene@naralabs.com>
Date: Wed, 21 Feb 2018 23:58:40 +0100
Subject: [PATCH 3/6] Force the aggregated list of analyses to read-only

The correct way to introduce analysis results is by using Worksheets. The
introduction of results through this list is meaningless.
---
 .../lims/browser/aggregatedanalyses/aggregatedanalyses.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/bika/lims/browser/aggregatedanalyses/aggregatedanalyses.py b/bika/lims/browser/aggregatedanalyses/aggregatedanalyses.py
index 55418505c7..772fad046f 100644
--- a/bika/lims/browser/aggregatedanalyses/aggregatedanalyses.py
+++ b/bika/lims/browser/aggregatedanalyses/aggregatedanalyses.py
@@ -44,7 +44,7 @@ def __init__(self, context, request, **kwargs):
 
         # each editable item needs it's own allow_edit
         # which is a list of field names.
-        self.allow_edit = True
+        self.allow_edit = False
 
         self.columns['AnalysisRequest'] = {
             'title': _('Analysis Request'),
@@ -144,6 +144,12 @@ def __init__(self, context, request, **kwargs):
     def getPOSTAction(self):
         return 'aggregatedanalyses_workflow_action'
 
+    def is_analysis_edition_allowed(self, analysis_brain):
+        """Overrides the function from the parent, cause we don't want in any
+        case the analyses displayed in aggregated list to be editable. The
+        correct way to introduce results is by using Worksheets!"""
+        return False
+
     def isItemAllowed(self, obj):
         """
         Checks if the passed in Analysis must be displayed in the list. If the

From 944058343a7b2e8c0111e8f168510493e1df5399 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= <jpuiggene@naralabs.com>
Date: Thu, 22 Feb 2018 00:03:16 +0100
Subject: [PATCH 4/6] Styling of analysis remarks field... that border was
 really ugly

---
 bika/lims/skins/bika/bika_listing.css.dtml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bika/lims/skins/bika/bika_listing.css.dtml b/bika/lims/skins/bika/bika_listing.css.dtml
index c188b8acc4..5d5c267408 100644
--- a/bika/lims/skins/bika/bika_listing.css.dtml
+++ b/bika/lims/skins/bika/bika_listing.css.dtml
@@ -81,8 +81,8 @@ table.bika-listing-table {
 }
 
 .bika-listing-table .listing_remarks {
+  color:#444;
   width:99%;
-  border:1px solid #ababab;
   margin:2px 0px;
 }
 

From b34072135d98ce0efdbf00bba7aace75e4ccc33b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= <jpuiggene@naralabs.com>
Date: Thu, 22 Feb 2018 00:05:42 +0100
Subject: [PATCH 5/6] Added #683 to CHANGES.rst

---
 CHANGES.rst | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGES.rst b/CHANGES.rst
index 416be0dd1c..3cb2348bba 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -11,6 +11,7 @@ Changelog
 
 **Changed**
 
+- #683 Aggregated lists of analyses have been set to read-only
 - #674 Dashboard with slightly better performance
 - #621 AnalysesView code refactoring
 - #668 AR Add: Debounce expensive XHR calls

From d81c5db43115fb13fc8c361572813a6c38a37969 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= <jp@naralabs.com>
Date: Wed, 21 Feb 2018 23:15:38 +0000
Subject: [PATCH 6/6] Update CHANGES.rst

---
 CHANGES.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/CHANGES.rst b/CHANGES.rst
index 3cb2348bba..6ed296e842 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -11,7 +11,7 @@ Changelog
 
 **Changed**
 
-- #683 Aggregated lists of analyses have been set to read-only
+- #684 Aggregated lists of analyses set to read-only mode
 - #674 Dashboard with slightly better performance
 - #621 AnalysesView code refactoring
 - #668 AR Add: Debounce expensive XHR calls