From a9a6f9c347d4d69f74285751f00148b9e47f2266 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Sun, 19 Jan 2020 13:38:06 +0100 Subject: [PATCH 01/63] Better handling of Specification + ResultsRange in Sample --- .../browser/fields/specificationsfield.py | 70 ++++++++++ .../widgets/analysisspecificationwidget.py | 3 +- bika/lims/content/analysisrequest.py | 126 ++++++++---------- bika/lims/content/analysisspec.py | 35 +---- 4 files changed, 126 insertions(+), 108 deletions(-) create mode 100644 bika/lims/browser/fields/specificationsfield.py diff --git a/bika/lims/browser/fields/specificationsfield.py b/bika/lims/browser/fields/specificationsfield.py new file mode 100644 index 0000000000..72b00abf34 --- /dev/null +++ b/bika/lims/browser/fields/specificationsfield.py @@ -0,0 +1,70 @@ +from operator import itemgetter + +from Products.ATExtensions.field import RecordsField + +from bika.lims import api +from bika.lims import bikaMessageFactory as _ +from bika.lims.browser.widgets import AnalysisSpecificationWidget + + +# A tuple of (subfield_id, subfield_label,) +SUB_FIELDS = ( + ("keyword", _("Analysis Service")), + ("min_operator", _("Min operator")), + ("min", _('Min')), + ("max_operator", _("Max operator")), + ("max", _('Max')), + ("warn_min", _('Min warn')), + ("warn_max", _('Max warn')), + ("hidemin", _('< Min')), + ("hidemax", _('> Max')), + ("rangecomment", _('Range Comment')), +) + + +class SpecificationsField(RecordsField): + """A field that stores a list of results ranges + """ + _properties = RecordsField._properties.copy() + _properties.update({ + "subfields": map(itemgetter(0), SUB_FIELDS), + "subfield_labels": dict(SUB_FIELDS), + "subfield_validators": { + "min": "analysisspecs_validator", + "max": "analysisspecs_validator", + }, + "required_subfields": ("keyword", ), + "widget": AnalysisSpecificationWidget, + }) + + def get(self, instance, **kwargs): + values = super(SpecificationsField, self).get(instance, **kwargs) + uid_keyword_service = kwargs.get("uid", None) + if uid_keyword_service: + return self.getResultsRange(values, uid_keyword_service) + return values or [] + + def getResultsRange(self, values, uid_keyword_service): + if not uid_keyword_service: + return None + + if api.is_object(uid_keyword_service): + uid_keyword_service = api.get_uid(uid_keyword_service) + + key = "keyword" + if api.is_uid(uid_keyword_service) and uid_keyword_service != "0": + # We always assume a uid of "0" refers to portal + key = "uid" + + # Find out the item for the given uid/keyword + value = filter(lambda v: v.get(key) == uid_keyword_service, values) + return value and value[0] or None + + def _to_dict(self, value): + """Convert the records to persistent dictionaries + """ + value = super(SpecificationsField, self)._to_dict(value) + + # Bail out items without "uid" key (uid is used everywhere to know the + # service the result range refers to) + return filter(lambda result_range: "uid" in result_range, value) diff --git a/bika/lims/browser/widgets/analysisspecificationwidget.py b/bika/lims/browser/widgets/analysisspecificationwidget.py index 20dd25fb38..bc6407db49 100644 --- a/bika/lims/browser/widgets/analysisspecificationwidget.py +++ b/bika/lims/browser/widgets/analysisspecificationwidget.py @@ -243,7 +243,8 @@ class AnalysisSpecificationWidget(TypesWidget): """ _properties = TypesWidget._properties.copy() _properties.update({ - 'macro': "bika_widgets/analysisspecificationwidget", + "checkbox_bound": 0, + "macro": "bika_widgets/analysisspecificationwidget", }) security = ClassSecurityInfo() diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 53d5255680..80fe46148e 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -25,6 +25,33 @@ from urlparse import urljoin from AccessControl import ClassSecurityInfo +from DateTime import DateTime +from Products.ATExtensions.field import RecordsField +from Products.Archetypes.Widget import RichWidget +from Products.Archetypes.atapi import BaseFolder +from Products.Archetypes.atapi import BooleanField +from Products.Archetypes.atapi import BooleanWidget +from Products.Archetypes.atapi import ComputedField +from Products.Archetypes.atapi import ComputedWidget +from Products.Archetypes.atapi import FileField +from Products.Archetypes.atapi import FileWidget +from Products.Archetypes.atapi import FixedPointField +from Products.Archetypes.atapi import ReferenceField +from Products.Archetypes.atapi import StringField +from Products.Archetypes.atapi import StringWidget +from Products.Archetypes.atapi import TextField +from Products.Archetypes.atapi import registerType +from Products.Archetypes.public import Schema +from Products.Archetypes.references import HoldingReference +from Products.CMFCore.permissions import ModifyPortalContent +from Products.CMFCore.permissions import View +from Products.CMFCore.utils import getToolByName +from Products.CMFPlone.utils import _createObjectByType +from Products.CMFPlone.utils import safe_unicode +from zope.interface import alsoProvides +from zope.interface import implements +from zope.interface import noLongerProvides + from bika.lims import api from bika.lims import bikaMessageFactory as _ from bika.lims import deprecated @@ -34,6 +61,7 @@ from bika.lims.browser.fields import DurationField from bika.lims.browser.fields import UIDReferenceField from bika.lims.browser.fields.remarksfield import RemarksField +from bika.lims.browser.fields.specificationsfield import SpecificationsField from bika.lims.browser.widgets import DateTimeWidget from bika.lims.browser.widgets import DecimalWidget from bika.lims.browser.widgets import PrioritySelectionWidget @@ -47,7 +75,6 @@ from bika.lims.catalog.bika_catalog import BIKA_CATALOG from bika.lims.config import PRIORITIES from bika.lims.config import PROJECTNAME -from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.content.bikaschema import BikaSchema from bika.lims.content.clientawaremixin import ClientAwareMixin from bika.lims.interfaces import IAnalysisRequest @@ -81,8 +108,8 @@ from bika.lims.permissions import FieldEditResultsInterpretation from bika.lims.permissions import FieldEditSampleCondition from bika.lims.permissions import FieldEditSamplePoint -from bika.lims.permissions import FieldEditSampler from bika.lims.permissions import FieldEditSampleType +from bika.lims.permissions import FieldEditSampler from bika.lims.permissions import FieldEditSamplingDate from bika.lims.permissions import FieldEditSamplingDeviation from bika.lims.permissions import FieldEditSamplingRound @@ -97,32 +124,6 @@ from bika.lims.utils import user_fullname from bika.lims.workflow import getTransitionDate from bika.lims.workflow import getTransitionUsers -from DateTime import DateTime -from Products.Archetypes.atapi import BaseFolder -from Products.Archetypes.atapi import BooleanField -from Products.Archetypes.atapi import BooleanWidget -from Products.Archetypes.atapi import ComputedField -from Products.Archetypes.atapi import ComputedWidget -from Products.Archetypes.atapi import FileField -from Products.Archetypes.atapi import FileWidget -from Products.Archetypes.atapi import FixedPointField -from Products.Archetypes.atapi import ReferenceField -from Products.Archetypes.atapi import StringField -from Products.Archetypes.atapi import StringWidget -from Products.Archetypes.atapi import TextField -from Products.Archetypes.atapi import registerType -from Products.Archetypes.public import Schema -from Products.Archetypes.references import HoldingReference -from Products.Archetypes.Widget import RichWidget -from Products.ATExtensions.field import RecordsField -from Products.CMFCore.permissions import ModifyPortalContent -from Products.CMFCore.permissions import View -from Products.CMFCore.utils import getToolByName -from Products.CMFPlone.utils import _createObjectByType -from Products.CMFPlone.utils import safe_unicode -from zope.interface import alsoProvides -from zope.interface import implements -from zope.interface import noLongerProvides IMG_SRC_RX = re.compile(r' Max'), - 'rangecomment': _('Range Comment'), - }, widget=AnalysisSpecificationWidget( - checkbox_bound=0, label=_("Specifications"), description=_( "'Min' and 'Max' values indicate a valid results range. Any " From 9f965bf0b9f1fa99583ff9cf9c3e9090135acffb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Sun, 19 Jan 2020 13:39:44 +0100 Subject: [PATCH 02/63] Added viewlet for Specification vs Results Range differences --- bika/lims/browser/viewlets/analysisrequest.py | 38 +++++++++++++++++ bika/lims/browser/viewlets/configure.zcml | 11 +++++ .../resultsranges_out_of_date_viewlet.pt | 41 +++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index dbd2afd941..cf7c029897 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -20,7 +20,11 @@ from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile from plone.app.layout.viewlets import ViewletBase + +from bika.lims import FieldEditSpecification from bika.lims import api +from bika.lims import logger +from bika.lims.api.security import check_permission class InvalidAnalysisRequestViewlet(ViewletBase): @@ -73,3 +77,37 @@ class DetachedPartitionViewlet(ViewletBase): detached from """ template = ViewPageTemplateFile("templates/detached_partition_viewlet.pt") + + +class ResultsRangesOutOfDateViewlet(ViewletBase): + """Print a viewlet that displays if results ranges from Sample are different + from results ranges initially set through Specifications field. If so, this + means the Specifications initially set have changed since they were set and + for new analyses, the old specifications will be kept + """ + + def is_specification_editable(self): + """Returns whether the Specification field is editable or not + """ + return check_permission(FieldEditSpecification, self.context) + + def is_results_ranges_out_of_date(self): + """Returns whether the value for ResultsRange field does not match with + the results ranges that come from the Specification assigned + """ + sample = self.context + sample_rr = sample.getResultsRange() + if not sample_rr: + # No results ranges set to this Sample, do nothing + return False + + specifications = sample.getSpecification() + if not specifications: + # This should never happen + logger.error("No specifications, but results ranges set for {}" + .format(api.get_id(sample))) + return False + + # Compare if results ranges are different + spec_rr = specifications.getResultsRange() + return sample_rr != spec_rr diff --git a/bika/lims/browser/viewlets/configure.zcml b/bika/lims/browser/viewlets/configure.zcml index 180908b0c5..fb1fee9858 100644 --- a/bika/lims/browser/viewlets/configure.zcml +++ b/bika/lims/browser/viewlets/configure.zcml @@ -171,6 +171,17 @@ layer="bika.lims.interfaces.IBikaLIMS" /> + + + + +
+ +
+ +
+ + Info + Warning +

+ + Specification ranges have changed since they were assigned. + +

+

+ + New ranges won't be applied to neither new nor current analyses. The + reassignment of the Specification will apply latest changes, but will + also override those that have been manually set to analyses. + +
+ + Visit the Specification's changes history for additional information: + + +

+
+
+ From 3223886acd04ac33daf0eb73bdcb283b7c01bbf8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Mon, 20 Jan 2020 10:17:40 +0100 Subject: [PATCH 03/63] Viewlet for when field change is populated to partitions --- bika/lims/browser/viewlets/analysisrequest.py | 22 +++++++++++ .../viewlets/templates/primary_ar_viewlet.pt | 39 ++++++++++++++++--- .../resultsranges_out_of_date_viewlet.pt | 8 ++-- bika/lims/content/analysisrequest.py | 9 +++++ 4 files changed, 67 insertions(+), 11 deletions(-) diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index cf7c029897..21d61b3af8 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -25,6 +25,7 @@ from bika.lims import api from bika.lims import logger from bika.lims.api.security import check_permission +from zope.schema import getFields class InvalidAnalysisRequestViewlet(ViewletBase): @@ -53,6 +54,27 @@ def get_partitions(self): return [] return self.context.getDescendants() + def get_primary_bound_fields(self): + """Returns a list with the names of the fields the current user has + write priveleges and for which changes in primary sample will apply to + its descendants too + """ + bound = [] + schema = api.get_schema(self.context) + for field in schema.fields(): + if not hasattr(field, "primary_bound"): + continue + + if not check_permission(field.write_permission, self.context): + # Current user does not have permission to edit this field + continue + + if field.primary_bound: + # Change in this field will populate to partitions + bound.append(field.widget.label) + + return bound + class PartitionAnalysisRequestViewlet(ViewletBase): """ Current Analysis Request is a partition. Display the link to primary diff --git a/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt b/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt index bca8cd46cc..a4bdae30f4 100644 --- a/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt +++ b/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt @@ -5,21 +5,48 @@
-
+
+
- Info

- The following partitions have been created from this Sample: + + Sample with partitions + +

+

+ + The following partitions have been created from this Sample: + - + tal:content="python:part.getId()" /> +

+ +
+ +

+ + Some changes might override partitions + +

+

+ + Changes to any of the following fields will also be applied to the + partitions in which the field is editable, even if they have a + different value already set: + + +

+
diff --git a/bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt b/bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt index 0fe39faaf8..38d63578c2 100644 --- a/bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt +++ b/bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt @@ -15,12 +15,10 @@ - Info - Warning

- - Specification ranges have changed since they were assigned. - + + Specification ranges have changed since they were assigned +

diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 80fe46148e..da7cfd7e6b 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -56,6 +56,7 @@ from bika.lims import bikaMessageFactory as _ from bika.lims import deprecated from bika.lims import logger +from bika.lims.api.security import check_permission from bika.lims.browser.fields import ARAnalysesField from bika.lims.browser.fields import DateTimeField from bika.lims.browser.fields import DurationField @@ -669,6 +670,7 @@ ReferenceField( 'Specification', required=0, + primary_bound=True, allowed_types='AnalysisSpec', relationship='AnalysisRequestAnalysisSpec', mode="rw", @@ -1437,6 +1439,7 @@ def setSpecification(self, value): """Sets the Specifications and ResultRange values """ self.getField("Specification").set(self, value) + # Set the value for field ResultsRange, so results ranges defined by # the specification won't change for this Sample, even if they are # changed later in the Specification object or result ranges are @@ -1445,6 +1448,12 @@ def setSpecification(self, value): results_range = spec and spec.getResultsRange() or [] self.getField("ResultsRange").set(self, results_range) + # Apply same change to partitions + permission = self.getField("Specification").write_permission + for descendant in self.getDescendants(all_descendants=True): + if check_permission(permission, descendant): + descendant.setSpecification(spec) + def getClient(self): """Returns the client this object is bound to. We override getClient from ClientAwareMixin because the "Client" schema field is only used to From c6ecd41b51914ec66896a7e9c079a587327e7dc7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Mon, 20 Jan 2020 11:25:37 +0100 Subject: [PATCH 04/63] Added IAnalysisRequestWithPartitions interface --- bika/lims/browser/viewlets/analysisrequest.py | 1 - bika/lims/browser/viewlets/configure.zcml | 6 ++-- bika/lims/content/analysisrequest.py | 6 ++++ bika/lims/interfaces/__init__.py | 15 ++++++++++ bika/lims/upgrade/v01_03_003.py | 28 +++++++++++++++++++ 5 files changed, 52 insertions(+), 4 deletions(-) diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index 21d61b3af8..ee5ee869ed 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -25,7 +25,6 @@ from bika.lims import api from bika.lims import logger from bika.lims.api.security import check_permission -from zope.schema import getFields class InvalidAnalysisRequestViewlet(ViewletBase): diff --git a/bika/lims/browser/viewlets/configure.zcml b/bika/lims/browser/viewlets/configure.zcml index fb1fee9858..f8d2e146e8 100644 --- a/bika/lims/browser/viewlets/configure.zcml +++ b/bika/lims/browser/viewlets/configure.zcml @@ -140,7 +140,7 @@ Date: Mon, 20 Jan 2020 16:56:19 +0100 Subject: [PATCH 05/63] Store ResultsRange directly to Analyses --- bika/lims/browser/fields/aranalysesfield.py | 174 +++++++++++------- bika/lims/browser/fields/configure.zcml | 12 +- .../browser/fields/specificationsfield.py | 69 ++++++- bika/lims/content/abstractanalysis.py | 14 +- bika/lims/content/abstractroutineanalysis.py | 45 +---- bika/lims/content/analysisrequest.py | 22 ++- bika/lims/content/referenceanalysis.py | 4 +- bika/lims/workflow/analysisrequest/events.py | 6 + 8 files changed, 226 insertions(+), 120 deletions(-) diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index f3fbfd6082..5bdf32656a 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -136,7 +136,7 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): services = set(services + dependencies) # Modify existing AR specs with new form values of selected analyses. - self._update_specs(instance, specs) + specs = self.resolve_specs(instance, specs) # Create a mapping of Service UID -> Hidden status if hidden is None: @@ -148,10 +148,11 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): prices = dict() # Add analyses - new_analyses = map(lambda service: - self.add_analysis(instance, service, prices, hidden), - services) - new_analyses = filter(None, new_analyses) + new_analyses = [] + for service in services: + an = self.add_analysis(instance, service, prices, hidden, specs) + if an: + new_analyses.append(an) # Remove analyses # Since Manage Analyses view displays the analyses from partitions, we @@ -202,42 +203,122 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): return new_analyses - def add_analysis(self, instance, service, prices, hidden): + def resolve_specs(self, instance, specs): + """Returns a dictionary where the key is the service_uid and the value + is its results range. If a result range for a given service does not + exist or does not match with the specs defined in the sample, the + function resets the Sample's Specification field to guarantee compliance + with the Specification value actually set + """ + form_specs = specs or [] + + # Copy the passed in dict, otherwise the specification might be written + # directly to the attached specification of the sample + sample_specs = dict(map(lambda rr: (rr["uid"], rr.copy()), + instance.getResultsRange())) + + out_specs = dict() + reset_specification = False + for form_rr in form_specs: + service_uid = form_rr["uid"] + sample_rr = sample_specs.get(service_uid) + if not sample_rr: + # Sample's ResultsRange (a "copy" of Specification initially set + # does not contain a result range for this service + logger.info("Result range for {} not present in Sample's" + .format(form_rr["keyword"])) + reset_specification = True + + else: + # Clean-up the result range passed in + form_rr = self.resolve_result_range(form_rr, sample_rr) + + if form_rr != sample_rr: + # Result range for this service has been changed manually, + # it does not match with sample's ResultRange + logger.warn("Result Range for {} do not match with Sample's" + .format(form_rr["keyword"])) + reset_specification = True + + # Generate the results_range format + out_specs[service_uid] = form_rr + + if reset_specification: + # Reset the specification, cause the sample won't longer be fully + # compliant with the Specifications it has assigned + instance.setSpecification(None) + + # We store the values in Sample's ResultsRange because although the + # specification is not met, we still want Sample's ResultsRange to + # act as a "template" for new analyses + instance.setResultsRange(out_specs.values()) + + return out_specs + + def resolve_result_range(self, result_range, original): + """Cleans up the result range passed-in to match with same keys as the + original result range, that presumably comes from a Specification + """ + if not original: + return result_range + + # Remove keys-values not present in original + extra_keys = filter(lambda key: key not in original, result_range) + for key in extra_keys: + del result_range[key] + + # Add keys-values not present in current result_range but in original + for key, val in original.items(): + if key not in result_range: + result_range[key] = val + + return result_range + + def add_analysis(self, instance, service, prices, hidden, specs=None): service_uid = api.get_uid(service) new_analysis = False # Gets the analysis or creates the analysis for this service - # Note this analysis might not belong to this current instance, but - # from a descendant (partition) - analysis = self.resolve_analysis(instance, service) - if not analysis: + # Note this returns a list, because is possible to have multiple + # partitions with same analysis + analyses = self.resolve_analyses(instance, service) + if not analyses: # Create the analysis new_analysis = True keyword = service.getKeyword() logger.info("Creating new analysis '{}'".format(keyword)) analysis = create_analysis(instance, service) + analyses.append(analysis) + + for analysis in analyses: + # Set the hidden status + analysis.setHidden(hidden.get(service_uid, False)) - # Set the hidden status - analysis.setHidden(hidden.get(service_uid, False)) + # Set the price of the Analysis + analysis.setPrice(prices.get(service_uid, service.getPrice())) - # Set the price of the Analysis - analysis.setPrice(prices.get(service_uid, service.getPrice())) + # Set the result range to the Analysis + analysis_rr = specs.get(service_uid) or analysis.getResultsRange() + analysis.setResultsRange(analysis_rr) + analysis.reindexObject() # Only return the analysis if is a new one if new_analysis: - return analysis + return analyses[0] return None - def resolve_analysis(self, instance, service): - """Resolves an analysis for the service and instance + def resolve_analyses(self, instance, service): + """Resolves analyses for the service and instance + It returns a list, cause for a given sample, multiple analyses for same + service can exist due to the possibility of having multiple partitions """ + analyses = [] + # Does the analysis exists in this instance already? analysis = self.get_from_instance(instance, service) if analysis: - keyword = service.getKeyword() - logger.info("Analysis for '{}' already exists".format(keyword)) - return analysis + analyses.append(analysis) # Does the analysis exists in an ancestor? from_ancestor = self.get_from_ancestor(instance, service) @@ -248,18 +329,12 @@ def resolve_analysis(self, instance, service): logger.info("Analysis {} is from an ancestor".format(analysis_id)) cp = from_ancestor.aq_parent.manage_cutObjects(analysis_id) instance.manage_pasteObjects(cp) - return instance._getOb(analysis_id) + analyses.append(instance._getOb(analysis_id)) - # Does the analysis exists in a descendant? + # Does the analysis exists in descendants? from_descendant = self.get_from_descendant(instance, service) - if from_descendant: - # The analysis already exists in a partition, keep it. The - # analysis from current instance will be masked otherwise - analysis_id = api.get_id(from_descendant) - logger.info("Analysis {} is from a descendant".format(analysis_id)) - return from_descendant - - return None + analyses.extend(from_descendant) + return analyses def get_analyses_from_descendants(self, instance): """Returns all the analyses from descendants @@ -289,20 +364,20 @@ def get_from_ancestor(self, instance, service): return analysis or self.get_from_ancestor(ancestor, service) def get_from_descendant(self, instance, service): - """Returns an analysis for the given service from descendants + """Returns analyses for the given service from descendants """ + analyses = [] for descendant in instance.getDescendants(): # Does the analysis exists in the current descendant? analysis = self.get_from_instance(descendant, service) if analysis: - return analysis + analyses.append(analysis) # Search in descendants from current descendant - analysis = self.get_from_descendant(descendant, service) - if analysis: - return analysis + from_descendant = self.get_from_descendant(descendant, service) + analyses.extend(from_descendant) - return None + return analyses def _get_services(self, full_objects=False): """Fetch and return analysis service objects @@ -345,33 +420,6 @@ def _to_service(self, thing): "The object will be dismissed.".format(portal_type)) return None - def _update_specs(self, instance, specs): - """Update AR specifications - - :param instance: Analysis Request - :param specs: List of Specification Records - """ - - if specs is None: - return - - # N.B. we copy the records here, otherwise the spec will be written to - # the attached specification of this AR - rr = {item["keyword"]: item.copy() - for item in instance.getResultsRange()} - for spec in specs: - keyword = spec.get("keyword") - if keyword in rr: - # overwrite the instance specification only, if the specific - # analysis spec has min/max values set - if all([spec.get("min"), spec.get("max")]): - rr[keyword].update(spec) - else: - rr[keyword] = spec - else: - rr[keyword] = spec - return instance.setResultsRange(rr.values()) - registerField(ARAnalysesField, title="Analyses", diff --git a/bika/lims/browser/fields/configure.zcml b/bika/lims/browser/fields/configure.zcml index 9ebfc6645e..27f9316ca3 100644 --- a/bika/lims/browser/fields/configure.zcml +++ b/bika/lims/browser/fields/configure.zcml @@ -1,7 +1,15 @@ + + + diff --git a/bika/lims/browser/fields/specificationsfield.py b/bika/lims/browser/fields/specificationsfield.py index 72b00abf34..dc1130c40c 100644 --- a/bika/lims/browser/fields/specificationsfield.py +++ b/bika/lims/browser/fields/specificationsfield.py @@ -1,13 +1,17 @@ from operator import itemgetter +from Products.ATExtensions.field import RecordField from Products.ATExtensions.field import RecordsField +from Products.Archetypes.Registry import registerField +from Products.Archetypes.interfaces import IFieldDefaultProvider +from zope.interface import implements from bika.lims import api from bika.lims import bikaMessageFactory as _ from bika.lims.browser.widgets import AnalysisSpecificationWidget - - # A tuple of (subfield_id, subfield_label,) +from bika.lims.interfaces.analysis import IRequestAnalysis + SUB_FIELDS = ( ("keyword", _("Analysis Service")), ("min_operator", _("Min operator")), @@ -22,11 +26,34 @@ ) +class ResultsRangeField(RecordField): + """A field that stores a results range + """ + _properties = RecordField._properties.copy() + _properties.update({ + "type": "results_range_field", + "subfields": map(itemgetter(0), SUB_FIELDS), + "subfield_labels": dict(SUB_FIELDS), + }) + + def get(self, instance, **kwargs): + from bika.lims.content.analysisspec import ResultsRangeDict + value = super(ResultsRangeField, self).get(instance, **kwargs) + if value: + return ResultsRangeDict(value) + return None + + +registerField(ResultsRangeField, title="ResultsRange", + description="Used for storing a results range",) + + class SpecificationsField(RecordsField): """A field that stores a list of results ranges """ _properties = RecordsField._properties.copy() _properties.update({ + "type": "specifications", "subfields": map(itemgetter(0), SUB_FIELDS), "subfield_labels": dict(SUB_FIELDS), "subfield_validators": { @@ -38,13 +65,17 @@ class SpecificationsField(RecordsField): }) def get(self, instance, **kwargs): + from bika.lims.content.analysisspec import ResultsRangeDict values = super(SpecificationsField, self).get(instance, **kwargs) - uid_keyword_service = kwargs.get("uid", None) + uid_keyword_service = kwargs.get("uid", kwargs.get("keyword", None)) if uid_keyword_service: return self.getResultsRange(values, uid_keyword_service) - return values or [] + if values: + return map(lambda val: ResultsRangeDict(val), values) + return [] def getResultsRange(self, values, uid_keyword_service): + from bika.lims.content.analysisspec import ResultsRangeDict if not uid_keyword_service: return None @@ -58,7 +89,7 @@ def getResultsRange(self, values, uid_keyword_service): # Find out the item for the given uid/keyword value = filter(lambda v: v.get(key) == uid_keyword_service, values) - return value and value[0] or None + return value and ResultsRangeDict(value[0]) or None def _to_dict(self, value): """Convert the records to persistent dictionaries @@ -68,3 +99,31 @@ def _to_dict(self, value): # Bail out items without "uid" key (uid is used everywhere to know the # service the result range refers to) return filter(lambda result_range: "uid" in result_range, value) + + +class DefaultResultsRangeProvider(object): + """Default Results Range provider for analyses + This is used for backwards-compatibility for when the analysis' ResultsRange + was obtained directly from Sample's ResultsRanges field, before this: + https://github.com/senaite/senaite.core/pull/1506 + """ + implements(IFieldDefaultProvider) + + def __init__(self, context): + self.context = context + + def __call__(self): + """Get the default value. + """ + if not IRequestAnalysis.providedBy(self.context): + return None + + keyword = self.context.getKeyword() + sample = self.context.getRequest() + field = sample.getField("ResultsRange") + rr = field.get(sample, keyword=keyword) + if rr: + self.context.setResultsRange(rr) + return self.context.getResultsRange() + + return None diff --git a/bika/lims/content/abstractanalysis.py b/bika/lims/content/abstractanalysis.py index 279c9f0452..c0e4dc231c 100644 --- a/bika/lims/content/abstractanalysis.py +++ b/bika/lims/content/abstractanalysis.py @@ -40,6 +40,7 @@ from bika.lims.browser.fields import HistoryAwareReferenceField from bika.lims.browser.fields import InterimFieldsField from bika.lims.browser.fields import UIDReferenceField +from bika.lims.browser.fields.specificationsfield import ResultsRangeField from bika.lims.browser.fields.uidreferencefield import get_backreferences from bika.lims.browser.widgets import RecordsWidget from bika.lims.config import LDL @@ -148,6 +149,12 @@ ) ) +# Results Range that applies to this analysis +ResultsRange = ResultsRangeField( + "ResultsRange", + required=0 +) + schema = schema.copy() + Schema(( AnalysisService, Analyst, @@ -160,7 +167,8 @@ RetestOf, Uncertainty, Calculation, - InterimFields + InterimFields, + ResultsRange, )) @@ -484,10 +492,6 @@ def setResult(self, value): # Set the result field self.getField("Result").set(self, val) - @security.public - def getResultsRange(self): - raise NotImplementedError("getResultsRange is not implemented.") - @security.public def calculateResult(self, override=False, cascade=False): """Calculates the result for the current analysis if it depends of diff --git a/bika/lims/content/abstractroutineanalysis.py b/bika/lims/content/abstractroutineanalysis.py index 8e160b40d7..ad833d6338 100644 --- a/bika/lims/content/abstractroutineanalysis.py +++ b/bika/lims/content/abstractroutineanalysis.py @@ -21,6 +21,15 @@ from datetime import timedelta from AccessControl import ClassSecurityInfo +from Products.ATContentTypes.utils import DT2dt +from Products.ATContentTypes.utils import dt2DT +from Products.Archetypes.Field import BooleanField +from Products.Archetypes.Field import FixedPointField +from Products.Archetypes.Field import StringField +from Products.Archetypes.Schema import Schema +from Products.CMFCore.permissions import View +from zope.interface import implements + from bika.lims import api from bika.lims import bikaMessageFactory as _ from bika.lims.browser.fields import UIDReferenceField @@ -28,7 +37,6 @@ from bika.lims.catalog.indexers.baseanalysis import sortable_title from bika.lims.content.abstractanalysis import AbstractAnalysis from bika.lims.content.abstractanalysis import schema -from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.content.clientawaremixin import ClientAwareMixin from bika.lims.content.reflexrule import doReflexRuleAction from bika.lims.interfaces import IAnalysis @@ -36,15 +44,6 @@ from bika.lims.interfaces import IRoutineAnalysis from bika.lims.interfaces.analysis import IRequestAnalysis from bika.lims.workflow import getTransitionDate -from Products.Archetypes.Field import BooleanField -from Products.Archetypes.Field import FixedPointField -from Products.Archetypes.Field import StringField -from Products.Archetypes.Schema import Schema -from Products.ATContentTypes.utils import DT2dt -from Products.ATContentTypes.utils import dt2DT -from Products.CMFCore.permissions import View -from zope.interface import implements - # True if the analysis is created by a reflex rule IsReflexAnalysis = BooleanField( @@ -112,7 +111,6 @@ default=False, ) - schema = schema.copy() + Schema(( IsReflexAnalysis, OriginalReflexedAnalysis, @@ -332,31 +330,6 @@ def getAnalysisRequestPrintStatus(self): if request: return request.getPrinted() - @security.public - def getResultsRange(self): - """Returns the valid result range for this routine analysis based on the - results ranges defined in the Analysis Request this routine analysis is - assigned to. - - A routine analysis will be considered out of range if it result falls - out of the range defined in "min" and "max". If there are values set for - "warn_min" and "warn_max", these are used to compute the shoulders in - both ends of the range. Thus, an analysis can be out of range, but be - within shoulders still. - :return: A dictionary with keys "min", "max", "warn_min" and "warn_max" - :rtype: dict - """ - specs = ResultsRangeDict() - analysis_request = self.getRequest() - if not analysis_request: - return specs - - keyword = self.getKeyword() - ar_ranges = analysis_request.getResultsRange() - # Get the result range that corresponds to this specific analysis - an_range = [rr for rr in ar_ranges if rr.get('keyword', '') == keyword] - return an_range and an_range[0] or specs - @security.public def getSiblings(self, retracted=False): """ diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 1eafd211bd..ba02ff588b 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -1441,15 +1441,21 @@ def setSpecification(self, value): """ self.getField("Specification").set(self, value) - # Set the value for field ResultsRange, so results ranges defined by - # the specification won't change for this Sample, even if they are - # changed later in the Specification object or result ranges are - # manually changed for the analyses this Sample contains + # Set the value for field ResultsRange, cause Specification is only used + # as a template: all the results range logic relies on ResultsRange + # field, so changes in setup's Specification object won't have effect to + # already created samples spec = self.getSpecification() - results_range = spec and spec.getResultsRange() or [] - self.getField("ResultsRange").set(self, results_range) - - # Apply same change to partitions + if spec: + # Update only results ranges if specs is not None, so results ranges + # manually set previously (e.g. via ManageAnalyses view) are + # preserved unless a new Specification overrides them + self.getField("ResultsRange").set(self, spec.getResultsRange()) + + # Cascade the changes to partitions, but only to those for which that + # are in a status in which the specification can be updated. This + # prevents the re-assignment of Specifications to already verified or + # published samples permission = self.getField("Specification").write_permission for descendant in self.getDescendants(all_descendants=True): if check_permission(permission, descendant): diff --git a/bika/lims/content/referenceanalysis.py b/bika/lims/content/referenceanalysis.py index 2e41eed68e..94aa0d7228 100644 --- a/bika/lims/content/referenceanalysis.py +++ b/bika/lims/content/referenceanalysis.py @@ -120,7 +120,9 @@ def getResultsRange(self): :return: A dictionary with the keys min and max :rtype: dict """ - specs = ResultsRangeDict(result="") + specs = ResultsRangeDict(uid=api.get_uid(self), + keyword=self.getKeyword(), + result="") sample = self.getSample() if not sample: return specs diff --git a/bika/lims/workflow/analysisrequest/events.py b/bika/lims/workflow/analysisrequest/events.py index 8edc785c19..2b570ac17c 100644 --- a/bika/lims/workflow/analysisrequest/events.py +++ b/bika/lims/workflow/analysisrequest/events.py @@ -202,3 +202,9 @@ def after_detach(analysis_request): # Reindex both the parent and the detached one analysis_request.reindexObject() parent.reindexObject() + + # And the analyses too. aranalysesfield relies on a search against the + # catalog to return the analyses: calling `getAnalyses` to the parent + # will return all them, so no need to do the same with the detached + analyses = parent.getAnalyses(full_objects=True) + map(lambda an: an.reindexObject(), analyses) From fce8121d6d5cc75038e047c45ff3ecc90361cf4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:12:33 +0100 Subject: [PATCH 06/63] Typo --- bika/lims/utils/analysis.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bika/lims/utils/analysis.py b/bika/lims/utils/analysis.py index f3aa92d3cb..84948b68b1 100644 --- a/bika/lims/utils/analysis.py +++ b/bika/lims/utils/analysis.py @@ -78,6 +78,7 @@ def copy_analysis_field_values(source, analysis, **kwargs): mutator = getattr(analysis, mutator_name) mutator(value) + def create_analysis(context, source, **kwargs): """Create a new Analysis. The source can be an Analysis Service or an existing Analysis, and all possible field values will be set to the @@ -85,7 +86,7 @@ def create_analysis(context, source, **kwargs): :param context: The analysis will be created inside this object. :param source: The schema of this object will be used to populate analysis. :param kwargs: The values of any keys which match schema fieldnames will - be inserted into the corrosponding fields in the new analysis. + be inserted into the corresponding fields in the new analysis. :returns: Analysis object that was created :rtype: Analysis """ From 78ad28a5720ae59c8423640bd46a8ce619494c2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:15:59 +0100 Subject: [PATCH 07/63] Add uid property in ResultsRangeDict --- bika/lims/content/analysisspec.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index d33ace51d4..ad8ea7fc95 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -20,7 +20,6 @@ from AccessControl import ClassSecurityInfo from Products.ATContentTypes.lib.historyaware import HistoryAwareMixin -from Products.ATExtensions.field.records import RecordsField from Products.Archetypes import atapi from Products.Archetypes.public import BaseFolder from Products.Archetypes.public import Schema @@ -143,6 +142,7 @@ class ResultsRangeDict(dict): def __init__(self, *arg, **kw): super(ResultsRangeDict, self).__init__(*arg, **kw) + self["uid"] = self.uid self["min"] = self.min self["max"] = self.max self["error"] = self.error @@ -151,6 +151,12 @@ def __init__(self, *arg, **kw): self["min_operator"] = self.min_operator self["max_operator"] = self.max_operator + @property + def uid(self): + """The uid of the service this ResultsRange refers to + """ + return self.get("uid", '') + @property def min(self): return self.get("min", '') From b8aed865417443be337d748638d5db8815892f47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:17:05 +0100 Subject: [PATCH 08/63] When a new Specification is applied, apply changes to analyses too --- bika/lims/content/analysisrequest.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index ba02ff588b..66b97fa4eb 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -1450,7 +1450,7 @@ def setSpecification(self, value): # Update only results ranges if specs is not None, so results ranges # manually set previously (e.g. via ManageAnalyses view) are # preserved unless a new Specification overrides them - self.getField("ResultsRange").set(self, spec.getResultsRange()) + self.setResultsRange(spec.getResultsRange()) # Cascade the changes to partitions, but only to those for which that # are in a status in which the specification can be updated. This @@ -1461,6 +1461,16 @@ def setSpecification(self, value): if check_permission(permission, descendant): descendant.setSpecification(spec) + def setResultsRange(self, value): + field = self.getField("ResultsRange") + field.set(self, value) + + # Reset results ranges from analyses + for analysis in self.objectValues("Analysis"): + service_uid = analysis.getRawAnalysisService() + result_range = field.get(self, uid=service_uid) + analysis.setResultsRange(result_range) + def getClient(self): """Returns the client this object is bound to. We override getClient from ClientAwareMixin because the "Client" schema field is only used to From 045e41326586ed3e771a01cf61bf4242023f78f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:19:24 +0100 Subject: [PATCH 09/63] Refactor create_analysisrequest function --- bika/lims/utils/analysisrequest.py | 203 +++++++++++++---------------- 1 file changed, 90 insertions(+), 113 deletions(-) diff --git a/bika/lims/utils/analysisrequest.py b/bika/lims/utils/analysisrequest.py index be727a96d6..dae6228aa3 100644 --- a/bika/lims/utils/analysisrequest.py +++ b/bika/lims/utils/analysisrequest.py @@ -18,12 +18,14 @@ # Copyright 2018-2019 by it's authors. # Some rights reserved, see README and LICENSE. +import six import itertools import os import tempfile from email.mime.multipart import MIMEMultipart from email.mime.text import MIMEText +from Products.Archetypes.config import UID_CATALOG from Products.CMFCore.utils import getToolByName from Products.CMFPlone.utils import _createObjectByType from Products.CMFPlone.utils import safe_unicode @@ -34,6 +36,7 @@ from bika.lims import api from bika.lims import bikaMessageFactory as _ from bika.lims import logger +from bika.lims.catalog import SETUP_CATALOG from bika.lims.idserver import renameAfterCreation from bika.lims.interfaces import IAnalysisRequest from bika.lims.interfaces import IAnalysisRequestRetest @@ -56,41 +59,36 @@ def create_analysisrequest(client, request, values, analyses=None, - partitions=None, specifications=None, prices=None): - """This is meant for general use and should do everything necessary to - create and initialise an AR and any other required auxilliary objects - (Sample, SamplePartition, Analysis...) - :param client: - The container (Client) in which the ARs will be created. - :param request: - The current Request object. - :param values: - a dict, where keys are AR|Sample schema field names. - :param analyses: - Analysis services list. If specified, augments the values in - values['Analyses']. May consist of service objects, UIDs, or Keywords. - :param partitions: - A list of dictionaries, if specific partitions are required. If not - specified, AR's sample is created with a single partition. - :param specifications: - These values augment those found in values['Specifications'] - :param prices: - Allow different prices to be set for analyses. If not set, prices + results_ranges=None, prices=None): + """Creates a new AnalysisRequest (a Sample) object + :param client: The container where the Sample will be created + :param request: The current Http Request object + :param values: A dict, with keys as AnalaysisRequest's schema field names + :param analyses: List of Services or Analyses (brains, objects, UIDs, + keywords). Extends the list from values["Analyses"] + :param results_ranges: List of Results Ranges. Extends the results ranges + from the Specification object defined in values["Specification"] + :param prices: Mapping of AnalysisService UID -> price. If not set, prices are read from the associated analysis service. """ # Don't pollute the dict param passed in values = dict(values.items()) - # Create the Analysis Request - ar = _createObjectByType('AnalysisRequest', client, tmpID()) + # Resolve the Service uids of analyses to be added in the Sample. Values + # passed-in might contain Profiles and also values that are not uids. Also, + # additional analyses can be passed-in through either values or services + service_uids = to_services_uids(values=values, services=analyses) + + # Remove the Analyses from values. We will add them manually + values.update({"Analyses": []}) - # Resolve the services uids and set the analyses for this Analysis Request - service_uids = get_services_uids(context=client, values=values, - analyses_serv=analyses) - ar.setAnalyses(service_uids, prices=prices, specs=specifications) - values.update({"Analyses": service_uids}) + # Create the Analysis Request and submit the form + ar = _createObjectByType('AnalysisRequest', client, tmpID()) ar.processForm(REQUEST=request, values=values) + # Set the analyses manually + ar.setAnalyses(service_uids, prices=prices, specs=results_ranges) + # Handle hidden analyses from template and profiles # https://github.com/senaite/senaite.core/issues/1437 # https://github.com/senaite/senaite.core/issues/1326 @@ -189,93 +187,78 @@ def get_hidden_service_uids(profile_or_template): return map(lambda setting: setting["uid"], hidden) -def get_services_uids(context=None, analyses_serv=None, values=None): +def to_services_uids(services=None, values=None): """ - This function returns a list of UIDs from analyses services from its - parameters. - :param analyses_serv: A list (or one object) of service-related info items. - see _resolve_items_to_service_uids() docstring. - :type analyses_serv: list + Returns a list of Analysis Services uids + :param services: A list of service items (uid, keyword, brain, obj, title) :param values: a dict, where keys are AR|Sample schema field names. - :type values: dict - :returns: a list of analyses services UIDs + :returns: a list of Analyses Services UIDs """ - if not analyses_serv: - analyses_serv = [] - if not values: - values = {} + def to_list(value): + if not value: + return [] + if isinstance(value, six.string_types): + return [value] + if isinstance(value, (list, tuple)): + return value + logger.warn("Cannot convert to a list: {}".format(value)) + return [] - if not context or (not analyses_serv and not values): - raise RuntimeError( - "get_services_uids: Missing or wrong parameters.") + services = services or [] + values = values or {} # Merge analyses from analyses_serv and values into one list - analyses_services = analyses_serv + (values.get("Analyses", None) or []) - - # It is possible to create analysis requests - # by JSON petitions and services, profiles or types aren't allways send. - # Sometimes we can get analyses and profiles that doesn't match and we - # should act in consequence. - # Getting the analyses profiles - analyses_profiles = values.get('Profiles', []) - if not isinstance(analyses_profiles, (list, tuple)): - # Plone converts the incoming form value to a list, if there are - # multiple values; but if not, it will send a string (a single UID). - analyses_profiles = [analyses_profiles] - - if not analyses_services and not analyses_profiles: - return [] + uids = to_list(services) + to_list(values.get("Analyses")) + + # Convert them to a list of service uids + uids = filter(None, map(to_service_uid, uids)) - # Add analysis services UIDs from profiles to analyses_services variable. - if analyses_profiles: - uid_catalog = getToolByName(context, 'uid_catalog') - for brain in uid_catalog(UID=analyses_profiles): + # Extend with service uids from profiles + profiles = to_list(values.get("Profiles")) + if profiles: + uid_catalog = api.get_tool(UID_CATALOG) + for brain in uid_catalog(UID=profiles): profile = api.get_object(brain) - # Only services UIDs - services_uids = profile.getRawService() - # _resolve_items_to_service_uids() will remove duplicates - analyses_services += services_uids - - return _resolve_items_to_service_uids(analyses_services) - - -def _resolve_items_to_service_uids(items): - """ Returns a list of service uids without duplicates based on the items - :param items: - A list (or one object) of service-related info items. The list can be - heterogeneous and each item can be: - - Analysis Service instance - - Analysis instance - - Analysis Service title - - Analysis Service UID - - Analysis Service Keyword - If an item that doesn't match any of the criterias above is found, the - function will raise a RuntimeError + uids.extend(profile.getRawService() or []) + + # Get the service uids without duplicates, but preserving the order + return list(dict.fromkeys(uids).keys()) + + +def to_service_uid(uid_brain_obj_str): + """Resolves the passed in element to a valid uid. Returns None if the value + cannot be resolved to a valid uid """ - def resolve_to_uid(item): - if api.is_uid(item): - return item - elif IAnalysisService.providedBy(item): - return item.UID() - elif IRoutineAnalysis.providedBy(item): - return item.getServiceUID() - - bsc = api.get_tool("bika_setup_catalog") - brains = bsc(portal_type='AnalysisService', getKeyword=item) - if brains: - return brains[0].UID - brains = bsc(portal_type='AnalysisService', title=item) - if brains: - return brains[0].UID - raise RuntimeError( - str(item) + " should be the UID, title, keyword " - " or title of an AnalysisService.") - - # Maybe only a single item was passed - if type(items) not in (list, tuple): - items = [items, ] - service_uids = map(resolve_to_uid, list(set(items))) - return list(set(service_uids)) + if api.is_uid(uid_brain_obj_str) and uid_brain_obj_str != "0": + return uid_brain_obj_str + + if api.is_object(uid_brain_obj_str): + obj = api.get_object(uid_brain_obj_str) + + if IAnalysisService.providedBy(obj): + return api.get_uid(obj) + + elif IRoutineAnalysis.providedBy(obj): + return obj.getServiceUID() + + else: + logger.error("Type not supported: {}".format(obj.portal_type)) + return None + + if isinstance(uid_brain_obj_str, six.string_types): + # Maybe is a keyword? + query = dict(portal_type="AnalysisService", getKeyword=uid_brain_obj_str) + brains = api.search(query, SETUP_CATALOG) + if len(brains) == 1: + return api.get_uid(brains[0]) + + # Or maybe a title + query = dict(portal_type="AnalysisService", title=uid_brain_obj_str) + brains = api.search(query, SETUP_CATALOG) + if len(brains) == 1: + return api.get_uid(brains[0]) + + return None def notify_rejection(analysisrequest): @@ -432,7 +415,7 @@ def create_retest(ar): def create_partition(analysis_request, request, analyses, sample_type=None, container=None, preservation=None, skip_fields=None, - remove_primary_analyses=True, internal_use=True): + internal_use=True): """ Creates a partition for the analysis_request (primary) passed in :param analysis_request: uid/brain/object of IAnalysisRequest type @@ -442,7 +425,6 @@ def create_partition(analysis_request, request, analyses, sample_type=None, :param container: uid/brain/object of Container :param preservation: uid/brain/object of Preservation :param skip_fields: names of fields to be skipped on copy from primary - :param remove_primary_analyses: removes the analyses from the parent :return: the new partition """ partition_skip_fields = [ @@ -491,12 +473,7 @@ def create_partition(analysis_request, request, analyses, sample_type=None, specs = ar.getSpecification() specs = specs and specs.getResultsRange() or [] partition = create_analysisrequest(client, request=request, values=record, - analyses=services, specifications=specs) - - # Remove analyses from the primary - if remove_primary_analyses: - analyses_ids = map(api.get_id, analyses) - ar.manage_delObjects(analyses_ids) + analyses=services, results_ranges=specs) # Reindex Parent Analysis Request ar.reindexObject(idxs=["isRootAncestor"]) From d17d8ff274b85fd2a25f7b46e8d8709d74b6c61e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:37:30 +0100 Subject: [PATCH 10/63] Refactor aranalysesfield --- bika/lims/browser/fields/aranalysesfield.py | 148 +++++++++++++------- 1 file changed, 101 insertions(+), 47 deletions(-) diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index 5bdf32656a..796e1f5384 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -25,15 +25,19 @@ from Products.Archetypes.Registry import registerField from Products.Archetypes.public import Field from Products.Archetypes.public import ObjectField +from zope.interface import alsoProvides from zope.interface import implements +from zope.interface import noLongerProvides from bika.lims import api from bika.lims import logger from bika.lims.api.security import check_permission from bika.lims.catalog import CATALOG_ANALYSIS_LISTING +from bika.lims.catalog import SETUP_CATALOG from bika.lims.interfaces import IARAnalysesField from bika.lims.interfaces import IAnalysis from bika.lims.interfaces import IAnalysisService +from bika.lims.interfaces import IInternalUse from bika.lims.interfaces import ISubmitted from bika.lims.permissions import AddAnalysis from bika.lims.utils.analysis import create_analysis @@ -135,18 +139,9 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): # Merge dependencies and services services = set(services + dependencies) - # Modify existing AR specs with new form values of selected analyses. + # Modify existing AR specs with new form values of selected analyses specs = self.resolve_specs(instance, specs) - # Create a mapping of Service UID -> Hidden status - if hidden is None: - hidden = [] - hidden = dict(map(lambda d: (d.get("uid"), d.get("hidden")), hidden)) - - # Ensure we have a prices dictionary - if prices is None: - prices = dict() - # Add analyses new_analyses = [] for service in services: @@ -203,57 +198,96 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): return new_analyses - def resolve_specs(self, instance, specs): + def resolve_specs(self, instance, results_ranges): """Returns a dictionary where the key is the service_uid and the value is its results range. If a result range for a given service does not exist or does not match with the specs defined in the sample, the function resets the Sample's Specification field to guarantee compliance with the Specification value actually set """ - form_specs = specs or [] - - # Copy the passed in dict, otherwise the specification might be written - # directly to the attached specification of the sample - sample_specs = dict(map(lambda rr: (rr["uid"], rr.copy()), - instance.getResultsRange())) - - out_specs = dict() - reset_specification = False - for form_rr in form_specs: - service_uid = form_rr["uid"] - sample_rr = sample_specs.get(service_uid) - if not sample_rr: - # Sample's ResultsRange (a "copy" of Specification initially set - # does not contain a result range for this service - logger.info("Result range for {} not present in Sample's" - .format(form_rr["keyword"])) - reset_specification = True + rrs = results_ranges or [] - else: - # Clean-up the result range passed in - form_rr = self.resolve_result_range(form_rr, sample_rr) + # Sample's Results ranges + sample_rrs = instance.getResultsRange() - if form_rr != sample_rr: - # Result range for this service has been changed manually, - # it does not match with sample's ResultRange - logger.warn("Result Range for {} do not match with Sample's" - .format(form_rr["keyword"])) - reset_specification = True + # Resolve results_ranges passed-in to make sure they contain uid + rrs = map(lambda rr: self.resolve_uid(rr), rrs) - # Generate the results_range format - out_specs[service_uid] = form_rr + # Append those from sample that are missing in the ranges passed-in + service_uids = map(lambda rr: rr["uid"], rrs) + rrs.extend(filter(lambda rr: rr not in service_uids, sample_rrs)) - if reset_specification: - # Reset the specification, cause the sample won't longer be fully - # compliant with the Specifications it has assigned + # Do the results ranges passed-in are compliant with Sample's spec? + if not self.is_compliant_with_specification(instance, rrs): + # Reset the specification, we cannot keep a Specification assigned + # to a Sample if there are results ranges set not compliant instance.setSpecification(None) # We store the values in Sample's ResultsRange because although the # specification is not met, we still want Sample's ResultsRange to # act as a "template" for new analyses - instance.setResultsRange(out_specs.values()) + instance.setResultsRange(rrs) - return out_specs + # Create a dict for easy access to results ranges + return dict(map(lambda rr: (rr["uid"], rr), rrs)) + + def resolve_uid(self, result_range): + """Resolves the uid key for the result_range passed in if it does not + exist when contains a keyword + """ + value = result_range.copy() + uid = value.get("uid") + if api.is_uid(uid) and uid != "0": + return value + + # uid key does not exist or is not valid, try to infere from keyword + keyword = value.get("keyword") + if keyword: + query = dict(portal_type="AnalysisService", getKeyword=keyword) + brains = api.search(query, SETUP_CATALOG) + if len(brains) == 1: + uid = api.get_uid(brains[0]) + value["uid"] = uid + return value + + def is_compliant_with_specification(self, instance, results_range): + """Returns whether the results_range passed-in are compliant with the + instance's Specification results ranges. This, is the results ranges + for each service match with those from the instance + """ + specification = instance.getSpecification() + if not specification: + # If there is no specification set, assume is compliant + return True + + # Get the results ranges to check against for compliance + sample_rrs = instance.getResultsRange() + sample_rrs = sample_rrs or specification.getResultsRanges() + + # Create a dict for easy access to results ranges + sample_rrs = dict(map(lambda rr: (rr["uid"], rr), sample_rrs)) + + # The Sample has Specification set: the ResultsRange from the sample is + # a copy of those from the Specification. If so, we need to check that + # there is no result range passed-in violating the Spec + for rr in results_range: + service_uid = rr["uid"] + sample_rr = sample_rrs.get(service_uid) + if not sample_rr: + # Sample's ResultsRange (a "copy" of Specification initially set + # does not contain a result range for this service + return False + + else: + # Clean-up the result range passed in + form_rr = self.resolve_result_range(rr, sample_rr) + if form_rr != sample_rr: + # Result range for this service has been changed manually, + # it does not match with sample's ResultRange + return False + + # No anomalies found, compliant + return True def resolve_result_range(self, result_range, original): """Cleans up the result range passed-in to match with same keys as the @@ -278,6 +312,19 @@ def add_analysis(self, instance, service, prices, hidden, specs=None): service_uid = api.get_uid(service) new_analysis = False + # Ensure we have suitable parameters + specs = specs or {} + + # Get the hidden status for the service + hidden = filter(lambda d: d.get("uid") == service_uid, hidden or []) + hidden = hidden and hidden[0] or service.getHidden() + + # Get the price for the service + price = prices and prices.get(service_uid) or service.getPrice() + + # Does analyses are for internal use + internal_use = instance.getInternalUse() + # Gets the analysis or creates the analysis for this service # Note this returns a list, because is possible to have multiple # partitions with same analysis @@ -292,10 +339,17 @@ def add_analysis(self, instance, service, prices, hidden, specs=None): for analysis in analyses: # Set the hidden status - analysis.setHidden(hidden.get(service_uid, False)) + analysis.setHidden(hidden) # Set the price of the Analysis - analysis.setPrice(prices.get(service_uid, service.getPrice())) + analysis.setPrice(price) + + # Set internal use + analysis.setInternalUse(internal_use) + if internal_use and not IInternalUse.providedBy(analysis): + alsoProvides(analysis, IInternalUse) + elif not internal_use and IInternalUse.providedBy(analysis): + noLongerProvides(analysis, IInternalUse) # Set the result range to the Analysis analysis_rr = specs.get(service_uid) or analysis.getResultsRange() From ce062e384dac54cd2511220105ef3e0d9eb07ee5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:38:46 +0100 Subject: [PATCH 11/63] SpecificationsField must return empty dict instead of None --- .../browser/fields/specificationsfield.py | 61 +++++++++++++------ 1 file changed, 42 insertions(+), 19 deletions(-) diff --git a/bika/lims/browser/fields/specificationsfield.py b/bika/lims/browser/fields/specificationsfield.py index dc1130c40c..ee05d41881 100644 --- a/bika/lims/browser/fields/specificationsfield.py +++ b/bika/lims/browser/fields/specificationsfield.py @@ -9,9 +9,10 @@ from bika.lims import api from bika.lims import bikaMessageFactory as _ from bika.lims.browser.widgets import AnalysisSpecificationWidget -# A tuple of (subfield_id, subfield_label,) +from bika.lims.catalog import SETUP_CATALOG from bika.lims.interfaces.analysis import IRequestAnalysis +# A tuple of (subfield_id, subfield_label,) SUB_FIELDS = ( ("keyword", _("Analysis Service")), ("min_operator", _("Min operator")), @@ -41,7 +42,7 @@ def get(self, instance, **kwargs): value = super(ResultsRangeField, self).get(instance, **kwargs) if value: return ResultsRangeDict(value) - return None + return {} registerField(ResultsRangeField, title="ResultsRange", @@ -67,12 +68,16 @@ class SpecificationsField(RecordsField): def get(self, instance, **kwargs): from bika.lims.content.analysisspec import ResultsRangeDict values = super(SpecificationsField, self).get(instance, **kwargs) - uid_keyword_service = kwargs.get("uid", kwargs.get("keyword", None)) - if uid_keyword_service: - return self.getResultsRange(values, uid_keyword_service) - if values: - return map(lambda val: ResultsRangeDict(val), values) - return [] + + # If a keyword or an uid has been specified, return the result range + # for that uid or keyword only + uid = kwargs.get("uid") + keyword = kwargs.get("keyword") + if uid or keyword: + return self.getResultsRange(values, uid or keyword) + + # Convert the dict items to ResultRangeDict for easy handling + return map(lambda val: ResultsRangeDict(val), values) def getResultsRange(self, values, uid_keyword_service): from bika.lims.content.analysisspec import ResultsRangeDict @@ -94,11 +99,25 @@ def getResultsRange(self, values, uid_keyword_service): def _to_dict(self, value): """Convert the records to persistent dictionaries """ + # Resolve items to guarantee all them have the key uid value = super(SpecificationsField, self)._to_dict(value) + return map(self.resolve_uid, value) - # Bail out items without "uid" key (uid is used everywhere to know the - # service the result range refers to) - return filter(lambda result_range: "uid" in result_range, value) + def resolve_uid(self, raw_dict): + value = raw_dict.copy() + uid = value.get("uid") + if api.is_uid(uid) and uid != "0": + return value + + # uid key does not exist or is not valid, try to infere from keyword + keyword = value.get("keyword") + if keyword: + query = dict(portal_type="AnalysisService", getKeyword=keyword) + brains = api.search(query, SETUP_CATALOG) + if len(brains) == 1: + uid = api.get_uid(brains[0]) + value["uid"] = uid + return value class DefaultResultsRangeProvider(object): @@ -116,14 +135,18 @@ def __call__(self): """Get the default value. """ if not IRequestAnalysis.providedBy(self.context): - return None + return {} keyword = self.context.getKeyword() sample = self.context.getRequest() - field = sample.getField("ResultsRange") - rr = field.get(sample, keyword=keyword) - if rr: - self.context.setResultsRange(rr) - return self.context.getResultsRange() - - return None + if sample and keyword: + field = sample.getField("ResultsRange") + rr = field.get(sample, keyword=keyword) + if rr: + #self.context.setResultsRange(rr) + return rr + #return self.context.getResultsRange() + + return {} + #from bika.lims.content.analysisspec import ResultsRangeDict + #return ResultsRangeDict(uid=api.get_uid(self.context), keyword=keyword) From 7953041342efcd758f6ddb52bc0092eda114fb03 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:41:25 +0100 Subject: [PATCH 12/63] When Specification's results ranges are changed, reapply to Sample Analyses from the sample will use the results ranges set initially, so if the Specification object is change afterwards, this will not have any impact to previously created samples and tests --- bika/lims/tests/doctests/API_analysis.rst | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/bika/lims/tests/doctests/API_analysis.rst b/bika/lims/tests/doctests/API_analysis.rst index aeb220f9a2..84023b2f2a 100644 --- a/bika/lims/tests/doctests/API_analysis.rst +++ b/bika/lims/tests/doctests/API_analysis.rst @@ -512,6 +512,10 @@ Set open interval for min and max from water specification ... range['max_operator'] = 'lt' >>> specification.setResultsRange(ranges) +We need to re-apply the Specification for the changes to take effect: + + >>> ar.setSpecification(specification) + First, get the analyses from slot 1 and sort them asc: >>> analyses = worksheet.get_analyses_at(1) @@ -540,6 +544,10 @@ Set left-open interval for min and max from water specification ... range['max_operator'] = 'lt' >>> specification.setResultsRange(ranges) +We need to re-apply the Specification for the changes to take effect: + + >>> ar.setSpecification(specification) + First, get the analyses from slot 1 and sort them asc: >>> analyses = worksheet.get_analyses_at(1) @@ -568,6 +576,10 @@ Set right-open interval for min and max from water specification ... range['max_operator'] = 'leq' >>> specification.setResultsRange(ranges) +We need to re-apply the Specification for the changes to take effect: + + >>> ar.setSpecification(specification) + First, get the analyses from slot 1 and sort them asc: >>> analyses = worksheet.get_analyses_at(1) From 85afa6849334f344d6803b8e54aa4bc20f1aa99a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:43:03 +0100 Subject: [PATCH 13/63] Compare with previous snapshot, not with the first one create_analysisrequest has been refactored so the analyses are not passed in through values['Analyses'] anymore, rather they are set manually afterwards. This prevents a lot of inconsistencies and makes the creation cleaner. As a result, the first version of a Sample won't never contain analyses. --- bika/lims/tests/doctests/API_snapshot.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bika/lims/tests/doctests/API_snapshot.rst b/bika/lims/tests/doctests/API_snapshot.rst index 2f76c63835..94a93b1c07 100644 --- a/bika/lims/tests/doctests/API_snapshot.rst +++ b/bika/lims/tests/doctests/API_snapshot.rst @@ -270,7 +270,7 @@ Comparing Snapshots The changes of two snapshots can be compared with `compare_snapshots`: - >>> snap0 = get_snapshot_by_version(sample, 0) + >>> snap0 = get_snapshot_by_version(sample, 2) Add 2 more analyses (Mg and Ca): From 7720f3e36f205e2650939943d21de2a0bf1399e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:45:59 +0100 Subject: [PATCH 14/63] Sample's ResultsRange does not get updated when applied to single analysis --- bika/lims/tests/doctests/ARAnalysesField.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bika/lims/tests/doctests/ARAnalysesField.rst b/bika/lims/tests/doctests/ARAnalysesField.rst index 10b19182e7..e7dd1a8cdf 100644 --- a/bika/lims/tests/doctests/ARAnalysesField.rst +++ b/bika/lims/tests/doctests/ARAnalysesField.rst @@ -364,10 +364,10 @@ Request and have precedence over the lab specifications: >>> myspec3.get("rangecomment") 'My CA Spec' -All Result Ranges are set on the AR: +Result Ranges are set to analyses level, but not present in the AR: >>> sorted(map(lambda r: r.get("rangecomment"), ar.getResultsRange())) - ['My CA Spec', 'My MG Spec', 'My PH Spec'] + [] Now we simulate the form input data of the ARs "Manage Analysis" form, so that the User only selected the `PH` service and gave some custom specifications for From a1ddfcf1aa2ed15c06448c11e9aea3f6c570701a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 17:47:08 +0100 Subject: [PATCH 15/63] Some functions from aranalysesfield return a list now --- .../ARAnalysesFieldWithPartitions.rst | 50 +++++++------------ 1 file changed, 19 insertions(+), 31 deletions(-) diff --git a/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst b/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst index c02f3c8b2e..4f1da3e3e4 100644 --- a/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst +++ b/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst @@ -166,28 +166,24 @@ get_from_descendant When asked for `Fe` to primary, it returns None because there is no descendant containing `Fe`: - >>> fe = field.get_from_descendant(sample, Fe) - >>> fe is None - True + >>> field.get_from_descendant(sample, Fe) + [] And same with partition: - >>> fe = field.get_from_descendant(partition, Fe) - >>> fe is None - True + >>> field.get_from_descendant(partition, Fe) + [] When asked for `Cu` to primary, it returns the analysis, because it lives in a descendant (partition): - >>> cu = field.get_from_descendant(sample, Cu) - >>> cu.getServiceUID() == api.get_uid(Cu) - True + >>> field.get_from_descendant(sample, Cu) + [] But returns None if I ask to the partition: - >>> cu = field.get_from_descendant(partition, Cu) - >>> cu is None - True + >>> field.get_from_descendant(partition, Cu) + [] get_analyses_from_descendants ............................. @@ -204,37 +200,29 @@ It returns the analyses contained by the descendants: Resolution of analyses from the Sample lineage ---------------------------------------------- -resolve_analysis +resolve_analyses ................ Resolves the analysis from the sample lineage if exists: - >>> fe = field.resolve_analysis(sample, Fe) - >>> fe.getServiceUID() == api.get_uid(Fe) - True - >>> fe.aq_parent == sample - True + >>> field.resolve_analyses(sample, Fe) + [] - >>> cu = field.resolve_analysis(sample, Cu) - >>> cu.getServiceUID() == api.get_uid(Cu) - True - >>> cu.aq_parent == partition - True + >>> field.resolve_analyses(sample, Cu) + [] - >>> au = field.resolve_analysis(sample, Au) - >>> au is None - True + >>> field.resolve_analyses(sample, Au) + [] But when we use the partition and the analysis is found in an ancestor, it moves the analysis into the partition: - >>> fe = field.resolve_analysis(partition, Fe) - >>> fe.getServiceUID() == api.get_uid(Fe) - True - >>> fe.aq_parent == partition - True + >>> field.resolve_analyses(partition, Fe) + [] + >>> sample.objectValues("Analysis") [] + >>> partition.objectValues("Analysis") [, ] From d67889970d8db70dcfb0c1029b15427db87674ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 18:00:46 +0100 Subject: [PATCH 16/63] Fix NameError: global name 'api' is not defined --- bika/lims/content/referenceanalysis.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/bika/lims/content/referenceanalysis.py b/bika/lims/content/referenceanalysis.py index 94aa0d7228..a2443455da 100644 --- a/bika/lims/content/referenceanalysis.py +++ b/bika/lims/content/referenceanalysis.py @@ -19,18 +19,20 @@ # Some rights reserved, see README and LICENSE. from AccessControl import ClassSecurityInfo +from DateTime import DateTime +from Products.Archetypes.Field import StringField +from Products.Archetypes.public import Schema +from Products.Archetypes.public import registerType +from plone.app.blob.field import BlobField +from zope.interface import implements + +from bika.lims import api from bika.lims.config import PROJECTNAME from bika.lims.config import STD_TYPES from bika.lims.content.abstractanalysis import AbstractAnalysis from bika.lims.content.abstractanalysis import schema from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.interfaces import IReferenceAnalysis -from DateTime import DateTime -from plone.app.blob.field import BlobField -from Products.Archetypes.Field import StringField -from Products.Archetypes.public import Schema -from Products.Archetypes.public import registerType -from zope.interface import implements schema = schema.copy() + Schema(( StringField( From c3d5bd1749e166bb0c3ab8a3c30a794bd7384505 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 20:49:51 +0100 Subject: [PATCH 17/63] Ensure ResultsRangeField always return a copy --- bika/lims/browser/fields/specificationsfield.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bika/lims/browser/fields/specificationsfield.py b/bika/lims/browser/fields/specificationsfield.py index ee05d41881..fd6be4978a 100644 --- a/bika/lims/browser/fields/specificationsfield.py +++ b/bika/lims/browser/fields/specificationsfield.py @@ -41,7 +41,7 @@ def get(self, instance, **kwargs): from bika.lims.content.analysisspec import ResultsRangeDict value = super(ResultsRangeField, self).get(instance, **kwargs) if value: - return ResultsRangeDict(value) + return ResultsRangeDict(dict(value.items())) return {} @@ -77,7 +77,7 @@ def get(self, instance, **kwargs): return self.getResultsRange(values, uid or keyword) # Convert the dict items to ResultRangeDict for easy handling - return map(lambda val: ResultsRangeDict(val), values) + return map(lambda val: ResultsRangeDict(dict(val.items())), values) def getResultsRange(self, values, uid_keyword_service): from bika.lims.content.analysisspec import ResultsRangeDict @@ -94,7 +94,7 @@ def getResultsRange(self, values, uid_keyword_service): # Find out the item for the given uid/keyword value = filter(lambda v: v.get(key) == uid_keyword_service, values) - return value and ResultsRangeDict(value[0]) or None + return value and ResultsRangeDict(dict(value[0].items())) or None def _to_dict(self, value): """Convert the records to persistent dictionaries From e977716fdc8f0e0062d85cf4f7ed5d22f02c5e5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 21:03:48 +0100 Subject: [PATCH 18/63] Added doctest --- .../SpecificationAndResultsRanges.rst | 168 ++++++++++++++++++ 1 file changed, 168 insertions(+) create mode 100644 bika/lims/tests/doctests/SpecificationAndResultsRanges.rst diff --git a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst new file mode 100644 index 0000000000..37c6242abd --- /dev/null +++ b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst @@ -0,0 +1,168 @@ +Specification and Results Ranges with Samples and analyses +========================================================== + +Specification is an object containing a list of results ranges, each one refers +to the min/max/min_warn/max_warn values to apply for a given analysis service. +User can assign a Specification to a Sample, so the results of it's Analyses +will be checked against the results ranges provided by the Specification. + +Running this test from the buildout directory: + + bin/test test_textual_doctests -t SpecificationAndResultsRanges.rst + +Test Setup +---------- + +Needed imports: + + >>> import transaction + >>> from DateTime import DateTime + >>> from plone.app.testing import setRoles + >>> from plone.app.testing import TEST_USER_ID + >>> from plone.app.testing import TEST_USER_PASSWORD + >>> from bika.lims import api + >>> from bika.lims.utils.analysisrequest import create_analysisrequest + >>> from bika.lims.utils.analysisrequest import create_partition + >>> from bika.lims.workflow import doActionFor as do_action_for + >>> from zope.interface import alsoProvides + >>> from zope.interface import noLongerProvides + +Functional Helpers: + + >>> def new_sample(services, specification=None, results_ranges=None): + ... values = { + ... 'Client': client.UID(), + ... 'Contact': contact.UID(), + ... 'DateSampled': DateTime().strftime("%Y-%m-%d"), + ... 'SampleType': sampletype.UID(), + ... 'Analyses': map(api.get_uid, services), + ... 'Specification': specification or None } + ... + ... ar = create_analysisrequest(client, request, values, results_ranges=results_ranges) + ... transitioned = do_action_for(ar, "receive") + ... return ar + + >>> def get_analysis_from(sample, service): + ... service_uid = api.get_uid(service) + ... for analysis in sample.getAnalyses(full_objects=True): + ... if analysis.getServiceUID() == service_uid: + ... return analysis + ... return None + + >>> def get_results_range_from(obj, service): + ... field = obj.getField("ResultsRange") + ... return field.get(obj, uid=api.get_uid(service)) + + >>> def set_results_range_for(obj, results_range): + ... rrs = obj.getResultsRange() + ... uid = results_range["uid"] + ... rrs = filter(lambda rr: rr["uid"] != uid, rrs) + ... rrs.append(results_range) + ... obj.setResultsRange(rrs) + + +Variables: + + >>> portal = self.portal + >>> request = self.request + >>> setup = api.get_setup() + +Create some basic objects for the test: + + >>> setRoles(portal, TEST_USER_ID, ['Manager',]) + >>> client = api.create(portal.clients, "Client", Name="Happy Hills", ClientID="HH", MemberDiscountApplies=True) + >>> contact = api.create(client, "Contact", Firstname="Rita", Lastname="Mohale") + >>> sampletype = api.create(setup.bika_sampletypes, "SampleType", title="Water", Prefix="W") + >>> labcontact = api.create(setup.bika_labcontacts, "LabContact", Firstname="Lab", Lastname="Manager") + >>> department = api.create(setup.bika_departments, "Department", title="Chemistry", Manager=labcontact) + >>> category = api.create(setup.bika_analysiscategories, "AnalysisCategory", title="Metals", Department=department) + >>> Au = api.create(setup.bika_analysisservices, "AnalysisService", title="Gold", Keyword="Au", Price="20", Category=category.UID()) + >>> Cu = api.create(setup.bika_analysisservices, "AnalysisService", title="Copper", Keyword="Cu", Price="15", Category=category.UID()) + >>> Fe = api.create(setup.bika_analysisservices, "AnalysisService", title="Iron", Keyword="Fe", Price="10", Category=category.UID()) + >>> Mg = api.create(setup.bika_analysisservices, "AnalysisService", title="Magnesium", Keyword="Mg", Price="20", Category=category.UID()) + +Create an Analysis Specification for `Water`: + + >>> sampletype_uid = api.get_uid(sampletype) + >>> rr1 = {"uid": api.get_uid(Au), "min": 10, "max": 20, "warn_min": 5, "warn_max": 25} + >>> rr2 = {"uid": api.get_uid(Cu), "min": 20, "max": 30, "warn_min": 15, "warn_max": 35} + >>> rr3 = {"uid": api.get_uid(Fe), "min": 30, "max": 40, "warn_min": 25, "warn_max": 45} + >>> rr4 = {"uid": api.get_uid(Mg), "min": 40, "max": 50, "warn_min": 35, "warn_max": 55} + >>> rr = [rr1, rr2, rr3, rr4] + >>> specification = api.create(setup.bika_analysisspecs, "AnalysisSpec", title="Lab Water Spec", SampleType=sampletype_uid, ResultsRange=rr) + + +Creation of a Sample with Specification +--------------------------------------- + +Create a Sample and receive: + + >>> services = [Au, Cu, Fe, Mg] + >>> sample = new_sample(services, specification=specification) + +The sample has the specification assigned: + + >>> sample.getSpecification() + + +And its results ranges match with the sample's `ResultsRange` field value: + + >>> specification.getResultsRange() == sample.getResultsRange() + True + +And the analyses the sample contains have the results ranges properly set: + + >>> au = get_analysis_from(sample, Au) + >>> au.getResultsRange() == get_results_range_from(specification, Au) + True + + >>> cu = get_analysis_from(sample, Cu) + >>> cu.getResultsRange() == get_results_range_from(specification, Cu) + True + + >>> fe = get_analysis_from(sample, Fe) + >>> fe.getResultsRange() == get_results_range_from(specification, Fe) + True + + >>> mg = get_analysis_from(sample, Mg) + >>> mg.getResultsRange() == get_results_range_from(specification, Mg) + True + +We can change a result range by using properties: + + >>> rr_au = au.getResultsRange() + >>> rr_au.min = 11 + >>> rr_au.max = 21 + >>> (rr_au.min, rr_au.max) + (11, 21) + +Or using it as a dict: + + >>> rr_au["min"] = 15 + >>> rr_au["max"] = 25 + >>> (rr_au["min"], rr_au["max"]) + (15, 25) + +If we change this results range in the Specification object, this won't take any +effect to neither the Sample nor analyses: + + >>> set_results_range_for(specification, rr_au) + >>> specification.getResultsRange() == sample.getResultsRange() + False + + >>> au = get_analysis_from(sample, Au) + >>> au.getResultsRange() == get_results_range_from(specification, Au) + False + + >>> get_results_range_from(sample, Au) == au.getResultsRange() + True + + >>> rr_sample_au = au.getResultsRange() + >>> (rr_sample_au.min, rr_sample_au.max) + (10, 20) + + + + + + From 902762e49ddb9b4655c4e3eb60778a7a66b2648d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 21:07:14 +0100 Subject: [PATCH 19/63] More rules in doctest --- .../tests/doctests/SpecificationAndResultsRanges.rst | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst index 37c6242abd..e38f466d54 100644 --- a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst +++ b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst @@ -150,7 +150,6 @@ effect to neither the Sample nor analyses: >>> specification.getResultsRange() == sample.getResultsRange() False - >>> au = get_analysis_from(sample, Au) >>> au.getResultsRange() == get_results_range_from(specification, Au) False @@ -161,8 +160,17 @@ effect to neither the Sample nor analyses: >>> (rr_sample_au.min, rr_sample_au.max) (10, 20) +We need to re-apply the Specification for the Sample's results range to update: + >>> sample.setSpecification(specification) + >>> specification.getResultsRange() == sample.getResultsRange() + True +As well as the analyses the sample contains: + >>> au.getResultsRange() == get_results_range_from(specification, Au) + True - + >>> rr_sample_au = au.getResultsRange() + >>> (rr_sample_au.min, rr_sample_au.max) + (15, 25) From f08795ed0bdbd74cd1b5278119350b9de71429b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 22:53:13 +0100 Subject: [PATCH 20/63] Ensure no duplicates are stored in Sample's ResultsRange --- bika/lims/browser/fields/aranalysesfield.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index 796e1f5384..ba75d3767f 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -215,7 +215,7 @@ def resolve_specs(self, instance, results_ranges): # Append those from sample that are missing in the ranges passed-in service_uids = map(lambda rr: rr["uid"], rrs) - rrs.extend(filter(lambda rr: rr not in service_uids, sample_rrs)) + rrs.extend(filter(lambda rr: rr["uid"] not in service_uids, sample_rrs)) # Do the results ranges passed-in are compliant with Sample's spec? if not self.is_compliant_with_specification(instance, rrs): From c9e4847211c9afc34e4fb36e4da8a0ed1b06dc84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 22:53:29 +0100 Subject: [PATCH 21/63] More test use cases --- .../SpecificationAndResultsRanges.rst | 95 ++++++++++++++++++- 1 file changed, 92 insertions(+), 3 deletions(-) diff --git a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst index e38f466d54..22ec2e99bf 100644 --- a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst +++ b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst @@ -24,8 +24,6 @@ Needed imports: >>> from bika.lims.utils.analysisrequest import create_analysisrequest >>> from bika.lims.utils.analysisrequest import create_partition >>> from bika.lims.workflow import doActionFor as do_action_for - >>> from zope.interface import alsoProvides - >>> from zope.interface import noLongerProvides Functional Helpers: @@ -80,6 +78,7 @@ Create some basic objects for the test: >>> Cu = api.create(setup.bika_analysisservices, "AnalysisService", title="Copper", Keyword="Cu", Price="15", Category=category.UID()) >>> Fe = api.create(setup.bika_analysisservices, "AnalysisService", title="Iron", Keyword="Fe", Price="10", Category=category.UID()) >>> Mg = api.create(setup.bika_analysisservices, "AnalysisService", title="Magnesium", Keyword="Mg", Price="20", Category=category.UID()) + >>> Zn = api.create(setup.bika_analysisservices, "AnalysisService", title="Zinc", Keyword="Zn", Price="10", Category=category.UID()) Create an Analysis Specification for `Water`: @@ -88,13 +87,23 @@ Create an Analysis Specification for `Water`: >>> rr2 = {"uid": api.get_uid(Cu), "min": 20, "max": 30, "warn_min": 15, "warn_max": 35} >>> rr3 = {"uid": api.get_uid(Fe), "min": 30, "max": 40, "warn_min": 25, "warn_max": 45} >>> rr4 = {"uid": api.get_uid(Mg), "min": 40, "max": 50, "warn_min": 35, "warn_max": 55} - >>> rr = [rr1, rr2, rr3, rr4] + >>> rr5 = {"uid": api.get_uid(Zn), "min": 50, "max": 60, "warn_min": 45, "warn_max": 65} + >>> rr = [rr1, rr2, rr3, rr4, rr5] >>> specification = api.create(setup.bika_analysisspecs, "AnalysisSpec", title="Lab Water Spec", SampleType=sampletype_uid, ResultsRange=rr) Creation of a Sample with Specification --------------------------------------- +A given Specification can be assigned to the Sample during the creation process. +The results ranges of the mentioned Specification will be stored in ResultsRange +field from the Sample and the analyses will acquire those results ranges +individually. + +Specification from Sample is history-aware, so even if the Specification object +is changed after its assignment to the Sample, the Results Ranges from either +the Sample and its Analyses will remain untouched. + Create a Sample and receive: >>> services = [Au, Cu, Fe, Mg] @@ -174,3 +183,83 @@ As well as the analyses the sample contains: >>> rr_sample_au = au.getResultsRange() >>> (rr_sample_au.min, rr_sample_au.max) (15, 25) + +Removal of Analyses from a Sample with Specifications +----------------------------------------------------- + +User can remove analyses from the Sample. If the user removes one of the +analyses, the Specification assigned to the Sample will remain intact, as well +as Sample's Results Range: + + >>> sample.setAnalyses([Au, Cu, Fe]) + [] + + >>> analyses = sample.objectValues() + >>> sorted(analyses, key=lambda an: an.getKeyword()) + [, , ] + + >>> sample.getSpecification() + + + >>> specification.getResultsRange() == sample.getResultsRange() + True + + +Addition of Analyses to a Sample with Specifications +---------------------------------------------------- + +User can add new analyses to the Sample as well. If the Sample has an +Specification set and the specification had a results range registered for +such analysis, the result range for the new analysis will be set automatically: + + >>> sample.setAnalyses([Au, Cu, Fe, Zn]) + [] + + >>> sample.getSpecification() + + + >>> zn = get_analysis_from(sample, Zn) + >>> zn.getResultsRange() == get_results_range_from(specification, Zn) + True + +But if we reset an Analysis with it's own ResultsRange, different from the +range defined by the Specification, the system will clear the Specification to +guarantee compliance: + + >>> rr_zn = zn.getResultsRange() + >>> rr_zn.min = 55 + >>> sample.setAnalyses([Au, Cu, Fe, Zn], specs=[rr_zn]) + [] + + >>> sample.getSpecification() is None + True + +Nevertheless, Sample's ResultsRange is kept unchanged: + + >>> sample_rr = sample.getResultsRange() + >>> len(sample_rr) + 5 + +but with the results range for `Zn` updated, different from the Specification: + + >>> sample_rr_zn = filter(lambda rr: rr["uid"] == api.get_uid(Zn), sample_rr)[0] + >>> sample_rr_zn.min + 55 + +As well as for the analysis itself: + + >>> zn.getResultsRange().min + 55 + +If we re-apply the Specification, the result range for `Zn`, as well as for the +Sample, are reestablished: + + >>> sample.setSpecification(specification) + >>> specification.getResultsRange() == sample.getResultsRange() + True + + >>> zn.getResultsRange() == get_results_range_from(specification, Zn) + True + + >>> zn.getResultsRange().min + 50 From 104a865bffda23014dbab840fe88f7e1c05829bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 23:25:37 +0100 Subject: [PATCH 22/63] Populate Sample ResultsRange settings to partitions --- bika/lims/content/analysisrequest.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 66b97fa4eb..9ef785fdec 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -85,6 +85,7 @@ from bika.lims.interfaces import ICancellable from bika.lims.interfaces import IClient from bika.lims.interfaces import ISubmitted +from bika.lims.permissions import EditResults from bika.lims.permissions import FieldEditBatch from bika.lims.permissions import FieldEditClient from bika.lims.permissions import FieldEditClientOrderNumber @@ -1450,26 +1451,34 @@ def setSpecification(self, value): # Update only results ranges if specs is not None, so results ranges # manually set previously (e.g. via ManageAnalyses view) are # preserved unless a new Specification overrides them - self.setResultsRange(spec.getResultsRange()) + self.setResultsRange(spec.getResultsRange(), recursive=False) # Cascade the changes to partitions, but only to those for which that # are in a status in which the specification can be updated. This # prevents the re-assignment of Specifications to already verified or # published samples permission = self.getField("Specification").write_permission - for descendant in self.getDescendants(all_descendants=True): + for descendant in self.getDescendants(): if check_permission(permission, descendant): descendant.setSpecification(spec) - def setResultsRange(self, value): + def setResultsRange(self, value, recursive=True): field = self.getField("ResultsRange") field.set(self, value) # Reset results ranges from analyses for analysis in self.objectValues("Analysis"): - service_uid = analysis.getRawAnalysisService() - result_range = field.get(self, uid=service_uid) - analysis.setResultsRange(result_range) + if check_permission(EditResults, analysis): + service_uid = analysis.getRawAnalysisService() + result_range = field.get(self, uid=service_uid) + analysis.setResultsRange(result_range) + + if recursive: + # Cascade the changes to partitions + permission = self.getField("Specification").write_permission + for descendant in self.getDescendants(): + if check_permission(permission, descendant): + descendant.setResultsRange(value) def getClient(self): """Returns the client this object is bound to. We override getClient From 53ca631e26a661bd8037f792b7a21eb24797a21b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 21 Jan 2020 23:26:27 +0100 Subject: [PATCH 23/63] Use case for when Partitions are used with specs assignment --- .../SpecificationAndResultsRanges.rst | 77 +++++++++++++++++++ 1 file changed, 77 insertions(+) diff --git a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst index 22ec2e99bf..351002c6e6 100644 --- a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst +++ b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst @@ -263,3 +263,80 @@ Sample, are reestablished: >>> zn.getResultsRange().min 50 + + +Sample with Specifications and Partitions +----------------------------------------- + +When a sample has partitions, the Specification set to the root Sample is +populated to all its descendants: + + >>> partition = create_partition(sample, request, [zn]) + >>> partition + + + >>> zn = get_analysis_from(partition, Zn) + >>> zn + + +The partition keeps the Specification and ResultsRange by its own: + + >>> partition.getSpecification() + + + >>> partition.getResultsRange() == specification.getResultsRange() + True + +If we reset an Analysis with it's own ResultsRange, different from the range +defined by the Specification, the system will clear the Specification from +both the root sample and the partition to guarantee compliance: + + >>> rr_zn = zn.getResultsRange() + >>> rr_zn.min = 56 + >>> partition.setAnalyses([Zn], specs=[rr_zn]) + [] + + >>> partition.getSpecification() is None + True + +But the root sample will keep its own ResultsRange and Specification untouched: + + >>> sample.getSpecification() + + + >>> sample.getResultsRange() == specification.getResultsRange() + True + +We can re-assign the Specification to the partition, though: + + >>> partition.setSpecification(specification) + >>> specification.getResultsRange() == partition.getResultsRange() + True + + >>> zn.getResultsRange() == get_results_range_from(specification, Zn) + True + + >>> zn.getResultsRange().min + 50 + +If we reset the same analysis, but in the root sample, both root and partition +loose the Specification: + + >>> rr_zn = zn.getResultsRange() + >>> rr_zn.min = 57 + >>> sample.setAnalyses([Au, Cu, Fe, Zn], specs=[rr_zn]) + [] + + >>> sample.getSpecification() is None + True + + >>> partition.getSpecification() is None + True + +And ResultsRange for Zn is stored in both the root and the partition: + + >>> get_results_range_from(sample, Zn).min + 57 + + >>> get_results_range_from(partition, Zn).min + 57 From be07d6280a169bbe397042cee37ec10321ea1ad9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 10:41:18 +0100 Subject: [PATCH 24/63] aranalysesfield clean-up --- bika/lims/browser/fields/aranalysesfield.py | 89 ++++++++++----------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index ba75d3767f..41185b3bdf 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -143,58 +143,19 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): specs = self.resolve_specs(instance, specs) # Add analyses - new_analyses = [] - for service in services: - an = self.add_analysis(instance, service, prices, hidden, specs) - if an: - new_analyses.append(an) + new_analyses = self.add_analyses(instance, services, prices, hidden, specs) - # Remove analyses - # Since Manage Analyses view displays the analyses from partitions, we - # also need to take them into consideration here. Analyses from - # ancestors can be omitted. + # Get all analyses (those from descendants included) analyses = instance.objectValues("Analysis") analyses.extend(self.get_analyses_from_descendants(instance)) - # Service UIDs - service_uids = map(api.get_uid, services) - - # Assigned Attachments - assigned_attachments = [] - - for analysis in analyses: - service_uid = analysis.getServiceUID() - - # Skip if the Service is selected - if service_uid in service_uids: - continue + # Bail out those not in services list or submitted + uids = map(api.get_uid, services) + to_remove = filter(lambda an: an.getServiceUID() not in uids, analyses) + to_remove = filter(lambda an: not ISubmitted.providedBy(an), to_remove) - # Skip non-open Analyses - if ISubmitted.providedBy(analysis): - continue - - # Remember assigned attachments - # https://github.com/senaite/senaite.core/issues/1025 - assigned_attachments.extend(analysis.getAttachment()) - analysis.setAttachment([]) - - # If it is assigned to a worksheet, unassign it before deletion. - worksheet = analysis.getWorksheet() - if worksheet: - worksheet.removeAnalysis(analysis) - - # Remove the analysis - # Note the analysis might belong to a partition - analysis.aq_parent.manage_delObjects(ids=[api.get_id(analysis)]) - - # Remove orphaned attachments - for attachment in assigned_attachments: - # only delete attachments which are no further linked - if not attachment.getLinkedAnalyses(): - logger.info( - "Deleting attachment: {}".format(attachment.getId())) - attachment_id = api.get_id(attachment) - api.get_parent(attachment).manage_delObjects(attachment_id) + # Remove analyses + map(self.remove_analysis, to_remove) return new_analyses @@ -308,6 +269,14 @@ def resolve_result_range(self, result_range, original): return result_range + def add_analyses(self, instance, services, prices, hidden, specs): + new_analyses = [] + for service in services: + an = self.add_analysis(instance, service, prices, hidden, specs) + if an: + new_analyses.append(an) + return new_analyses + def add_analysis(self, instance, service, prices, hidden, specs=None): service_uid = api.get_uid(service) new_analysis = False @@ -362,6 +331,32 @@ def add_analysis(self, instance, service, prices, hidden, specs=None): return None + def remove_analysis(self, analysis): + """Removes a given analysis from the instance + """ + # Remember assigned attachments + # https://github.com/senaite/senaite.core/issues/1025 + attachments = analysis.getAttachment() + analysis.setAttachment([]) + + # If assigned to a worksheet, unassign it before deletion + worksheet = analysis.getWorksheet() + if worksheet: + worksheet.removeAnalysis(analysis) + + # Remove the analysis + # Note the analysis might belong to a partition + analysis.aq_parent.manage_delObjects(ids=[api.get_id(analysis)]) + + # Remove orphaned attachments + for attachment in attachments: + if not attachment.getLinkedAnalyses(): + # only delete attachments which are no further linked + logger.info( + "Deleting attachment: {}".format(attachment.getId())) + attachment_id = api.get_id(attachment) + api.get_parent(attachment).manage_delObjects(attachment_id) + def resolve_analyses(self, instance, service): """Resolves analyses for the service and instance It returns a list, cause for a given sample, multiple analyses for same From 64eb91873c93ec75b86929e805c66fa39e54e410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 12:08:39 +0100 Subject: [PATCH 25/63] Fix possible recursion error when fetching calculation dependencies --- bika/lims/browser/fields/aranalysesfield.py | 2 - bika/lims/content/calculation.py | 31 +++- .../doctests/ServicesCalculationRecursion.rst | 143 ++++++++++++++++++ 3 files changed, 171 insertions(+), 5 deletions(-) create mode 100644 bika/lims/tests/doctests/ServicesCalculationRecursion.rst diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index 41185b3bdf..de4dd430ed 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -131,8 +131,6 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): services = filter(None, map(self._to_service, items)) # Calculate dependencies - # FIXME Infinite recursion error possible here, if the formula includes - # the Keyword of the Service that includes the Calculation dependencies = map(lambda s: s.getServiceDependencies(), services) dependencies = list(itertools.chain.from_iterable(dependencies)) diff --git a/bika/lims/content/calculation.py b/bika/lims/content/calculation.py index 0ba888b09e..c61a4fcfaa 100644 --- a/bika/lims/content/calculation.py +++ b/bika/lims/content/calculation.py @@ -252,14 +252,39 @@ def getCalculationDependencies(self, flat=False, deps=None): if deps is None: deps = [] if flat is True else {} + def get_fetched(deps): + if isinstance(deps, list): + return map(api.get_uid, deps) + if isinstance(deps, dict): + fetched = deps.keys() + for value in deps.values(): + fetched.extend(get_fetched(value)) + return fetched + return [] + + # List of service uids that have been grabbed already. This is used to + # prevent an infinite recursion error when the formula includes the + # Keyword of the Service that includes the Calculation + fetched = get_fetched(deps) + for service in self.getDependentServices(): - calc = service.getCalculation() - if calc: - calc.getCalculationDependencies(flat, deps) + if api.get_uid(service) in fetched: + # Processed already. Omit to prevent recursion + continue + if flat: deps.append(service) else: deps[service.UID()] = {} + + calc = service.getCalculation() + if calc: + calc.getCalculationDependencies(flat, deps) + + if flat: + # Remove duplicates + deps = list(set(deps)) + return deps def getCalculationDependants(self, deps=None): diff --git a/bika/lims/tests/doctests/ServicesCalculationRecursion.rst b/bika/lims/tests/doctests/ServicesCalculationRecursion.rst new file mode 100644 index 0000000000..f536df381f --- /dev/null +++ b/bika/lims/tests/doctests/ServicesCalculationRecursion.rst @@ -0,0 +1,143 @@ +Infinite recursion when fetching dependencies from Service +========================================================== + +This test checks that no infinite recursion error arises when fetching the +dependencies of a Service (via Calculation) that itself contains a keyword in +a calculation from another service bound to a calculation that refers to the +first one as well. + +Running this test from the buildout directory: + + bin/test test_textual_doctests -t ServicesCalculationRecursion.rst + +Test Setup +---------- + +Needed imports: + + >>> import transaction + >>> from DateTime import DateTime + >>> from plone.app.testing import setRoles + >>> from plone.app.testing import TEST_USER_ID + >>> from plone.app.testing import TEST_USER_PASSWORD + >>> from bika.lims import api + >>> from bika.lims.utils.analysisrequest import create_analysisrequest + >>> from bika.lims.utils.analysisrequest import create_partition + >>> from bika.lims.workflow import doActionFor as do_action_for + +Functional Helpers: + + >>> def new_sample(services, specification=None, results_ranges=None): + ... values = { + ... 'Client': client.UID(), + ... 'Contact': contact.UID(), + ... 'DateSampled': DateTime().strftime("%Y-%m-%d"), + ... 'SampleType': sampletype.UID(), + ... 'Analyses': map(api.get_uid, services), + ... 'Specification': specification or None } + ... + ... ar = create_analysisrequest(client, request, values, results_ranges=results_ranges) + ... transitioned = do_action_for(ar, "receive") + ... return ar + + >>> def get_analysis_from(sample, service): + ... service_uid = api.get_uid(service) + ... for analysis in sample.getAnalyses(full_objects=True): + ... if analysis.getServiceUID() == service_uid: + ... return analysis + ... return None + + >>> def get_results_range_from(obj, service): + ... field = obj.getField("ResultsRange") + ... return field.get(obj, uid=api.get_uid(service)) + + >>> def set_results_range_for(obj, results_range): + ... rrs = obj.getResultsRange() + ... uid = results_range["uid"] + ... rrs = filter(lambda rr: rr["uid"] != uid, rrs) + ... rrs.append(results_range) + ... obj.setResultsRange(rrs) + + +Variables: + + >>> portal = self.portal + >>> request = self.request + >>> setup = api.get_setup() + +Create some basic objects for the test: + + >>> setRoles(portal, TEST_USER_ID, ['Manager',]) + >>> client = api.create(portal.clients, "Client", Name="Happy Hills", ClientID="HH", MemberDiscountApplies=True) + >>> contact = api.create(client, "Contact", Firstname="Rita", Lastname="Mohale") + >>> sampletype = api.create(setup.bika_sampletypes, "SampleType", title="Water", Prefix="W") + >>> labcontact = api.create(setup.bika_labcontacts, "LabContact", Firstname="Lab", Lastname="Manager") + >>> department = api.create(setup.bika_departments, "Department", title="Chemistry", Manager=labcontact) + >>> category = api.create(setup.bika_analysiscategories, "AnalysisCategory", title="Metals", Department=department) + + +Creation of Service with a Calculation that refers to itself +------------------------------------------------------------ + +The most common case is when the Calculation is assigned to the same Analysis +that is referred in the Calculation's formula: + + >>> Ca = api.create(setup.bika_analysisservices, "AnalysisService", title="Calcium", Keyword="Ca", Price="20", Category=category.UID()) + >>> Mg = api.create(setup.bika_analysisservices, "AnalysisService", title="Magnesium", Keyword="Mg", Price="20", Category=category.UID()) + + >>> calc = api.create(setup.bika_calculations, "Calculation", title="Total Hardness") + >>> calc.setFormula("[Ca] + [Mg]") + >>> calc.getFormula() + '[Ca] + [Mg]' + + >>> Ca.setCalculation(calc) + >>> Ca.getCalculation() + + + >>> deps = Ca.getServiceDependencies() + >>> sorted(map(lambda d: d.getKeyword(), deps)) + ['Ca', 'Mg'] + + >>> deps = calc.getCalculationDependencies() + >>> len(deps.keys()) + 2 + + >>> deps = calc.getCalculationDependencies(flat=True) + >>> sorted(map(lambda d: d.getKeyword(), deps)) + ['Ca', 'Mg'] + +The other case is when the initial Service is referred indirectly, through a +calculation a dependency is bound to: + + >>> calc_mg = api.create(setup.bika_calculations, "Calculation", title="Test") + >>> calc_mg.setFormula("[Ca] + [Ca]") + >>> calc_mg.getFormula() + '[Ca] + [Ca]' + + >>> Mg.setCalculation(calc_mg) + >>> Mg.getCalculation() + + + >>> deps = Mg.getServiceDependencies() + >>> sorted(map(lambda d: d.getKeyword(), deps)) + ['Ca', 'Mg'] + + >>> deps = calc_mg.getCalculationDependencies() + >>> len(deps.keys()) + 2 + + >>> deps = calc_mg.getCalculationDependencies(flat=True) + >>> sorted(map(lambda d: d.getKeyword(), deps)) + ['Ca', 'Mg'] + + >>> deps = Ca.getServiceDependencies() + >>> sorted(map(lambda d: d.getKeyword(), deps)) + ['Ca', 'Mg'] + + >>> deps = calc.getCalculationDependencies() + >>> len(deps.keys()) + 2 + + >>> deps = calc.getCalculationDependencies(flat=True) + >>> sorted(map(lambda d: d.getKeyword(), deps)) + ['Ca', 'Mg'] \ No newline at end of file From 7d2ab9b8f9fae864f01e1fd3822d3290c29a320f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 12:11:32 +0100 Subject: [PATCH 26/63] Remove unnecessary stuff from the doctest --- .../doctests/ServicesCalculationRecursion.rst | 42 ------------------- 1 file changed, 42 deletions(-) diff --git a/bika/lims/tests/doctests/ServicesCalculationRecursion.rst b/bika/lims/tests/doctests/ServicesCalculationRecursion.rst index f536df381f..c55d5ce181 100644 --- a/bika/lims/tests/doctests/ServicesCalculationRecursion.rst +++ b/bika/lims/tests/doctests/ServicesCalculationRecursion.rst @@ -15,49 +15,10 @@ Test Setup Needed imports: - >>> import transaction - >>> from DateTime import DateTime >>> from plone.app.testing import setRoles >>> from plone.app.testing import TEST_USER_ID >>> from plone.app.testing import TEST_USER_PASSWORD >>> from bika.lims import api - >>> from bika.lims.utils.analysisrequest import create_analysisrequest - >>> from bika.lims.utils.analysisrequest import create_partition - >>> from bika.lims.workflow import doActionFor as do_action_for - -Functional Helpers: - - >>> def new_sample(services, specification=None, results_ranges=None): - ... values = { - ... 'Client': client.UID(), - ... 'Contact': contact.UID(), - ... 'DateSampled': DateTime().strftime("%Y-%m-%d"), - ... 'SampleType': sampletype.UID(), - ... 'Analyses': map(api.get_uid, services), - ... 'Specification': specification or None } - ... - ... ar = create_analysisrequest(client, request, values, results_ranges=results_ranges) - ... transitioned = do_action_for(ar, "receive") - ... return ar - - >>> def get_analysis_from(sample, service): - ... service_uid = api.get_uid(service) - ... for analysis in sample.getAnalyses(full_objects=True): - ... if analysis.getServiceUID() == service_uid: - ... return analysis - ... return None - - >>> def get_results_range_from(obj, service): - ... field = obj.getField("ResultsRange") - ... return field.get(obj, uid=api.get_uid(service)) - - >>> def set_results_range_for(obj, results_range): - ... rrs = obj.getResultsRange() - ... uid = results_range["uid"] - ... rrs = filter(lambda rr: rr["uid"] != uid, rrs) - ... rrs.append(results_range) - ... obj.setResultsRange(rrs) - Variables: @@ -68,9 +29,6 @@ Variables: Create some basic objects for the test: >>> setRoles(portal, TEST_USER_ID, ['Manager',]) - >>> client = api.create(portal.clients, "Client", Name="Happy Hills", ClientID="HH", MemberDiscountApplies=True) - >>> contact = api.create(client, "Contact", Firstname="Rita", Lastname="Mohale") - >>> sampletype = api.create(setup.bika_sampletypes, "SampleType", title="Water", Prefix="W") >>> labcontact = api.create(setup.bika_labcontacts, "LabContact", Firstname="Lab", Lastname="Manager") >>> department = api.create(setup.bika_departments, "Department", title="Chemistry", Manager=labcontact) >>> category = api.create(setup.bika_analysiscategories, "AnalysisCategory", title="Metals", Department=department) From 25aa1c094d35354b2b9408d44bfb4ee55b1c2c4e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 13:27:36 +0100 Subject: [PATCH 27/63] Make setAnalyses from ARAnalysesField to not return anything --- bika/lims/browser/fields/aranalysesfield.py | 40 +++-------- bika/lims/tests/doctests/ARAnalysesField.rst | 71 +++++++++---------- .../ARAnalysesFieldWithPartitions.rst | 34 ++------- .../RemoveAnalysesFromAnalysisRequest.rst | 10 +-- .../SpecificationAndResultsRanges.rst | 10 --- 5 files changed, 52 insertions(+), 113 deletions(-) diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index de4dd430ed..4a141ff83e 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -141,7 +141,8 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): specs = self.resolve_specs(instance, specs) # Add analyses - new_analyses = self.add_analyses(instance, services, prices, hidden, specs) + params = dict(prices=prices, hidden=hidden, specs=specs) + map(lambda serv: self.add_analysis(instance, serv, **params), services) # Get all analyses (those from descendants included) analyses = instance.objectValues("Analysis") @@ -155,8 +156,6 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): # Remove analyses map(self.remove_analysis, to_remove) - return new_analyses - def resolve_specs(self, instance, results_ranges): """Returns a dictionary where the key is the service_uid and the value is its results range. If a result range for a given service does not @@ -267,27 +266,20 @@ def resolve_result_range(self, result_range, original): return result_range - def add_analyses(self, instance, services, prices, hidden, specs): - new_analyses = [] - for service in services: - an = self.add_analysis(instance, service, prices, hidden, specs) - if an: - new_analyses.append(an) - return new_analyses - - def add_analysis(self, instance, service, prices, hidden, specs=None): + def add_analysis(self, instance, service, **kwargs): service_uid = api.get_uid(service) - new_analysis = False # Ensure we have suitable parameters - specs = specs or {} + specs = kwargs.get("specs") or {} # Get the hidden status for the service - hidden = filter(lambda d: d.get("uid") == service_uid, hidden or []) + hidden = kwargs.get("hidden") or [] + hidden = filter(lambda d: d.get("uid") == service_uid, hidden) hidden = hidden and hidden[0] or service.getHidden() # Get the price for the service - price = prices and prices.get(service_uid) or service.getPrice() + prices = kwargs.get("prices") or {} + price = prices.get(service_uid) or service.getPrice() # Does analyses are for internal use internal_use = instance.getInternalUse() @@ -298,7 +290,6 @@ def add_analysis(self, instance, service, prices, hidden, specs=None): analyses = self.resolve_analyses(instance, service) if not analyses: # Create the analysis - new_analysis = True keyword = service.getKeyword() logger.info("Creating new analysis '{}'".format(keyword)) analysis = create_analysis(instance, service) @@ -323,12 +314,6 @@ def add_analysis(self, instance, service, prices, hidden, specs=None): analysis.setResultsRange(analysis_rr) analysis.reindexObject() - # Only return the analysis if is a new one - if new_analysis: - return analyses[0] - - return None - def remove_analysis(self, analysis): """Removes a given analysis from the instance """ @@ -426,15 +411,6 @@ def get_from_descendant(self, instance, service): return analyses - def _get_services(self, full_objects=False): - """Fetch and return analysis service objects - """ - bsc = api.get_tool("bika_setup_catalog") - brains = bsc(portal_type="AnalysisService") - if full_objects: - return map(api.get_object, brains) - return brains - def _to_service(self, thing): """Convert to Analysis Service diff --git a/bika/lims/tests/doctests/ARAnalysesField.rst b/bika/lims/tests/doctests/ARAnalysesField.rst index e7dd1a8cdf..330e39cd0c 100644 --- a/bika/lims/tests/doctests/ARAnalysesField.rst +++ b/bika/lims/tests/doctests/ARAnalysesField.rst @@ -41,6 +41,13 @@ Functional Helpers: >>> def timestamp(format="%Y-%m-%d"): ... return DateTime().strftime(format) + >>> def get_analyses_from(sample, services): + ... if not isinstance(services, (list, tuple)): + ... services = [services] + ... uids = map(api.get_uid, services) + ... analyses = sample.getAnalyses(full_objects=True) + ... return filter(lambda an: an.getServiceUID() in uids, analyses) + Variables:: >>> date_now = timestamp() @@ -241,23 +248,16 @@ The field takes the following parameters: Pass in all prior created Analysis Services: >>> all_services = [analysisservice1, analysisservice2, analysisservice3] - >>> new_analyses = field.set(ar, all_services) + >>> field.set(ar, all_services) We expect to have now the `CA` and `MG` Analyses as well: - >>> sorted(new_analyses, key=methodcaller('getId')) - [, ] - -In the Analyis Request should be now three Analyses: - - >>> len(ar.objectValues("Analysis")) - 3 + >>> sorted(ar.objectValues("Analysis"), key=methodcaller('getId')) + [, , ] Removing Analyses is done by omitting those from the `items` list: - >>> new_analyses = field.set(ar, [analysisservice1]) - >>> sorted(new_analyses, key=methodcaller('getId')) - [] + >>> field.set(ar, [analysisservice1]) Now there should be again only one Analysis assigned: @@ -272,7 +272,7 @@ We expect to have just the `PH` Analysis again: The field can also handle UIDs of Analyses Services: >>> service_uids = map(api.get_uid, all_services) - >>> new_analyses = field.set(ar, service_uids) + >>> field.set(ar, service_uids) We expect again to have all the three Analyses: @@ -289,7 +289,7 @@ The field should also handle catalog brains: >>> api.get_title(brain) 'Calcium' - >>> new_analyses = field.set(ar, [brain]) + >>> field.set(ar, [brain]) We expect now to have just the `CA` analysis assigned: @@ -298,7 +298,7 @@ We expect now to have just the `CA` analysis assigned: Now let's try int mixed, one catalog brain and one object: - >>> new_analyses = field.set(ar, [analysisservice1, brain]) + >>> field.set(ar, [analysisservice1, brain]) We expect now to have now `PH` and `CA`: @@ -308,7 +308,7 @@ We expect now to have now `PH` and `CA`: Finally, we test it with an `Analysis` object: >>> analysis1 = ar["PH"] - >>> new_analyses = field.set(ar, [analysis1]) + >>> field.set(ar, [analysis1]) >>> sorted(ar.objectValues("Analysis"), key=methodcaller("getId")) [] @@ -330,7 +330,7 @@ It is a dictionary with the following keys and values: Each Analysis can request its own Specification (Result Range): - >>> new_analyses = field.set(ar, all_services) + >>> field.set(ar, all_services) >>> analysis1 = ar[analysisservice1.getKeyword()] >>> analysis2 = ar[analysisservice2.getKeyword()] @@ -350,7 +350,7 @@ Request and have precedence over the lab specifications: >>> arr = [arr1, arr2, arr3] >>> all_analyses = [analysis1, analysis2, analysis3] - >>> new_analyses = field.set(ar, all_analyses, specs=arr) + >>> field.set(ar, all_analyses, specs=arr) >>> myspec1 = analysis1.getResultsRange() >>> myspec1.get("rangecomment") @@ -376,7 +376,7 @@ this Analysis. The specifications get applied if the keyword matches: >>> ph_specs = {"keyword": analysis1.getKeyword(), "min": 5.2, "max": 7.9, "error": 3} - >>> new_analyses = field.set(ar, [analysis1], specs=[ph_specs]) + >>> field.set(ar, [analysis1], specs=[ph_specs]) We expect to have now just one Analysis set: @@ -415,7 +415,7 @@ Prices are primarily defined on Analyses Services: Created Analyses inherit that price: - >>> new_analyses = field.set(ar, all_services) + >>> field.set(ar, all_services) >>> analysis1 = ar[analysisservice1.getKeyword()] >>> analysis2 = ar[analysisservice2.getKeyword()] @@ -440,7 +440,7 @@ The `setter` also allows to set custom prices for the Analyses: Now we set the field with all analyses services and new prices: - >>> new_analyses = field.set(ar, all_services, prices=prices) + >>> field.set(ar, all_services, prices=prices) The Analyses have now the new prices: @@ -491,7 +491,8 @@ Append interim field `B` to the `Total Hardness` Analysis Service: Now we assign the `Total Hardness` Analysis Service: - >>> new_analyses = field.set(ar, [analysisservice4]) + >>> field.set(ar, [analysisservice4]) + >>> new_analyses = get_analyses_from(ar, analysisservice4) >>> analysis = new_analyses[0] >>> analysis @@ -532,12 +533,7 @@ The Analysis Service returns the interim fields from the Calculation too: Update the AR with the new Analysis Service: - >>> new_analyses = field.set(ar, [analysisservice4]) - -Since no new Analyses were created, the field should return an empty list: - - >>> new_analyses - [] + >>> field.set(ar, [analysisservice4]) The Analysis should be still there: @@ -571,9 +567,8 @@ is removed from an Analysis Request. Assign the `PH` Analysis: - >>> new_analyses = field.set(ar, [analysisservice1]) - >>> new_analyses - [] + >>> field.set(ar, [analysisservice1]) + >>> new_analyses = ar.getAnalyses(full_objects=True) Create a new Worksheet and assign the Analysis to it: @@ -607,9 +602,7 @@ The worksheet contains now the Analysis: Removing the analysis from the AR also unassignes it from the worksheet: - >>> new_analyses = field.set(ar, [analysisservice2]) - >>> new_analyses - [] + >>> field.set(ar, [analysisservice2]) >>> ws.getAnalyses() [] @@ -635,7 +628,7 @@ Get the dependent services: We expect that dependent services get automatically set: - >>> new_analyses = field.set(ar, [analysisservice4]) + >>> field.set(ar, [analysisservice4]) >>> sorted(ar.objectValues("Analysis"), key=methodcaller('getId')) [, , ] @@ -681,7 +674,7 @@ We create a new attachment in the client and assign it to this specific analysis Now we remove the *PH* analysis. Since it is prohibited by the field to remove all analyses from an AR, we will set here some other analyses instead: - >>> new_analyses = field.set(ar2, [analysisservice2, analysisservice3]) + >>> field.set(ar2, [analysisservice2, analysisservice3]) The attachment should be deleted from the client folder as well: @@ -690,14 +683,14 @@ The attachment should be deleted from the client folder as well: Re-adding the *PH* analysis should start with no attachments: - >>> new_analyses = field.set(ar2, [analysisservice1, analysisservice2, analysisservice3]) + >>> field.set(ar2, [analysisservice1, analysisservice2, analysisservice3]) >>> an1 = ar2[analysisservice1.getKeyword()] >>> an1.getAttachment() [] This should work as well when multiple attachments are assigned. - >>> new_analyses = field.set(ar2, [analysisservice1, analysisservice2]) + >>> field.set(ar2, [analysisservice1, analysisservice2]) >>> an1 = ar2[analysisservice1.getKeyword()] >>> an2 = ar2[analysisservice2.getKeyword()] @@ -724,7 +717,7 @@ Assign the second half of the attachments to the *Magnesium* analysis: Removing the *PH* analysis should also remove all the assigned attachments: - >>> new_analyses = field.set(ar2, [analysisservice2]) + >>> field.set(ar2, [analysisservice2]) >>> att2.getId() in ar2.getClient().objectIds() False @@ -837,7 +830,7 @@ And all contained Analyses of the retest keep references to the same Attachments This means that removing that attachment from the retest should **not** delete the attachment from the original AR: - >>> new_analyses = field.set(ar_retest, [analysisservice1]) + >>> field.set(ar_retest, [analysisservice1]) >>> an.getAttachment() [] diff --git a/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst b/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst index 4f1da3e3e4..f14ab0c235 100644 --- a/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst +++ b/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst @@ -233,30 +233,20 @@ Addition of analyses add_analysis ............ -Setup required parameters: - - >>> prices = hidden = dict() - If we try to add now an analysis that already exists, either in the partition or in the primary, the analysis won't be added: - >>> added = field.add_analysis(sample, Fe, prices, hidden) - >>> added is None - True + >>> field.add_analysis(sample, Fe) >>> sample.objectValues("Analysis") [] - >>> added = field.add_analysis(partition, Fe, prices, hidden) - >>> added is None - True + >>> field.add_analysis(partition, Fe) >>> partition.objectValues("Analysis") [, ] If we add a new analysis, this will be added in the sample we are working with: - >>> au = field.add_analysis(sample, Au, prices, hidden) - >>> au.getServiceUID() == api.get_uid(Au) - True + >>> field.add_analysis(sample, Au) >>> sample.objectValues("Analysis") [] >>> partition.objectValues("Analysis") @@ -269,9 +259,7 @@ Apply the changes: If I try to add an analysis that exists in an ancestor, the analysis gets moved while the function returns None: - >>> added = field.add_analysis(partition, Au, prices, hidden) - >>> added is None - True + >>> field.add_analysis(partition, Au) >>> sample.objectValues("Analysis") [] >>> partition.objectValues("Analysis") @@ -285,7 +273,6 @@ If we try to set same analyses as before to the root sample, nothing happens because the analyses are already there: >>> field.set(sample, [Cu, Fe, Au]) - [] The analyses still belong to the partition though: @@ -297,7 +284,6 @@ The analyses still belong to the partition though: Same result if I set the analyses to the partition: >>> field.set(partition, [Cu, Fe, Au]) - [] >>> sample.objectValues("Analysis") [] >>> partition.objectValues("Analysis") @@ -306,7 +292,6 @@ Same result if I set the analyses to the partition: If I add a new analysis in the list, the analysis is successfully added: >>> field.set(sample, [Cu, Fe, Au, Mg]) - [] >>> sample.objectValues("Analysis") [] @@ -319,13 +304,10 @@ Apply the changes: >>> transaction.commit() -If I set the same analyses to the partition, I don't get any result: +If I set the same analyses to the partition, the `Mg` analysis is moved into +the partition: >>> field.set(partition, [Cu, Fe, Au, Mg]) - [] - -but, the `Mg` analysis has been moved into the partition: - >>> sample.objectValues("Analysis") [] >>> partition.objectValues("Analysis") @@ -334,7 +316,6 @@ but, the `Mg` analysis has been moved into the partition: To remove `Mg` analysis, pass the list without `Mg`: >>> field.set(sample, [Cu, Fe, Au]) - [] The analysis `Mg` has been removed, although it belonged to the partition: @@ -347,10 +328,9 @@ But if I add a new analysis to the primary and I try to remove it from the partition, nothing will happen: >>> field.set(sample, [Cu, Fe, Au, Mg]) - [] >>> field.set(partition, [Cu, Fe, Au]) - [] + >>> sample.objectValues("Analysis") [] >>> partition.objectValues("Analysis") diff --git a/bika/lims/tests/doctests/RemoveAnalysesFromAnalysisRequest.rst b/bika/lims/tests/doctests/RemoveAnalysesFromAnalysisRequest.rst index a322063e75..c05794b4b0 100644 --- a/bika/lims/tests/doctests/RemoveAnalysesFromAnalysisRequest.rst +++ b/bika/lims/tests/doctests/RemoveAnalysesFromAnalysisRequest.rst @@ -80,7 +80,7 @@ Create a new Analysis Request: And remove two analyses (`Cu` and `Fe`): - >>> new_analyses = ar.setAnalyses([Au]) + >>> ar.setAnalyses([Au]) >>> map(lambda an: an.getKeyword(), ar.getAnalyses(full_objects=True)) ['Au'] @@ -144,7 +144,7 @@ Again, the Analysis Request status is still `sample_received`: But if we remove the analysis without result (`Cu`), the Analysis Request transitions to "to_be_verified" because follows `Fe`: - >>> new_analyses = ar.setAnalyses([Fe, Au]) + >>> ar.setAnalyses([Fe, Au]) >>> api.get_workflow_status_of(ar) 'to_be_verified' @@ -153,7 +153,7 @@ Therefore, if we try to remove the analysis `Fe` (in `to_be_verified` state), the Analysis Request will stay in `to_be_verified` and the Analysis will still be assigned: - >>> new_analyses = ar.setAnalyses([Au]) + >>> ar.setAnalyses([Au]) >>> analysis_fe in ar.objectValues() True @@ -173,7 +173,7 @@ The only way to remove the `Fe` analysis is to retract it first: And if we remove analysis `Fe`, the Analysis Request will follow `Au` analysis (that is `verified`): - >>> new_analyses = ar.setAnalyses([Au]) + >>> ar.setAnalyses([Au]) >>> api.get_workflow_status_of(ar) 'verified' @@ -228,6 +228,6 @@ The Analysis Request status is still `sample_received`: But if we remove the analysis without result (`Cu`), the Analysis Request transitions to "verfied" because follows `Fe` and `Au`: - >>> new_analyses = ar.setAnalyses([Fe, Au]) + >>> ar.setAnalyses([Fe, Au]) >>> api.get_workflow_status_of(ar) 'verified' diff --git a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst index 351002c6e6..a388a78492 100644 --- a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst +++ b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst @@ -192,8 +192,6 @@ analyses, the Specification assigned to the Sample will remain intact, as well as Sample's Results Range: >>> sample.setAnalyses([Au, Cu, Fe]) - [] - >>> analyses = sample.objectValues() >>> sorted(analyses, key=lambda an: an.getKeyword()) [, , ] @@ -213,8 +211,6 @@ Specification set and the specification had a results range registered for such analysis, the result range for the new analysis will be set automatically: >>> sample.setAnalyses([Au, Cu, Fe, Zn]) - [] - >>> sample.getSpecification() @@ -229,8 +225,6 @@ guarantee compliance: >>> rr_zn = zn.getResultsRange() >>> rr_zn.min = 55 >>> sample.setAnalyses([Au, Cu, Fe, Zn], specs=[rr_zn]) - [] - >>> sample.getSpecification() is None True @@ -294,8 +288,6 @@ both the root sample and the partition to guarantee compliance: >>> rr_zn = zn.getResultsRange() >>> rr_zn.min = 56 >>> partition.setAnalyses([Zn], specs=[rr_zn]) - [] - >>> partition.getSpecification() is None True @@ -325,8 +317,6 @@ loose the Specification: >>> rr_zn = zn.getResultsRange() >>> rr_zn.min = 57 >>> sample.setAnalyses([Au, Cu, Fe, Zn], specs=[rr_zn]) - [] - >>> sample.getSpecification() is None True From 49565e29a3897945d0cffd1ae4f127361d2b0649 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 15:22:55 +0100 Subject: [PATCH 28/63] SpecificationsField -> ResultsRangesField --- bika/lims/browser/fields/__init__.py | 2 + bika/lims/browser/fields/configure.zcml | 4 +- bika/lims/browser/fields/resultrangefield.py | 98 +++++++++++++++++ ...icationsfield.py => resultsrangesfield.py} | 100 +++++------------- bika/lims/content/abstractanalysis.py | 4 +- bika/lims/content/analysisrequest.py | 4 +- bika/lims/content/analysisspec.py | 4 +- 7 files changed, 134 insertions(+), 82 deletions(-) create mode 100644 bika/lims/browser/fields/resultrangefield.py rename bika/lims/browser/fields/{specificationsfield.py => resultsrangesfield.py} (51%) diff --git a/bika/lims/browser/fields/__init__.py b/bika/lims/browser/fields/__init__.py index 5c37b80e51..fb6fb5716d 100644 --- a/bika/lims/browser/fields/__init__.py +++ b/bika/lims/browser/fields/__init__.py @@ -29,3 +29,5 @@ from .reflexrulefield import ReflexRuleField from .proxyfield import ProxyField from .uidreferencefield import UIDReferenceField +from .resultrangefield import ResultRangeField +from .resultsrangesfield import ResultsRangesField diff --git a/bika/lims/browser/fields/configure.zcml b/bika/lims/browser/fields/configure.zcml index 27f9316ca3..c2a8a090a9 100644 --- a/bika/lims/browser/fields/configure.zcml +++ b/bika/lims/browser/fields/configure.zcml @@ -2,12 +2,12 @@ xmlns="http://namespaces.zope.org/zope" i18n_domain="senaite.core"> - diff --git a/bika/lims/browser/fields/resultrangefield.py b/bika/lims/browser/fields/resultrangefield.py new file mode 100644 index 0000000000..3117f61310 --- /dev/null +++ b/bika/lims/browser/fields/resultrangefield.py @@ -0,0 +1,98 @@ +# -*- coding: utf-8 -*- +# +# This file is part of SENAITE.CORE. +# +# SENAITE.CORE is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free Software +# Foundation, version 2. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Copyright 2018-2020 by it's authors. +# Some rights reserved, see README and LICENSE. + +from operator import itemgetter + +from Products.ATExtensions.field import RecordField +from Products.Archetypes.Registry import registerField +from Products.Archetypes.interfaces import IFieldDefaultProvider +from zope.interface import implements + +from bika.lims import bikaMessageFactory as _ +from bika.lims.interfaces.analysis import IRequestAnalysis + + +# A tuple of (subfield_id, subfield_label,) +SUB_FIELDS = ( + ("keyword", _("Analysis Service")), + ("min_operator", _("Min operator")), + ("min", _('Min')), + ("max_operator", _("Max operator")), + ("max", _('Max')), + ("warn_min", _('Min warn')), + ("warn_max", _('Max warn')), + ("hidemin", _('< Min')), + ("hidemax", _('> Max')), + ("rangecomment", _('Range Comment')), +) + + +class ResultRangeField(RecordField): + """A field that stores a results range + """ + _properties = RecordField._properties.copy() + _properties.update({ + "type": "results_range_field", + "subfields": map(itemgetter(0), SUB_FIELDS), + "subfield_labels": dict(SUB_FIELDS), + }) + + def get(self, instance, **kwargs): + from bika.lims.content.analysisspec import ResultsRangeDict + value = super(ResultRangeField, self).get(instance, **kwargs) + if value: + return ResultsRangeDict(dict(value.items())) + return {} + + +registerField(ResultRangeField, title="ResultRange", + description="Used for storing a result range",) + + +class DefaultResultsRangeProvider(object): + """Default Results Range provider for analyses + This is used for backwards-compatibility for when the analysis' ResultsRange + was obtained directly from Sample's ResultsRanges field, before this: + https://github.com/senaite/senaite.core/pull/1506 + """ + implements(IFieldDefaultProvider) + + def __init__(self, context): + self.context = context + + def __call__(self): + """Get the default value. + """ + if not IRequestAnalysis.providedBy(self.context): + return {} + + keyword = self.context.getKeyword() + sample = self.context.getRequest() + if sample and keyword: + field = sample.getField("ResultsRange") + rr = field.get(sample, keyword=keyword) + if rr: + #self.context.setResultsRange(rr) + return rr + #return self.context.getResultsRange() + + return {} + #from bika.lims.content.analysisspec import ResultsRangeDict + #return ResultsRangeDict(uid=api.get_uid(self.context), keyword=keyword) diff --git a/bika/lims/browser/fields/specificationsfield.py b/bika/lims/browser/fields/resultsrangesfield.py similarity index 51% rename from bika/lims/browser/fields/specificationsfield.py rename to bika/lims/browser/fields/resultsrangesfield.py index fd6be4978a..645b1391c7 100644 --- a/bika/lims/browser/fields/specificationsfield.py +++ b/bika/lims/browser/fields/resultsrangesfield.py @@ -1,55 +1,35 @@ +# -*- coding: utf-8 -*- +# +# This file is part of SENAITE.CORE. +# +# SENAITE.CORE is free software: you can redistribute it and/or modify it under +# the terms of the GNU General Public License as published by the Free Software +# Foundation, version 2. +# +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS +# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more +# details. +# +# You should have received a copy of the GNU General Public License along with +# this program; if not, write to the Free Software Foundation, Inc., 51 +# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. +# +# Copyright 2018-2020 by it's authors. +# Some rights reserved, see README and LICENSE. + from operator import itemgetter -from Products.ATExtensions.field import RecordField from Products.ATExtensions.field import RecordsField from Products.Archetypes.Registry import registerField -from Products.Archetypes.interfaces import IFieldDefaultProvider -from zope.interface import implements from bika.lims import api -from bika.lims import bikaMessageFactory as _ +from bika.lims.browser.fields.resultrangefield import SUB_FIELDS from bika.lims.browser.widgets import AnalysisSpecificationWidget from bika.lims.catalog import SETUP_CATALOG -from bika.lims.interfaces.analysis import IRequestAnalysis - -# A tuple of (subfield_id, subfield_label,) -SUB_FIELDS = ( - ("keyword", _("Analysis Service")), - ("min_operator", _("Min operator")), - ("min", _('Min')), - ("max_operator", _("Max operator")), - ("max", _('Max')), - ("warn_min", _('Min warn')), - ("warn_max", _('Max warn')), - ("hidemin", _('< Min')), - ("hidemax", _('> Max')), - ("rangecomment", _('Range Comment')), -) - - -class ResultsRangeField(RecordField): - """A field that stores a results range - """ - _properties = RecordField._properties.copy() - _properties.update({ - "type": "results_range_field", - "subfields": map(itemgetter(0), SUB_FIELDS), - "subfield_labels": dict(SUB_FIELDS), - }) - def get(self, instance, **kwargs): - from bika.lims.content.analysisspec import ResultsRangeDict - value = super(ResultsRangeField, self).get(instance, **kwargs) - if value: - return ResultsRangeDict(dict(value.items())) - return {} - -registerField(ResultsRangeField, title="ResultsRange", - description="Used for storing a results range",) - - -class SpecificationsField(RecordsField): +class ResultsRangesField(RecordsField): """A field that stores a list of results ranges """ _properties = RecordsField._properties.copy() @@ -67,7 +47,7 @@ class SpecificationsField(RecordsField): def get(self, instance, **kwargs): from bika.lims.content.analysisspec import ResultsRangeDict - values = super(SpecificationsField, self).get(instance, **kwargs) + values = super(ResultsRangesField, self).get(instance, **kwargs) # If a keyword or an uid has been specified, return the result range # for that uid or keyword only @@ -100,7 +80,7 @@ def _to_dict(self, value): """Convert the records to persistent dictionaries """ # Resolve items to guarantee all them have the key uid - value = super(SpecificationsField, self)._to_dict(value) + value = super(ResultsRangesField, self)._to_dict(value) return map(self.resolve_uid, value) def resolve_uid(self, raw_dict): @@ -120,33 +100,5 @@ def resolve_uid(self, raw_dict): return value -class DefaultResultsRangeProvider(object): - """Default Results Range provider for analyses - This is used for backwards-compatibility for when the analysis' ResultsRange - was obtained directly from Sample's ResultsRanges field, before this: - https://github.com/senaite/senaite.core/pull/1506 - """ - implements(IFieldDefaultProvider) - - def __init__(self, context): - self.context = context - - def __call__(self): - """Get the default value. - """ - if not IRequestAnalysis.providedBy(self.context): - return {} - - keyword = self.context.getKeyword() - sample = self.context.getRequest() - if sample and keyword: - field = sample.getField("ResultsRange") - rr = field.get(sample, keyword=keyword) - if rr: - #self.context.setResultsRange(rr) - return rr - #return self.context.getResultsRange() - - return {} - #from bika.lims.content.analysisspec import ResultsRangeDict - #return ResultsRangeDict(uid=api.get_uid(self.context), keyword=keyword) +registerField(ResultsRangesField, title="ResultsRanges", + description="Used for storing a results ranges",) diff --git a/bika/lims/content/abstractanalysis.py b/bika/lims/content/abstractanalysis.py index c0e4dc231c..fac63cd391 100644 --- a/bika/lims/content/abstractanalysis.py +++ b/bika/lims/content/abstractanalysis.py @@ -40,7 +40,7 @@ from bika.lims.browser.fields import HistoryAwareReferenceField from bika.lims.browser.fields import InterimFieldsField from bika.lims.browser.fields import UIDReferenceField -from bika.lims.browser.fields.specificationsfield import ResultsRangeField +from bika.lims.browser.fields import ResultRangeField from bika.lims.browser.fields.uidreferencefield import get_backreferences from bika.lims.browser.widgets import RecordsWidget from bika.lims.config import LDL @@ -150,7 +150,7 @@ ) # Results Range that applies to this analysis -ResultsRange = ResultsRangeField( +ResultsRange = ResultRangeField( "ResultsRange", required=0 ) diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 9ef785fdec..836ed2993f 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -62,7 +62,7 @@ from bika.lims.browser.fields import DurationField from bika.lims.browser.fields import UIDReferenceField from bika.lims.browser.fields.remarksfield import RemarksField -from bika.lims.browser.fields.specificationsfield import SpecificationsField +from bika.lims.browser.fields import ResultsRangesField from bika.lims.browser.widgets import DateTimeWidget from bika.lims.browser.widgets import DecimalWidget from bika.lims.browser.widgets import PrioritySelectionWidget @@ -715,7 +715,7 @@ # This field does not consider result ranges manually set to analyses. # Therefore, is also used to "detect" changes between the result ranges # specifically set to analyses and the results ranges set to the sample - SpecificationsField( + ResultsRangesField( "ResultsRange", write_permission=FieldEditSpecification, widget=ComputedWidget(visible=False), diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index ad8ea7fc95..c39243645d 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -29,7 +29,7 @@ from bika.lims import bikaMessageFactory as _ from bika.lims.browser.fields import UIDReferenceField -from bika.lims.browser.fields.specificationsfield import SpecificationsField +from bika.lims.browser.fields import ResultsRangesField from bika.lims.browser.widgets import AnalysisSpecificationWidget from bika.lims.browser.widgets import ReferenceWidget from bika.lims.catalog.bikasetup_catalog import SETUP_CATALOG @@ -60,7 +60,7 @@ )) + BikaSchema.copy() + Schema(( - SpecificationsField( + ResultsRangesField( 'ResultsRange', required=1, widget=AnalysisSpecificationWidget( From 1abd2a22f9af92bd87ab56d6b7ed2886059861a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 15:38:11 +0100 Subject: [PATCH 29/63] A bit of clean-up --- bika/lims/browser/fields/resultrangefield.py | 10 +++------- bika/lims/browser/fields/resultsrangesfield.py | 11 +++++++---- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/bika/lims/browser/fields/resultrangefield.py b/bika/lims/browser/fields/resultrangefield.py index 3117f61310..505816c3ea 100644 --- a/bika/lims/browser/fields/resultrangefield.py +++ b/bika/lims/browser/fields/resultrangefield.py @@ -83,16 +83,12 @@ def __call__(self): if not IRequestAnalysis.providedBy(self.context): return {} - keyword = self.context.getKeyword() + service_uid = self.context.getServiceUID() sample = self.context.getRequest() - if sample and keyword: + if sample and service_uid: field = sample.getField("ResultsRange") - rr = field.get(sample, keyword=keyword) + rr = field.get(sample, uid=service_uid) if rr: - #self.context.setResultsRange(rr) return rr - #return self.context.getResultsRange() return {} - #from bika.lims.content.analysisspec import ResultsRangeDict - #return ResultsRangeDict(uid=api.get_uid(self.context), keyword=keyword) diff --git a/bika/lims/browser/fields/resultsrangesfield.py b/bika/lims/browser/fields/resultsrangesfield.py index 645b1391c7..5d05538153 100644 --- a/bika/lims/browser/fields/resultsrangesfield.py +++ b/bika/lims/browser/fields/resultsrangesfield.py @@ -46,7 +46,6 @@ class ResultsRangesField(RecordsField): }) def get(self, instance, **kwargs): - from bika.lims.content.analysisspec import ResultsRangeDict values = super(ResultsRangesField, self).get(instance, **kwargs) # If a keyword or an uid has been specified, return the result range @@ -54,13 +53,13 @@ def get(self, instance, **kwargs): uid = kwargs.get("uid") keyword = kwargs.get("keyword") if uid or keyword: - return self.getResultsRange(values, uid or keyword) + return self.getResultRange(values, uid or keyword) # Convert the dict items to ResultRangeDict for easy handling + from bika.lims.content.analysisspec import ResultsRangeDict return map(lambda val: ResultsRangeDict(dict(val.items())), values) - def getResultsRange(self, values, uid_keyword_service): - from bika.lims.content.analysisspec import ResultsRangeDict + def getResultRange(self, values, uid_keyword_service): if not uid_keyword_service: return None @@ -73,6 +72,7 @@ def getResultsRange(self, values, uid_keyword_service): key = "uid" # Find out the item for the given uid/keyword + from bika.lims.content.analysisspec import ResultsRangeDict value = filter(lambda v: v.get(key) == uid_keyword_service, values) return value and ResultsRangeDict(dict(value[0].items())) or None @@ -84,6 +84,9 @@ def _to_dict(self, value): return map(self.resolve_uid, value) def resolve_uid(self, raw_dict): + """Returns a copy of the raw dictionary passed in, but with additional + key "uid". It's value is inferred from "keyword" if present + """ value = raw_dict.copy() uid = value.get("uid") if api.is_uid(uid) and uid != "0": From 2103bec7b7bec6b84278617d5dc48d906b373c19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 16:55:35 +0100 Subject: [PATCH 30/63] Ensure ResultsRange value is always stored as a built-in dict --- bika/lims/browser/fields/resultrangefield.py | 32 ++++++++++++++----- .../lims/browser/fields/resultsrangesfield.py | 9 +++--- bika/lims/content/analysisrequest.py | 2 +- bika/lims/content/analysisspec.py | 11 ++----- .../SpecificationAndResultsRanges.rst | 2 +- 5 files changed, 33 insertions(+), 23 deletions(-) diff --git a/bika/lims/browser/fields/resultrangefield.py b/bika/lims/browser/fields/resultrangefield.py index 505816c3ea..446d2b317b 100644 --- a/bika/lims/browser/fields/resultrangefield.py +++ b/bika/lims/browser/fields/resultrangefield.py @@ -54,6 +54,15 @@ class ResultRangeField(RecordField): "subfield_labels": dict(SUB_FIELDS), }) + def set(self, instance, value, **kwargs): + from bika.lims.content.analysisspec import ResultsRangeDict + if isinstance(value, ResultsRangeDict): + # Better store a built-in dict so it will always be available even + # if ResultsRangeDict is removed or changed + value = dict(value) + + super(ResultRangeField, self).set(instance, value, **kwargs) + def get(self, instance, **kwargs): from bika.lims.content.analysisspec import ResultsRangeDict value = super(ResultRangeField, self).get(instance, **kwargs) @@ -83,12 +92,19 @@ def __call__(self): if not IRequestAnalysis.providedBy(self.context): return {} - service_uid = self.context.getServiceUID() - sample = self.context.getRequest() - if sample and service_uid: - field = sample.getField("ResultsRange") - rr = field.get(sample, uid=service_uid) - if rr: - return rr + # Get the AnalysisRequest to look at + analysis = self.context + sample = analysis.getRequest() + if not sample: + return {} - return {} + # Search by keyword + field = sample.getField("ResultsRange") + keyword = analysis.getKeyword() + rr = field.get(sample, search_by=keyword) + if rr: + return rr + + # Try with uid (this shouldn't be necessary) + service_uid = analysis.getServiceUID() + return field.get(sample, search_by=service_uid) or {} diff --git a/bika/lims/browser/fields/resultsrangesfield.py b/bika/lims/browser/fields/resultsrangesfield.py index 5d05538153..3e429274f4 100644 --- a/bika/lims/browser/fields/resultsrangesfield.py +++ b/bika/lims/browser/fields/resultsrangesfield.py @@ -50,10 +50,11 @@ def get(self, instance, **kwargs): # If a keyword or an uid has been specified, return the result range # for that uid or keyword only - uid = kwargs.get("uid") - keyword = kwargs.get("keyword") - if uid or keyword: - return self.getResultRange(values, uid or keyword) + if "search_by" in kwargs: + uid_or_keyword = kwargs.get("search_by") + if uid_or_keyword: + return self.getResultRange(values, uid_or_keyword) or {} + return {} # Convert the dict items to ResultRangeDict for easy handling from bika.lims.content.analysisspec import ResultsRangeDict diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 836ed2993f..e5a2131518 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -1470,7 +1470,7 @@ def setResultsRange(self, value, recursive=True): for analysis in self.objectValues("Analysis"): if check_permission(EditResults, analysis): service_uid = analysis.getRawAnalysisService() - result_range = field.get(self, uid=service_uid) + result_range = field.get(self, search_by=service_uid) analysis.setResultsRange(result_range) if recursive: diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index c39243645d..f73c8e6113 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -124,15 +124,8 @@ def getResultsRangeDict(self): 'warnmin': value, ... } """ - specs = {} - subfields = self.Schema()['ResultsRange'].subfields - for spec in self.getResultsRange(): - keyword = spec['keyword'] - specs[keyword] = {} - for key in subfields: - if key not in ['uid', 'keyword']: - specs[keyword][key] = spec.get(key, '') - return specs + results_range = self.getResultsRange() + return dict(map(lambda rr: (rr["keyword"], rr), results_range)) atapi.registerType(AnalysisSpec, PROJECTNAME) diff --git a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst index a388a78492..a88757044f 100644 --- a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst +++ b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst @@ -49,7 +49,7 @@ Functional Helpers: >>> def get_results_range_from(obj, service): ... field = obj.getField("ResultsRange") - ... return field.get(obj, uid=api.get_uid(service)) + ... return field.get(obj, search_by=api.get_uid(service)) >>> def set_results_range_for(obj, results_range): ... rrs = obj.getResultsRange() From aaa0485504b37b3784d11d3cca90a119578f7860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 17:44:18 +0100 Subject: [PATCH 31/63] Assume that an analysis is added w/o specs, it does not break compliance --- bika/lims/browser/fields/aranalysesfield.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index 4a141ff83e..08e9f95e38 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -232,9 +232,9 @@ def is_compliant_with_specification(self, instance, results_range): service_uid = rr["uid"] sample_rr = sample_rrs.get(service_uid) if not sample_rr: - # Sample's ResultsRange (a "copy" of Specification initially set - # does not contain a result range for this service - return False + # This service is not defined in Sample's ResultsRange, we + # assume this *does not* break the compliance + continue else: # Clean-up the result range passed in From 9c0b3c568c9518dd9fb2510f5003b60c7a80e9a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 17:44:44 +0100 Subject: [PATCH 32/63] Remove getResultsRangeDict from analysisspecs --- .../widgets/analysisspecificationwidget.py | 15 ++++++++++++++- bika/lims/content/analysisspec.py | 14 -------------- 2 files changed, 14 insertions(+), 15 deletions(-) diff --git a/bika/lims/browser/widgets/analysisspecificationwidget.py b/bika/lims/browser/widgets/analysisspecificationwidget.py index bc6407db49..300fb5190c 100644 --- a/bika/lims/browser/widgets/analysisspecificationwidget.py +++ b/bika/lims/browser/widgets/analysisspecificationwidget.py @@ -134,7 +134,7 @@ def update(self): """ super(AnalysisSpecificationView, self).update() self.allow_edit = self.is_edit_allowed() - self.specification = self.context.getResultsRangeDict() + self.specification = self.get_results_range_by_keyword() @view.memoize def is_edit_allowed(self): @@ -148,6 +148,19 @@ def show_categories_enabled(self): """ return self.context.bika_setup.getCategoriseAnalysisServices() + def get_results_range_by_keyword(self): + """Return a dictionary with the specification fields for each + service. The keys of the dictionary are the keywords of each + analysis service. Each service contains a dictionary in which + each key is the name of the spec field: + specs['keyword'] = {'min': value, + 'max': value, + 'warnmin': value, + ... } + """ + results_range = self.context.getResultsRange() + return dict(map(lambda rr: (rr["keyword"], rr), results_range)) + def get_editable_columns(self): """Return editable fields """ diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index f73c8e6113..a3ad2d28e9 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -113,20 +113,6 @@ def contextual_title(self): else: return self.title + " (" + translate(_("Client")) + ")" - @security.public - def getResultsRangeDict(self): - """Return a dictionary with the specification fields for each - service. The keys of the dictionary are the keywords of each - analysis service. Each service contains a dictionary in which - each key is the name of the spec field: - specs['keyword'] = {'min': value, - 'max': value, - 'warnmin': value, - ... } - """ - results_range = self.getResultsRange() - return dict(map(lambda rr: (rr["keyword"], rr), results_range)) - atapi.registerType(AnalysisSpec, PROJECTNAME) From bfa362abb0a8278a1256d66c2ef702f8484ca354 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 18:19:20 +0100 Subject: [PATCH 33/63] Populate Sample's ResultsRanges to Partitions on creation --- bika/lims/utils/analysisrequest.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/bika/lims/utils/analysisrequest.py b/bika/lims/utils/analysisrequest.py index dae6228aa3..b727622a55 100644 --- a/bika/lims/utils/analysisrequest.py +++ b/bika/lims/utils/analysisrequest.py @@ -470,10 +470,14 @@ def create_partition(analysis_request, request, analyses, sample_type=None, client = ar.getClient() analyses = list(set(map(api.get_object, analyses))) services = map(lambda an: an.getAnalysisService(), analyses) - specs = ar.getSpecification() - specs = specs and specs.getResultsRange() or [] - partition = create_analysisrequest(client, request=request, values=record, - analyses=services, results_ranges=specs) + + # Populate the root's ResultsRanges to partitions + results_ranges = ar.getResultsRange() or [] + partition = create_analysisrequest(client, + request=request, + values=record, + analyses=services, + results_ranges=results_ranges) # Reindex Parent Analysis Request ar.reindexObject(idxs=["isRootAncestor"]) From 894482be8e40f4156fa177c83622657610e702be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 18:28:55 +0100 Subject: [PATCH 34/63] Add some comments --- bika/lims/content/analysisrequest.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index e5a2131518..66963ff231 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -1453,20 +1453,25 @@ def setSpecification(self, value): # preserved unless a new Specification overrides them self.setResultsRange(spec.getResultsRange(), recursive=False) - # Cascade the changes to partitions, but only to those for which that - # are in a status in which the specification can be updated. This - # prevents the re-assignment of Specifications to already verified or - # published samples + # Cascade the changes to partitions, but only to those that are in a + # status in which the specification can be updated. This prevents the + # re-assignment of Specifications to already verified or published + # samples permission = self.getField("Specification").write_permission for descendant in self.getDescendants(): if check_permission(permission, descendant): descendant.setSpecification(spec) def setResultsRange(self, value, recursive=True): + """Sets the results range for this Sample and analyses it contains. + If recursive is True, then applies the results ranges to descendants + (partitions) as well as their analyses too + """ + # Set Results Range to the Sample field = self.getField("ResultsRange") field.set(self, value) - # Reset results ranges from analyses + # Set Results Range to analyses for analysis in self.objectValues("Analysis"): if check_permission(EditResults, analysis): service_uid = analysis.getRawAnalysisService() From 1d228e1eb4e1f49a8821c42f1d45012a40055862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 21:26:33 +0100 Subject: [PATCH 35/63] Revisit InternalUse --- bika/lims/browser/fields/aranalysesfield.py | 12 +++--------- bika/lims/content/abstractroutineanalysis.py | 13 +++++++++++++ bika/lims/subscribers/analysisrequest.py | 11 +++++------ 3 files changed, 21 insertions(+), 15 deletions(-) diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index 08e9f95e38..8c74c0d10c 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -281,9 +281,6 @@ def add_analysis(self, instance, service, **kwargs): prices = kwargs.get("prices") or {} price = prices.get(service_uid) or service.getPrice() - # Does analyses are for internal use - internal_use = instance.getInternalUse() - # Gets the analysis or creates the analysis for this service # Note this returns a list, because is possible to have multiple # partitions with same analysis @@ -302,12 +299,9 @@ def add_analysis(self, instance, service, **kwargs): # Set the price of the Analysis analysis.setPrice(price) - # Set internal use - analysis.setInternalUse(internal_use) - if internal_use and not IInternalUse.providedBy(analysis): - alsoProvides(analysis, IInternalUse) - elif not internal_use and IInternalUse.providedBy(analysis): - noLongerProvides(analysis, IInternalUse) + # Set the internal use status + parent_sample = analysis.getRequest() + analysis.setInternalUse(parent_sample.getInternalUse()) # Set the result range to the Analysis analysis_rr = specs.get(service_uid) or analysis.getResultsRange() diff --git a/bika/lims/content/abstractroutineanalysis.py b/bika/lims/content/abstractroutineanalysis.py index ad833d6338..c0310fe536 100644 --- a/bika/lims/content/abstractroutineanalysis.py +++ b/bika/lims/content/abstractroutineanalysis.py @@ -41,9 +41,12 @@ from bika.lims.content.reflexrule import doReflexRuleAction from bika.lims.interfaces import IAnalysis from bika.lims.interfaces import ICancellable +from bika.lims.interfaces import IInternalUse from bika.lims.interfaces import IRoutineAnalysis from bika.lims.interfaces.analysis import IRequestAnalysis from bika.lims.workflow import getTransitionDate +from zope.interface import noLongerProvides +from zope.interface import alsoProvides # True if the analysis is created by a reflex rule IsReflexAnalysis = BooleanField( @@ -451,6 +454,16 @@ def setHidden(self, hidden): self.setHiddenManually(True) self.getField('Hidden').set(self, hidden) + @security.public + def setInternalUse(self, internal_use): + """Applies the internal use of this Analysis. Analyses set for internal + use are not accessible to clients and are not visible in reports + """ + if internal_use: + alsoProvides(self, IInternalUse) + else: + noLongerProvides(self, IInternalUse) + @security.public def setReflexAnalysisOf(self, analysis): """Sets the analysis that has been reflexed in order to create this diff --git a/bika/lims/subscribers/analysisrequest.py b/bika/lims/subscribers/analysisrequest.py index ffdad450ed..04933a24e1 100644 --- a/bika/lims/subscribers/analysisrequest.py +++ b/bika/lims/subscribers/analysisrequest.py @@ -41,12 +41,7 @@ def ObjectModifiedEventHandler(instance, event): # Mark/Unmark all analyses with IInternalUse to control their # visibility in results reports for analysis in instance.objectValues("Analysis"): - if internal_use: - alsoProvides(analysis, IInternalUse) - else: - noLongerProvides(analysis, IInternalUse) - - # Reindex analysis security in catalogs + analysis.setInternalUse(internal_use) analysis.reindexObjectSecurity() # If internal use is True, cascade same setting to partitions @@ -62,6 +57,10 @@ def AfterTransitionEventHandler(instance, event): This function does not superseds workflow.analysisrequest.events, rather it only updates the permissions in accordance with InternalUse value """ + # Permissions for a given object change after transitions to meet with the + # workflow definition. InternalUse prevents Clients to access to Samples + # and analyses as well. Therefore, we have to update the permissions + # manually here to override those set by default update_internal_use_permissions(instance) From 48efff0509444fe495dc1cd2f830cb281fe4ec5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 22 Jan 2020 21:30:18 +0100 Subject: [PATCH 36/63] Apply result ranges to analyses that are not yet submitted only --- bika/lims/content/analysisrequest.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 66963ff231..10a1597091 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -60,9 +60,9 @@ from bika.lims.browser.fields import ARAnalysesField from bika.lims.browser.fields import DateTimeField from bika.lims.browser.fields import DurationField +from bika.lims.browser.fields import ResultsRangesField from bika.lims.browser.fields import UIDReferenceField from bika.lims.browser.fields.remarksfield import RemarksField -from bika.lims.browser.fields import ResultsRangesField from bika.lims.browser.widgets import DateTimeWidget from bika.lims.browser.widgets import DecimalWidget from bika.lims.browser.widgets import PrioritySelectionWidget @@ -85,7 +85,6 @@ from bika.lims.interfaces import ICancellable from bika.lims.interfaces import IClient from bika.lims.interfaces import ISubmitted -from bika.lims.permissions import EditResults from bika.lims.permissions import FieldEditBatch from bika.lims.permissions import FieldEditClient from bika.lims.permissions import FieldEditClientOrderNumber @@ -1473,7 +1472,7 @@ def setResultsRange(self, value, recursive=True): # Set Results Range to analyses for analysis in self.objectValues("Analysis"): - if check_permission(EditResults, analysis): + if not ISubmitted.providedBy(analysis): service_uid = analysis.getRawAnalysisService() result_range = field.get(self, search_by=service_uid) analysis.setResultsRange(result_range) From 50a4bf56a156944ae11d9c6cb7fae0deca86da7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 23 Jan 2020 10:37:14 +0100 Subject: [PATCH 37/63] Viewlet for when analyses with non-compliant ranges --- bika/lims/browser/fields/aranalysesfield.py | 76 +--------------- bika/lims/browser/viewlets/analysisrequest.py | 89 ++++++++++++++++++- bika/lims/browser/viewlets/configure.zcml | 28 +++++- .../specification_non_compliant_viewlet.pt | 37 ++++++++ 4 files changed, 152 insertions(+), 78 deletions(-) create mode 100644 bika/lims/browser/viewlets/templates/specification_non_compliant_viewlet.pt diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index 8c74c0d10c..82c5cd1fb7 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -158,10 +158,9 @@ def set(self, instance, items, prices=None, specs=None, hidden=None, **kw): def resolve_specs(self, instance, results_ranges): """Returns a dictionary where the key is the service_uid and the value - is its results range. If a result range for a given service does not - exist or does not match with the specs defined in the sample, the - function resets the Sample's Specification field to guarantee compliance - with the Specification value actually set + is its results range. The dictionary is made by extending the + results_ranges passed-in with the Sample's ResultsRanges (a copy of the + specifications initially set) """ rrs = results_ranges or [] @@ -175,17 +174,6 @@ def resolve_specs(self, instance, results_ranges): service_uids = map(lambda rr: rr["uid"], rrs) rrs.extend(filter(lambda rr: rr["uid"] not in service_uids, sample_rrs)) - # Do the results ranges passed-in are compliant with Sample's spec? - if not self.is_compliant_with_specification(instance, rrs): - # Reset the specification, we cannot keep a Specification assigned - # to a Sample if there are results ranges set not compliant - instance.setSpecification(None) - - # We store the values in Sample's ResultsRange because although the - # specification is not met, we still want Sample's ResultsRange to - # act as a "template" for new analyses - instance.setResultsRange(rrs) - # Create a dict for easy access to results ranges return dict(map(lambda rr: (rr["uid"], rr), rrs)) @@ -208,64 +196,6 @@ def resolve_uid(self, result_range): value["uid"] = uid return value - def is_compliant_with_specification(self, instance, results_range): - """Returns whether the results_range passed-in are compliant with the - instance's Specification results ranges. This, is the results ranges - for each service match with those from the instance - """ - specification = instance.getSpecification() - if not specification: - # If there is no specification set, assume is compliant - return True - - # Get the results ranges to check against for compliance - sample_rrs = instance.getResultsRange() - sample_rrs = sample_rrs or specification.getResultsRanges() - - # Create a dict for easy access to results ranges - sample_rrs = dict(map(lambda rr: (rr["uid"], rr), sample_rrs)) - - # The Sample has Specification set: the ResultsRange from the sample is - # a copy of those from the Specification. If so, we need to check that - # there is no result range passed-in violating the Spec - for rr in results_range: - service_uid = rr["uid"] - sample_rr = sample_rrs.get(service_uid) - if not sample_rr: - # This service is not defined in Sample's ResultsRange, we - # assume this *does not* break the compliance - continue - - else: - # Clean-up the result range passed in - form_rr = self.resolve_result_range(rr, sample_rr) - if form_rr != sample_rr: - # Result range for this service has been changed manually, - # it does not match with sample's ResultRange - return False - - # No anomalies found, compliant - return True - - def resolve_result_range(self, result_range, original): - """Cleans up the result range passed-in to match with same keys as the - original result range, that presumably comes from a Specification - """ - if not original: - return result_range - - # Remove keys-values not present in original - extra_keys = filter(lambda key: key not in original, result_range) - for key in extra_keys: - del result_range[key] - - # Add keys-values not present in current result_range but in original - for key, val in original.items(): - if key not in result_range: - result_range[key] = val - - return result_range - def add_analysis(self, instance, service, **kwargs): service_uid = api.get_uid(service) diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index ee5ee869ed..53491646c6 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -103,8 +103,9 @@ class DetachedPartitionViewlet(ViewletBase): class ResultsRangesOutOfDateViewlet(ViewletBase): """Print a viewlet that displays if results ranges from Sample are different from results ranges initially set through Specifications field. If so, this - means the Specifications initially set have changed since they were set and - for new analyses, the old specifications will be kept + means the Specification initially set has changed since it was assigned to + the Sample and for new analyses, the ranges defined in the initial + specification ranges will be used instead of the new ones. """ def is_specification_editable(self): @@ -132,3 +133,87 @@ def is_results_ranges_out_of_date(self): # Compare if results ranges are different spec_rr = specifications.getResultsRange() return sample_rr != spec_rr + + +class SpecificationNotCompliantViewlet(ViewletBase): + """Print a viewlet that displays if the sample contains analyses that are + not compliant with the Specification initially set (stored in Sample's + ResultsRange field). If so, this means that user changed the results ranges + of the analyses manually, either by adding new ones or by modifying the + existing ones via "Manage analyses" view. And results range for those + analyses are different from the Specification initially set. + """ + + def is_specification_editable(self): + """Returns whether the Specification field is editable or not + """ + return check_permission(FieldEditSpecification, self.context) + + def get_non_compliant_analyses(self): + """Returns the list of analysis keywords from this sample with a + result range set not compliant with the result range of the Sample + """ + non_compliant = [] + sample = self.context + + # If no Specification is set, assume is compliant + specification = sample.getSpecification() + if not specification: + return [] + + # Get the Sample's ResultsRanges, that are copy of those from the + # Specification initially set. + sample_rrs = sample.getResultsRange() + if not sample_rrs: + # No results range set at Sample level. Assume is compliant + return [] + + # Create a dict for easy access to results ranges + sample_rrs = dict(map(lambda rr: (rr["uid"], rr), sample_rrs)) + + # Check if the results ranges set to analyses individually remain + # compliant with the Sample's ResultRange + analyses = sample.getAnalyses(full_objects=True) + for analysis in analyses: + rr = analysis.getResultsRange() + service_uid = rr.get("uid", None) + if not api.is_uid(service_uid): + continue + + sample_rr = sample_rrs.get(service_uid) + if not sample_rr: + # This service is not defined in Sample's ResultsRange, we + # assume this *does not* break the compliance + continue + + else: + # Clean-up the result range passed in + form_rr = self.resolve_result_range(rr, sample_rr) + if form_rr != sample_rr: + # Result range for this service has been changed manually, + # it does not match with sample's ResultRange + an_title = api.get_title(analysis) + keyword = analysis.getKeyword() + non_compliant.append("{} ({})".format(an_title, keyword)) + + # Return the list of keywords from non-compliant analyses + return list(set(non_compliant)) + + def resolve_result_range(self, result_range, original): + """Cleans up the result range passed-in to match with same keys as the + original result range, that presumably comes from a Specification + """ + if not original: + return result_range + + # Remove keys-values not present in original + extra_keys = filter(lambda key: key not in original, result_range) + for key in extra_keys: + del result_range[key] + + # Add keys-values not present in current result_range but in original + for key, val in original.items(): + if key not in result_range: + result_range[key] = val + + return result_range \ No newline at end of file diff --git a/bika/lims/browser/viewlets/configure.zcml b/bika/lims/browser/viewlets/configure.zcml index 16212d929b..845ab470e8 100644 --- a/bika/lims/browser/viewlets/configure.zcml +++ b/bika/lims/browser/viewlets/configure.zcml @@ -223,7 +223,13 @@ layer="bika.lims.interfaces.IBikaLIMS" /> - + + layer="bika.lims.interfaces.IBikaLIMS" /> + + + + +

+ +
+ +
+ +

+ + Ranges for some analyses are different from the Specification + +

+

+ + The ranges for the following analyses have been manually changed and + they are not compliant with the ranges from the Specification: +   + +
+ + The re-assignment of the Specification will override the ranges of + analyses. + +

+
+
+ From 6d2deef3942e5440d23f745a82cd0d7c6a5af459 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 23 Jan 2020 10:40:10 +0100 Subject: [PATCH 38/63] Fix unexpected keyword argument 'remove_primary_analyses' Traceback (innermost last): Module ZPublisher.Publish, line 138, in publish Module ZPublisher.mapply, line 77, in mapply Module ZPublisher.Publish, line 48, in call_object Module bika.lims.browser.partition_magic, line 100, in __call__ TypeError: create_partition() got an unexpected keyword argument 'remove_primary_analyses' --- bika/lims/browser/partition_magic.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/bika/lims/browser/partition_magic.py b/bika/lims/browser/partition_magic.py index bb829d8829..f55baf715e 100644 --- a/bika/lims/browser/partition_magic.py +++ b/bika/lims/browser/partition_magic.py @@ -86,8 +86,6 @@ def __call__(self): # The creation of partitions w/o analyses is allowed. Maybe the # user wants to add the analyses later manually or wants to keep # this partition stored in a freezer for some time - # Note we set "remove_primary_analyses" to False cause we want - # user to be able to add same analyses to different partitions. analyses_uids = partition.get("analyses", []) partition = create_partition( request=self.request, @@ -96,7 +94,6 @@ def __call__(self): container=container_uid, preservation=preservation_uid, analyses=analyses_uids, - remove_primary_analyses=False, internal_use=internal_use, ) partitions.append(partition) @@ -112,9 +109,6 @@ def __call__(self): # If no partitions were created, show a warning message return self.redirect(message=_("No partitions were created")) - # Remove analyses from primary Analysis Requests - self.remove_primary_analyses() - message = _("Created {} partitions: {}".format( len(partitions), ", ".join(map(api.get_title, partitions)))) return self.redirect(message=message) @@ -133,14 +127,6 @@ def push_primary_analyses_for_removal(self, analysis_request, analyses): to_remove.extend(analyses) self.analyses_to_remove[analysis_request] = list(set(to_remove)) - def remove_primary_analyses(self): - """Remove analyses relocated to partitions - """ - for ar, analyses in self.analyses_to_remove.items(): - analyses_ids = list(set(map(api.get_id, analyses))) - ar.manage_delObjects(analyses_ids) - self.analyses_to_remove = dict() - def get_ar_data(self): """Returns a list of AR data """ From d7aa06b65851cd38f6be4b866683dca472e4fe6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 23 Jan 2020 14:10:35 +0100 Subject: [PATCH 39/63] Better messages --- .../browser/viewlets/templates/primary_ar_viewlet.pt | 5 ++--- .../templates/resultsranges_out_of_date_viewlet.pt | 5 ++--- .../templates/specification_non_compliant_viewlet.pt | 9 ++++----- 3 files changed, 8 insertions(+), 11 deletions(-) diff --git a/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt b/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt index a4bdae30f4..506766bb8e 100644 --- a/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt +++ b/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt @@ -41,9 +41,8 @@

- Changes to any of the following fields will also be applied to the - partitions in which the field is editable, even if they have a - different value already set: + Changes to any of the following fields will override the value in + underlying partitions for which the given field is editable:

diff --git a/bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt b/bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt index 38d63578c2..dfe1b31dcd 100644 --- a/bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt +++ b/bika/lims/browser/viewlets/templates/resultsranges_out_of_date_viewlet.pt @@ -22,9 +22,8 @@

- New ranges won't be applied to neither new nor current analyses. The - reassignment of the Specification will apply latest changes, but will - also override those that have been manually set to analyses. + New ranges won't be applied to neither new nor current analyses. + Re-assign the Specification if you want to apply latest changes.
diff --git a/bika/lims/browser/viewlets/templates/specification_non_compliant_viewlet.pt b/bika/lims/browser/viewlets/templates/specification_non_compliant_viewlet.pt index 79fe5cc62e..d65e48cae9 100644 --- a/bika/lims/browser/viewlets/templates/specification_non_compliant_viewlet.pt +++ b/bika/lims/browser/viewlets/templates/specification_non_compliant_viewlet.pt @@ -23,13 +23,12 @@

The ranges for the following analyses have been manually changed and - they are not compliant with the ranges from the Specification: -   - + they are no longer compliant with the ranges of the Specification: + +
- The re-assignment of the Specification will override the ranges of - analyses. + Re-assign the Specification if you want to restore analysis ranges.

From 40310d1deacd303dca74747143008792dffc768d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 23 Jan 2020 20:11:13 +0100 Subject: [PATCH 40/63] Emphasize analyses with non-compliant ranges in analyses listing --- bika/lims/api/analysis.py | 32 ++++++++++ bika/lims/browser/analyses/view.py | 32 +++++++--- .../analysisrequest/manage_analyses.py | 22 +++++-- bika/lims/browser/viewlets/analysisrequest.py | 64 +++---------------- bika/lims/content/analysisrequest.py | 1 + bika/lims/content/analysisspec.py | 12 ++++ 6 files changed, 93 insertions(+), 70 deletions(-) diff --git a/bika/lims/api/analysis.py b/bika/lims/api/analysis.py index 0d141a90ab..e466e53600 100644 --- a/bika/lims/api/analysis.py +++ b/bika/lims/api/analysis.py @@ -27,6 +27,8 @@ IResultOutOfRange from zope.component._api import getAdapters +from bika.lims.interfaces.analysis import IRequestAnalysis + def is_out_of_range(brain_or_object, result=_marker): """Checks if the result for the analysis passed in is out of range and/or @@ -148,3 +150,33 @@ def get_formatted_interval(results_range, default=_marker): max_bracket = max_operator == 'leq' and ']' or ')' return "{}{};{}{}".format(min_bracket, min_str, max_str, max_bracket) + + +def is_result_range_compliant(analysis): + """Returns whether the result range from the analysis matches with the + result range for the service counterpart defined in the Sample + """ + if not IRequestAnalysis.providedBy(analysis): + return True + + rr = analysis.getResultsRange() + service_uid = rr.get("uid", None) + if not api.is_uid(service_uid): + return True + + # Compare with Sample + sample = analysis.getRequest() + + # If no Specification is set, assume is compliant + specification = sample.getRawSpecification() + if not specification: + return True + + # Compare with the Specification that was initially set to the Sample + sample_rr = sample.getResultsRange(search_by=service_uid) + if not sample_rr: + # This service is not defined in Sample's ResultsRange, we + # assume this *does not* break the compliance + return True + + return rr == sample_rr diff --git a/bika/lims/browser/analyses/view.py b/bika/lims/browser/analyses/view.py index 5a81de8420..89300e8244 100644 --- a/bika/lims/browser/analyses/view.py +++ b/bika/lims/browser/analyses/view.py @@ -25,11 +25,15 @@ from DateTime import DateTime from Products.Archetypes.config import REFERENCE_CATALOG from Products.CMFPlone.utils import safe_unicode +from plone.memoize import view as viewcache +from zope.component import getAdapters + from bika.lims import api from bika.lims import bikaMessageFactory as _ from bika.lims import logger from bika.lims.api.analysis import get_formatted_interval from bika.lims.api.analysis import is_out_of_range +from bika.lims.api.analysis import is_result_range_compliant from bika.lims.browser.bika_listing import BikaListingView from bika.lims.catalog import CATALOG_ANALYSIS_LISTING from bika.lims.config import LDL @@ -51,8 +55,6 @@ from bika.lims.utils import get_link from bika.lims.utils import t from bika.lims.utils.analysis import format_uncertainty -from plone.memoize import view as viewcache -from zope.component import getAdapters class AnalysesView(BikaListingView): @@ -1023,13 +1025,25 @@ def _folder_item_specifications(self, analysis_brain, item): # Show an icon if out of range out_range, out_shoulders = is_out_of_range(analysis_brain) - if not out_range: - return - # At least is out of range - img = get_image("exclamation.png", title=_("Result out of range")) - if not out_shoulders: - img = get_image("warning.png", title=_("Result in shoulder range")) - self._append_html_element(item, "Result", img) + if out_range: + msg = _("Result out of range") + img = get_image("exclamation.png", title=msg) + if not out_shoulders: + msg = _("Result in shoulder range") + img = get_image("warning.png", title=msg) + self._append_html_element(item, "Result", img) + + # Show an icon if the analysis range is different from the Sample spec + if IAnalysisRequest.providedBy(self.context): + analysis = self.get_object(analysis_brain) + if not is_result_range_compliant(analysis): + service_uid = analysis_brain.getServiceUID + original = self.context.getResultsRange(search_by=service_uid) + original = get_formatted_interval(original, "") + msg = _("Result range is different from Specification: {}" + .format(original)) + img = get_image("warning.png", title=msg) + self._append_html_element(item, "Specification", img) def _folder_item_verify_icons(self, analysis_brain, item): """Set the analysis' verification icons to the item passed in. diff --git a/bika/lims/browser/analysisrequest/manage_analyses.py b/bika/lims/browser/analysisrequest/manage_analyses.py index 1db380950c..cca18dafb8 100644 --- a/bika/lims/browser/analysisrequest/manage_analyses.py +++ b/bika/lims/browser/analysisrequest/manage_analyses.py @@ -142,12 +142,24 @@ def show_ar_specs(self): @view.memoize def get_results_range(self): - """Get the results Range from the AR + """Get the results Range from the Sample, but gives priority to the + result ranges set in analyses. This guarantees that result ranges for + already present analyses are not overriden after form submission """ - spec = self.context.getResultsRange() - if spec: - return dicts_to_dict(spec, "keyword") - return ResultsRangeDict() + # Extract the result ranges from Sample analyses + analyses = self.analyses.values() + analyses_rrs = map(lambda an: an.getResultsRange(), analyses) + analyses_rrs = filter(None, analyses_rrs) + rrs = dicts_to_dict(analyses_rrs, "keyword") + + # Bail out ranges from Sample that are already present in analyses + sample_rrs = self.context.getResultsRange() + sample_rrs = filter(lambda rr: rr["keyword"] not in rrs, sample_rrs) + sample_rrs = dicts_to_dict(sample_rrs, "keyword") + + # Extend result ranges with those from Sample + rrs.update(sample_rrs) + return rrs @view.memoize def get_currency_symbol(self): diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index 53491646c6..5b8abbad67 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -24,6 +24,7 @@ from bika.lims import FieldEditSpecification from bika.lims import api from bika.lims import logger +from bika.lims.api.analysis import is_result_range_compliant from bika.lims.api.security import check_permission @@ -154,66 +155,17 @@ def get_non_compliant_analyses(self): result range set not compliant with the result range of the Sample """ non_compliant = [] - sample = self.context - - # If no Specification is set, assume is compliant - specification = sample.getSpecification() - if not specification: - return [] - - # Get the Sample's ResultsRanges, that are copy of those from the - # Specification initially set. - sample_rrs = sample.getResultsRange() - if not sample_rrs: - # No results range set at Sample level. Assume is compliant - return [] - - # Create a dict for easy access to results ranges - sample_rrs = dict(map(lambda rr: (rr["uid"], rr), sample_rrs)) # Check if the results ranges set to analyses individually remain # compliant with the Sample's ResultRange - analyses = sample.getAnalyses(full_objects=True) + analyses = self.context.getAnalyses(full_objects=True) for analysis in analyses: - rr = analysis.getResultsRange() - service_uid = rr.get("uid", None) - if not api.is_uid(service_uid): - continue - - sample_rr = sample_rrs.get(service_uid) - if not sample_rr: - # This service is not defined in Sample's ResultsRange, we - # assume this *does not* break the compliance - continue - - else: - # Clean-up the result range passed in - form_rr = self.resolve_result_range(rr, sample_rr) - if form_rr != sample_rr: - # Result range for this service has been changed manually, - # it does not match with sample's ResultRange - an_title = api.get_title(analysis) - keyword = analysis.getKeyword() - non_compliant.append("{} ({})".format(an_title, keyword)) + if not is_result_range_compliant(analysis): + # Result range for this service has been changed manually, + # it does not match with sample's ResultRange + an_title = api.get_title(analysis) + keyword = analysis.getKeyword() + non_compliant.append("{} ({})".format(an_title, keyword)) # Return the list of keywords from non-compliant analyses return list(set(non_compliant)) - - def resolve_result_range(self, result_range, original): - """Cleans up the result range passed-in to match with same keys as the - original result range, that presumably comes from a Specification - """ - if not original: - return result_range - - # Remove keys-values not present in original - extra_keys = filter(lambda key: key not in original, result_range) - for key in extra_keys: - del result_range[key] - - # Add keys-values not present in current result_range but in original - for key, val in original.items(): - if key not in result_range: - result_range[key] = val - - return result_range \ No newline at end of file diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 10a1597091..8996e5e0b5 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -1476,6 +1476,7 @@ def setResultsRange(self, value, recursive=True): service_uid = analysis.getRawAnalysisService() result_range = field.get(self, search_by=service_uid) analysis.setResultsRange(result_range) + analysis.reindexObject() if recursive: # Cascade the changes to partitions diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index a3ad2d28e9..2efc5adca4 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -187,3 +187,15 @@ def min_operator(self, value): @max_operator.setter def max_operator(self, value): self['max_operator'] = value + + def __eq__(self, other): + if isinstance(other, dict): + other = ResultsRangeDict(other) + + if isinstance(other, ResultsRangeDict): + # Balance both dicts with same keys, but without corrupting them + current = dict(filter(lambda o: o[0] in other, self.items())) + other_dict = dict(filter(lambda o: o[0] in current, other.items())) + return current == other_dict + + return super(ResultsRangeDict, self).__eq__(other) From d08bfeb8745c0082ceaba6d1552f9eea020d781d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 23 Jan 2020 20:28:59 +0100 Subject: [PATCH 41/63] Fix test --- .../SpecificationAndResultsRanges.rst | 66 +++++++------------ 1 file changed, 23 insertions(+), 43 deletions(-) diff --git a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst index a88757044f..26885dc3ff 100644 --- a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst +++ b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst @@ -172,6 +172,9 @@ effect to neither the Sample nor analyses: We need to re-apply the Specification for the Sample's results range to update: >>> sample.setSpecification(specification) + +And the ResultsRange value from Sample is updated accordingly: + >>> specification.getResultsRange() == sample.getResultsRange() True @@ -218,29 +221,28 @@ such analysis, the result range for the new analysis will be set automatically: >>> zn.getResultsRange() == get_results_range_from(specification, Zn) True -But if we reset an Analysis with it's own ResultsRange, different from the -range defined by the Specification, the system will clear the Specification to -guarantee compliance: +If we reset an Analysis with it's own ResultsRange, different from the range +defined by the Specification, the system does not clear the Specification: >>> rr_zn = zn.getResultsRange() >>> rr_zn.min = 55 >>> sample.setAnalyses([Au, Cu, Fe, Zn], specs=[rr_zn]) - >>> sample.getSpecification() is None - True + >>> sample.getSpecification() + -Nevertheless, Sample's ResultsRange is kept unchanged: +and Sample's ResultsRange is kept unchanged: >>> sample_rr = sample.getResultsRange() >>> len(sample_rr) 5 -but with the results range for `Zn` updated, different from the Specification: +with result range for `Zn` unchanged: - >>> sample_rr_zn = filter(lambda rr: rr["uid"] == api.get_uid(Zn), sample_rr)[0] + >>> sample_rr_zn = sample.getResultsRange(search_by=api.get_uid(Zn)) >>> sample_rr_zn.min - 55 + 50 -As well as for the analysis itself: +But analysis' result range has indeed changed: >>> zn.getResultsRange().min 55 @@ -282,51 +284,29 @@ The partition keeps the Specification and ResultsRange by its own: True If we reset an Analysis with it's own ResultsRange, different from the range -defined by the Specification, the system will clear the Specification from -both the root sample and the partition to guarantee compliance: +defined by the Specification, the system does not clear the Specification, +neither from the root sample nor the partition: >>> rr_zn = zn.getResultsRange() >>> rr_zn.min = 56 >>> partition.setAnalyses([Zn], specs=[rr_zn]) - >>> partition.getSpecification() is None - True - -But the root sample will keep its own ResultsRange and Specification untouched: >>> sample.getSpecification() - >>> sample.getResultsRange() == specification.getResultsRange() - True + >>> partition.getSpecification() + -We can re-assign the Specification to the partition, though: +And Results Range from both Sample and partition are kept untouched: - >>> partition.setSpecification(specification) - >>> specification.getResultsRange() == partition.getResultsRange() - True + >>> sample.getSpecification() + - >>> zn.getResultsRange() == get_results_range_from(specification, Zn) + >>> sample.getResultsRange() == specification.getResultsRange() True - >>> zn.getResultsRange().min - 50 - -If we reset the same analysis, but in the root sample, both root and partition -loose the Specification: - - >>> rr_zn = zn.getResultsRange() - >>> rr_zn.min = 57 - >>> sample.setAnalyses([Au, Cu, Fe, Zn], specs=[rr_zn]) - >>> sample.getSpecification() is None - True + >>> partition.getSpecification() + - >>> partition.getSpecification() is None + >>> partition.getResultsRange() == specification.getResultsRange() True - -And ResultsRange for Zn is stored in both the root and the partition: - - >>> get_results_range_from(sample, Zn).min - 57 - - >>> get_results_range_from(partition, Zn).min - 57 From 7b5e91805198203d121769f2f0602f419ea4a850 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 23 Jan 2020 20:45:49 +0100 Subject: [PATCH 42/63] Changelog --- CHANGES.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 6fcc088bda..ba1028bc55 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,6 +7,9 @@ Changelog **Added** +- #1506 Specification non-compliant viewlet in Sample +- #1506 Sample results ranges out-of-date viewlet in Sample +- #1506 Warn icon in analyses when range is not compliant with Specification - #1499 Moved navigation portlet into core - #1498 Moved all viewlets from senaite.lims to senaite.core - #1505 Display partition link in analyses listing @@ -33,6 +36,9 @@ Changelog **Fixed** +- #1506 Changes via manage results don't get applied to partitions +- #1506 Fix recursion error when getting dependencies through Calculation +- #1506 setter from ARAnalysisField does no longer return values - #1505 Manage Analyses Form re-applies partitioned Analyses back to the Root - #1503 Avoid duplicate CSS IDs in multi-column Add form - #1501 Fix Attribute Error in Reference Sample Popup From b7674362a0d97dd14651c97428f96bbf526eeb9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 23 Jan 2020 21:13:03 +0100 Subject: [PATCH 43/63] Remove unnecessary code --- .../widgets/analysisspecificationwidget.py | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-) diff --git a/bika/lims/browser/widgets/analysisspecificationwidget.py b/bika/lims/browser/widgets/analysisspecificationwidget.py index 300fb5190c..9f2a06f540 100644 --- a/bika/lims/browser/widgets/analysisspecificationwidget.py +++ b/bika/lims/browser/widgets/analysisspecificationwidget.py @@ -29,6 +29,7 @@ from bika.lims.config import MAX_OPERATORS from bika.lims.config import MIN_OPERATORS from bika.lims.permissions import FieldEditSpecification +from bika.lims.utils import dicts_to_dict from bika.lims.utils import get_image from bika.lims.utils import get_link from bika.lims.utils import to_choices @@ -134,7 +135,8 @@ def update(self): """ super(AnalysisSpecificationView, self).update() self.allow_edit = self.is_edit_allowed() - self.specification = self.get_results_range_by_keyword() + results_range = self.context.getResultsRange() + self.specification = dicts_to_dict(results_range, "keyword") @view.memoize def is_edit_allowed(self): @@ -148,19 +150,6 @@ def show_categories_enabled(self): """ return self.context.bika_setup.getCategoriseAnalysisServices() - def get_results_range_by_keyword(self): - """Return a dictionary with the specification fields for each - service. The keys of the dictionary are the keywords of each - analysis service. Each service contains a dictionary in which - each key is the name of the spec field: - specs['keyword'] = {'min': value, - 'max': value, - 'warnmin': value, - ... } - """ - results_range = self.context.getResultsRange() - return dict(map(lambda rr: (rr["keyword"], rr), results_range)) - def get_editable_columns(self): """Return editable fields """ From d91f8ac82a34e6ed69b7acd10c30385a86aea2a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Tue, 28 Jan 2020 23:00:06 +0100 Subject: [PATCH 44/63] Sample can have multiple analyses with same Keyword --- bika/lims/browser/analysisspec.py | 7 ++-- bika/lims/browser/fields/aranalysesfield.py | 44 +++++++++++---------- 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/bika/lims/browser/analysisspec.py b/bika/lims/browser/analysisspec.py index 3608e2cd84..59df2b0843 100644 --- a/bika/lims/browser/analysisspec.py +++ b/bika/lims/browser/analysisspec.py @@ -18,12 +18,13 @@ # Copyright 2018-2019 by it's authors. # Some rights reserved, see README and LICENSE. -from bika.lims.config import POINTS_OF_CAPTURE -from bika.lims.interfaces import IAnalysisSpec -from bika.lims.interfaces import IJSONReadExtender from zope.component import adapts from zope.interface import implements +from bika.lims.interfaces import IAnalysisSpec +from bika.lims.interfaces import IJSONReadExtender + + class JSONReadExtender(object): """Adds the UID to the ResultsRange dict. This will go away when we stop using keywords for this stuff. diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index 82c5cd1fb7..32f13d5ce3 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -205,7 +205,7 @@ def add_analysis(self, instance, service, **kwargs): # Get the hidden status for the service hidden = kwargs.get("hidden") or [] hidden = filter(lambda d: d.get("uid") == service_uid, hidden) - hidden = hidden and hidden[0] or service.getHidden() + hidden = hidden and hidden[0].get("hidden") or service.getHidden() # Get the price for the service prices = kwargs.get("prices") or {} @@ -222,6 +222,10 @@ def add_analysis(self, instance, service, **kwargs): analysis = create_analysis(instance, service) analyses.append(analysis) + # Bail out analyses to better not be modified + skip = ["cancelled", "retracted", "rejected"] + analyses = filter(lambda a: api.get_review_status(a) in skip, analyses) + for analysis in analyses: # Set the hidden status analysis.setHidden(hidden) @@ -233,7 +237,7 @@ def add_analysis(self, instance, service, **kwargs): parent_sample = analysis.getRequest() analysis.setInternalUse(parent_sample.getInternalUse()) - # Set the result range to the Analysis + # Set the result range to the analysis analysis_rr = specs.get(service_uid) or analysis.getResultsRange() analysis.setResultsRange(analysis_rr) analysis.reindexObject() @@ -272,18 +276,18 @@ def resolve_analyses(self, instance, service): analyses = [] # Does the analysis exists in this instance already? - analysis = self.get_from_instance(instance, service) - if analysis: - analyses.append(analysis) + instance_analyses = self.get_from_instance(instance, service) + if instance_analyses: + analyses.extend(instance_analyses) # Does the analysis exists in an ancestor? from_ancestor = self.get_from_ancestor(instance, service) - if from_ancestor: + for ancestor_analysis in from_ancestor: # Move the analysis into this instance. The ancestor's # analysis will be masked otherwise - analysis_id = api.get_id(from_ancestor) + analysis_id = api.get_id(ancestor_analysis) logger.info("Analysis {} is from an ancestor".format(analysis_id)) - cp = from_ancestor.aq_parent.manage_cutObjects(analysis_id) + cp = ancestor_analysis.aq_parent.manage_cutObjects(analysis_id) instance.manage_pasteObjects(cp) analyses.append(instance._getOb(analysis_id)) @@ -301,23 +305,23 @@ def get_analyses_from_descendants(self, instance): return analyses def get_from_instance(self, instance, service): - """Returns an analysis for the given service from the instance + """Returns analyses for the given service from the instance """ service_uid = api.get_uid(service) - for analysis in instance.objectValues("Analysis"): - if analysis.getServiceUID() == service_uid: - return analysis - return None + analyses = instance.objectValues("Analysis") + # Filter those analyses with same keyword. Note that a Sample can + # contain more than one analysis with same keyword because of retests + return filter(lambda an: an.getServiceUID() == service_uid, analyses) def get_from_ancestor(self, instance, service): - """Returns an analysis for the given service from ancestors + """Returns analyses for the given service from ancestors """ ancestor = instance.getParentAnalysisRequest() if not ancestor: - return None + return [] - analysis = self.get_from_instance(ancestor, service) - return analysis or self.get_from_ancestor(ancestor, service) + analyses = self.get_from_instance(ancestor, service) + return analyses or self.get_from_ancestor(ancestor, service) def get_from_descendant(self, instance, service): """Returns analyses for the given service from descendants @@ -325,9 +329,9 @@ def get_from_descendant(self, instance, service): analyses = [] for descendant in instance.getDescendants(): # Does the analysis exists in the current descendant? - analysis = self.get_from_instance(descendant, service) - if analysis: - analyses.append(analysis) + descendant_analyses = self.get_from_instance(descendant, service) + if descendant_analyses: + analyses.extend(descendant_analyses) # Search in descendants from current descendant from_descendant = self.get_from_descendant(descendant, service) From 098121798d2a9e2a6059ba406c55000cae6bcbf7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 29 Jan 2020 09:30:12 +0100 Subject: [PATCH 45/63] Skip invalid/cancelled analyses while setting results range --- bika/lims/browser/fields/aranalysesfield.py | 7 ++++--- bika/lims/browser/viewlets/analysisrequest.py | 5 +++++ 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/bika/lims/browser/fields/aranalysesfield.py b/bika/lims/browser/fields/aranalysesfield.py index 32f13d5ce3..49b0cf5acf 100644 --- a/bika/lims/browser/fields/aranalysesfield.py +++ b/bika/lims/browser/fields/aranalysesfield.py @@ -222,11 +222,12 @@ def add_analysis(self, instance, service, **kwargs): analysis = create_analysis(instance, service) analyses.append(analysis) - # Bail out analyses to better not be modified skip = ["cancelled", "retracted", "rejected"] - analyses = filter(lambda a: api.get_review_status(a) in skip, analyses) - for analysis in analyses: + # Skip analyses to better not modify + if api.get_review_status(analysis) in skip: + continue + # Set the hidden status analysis.setHidden(hidden) diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index 6219e6ebe9..a172f04244 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -164,11 +164,16 @@ def get_non_compliant_analyses(self): result range set not compliant with the result range of the Sample """ non_compliant = [] + skip = ["cancelled", "retracted", "rejected"] # Check if the results ranges set to analyses individually remain # compliant with the Sample's ResultRange analyses = self.context.getAnalyses(full_objects=True) for analysis in analyses: + # Skip non-valid/inactive analyses + if api.get_review_status(analysis) in skip: + continue + if not is_result_range_compliant(analysis): # Result range for this service has been changed manually, # it does not match with sample's ResultRange From ede9506638270a5a4c3df38330f94664367d8ebc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 29 Jan 2020 09:31:05 +0100 Subject: [PATCH 46/63] Always store results ranges as str --- bika/lims/content/analysisspec.py | 10 +++++----- bika/lims/upgrade/v01_03_003.py | 16 +++++++++++++++- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index 593cbd5837..4b2f3ed284 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -137,11 +137,11 @@ class ResultsRangeDict(dict): def __init__(self, *arg, **kw): super(ResultsRangeDict, self).__init__(*arg, **kw) self["uid"] = self.uid - self["min"] = self.min - self["max"] = self.max - self["error"] = self.error - self["warn_min"] = self.warn_min - self["warn_max"] = self.warn_max + self["min"] = str(self.min) + self["max"] = str(self.max) + self["error"] = str(self.error) + self["warn_min"] = str(self.warn_min) + self["warn_max"] = str(self.warn_max) self["min_operator"] = self.min_operator self["max_operator"] = self.max_operator diff --git a/bika/lims/upgrade/v01_03_003.py b/bika/lims/upgrade/v01_03_003.py index ce59a2af36..232c9e69bc 100644 --- a/bika/lims/upgrade/v01_03_003.py +++ b/bika/lims/upgrade/v01_03_003.py @@ -284,6 +284,9 @@ def upgrade(tool): setup.runImportStepFromProfile(profile, "controlpanel") add_dexterity_setup_items(portal) + # Reset the results ranges from Specification objects (to include uid) + reset_specifications_ranges(portal) + logger.info("{0} upgraded to version {1}".format(product, version)) return True @@ -569,4 +572,15 @@ def mark_samples_with_partitions(portal): alsoProvides(parent, IAnalysisRequestWithPartitions) logger.info("Marking Samples with partitions [DONE]") - api.search(query, CATALOG_ANALYSIS_REQUEST_LISTING) + + +def reset_specifications_ranges(portal): + """Reset the result ranges to existing Specification objects. Prior + versions were not storing the service uid in the result range + """ + logger.info("Add uids to Specification ranges subfields ...") + specifications = portal.bika_setup.bika_analysisspecs + for specification in specifications.objectValues("AnalysisSpec"): + specification.setResultsRange(specification.getResultsRange()) + logger.info("Add uids to Specification ranges subfields [DONE]") + From ed2db9a98b53134347f1561e606a60863e95c780 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 29 Jan 2020 09:38:10 +0100 Subject: [PATCH 47/63] Omit services not present in sample when checking if specs is out-of-date --- bika/lims/browser/viewlets/analysisrequest.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index a172f04244..3b18d8a45e 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -140,8 +140,13 @@ def is_results_ranges_out_of_date(self): .format(api.get_id(sample))) return False - # Compare if results ranges are different spec_rr = specifications.getResultsRange() + + # Omit services not present in current Sample + services = map(lambda an: an.getServiceUID, sample.getAnalyses()) + sample_rr = filter(lambda rr: rr.uid in services, sample_rr) + spec_rr = filter(lambda rr: rr.uid in services, spec_rr) + return sample_rr != spec_rr From ee1a3ca4e0a9f46a3de313488c7a6c58aacf12d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 29 Jan 2020 12:22:10 +0100 Subject: [PATCH 48/63] Added viewlet in Sample for when dynamic spec is assigned --- bika/lims/browser/viewlets/configure.zcml | 21 +++++++++--- bika/lims/browser/viewlets/dynamic_specs.py | 23 +++++++++++++ .../templates/dynamic_specs_viewlet.pt | 15 ++++++--- .../templates/sample_dynamic_specs_viewlet.pt | 32 +++++++++++++++++++ 4 files changed, 83 insertions(+), 8 deletions(-) create mode 100644 bika/lims/browser/viewlets/templates/sample_dynamic_specs_viewlet.pt diff --git a/bika/lims/browser/viewlets/configure.zcml b/bika/lims/browser/viewlets/configure.zcml index a8baf73a67..7b4dd5bcec 100644 --- a/bika/lims/browser/viewlets/configure.zcml +++ b/bika/lims/browser/viewlets/configure.zcml @@ -278,10 +278,10 @@ layer="bika.lims.interfaces.IBikaLIMS" /> - + + + + diff --git a/bika/lims/browser/viewlets/dynamic_specs.py b/bika/lims/browser/viewlets/dynamic_specs.py index 06c878ebec..df813b5715 100644 --- a/bika/lims/browser/viewlets/dynamic_specs.py +++ b/bika/lims/browser/viewlets/dynamic_specs.py @@ -18,6 +18,7 @@ # Copyright 2018-2020 by it's authors. # Some rights reserved, see README and LICENSE. +from bika.lims import api from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile from plone.app.layout.viewlets import ViewletBase @@ -28,3 +29,25 @@ class DynamicSpecsViewlet(ViewletBase): in the xls file from the Dynamic Specification """ template = ViewPageTemplateFile("templates/dynamic_specs_viewlet.pt") + + +class SampleDynamicSpecsViewlet(ViewletBase): + """Displays an informative message in Sample view when the assigned + specification has a dynamic specification assigned, so ranges set manually + might be overriden by the ranges provided in the xls file from the Dynamic + Specification + """ + template = ViewPageTemplateFile("templates/sample_dynamic_specs_viewlet.pt") + + def get_dynamic_specification(self): + """Returns the dynamic specification assigned to the Sample via + Specification, but only if the current view is manage analyses + """ + if not self.request.getURL().endswith("analyses"): + # Do not display the viewlet if not in manage analyses + return None + + spec = self.context.getSpecification() + if spec: + return spec.getDynamicAnalysisSpec() + return None diff --git a/bika/lims/browser/viewlets/templates/dynamic_specs_viewlet.pt b/bika/lims/browser/viewlets/templates/dynamic_specs_viewlet.pt index 84353a2ae5..4629b47b37 100644 --- a/bika/lims/browser/viewlets/templates/dynamic_specs_viewlet.pt +++ b/bika/lims/browser/viewlets/templates/dynamic_specs_viewlet.pt @@ -10,15 +10,22 @@ - - This Analysis Specification has a Dynamic Specification assigned -

+ + This Analysis Specification has a Dynamic Specification assigned + +

+

Be aware that the ranges provided in the spreadsheet file from the dynamic specification might override the ranges defined in the Specifications list below. - +
+ + Visit the Dynamic Specification for additional information: +   +

diff --git a/bika/lims/browser/viewlets/templates/sample_dynamic_specs_viewlet.pt b/bika/lims/browser/viewlets/templates/sample_dynamic_specs_viewlet.pt new file mode 100644 index 0000000000..9965e354d6 --- /dev/null +++ b/bika/lims/browser/viewlets/templates/sample_dynamic_specs_viewlet.pt @@ -0,0 +1,32 @@ + From b1e8c1f0acea0806887c26609bbc59b9c94a1ae0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 29 Jan 2020 12:25:13 +0100 Subject: [PATCH 49/63] Remove unnecessary code --- bika/lims/browser/analysisspec.py | 46 ----------------------------- bika/lims/browser/analysisspec.zcml | 12 -------- bika/lims/browser/configure.zcml | 1 - 3 files changed, 59 deletions(-) delete mode 100644 bika/lims/browser/analysisspec.py delete mode 100644 bika/lims/browser/analysisspec.zcml diff --git a/bika/lims/browser/analysisspec.py b/bika/lims/browser/analysisspec.py deleted file mode 100644 index 59df2b0843..0000000000 --- a/bika/lims/browser/analysisspec.py +++ /dev/null @@ -1,46 +0,0 @@ -# -*- coding: utf-8 -*- -# -# This file is part of SENAITE.CORE. -# -# SENAITE.CORE is free software: you can redistribute it and/or modify it under -# the terms of the GNU General Public License as published by the Free Software -# Foundation, version 2. -# -# This program is distributed in the hope that it will be useful, but WITHOUT -# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS -# FOR A PARTICULAR PURPOSE. See the GNU General Public License for more -# details. -# -# You should have received a copy of the GNU General Public License along with -# this program; if not, write to the Free Software Foundation, Inc., 51 -# Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. -# -# Copyright 2018-2019 by it's authors. -# Some rights reserved, see README and LICENSE. - -from zope.component import adapts -from zope.interface import implements - -from bika.lims.interfaces import IAnalysisSpec -from bika.lims.interfaces import IJSONReadExtender - - -class JSONReadExtender(object): - """Adds the UID to the ResultsRange dict. This will go away - when we stop using keywords for this stuff. - """ - - implements(IJSONReadExtender) - adapts(IAnalysisSpec) - - def __init__(self, context): - self.context = context - - def __call__(self, request, data): - bsc = self.context.bika_setup_catalog - rr = [] - for i, x in enumerate(data.get("ResultsRange", [])): - keyword = x.get("keyword") - proxies = bsc(portal_type="AnalysisService", getKeyword=keyword) - if proxies: - data['ResultsRange'][i]['uid'] = proxies[0].UID diff --git a/bika/lims/browser/analysisspec.zcml b/bika/lims/browser/analysisspec.zcml deleted file mode 100644 index d3951601f2..0000000000 --- a/bika/lims/browser/analysisspec.zcml +++ /dev/null @@ -1,12 +0,0 @@ - - - - - diff --git a/bika/lims/browser/configure.zcml b/bika/lims/browser/configure.zcml index f7bcd77559..a7eb065517 100644 --- a/bika/lims/browser/configure.zcml +++ b/bika/lims/browser/configure.zcml @@ -11,7 +11,6 @@ - From baac0b2a425397014b3890890f7236957c607f52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Wed, 29 Jan 2020 15:32:54 +0100 Subject: [PATCH 50/63] Fix test --- .../ARAnalysesFieldWithPartitions.rst | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst b/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst index f14ab0c235..fa546262d2 100644 --- a/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst +++ b/bika/lims/tests/doctests/ARAnalysesFieldWithPartitions.rst @@ -109,56 +109,51 @@ get_from_instance When asked for `Fe` when the primary is given, it returns the analysis, cause it lives in the primary: - >>> fe = field.get_from_instance(sample, Fe) + >>> fe = field.get_from_instance(sample, Fe)[0] >>> fe.getServiceUID() == api.get_uid(Fe) True -But when asked for `Cu` when the primary is given, it returns None, cause it +But when asked for `Cu` when the primary is given, it returns empty, cause it lives in the partition: - >>> cu = field.get_from_instance(sample, Cu) - >>> cu is None - True + >>> field.get_from_instance(sample, Cu) + [] While it returns the analysis when the partition is used: - >>> cu = field.get_from_instance(partition, Cu) + >>> cu = field.get_from_instance(partition, Cu)[0] >>> cu.getServiceUID() == api.get_uid(Cu) True -But when asking the partition for `Fe` it returns None, cause it lives in the +But when asking the partition for `Fe` it returns empty, cause it lives in the ancestor: - >>> fe = field.get_from_instance(partition, Fe) - >>> fe is None - True + >>> field.get_from_instance(partition, Fe) + [] get_from_ancestor ................. -When asked for `Fe` to primary, it returns None because there is no ancestor +When asked for `Fe` to primary, it returns empty because there is no ancestor containing `Fe`: - >>> fe = field.get_from_ancestor(sample, Fe) - >>> fe is None - True + >>> field.get_from_ancestor(sample, Fe) + [] But when asked for `Fe` to the partition, it returns the analysis, cause it it lives in an ancestor from the partition: - >>> fe = field.get_from_ancestor(partition, Fe) + >>> fe = field.get_from_ancestor(partition, Fe)[0] >>> fe.getServiceUID() == api.get_uid(Fe) True -If I ask for `Cu`, that lives in the partition, it will return None for both: +If I ask for `Cu`, that lives in the partition, it will return empty for both: - >>> cu = field.get_from_ancestor(sample, Cu) - >>> cu is None - True + >>> field.get_from_ancestor(sample, Cu) + [] - >>> cu = field.get_from_ancestor(partition, Cu) - >>> cu is None - True + >>> field.get_from_ancestor(partition, Cu) + [] get_from_descendant ................... From c78a3cd8414cbd8827b26d02b0be712ec534758f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 30 Jan 2020 18:35:09 +0100 Subject: [PATCH 51/63] Upgrade step --- bika/lims/upgrade/v01_03_003.py | 60 +++++++++++++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/bika/lims/upgrade/v01_03_003.py b/bika/lims/upgrade/v01_03_003.py index 232c9e69bc..8bcb69aa94 100644 --- a/bika/lims/upgrade/v01_03_003.py +++ b/bika/lims/upgrade/v01_03_003.py @@ -37,6 +37,8 @@ from bika.lims.upgrade.utils import UpgradeUtils from Products.Archetypes.config import UID_CATALOG +from bika.lims.utils import dicts_to_dict + version = "1.3.3" # Remember version number in metadata.xml and setup.py profile = "profile-{0}:default".format(product) @@ -285,8 +287,13 @@ def upgrade(tool): add_dexterity_setup_items(portal) # Reset the results ranges from Specification objects (to include uid) + # https://github.com/senaite/senaite.core/pull/1506 reset_specifications_ranges(portal) + # Update the ResultsRange field from Samples and their analyses as needed + # https://github.com/senaite/senaite.core/pull/1506 + update_samples_result_ranges(portal) + logger.info("{0} upgraded to version {1}".format(product, version)) return True @@ -584,3 +591,56 @@ def reset_specifications_ranges(portal): specification.setResultsRange(specification.getResultsRange()) logger.info("Add uids to Specification ranges subfields [DONE]") + +def update_samples_result_ranges(portal): + """Stores the result range field for those samples that have a + specification assigned. In prior versions, getResultsRange was relying + on Specification's ResultsRange + """ + query = dict(portal_type="AnalysisRequest") + brains = api.search(query, CATALOG_ANALYSIS_REQUEST_LISTING) + total = len(brains) + for num, brain in enumerate(brains): + if num and num % 1000 == 0: + logger.info("{}/{} samples processed".format(num, total)) + sample = api.get_object(brain) + + # Check if the ResultsRange field from sample contains values already + ar_range = sample.getResultsRange() + if ar_range: + # This sample has results range already set, probably assigned + # manually through Manage analyses + # Reassign the results range (for uid subfield resolution) + field = sample.getField("ResultsRange") + field.set(sample, ar_range) + + # Store the result range directly to their analyses + update_analyses_results_range(sample) + + # No need to go further + continue + + # Check if the Sample has Specification set + spec_uid = sample.getRawSpecification() + if not spec_uid: + # This sample does not have a specification set, skip + continue + + # Store the specification results range to the Sample + specification = sample.getSpecification() + result_range = specification.getResultsRange() + sample.getField("ResultsRange").set(sample, result_range) + + # Store the result range directly to their analyses + update_analyses_results_range(sample) + + +def update_analyses_results_range(sample): + field = sample.getField("ResultsRange") + for analysis in sample.objectValues("Analysis"): + service_uid = analysis.getRawAnalysisService() + analysis_rr = field.get(sample, search_by=service_uid) + if analysis_rr: + analysis = api.get_object(analysis) + analysis.setResultsRange(analysis_rr) + analysis.reindexObject() From ed6a0047338381eb836bb5b55e2469be04f8e73a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 30 Jan 2020 18:35:41 +0100 Subject: [PATCH 52/63] Bad comment --- bika/lims/content/abstractroutineanalysis.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bika/lims/content/abstractroutineanalysis.py b/bika/lims/content/abstractroutineanalysis.py index ac53d6be47..41fbdd752e 100644 --- a/bika/lims/content/abstractroutineanalysis.py +++ b/bika/lims/content/abstractroutineanalysis.py @@ -338,9 +338,7 @@ def getAnalysisRequestPrintStatus(self): @security.public def getResultsRange(self): - """Returns the valid result range for this routine analysis based on the - results ranges defined in the Analysis Request this routine analysis is - assigned to. + """Returns the valid result range for this routine analysis A routine analysis will be considered out of range if it result falls out of the range defined in "min" and "max". If there are values set for From b36e10f2c31caf022a8ef9b74707ebc4916faf44 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Thu, 30 Jan 2020 18:35:47 +0100 Subject: [PATCH 53/63] Better equals for ResultRangeDict --- bika/lims/content/analysisspec.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index 4b2f3ed284..c792205bc7 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -137,11 +137,11 @@ class ResultsRangeDict(dict): def __init__(self, *arg, **kw): super(ResultsRangeDict, self).__init__(*arg, **kw) self["uid"] = self.uid - self["min"] = str(self.min) - self["max"] = str(self.max) - self["error"] = str(self.error) - self["warn_min"] = str(self.warn_min) - self["warn_max"] = str(self.warn_max) + self["min"] = self.min + self["max"] = self.max + self["error"] = self.error + self["warn_min"] = self.warn_min + self["warn_max"] = self.warn_max self["min_operator"] = self.min_operator self["max_operator"] = self.max_operator @@ -210,7 +210,14 @@ def __eq__(self, other): if isinstance(other, ResultsRangeDict): # Balance both dicts with same keys, but without corrupting them current = dict(filter(lambda o: o[0] in other, self.items())) - other_dict = dict(filter(lambda o: o[0] in current, other.items())) - return current == other_dict + other = dict(filter(lambda o: o[0] in current, other.items())) + + # Ensure that all values are str (sometimes ranges are stored as + # numeric values and sometimes are stored as str) + current = dict(map(lambda o: (o[0], str(o[1])), current.items())) + other = dict(map(lambda o: (o[0], str(o[1])), other.items())) + + # Check if both are equal + return current == other return super(ResultsRangeDict, self).__eq__(other) From 04425c17c93e36f06c89863d197ea9034617f410 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 31 Jan 2020 00:39:40 +0100 Subject: [PATCH 54/63] specifications --> results_ranges Traceback (innermost last): Module ZPublisher.Publish, line 138, in publish Module ZPublisher.mapply, line 77, in mapply Module ZPublisher.Publish, line 48, in call_object Module bika.lims.browser.analysisrequest.add2, line 66, in decorator Module bika.lims.browser.analysisrequest.add2, line 734, in __call__ Module bika.lims.browser.analysisrequest.add2, line 1673, in ajax_submit TypeError: create_analysisrequest() got an unexpected keyword argument 'specifications' --- bika/lims/browser/analysisrequest/add2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bika/lims/browser/analysisrequest/add2.py b/bika/lims/browser/analysisrequest/add2.py index 460398c965..81a26a6320 100644 --- a/bika/lims/browser/analysisrequest/add2.py +++ b/bika/lims/browser/analysisrequest/add2.py @@ -1670,7 +1670,7 @@ def ajax_submit(self): client, self.request, record, - specifications=specifications + results_ranges=specifications ) except (KeyError, RuntimeError) as e: actions.resume() From 91467bcd52d3fa15c9e14500ef5b7bc4e136d39c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 31 Jan 2020 00:43:12 +0100 Subject: [PATCH 55/63] Remove internal imports of ResultsRangeDict --- bika/lims/api/analysis.py | 2 +- .../analysisrequest/manage_analyses.py | 2 +- bika/lims/browser/fields/resultrangefield.py | 93 ++++++++++++++++++- .../lims/browser/fields/resultsrangesfield.py | 3 +- bika/lims/browser/workflow/analysisrequest.py | 2 +- bika/lims/content/analysisspec.py | 91 ------------------ bika/lims/content/duplicateanalysis.py | 2 +- bika/lims/content/referenceanalysis.py | 2 +- 8 files changed, 97 insertions(+), 100 deletions(-) diff --git a/bika/lims/api/analysis.py b/bika/lims/api/analysis.py index e466e53600..5aebec39db 100644 --- a/bika/lims/api/analysis.py +++ b/bika/lims/api/analysis.py @@ -22,7 +22,7 @@ from bika.lims import api from bika.lims.api import _marker from bika.lims.config import MIN_OPERATORS, MAX_OPERATORS -from bika.lims.content.analysisspec import ResultsRangeDict +from bika.lims.browser.fields.resultrangefield import ResultsRangeDict from bika.lims.interfaces import IAnalysis, IReferenceAnalysis, \ IResultOutOfRange from zope.component._api import getAdapters diff --git a/bika/lims/browser/analysisrequest/manage_analyses.py b/bika/lims/browser/analysisrequest/manage_analyses.py index cca18dafb8..c7a9785b1e 100644 --- a/bika/lims/browser/analysisrequest/manage_analyses.py +++ b/bika/lims/browser/analysisrequest/manage_analyses.py @@ -23,7 +23,7 @@ from bika.lims import api from bika.lims import bikaMessageFactory as _ from bika.lims.browser.bika_listing import BikaListingView -from bika.lims.content.analysisspec import ResultsRangeDict +from bika.lims.browser.fields.resultrangefield import ResultsRangeDict from bika.lims.interfaces import ISubmitted from bika.lims.utils import dicts_to_dict from bika.lims.utils import get_image diff --git a/bika/lims/browser/fields/resultrangefield.py b/bika/lims/browser/fields/resultrangefield.py index 446d2b317b..f1d378ab71 100644 --- a/bika/lims/browser/fields/resultrangefield.py +++ b/bika/lims/browser/fields/resultrangefield.py @@ -55,7 +55,6 @@ class ResultRangeField(RecordField): }) def set(self, instance, value, **kwargs): - from bika.lims.content.analysisspec import ResultsRangeDict if isinstance(value, ResultsRangeDict): # Better store a built-in dict so it will always be available even # if ResultsRangeDict is removed or changed @@ -64,7 +63,6 @@ def set(self, instance, value, **kwargs): super(ResultRangeField, self).set(instance, value, **kwargs) def get(self, instance, **kwargs): - from bika.lims.content.analysisspec import ResultsRangeDict value = super(ResultRangeField, self).get(instance, **kwargs) if value: return ResultsRangeDict(dict(value.items())) @@ -108,3 +106,94 @@ def __call__(self): # Try with uid (this shouldn't be necessary) service_uid = analysis.getServiceUID() return field.get(sample, search_by=service_uid) or {} + + +class ResultsRangeDict(dict): + + def __init__(self, *arg, **kw): + super(ResultsRangeDict, self).__init__(*arg, **kw) + self["uid"] = self.uid + self["min"] = self.min + self["max"] = self.max + self["error"] = self.error + self["warn_min"] = self.warn_min + self["warn_max"] = self.warn_max + self["min_operator"] = self.min_operator + self["max_operator"] = self.max_operator + + @property + def uid(self): + """The uid of the service this ResultsRange refers to + """ + return self.get("uid", '') + + @property + def min(self): + return self.get("min", '') + + @property + def max(self): + return self.get("max", '') + + @property + def error(self): + return self.get("error", '') + + @property + def warn_min(self): + return self.get("warn_min", self.min) + + @property + def warn_max(self): + return self.get('warn_max', self.max) + + @property + def min_operator(self): + return self.get('min_operator', 'geq') + + @property + def max_operator(self): + return self.get('max_operator', 'leq') + + @min.setter + def min(self, value): + self["min"] = value + + @max.setter + def max(self, value): + self["max"] = value + + @warn_min.setter + def warn_min(self, value): + self['warn_min'] = value + + @warn_max.setter + def warn_max(self, value): + self['warn_max'] = value + + @min_operator.setter + def min_operator(self, value): + self['min_operator'] = value + + @max_operator.setter + def max_operator(self, value): + self['max_operator'] = value + + def __eq__(self, other): + if isinstance(other, dict): + other = ResultsRangeDict(other) + + if isinstance(other, ResultsRangeDict): + # Balance both dicts with same keys, but without corrupting them + current = dict(filter(lambda o: o[0] in other, self.items())) + other = dict(filter(lambda o: o[0] in current, other.items())) + + # Ensure that all values are str (sometimes ranges are stored as + # numeric values and sometimes are stored as str) + current = dict(map(lambda o: (o[0], str(o[1])), current.items())) + other = dict(map(lambda o: (o[0], str(o[1])), other.items())) + + # Check if both are equal + return current == other + + return super(ResultsRangeDict, self).__eq__(other) \ No newline at end of file diff --git a/bika/lims/browser/fields/resultsrangesfield.py b/bika/lims/browser/fields/resultsrangesfield.py index 3e429274f4..704cc9153b 100644 --- a/bika/lims/browser/fields/resultsrangesfield.py +++ b/bika/lims/browser/fields/resultsrangesfield.py @@ -24,6 +24,7 @@ from Products.Archetypes.Registry import registerField from bika.lims import api +from bika.lims.browser.fields.resultrangefield import ResultsRangeDict from bika.lims.browser.fields.resultrangefield import SUB_FIELDS from bika.lims.browser.widgets import AnalysisSpecificationWidget from bika.lims.catalog import SETUP_CATALOG @@ -57,7 +58,6 @@ def get(self, instance, **kwargs): return {} # Convert the dict items to ResultRangeDict for easy handling - from bika.lims.content.analysisspec import ResultsRangeDict return map(lambda val: ResultsRangeDict(dict(val.items())), values) def getResultRange(self, values, uid_keyword_service): @@ -73,7 +73,6 @@ def getResultRange(self, values, uid_keyword_service): key = "uid" # Find out the item for the given uid/keyword - from bika.lims.content.analysisspec import ResultsRangeDict value = filter(lambda v: v.get(key) == uid_keyword_service, values) return value and ResultsRangeDict(dict(value[0].items())) or None diff --git a/bika/lims/browser/workflow/analysisrequest.py b/bika/lims/browser/workflow/analysisrequest.py index a4d7156b61..b1952e3585 100644 --- a/bika/lims/browser/workflow/analysisrequest.py +++ b/bika/lims/browser/workflow/analysisrequest.py @@ -28,7 +28,7 @@ from bika.lims import logger from bika.lims.browser.workflow import RequestContextAware from bika.lims.browser.workflow import WorkflowActionGenericAdapter -from bika.lims.content.analysisspec import ResultsRangeDict +from bika.lims.browser.fields.resultrangefield import ResultsRangeDict from bika.lims.interfaces import IAnalysisRequest from bika.lims.interfaces import IWorkflowActionUIDsAdapter from bika.lims.utils import encode_header diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index c792205bc7..bb74703e4c 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -130,94 +130,3 @@ def contextual_title(self): atapi.registerType(AnalysisSpec, PROJECTNAME) - - -class ResultsRangeDict(dict): - - def __init__(self, *arg, **kw): - super(ResultsRangeDict, self).__init__(*arg, **kw) - self["uid"] = self.uid - self["min"] = self.min - self["max"] = self.max - self["error"] = self.error - self["warn_min"] = self.warn_min - self["warn_max"] = self.warn_max - self["min_operator"] = self.min_operator - self["max_operator"] = self.max_operator - - @property - def uid(self): - """The uid of the service this ResultsRange refers to - """ - return self.get("uid", '') - - @property - def min(self): - return self.get("min", '') - - @property - def max(self): - return self.get("max", '') - - @property - def error(self): - return self.get("error", '') - - @property - def warn_min(self): - return self.get("warn_min", self.min) - - @property - def warn_max(self): - return self.get('warn_max', self.max) - - @property - def min_operator(self): - return self.get('min_operator', 'geq') - - @property - def max_operator(self): - return self.get('max_operator', 'leq') - - @min.setter - def min(self, value): - self["min"] = value - - @max.setter - def max(self, value): - self["max"] = value - - @warn_min.setter - def warn_min(self, value): - self['warn_min'] = value - - @warn_max.setter - def warn_max(self, value): - self['warn_max'] = value - - @min_operator.setter - def min_operator(self, value): - self['min_operator'] = value - - @max_operator.setter - def max_operator(self, value): - self['max_operator'] = value - - def __eq__(self, other): - if isinstance(other, dict): - other = ResultsRangeDict(other) - - if isinstance(other, ResultsRangeDict): - # Balance both dicts with same keys, but without corrupting them - current = dict(filter(lambda o: o[0] in other, self.items())) - other = dict(filter(lambda o: o[0] in current, other.items())) - - # Ensure that all values are str (sometimes ranges are stored as - # numeric values and sometimes are stored as str) - current = dict(map(lambda o: (o[0], str(o[1])), current.items())) - other = dict(map(lambda o: (o[0], str(o[1])), other.items())) - - # Check if both are equal - return current == other - - return super(ResultsRangeDict, self).__eq__(other) diff --git a/bika/lims/content/duplicateanalysis.py b/bika/lims/content/duplicateanalysis.py index 97467e6e47..85f5f9d80b 100644 --- a/bika/lims/content/duplicateanalysis.py +++ b/bika/lims/content/duplicateanalysis.py @@ -26,7 +26,7 @@ from bika.lims.config import PROJECTNAME from bika.lims.content.abstractroutineanalysis import AbstractRoutineAnalysis from bika.lims.content.abstractroutineanalysis import schema -from bika.lims.content.analysisspec import ResultsRangeDict +from bika.lims.browser.fields.resultrangefield import ResultsRangeDict from bika.lims.interfaces import IDuplicateAnalysis from bika.lims.interfaces.analysis import IRequestAnalysis from bika.lims.workflow import in_state diff --git a/bika/lims/content/referenceanalysis.py b/bika/lims/content/referenceanalysis.py index a2443455da..d900f19fbb 100644 --- a/bika/lims/content/referenceanalysis.py +++ b/bika/lims/content/referenceanalysis.py @@ -31,7 +31,7 @@ from bika.lims.config import STD_TYPES from bika.lims.content.abstractanalysis import AbstractAnalysis from bika.lims.content.abstractanalysis import schema -from bika.lims.content.analysisspec import ResultsRangeDict +from bika.lims.browser.fields.resultrangefield import ResultsRangeDict from bika.lims.interfaces import IReferenceAnalysis schema = schema.copy() + Schema(( From c9b239f69fbff85d3ca9cc20c14dd37024554706 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 31 Jan 2020 01:11:27 +0100 Subject: [PATCH 56/63] Non-supported "error" subfield is imported in specs through setup data --- bika/lims/exportimport/setupdata/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/bika/lims/exportimport/setupdata/__init__.py b/bika/lims/exportimport/setupdata/__init__.py index 75bb1717c2..853e6d8ab1 100644 --- a/bika/lims/exportimport/setupdata/__init__.py +++ b/bika/lims/exportimport/setupdata/__init__.py @@ -1718,7 +1718,6 @@ def Import(self): "keyword": service.getKeyword(), "min": row["min"] if row["min"] else "0", "max": row["max"] if row["max"] else "0", - "error": row["error"] if row["error"] else "0" }) # write objects. for parent in bucket.keys(): From 656c9b188320f357c8c534e5f17e3ee961a5595c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 31 Jan 2020 09:32:09 +0100 Subject: [PATCH 57/63] Revert "Remove internal imports of ResultsRangeDict" This reverts commit 91467bcd52d3fa15c9e14500ef5b7bc4e136d39c. --- bika/lims/api/analysis.py | 2 +- .../analysisrequest/manage_analyses.py | 2 +- bika/lims/browser/fields/resultrangefield.py | 93 +------------------ .../lims/browser/fields/resultsrangesfield.py | 3 +- bika/lims/browser/workflow/analysisrequest.py | 2 +- bika/lims/content/analysisspec.py | 91 ++++++++++++++++++ bika/lims/content/duplicateanalysis.py | 2 +- bika/lims/content/referenceanalysis.py | 2 +- 8 files changed, 100 insertions(+), 97 deletions(-) diff --git a/bika/lims/api/analysis.py b/bika/lims/api/analysis.py index 5aebec39db..e466e53600 100644 --- a/bika/lims/api/analysis.py +++ b/bika/lims/api/analysis.py @@ -22,7 +22,7 @@ from bika.lims import api from bika.lims.api import _marker from bika.lims.config import MIN_OPERATORS, MAX_OPERATORS -from bika.lims.browser.fields.resultrangefield import ResultsRangeDict +from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.interfaces import IAnalysis, IReferenceAnalysis, \ IResultOutOfRange from zope.component._api import getAdapters diff --git a/bika/lims/browser/analysisrequest/manage_analyses.py b/bika/lims/browser/analysisrequest/manage_analyses.py index c7a9785b1e..cca18dafb8 100644 --- a/bika/lims/browser/analysisrequest/manage_analyses.py +++ b/bika/lims/browser/analysisrequest/manage_analyses.py @@ -23,7 +23,7 @@ from bika.lims import api from bika.lims import bikaMessageFactory as _ from bika.lims.browser.bika_listing import BikaListingView -from bika.lims.browser.fields.resultrangefield import ResultsRangeDict +from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.interfaces import ISubmitted from bika.lims.utils import dicts_to_dict from bika.lims.utils import get_image diff --git a/bika/lims/browser/fields/resultrangefield.py b/bika/lims/browser/fields/resultrangefield.py index f1d378ab71..446d2b317b 100644 --- a/bika/lims/browser/fields/resultrangefield.py +++ b/bika/lims/browser/fields/resultrangefield.py @@ -55,6 +55,7 @@ class ResultRangeField(RecordField): }) def set(self, instance, value, **kwargs): + from bika.lims.content.analysisspec import ResultsRangeDict if isinstance(value, ResultsRangeDict): # Better store a built-in dict so it will always be available even # if ResultsRangeDict is removed or changed @@ -63,6 +64,7 @@ def set(self, instance, value, **kwargs): super(ResultRangeField, self).set(instance, value, **kwargs) def get(self, instance, **kwargs): + from bika.lims.content.analysisspec import ResultsRangeDict value = super(ResultRangeField, self).get(instance, **kwargs) if value: return ResultsRangeDict(dict(value.items())) @@ -106,94 +108,3 @@ def __call__(self): # Try with uid (this shouldn't be necessary) service_uid = analysis.getServiceUID() return field.get(sample, search_by=service_uid) or {} - - -class ResultsRangeDict(dict): - - def __init__(self, *arg, **kw): - super(ResultsRangeDict, self).__init__(*arg, **kw) - self["uid"] = self.uid - self["min"] = self.min - self["max"] = self.max - self["error"] = self.error - self["warn_min"] = self.warn_min - self["warn_max"] = self.warn_max - self["min_operator"] = self.min_operator - self["max_operator"] = self.max_operator - - @property - def uid(self): - """The uid of the service this ResultsRange refers to - """ - return self.get("uid", '') - - @property - def min(self): - return self.get("min", '') - - @property - def max(self): - return self.get("max", '') - - @property - def error(self): - return self.get("error", '') - - @property - def warn_min(self): - return self.get("warn_min", self.min) - - @property - def warn_max(self): - return self.get('warn_max', self.max) - - @property - def min_operator(self): - return self.get('min_operator', 'geq') - - @property - def max_operator(self): - return self.get('max_operator', 'leq') - - @min.setter - def min(self, value): - self["min"] = value - - @max.setter - def max(self, value): - self["max"] = value - - @warn_min.setter - def warn_min(self, value): - self['warn_min'] = value - - @warn_max.setter - def warn_max(self, value): - self['warn_max'] = value - - @min_operator.setter - def min_operator(self, value): - self['min_operator'] = value - - @max_operator.setter - def max_operator(self, value): - self['max_operator'] = value - - def __eq__(self, other): - if isinstance(other, dict): - other = ResultsRangeDict(other) - - if isinstance(other, ResultsRangeDict): - # Balance both dicts with same keys, but without corrupting them - current = dict(filter(lambda o: o[0] in other, self.items())) - other = dict(filter(lambda o: o[0] in current, other.items())) - - # Ensure that all values are str (sometimes ranges are stored as - # numeric values and sometimes are stored as str) - current = dict(map(lambda o: (o[0], str(o[1])), current.items())) - other = dict(map(lambda o: (o[0], str(o[1])), other.items())) - - # Check if both are equal - return current == other - - return super(ResultsRangeDict, self).__eq__(other) \ No newline at end of file diff --git a/bika/lims/browser/fields/resultsrangesfield.py b/bika/lims/browser/fields/resultsrangesfield.py index 704cc9153b..3e429274f4 100644 --- a/bika/lims/browser/fields/resultsrangesfield.py +++ b/bika/lims/browser/fields/resultsrangesfield.py @@ -24,7 +24,6 @@ from Products.Archetypes.Registry import registerField from bika.lims import api -from bika.lims.browser.fields.resultrangefield import ResultsRangeDict from bika.lims.browser.fields.resultrangefield import SUB_FIELDS from bika.lims.browser.widgets import AnalysisSpecificationWidget from bika.lims.catalog import SETUP_CATALOG @@ -58,6 +57,7 @@ def get(self, instance, **kwargs): return {} # Convert the dict items to ResultRangeDict for easy handling + from bika.lims.content.analysisspec import ResultsRangeDict return map(lambda val: ResultsRangeDict(dict(val.items())), values) def getResultRange(self, values, uid_keyword_service): @@ -73,6 +73,7 @@ def getResultRange(self, values, uid_keyword_service): key = "uid" # Find out the item for the given uid/keyword + from bika.lims.content.analysisspec import ResultsRangeDict value = filter(lambda v: v.get(key) == uid_keyword_service, values) return value and ResultsRangeDict(dict(value[0].items())) or None diff --git a/bika/lims/browser/workflow/analysisrequest.py b/bika/lims/browser/workflow/analysisrequest.py index b1952e3585..a4d7156b61 100644 --- a/bika/lims/browser/workflow/analysisrequest.py +++ b/bika/lims/browser/workflow/analysisrequest.py @@ -28,7 +28,7 @@ from bika.lims import logger from bika.lims.browser.workflow import RequestContextAware from bika.lims.browser.workflow import WorkflowActionGenericAdapter -from bika.lims.browser.fields.resultrangefield import ResultsRangeDict +from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.interfaces import IAnalysisRequest from bika.lims.interfaces import IWorkflowActionUIDsAdapter from bika.lims.utils import encode_header diff --git a/bika/lims/content/analysisspec.py b/bika/lims/content/analysisspec.py index bb74703e4c..c792205bc7 100644 --- a/bika/lims/content/analysisspec.py +++ b/bika/lims/content/analysisspec.py @@ -130,3 +130,94 @@ def contextual_title(self): atapi.registerType(AnalysisSpec, PROJECTNAME) + + +class ResultsRangeDict(dict): + + def __init__(self, *arg, **kw): + super(ResultsRangeDict, self).__init__(*arg, **kw) + self["uid"] = self.uid + self["min"] = self.min + self["max"] = self.max + self["error"] = self.error + self["warn_min"] = self.warn_min + self["warn_max"] = self.warn_max + self["min_operator"] = self.min_operator + self["max_operator"] = self.max_operator + + @property + def uid(self): + """The uid of the service this ResultsRange refers to + """ + return self.get("uid", '') + + @property + def min(self): + return self.get("min", '') + + @property + def max(self): + return self.get("max", '') + + @property + def error(self): + return self.get("error", '') + + @property + def warn_min(self): + return self.get("warn_min", self.min) + + @property + def warn_max(self): + return self.get('warn_max', self.max) + + @property + def min_operator(self): + return self.get('min_operator', 'geq') + + @property + def max_operator(self): + return self.get('max_operator', 'leq') + + @min.setter + def min(self, value): + self["min"] = value + + @max.setter + def max(self, value): + self["max"] = value + + @warn_min.setter + def warn_min(self, value): + self['warn_min'] = value + + @warn_max.setter + def warn_max(self, value): + self['warn_max'] = value + + @min_operator.setter + def min_operator(self, value): + self['min_operator'] = value + + @max_operator.setter + def max_operator(self, value): + self['max_operator'] = value + + def __eq__(self, other): + if isinstance(other, dict): + other = ResultsRangeDict(other) + + if isinstance(other, ResultsRangeDict): + # Balance both dicts with same keys, but without corrupting them + current = dict(filter(lambda o: o[0] in other, self.items())) + other = dict(filter(lambda o: o[0] in current, other.items())) + + # Ensure that all values are str (sometimes ranges are stored as + # numeric values and sometimes are stored as str) + current = dict(map(lambda o: (o[0], str(o[1])), current.items())) + other = dict(map(lambda o: (o[0], str(o[1])), other.items())) + + # Check if both are equal + return current == other + + return super(ResultsRangeDict, self).__eq__(other) diff --git a/bika/lims/content/duplicateanalysis.py b/bika/lims/content/duplicateanalysis.py index 85f5f9d80b..97467e6e47 100644 --- a/bika/lims/content/duplicateanalysis.py +++ b/bika/lims/content/duplicateanalysis.py @@ -26,7 +26,7 @@ from bika.lims.config import PROJECTNAME from bika.lims.content.abstractroutineanalysis import AbstractRoutineAnalysis from bika.lims.content.abstractroutineanalysis import schema -from bika.lims.browser.fields.resultrangefield import ResultsRangeDict +from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.interfaces import IDuplicateAnalysis from bika.lims.interfaces.analysis import IRequestAnalysis from bika.lims.workflow import in_state diff --git a/bika/lims/content/referenceanalysis.py b/bika/lims/content/referenceanalysis.py index d900f19fbb..a2443455da 100644 --- a/bika/lims/content/referenceanalysis.py +++ b/bika/lims/content/referenceanalysis.py @@ -31,7 +31,7 @@ from bika.lims.config import STD_TYPES from bika.lims.content.abstractanalysis import AbstractAnalysis from bika.lims.content.abstractanalysis import schema -from bika.lims.browser.fields.resultrangefield import ResultsRangeDict +from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.interfaces import IReferenceAnalysis schema = schema.copy() + Schema(( From 98df11af206d11bcb42591d16ba3294552fb219b Mon Sep 17 00:00:00 2001 From: Ramon Bartl Date: Fri, 31 Jan 2020 15:10:30 +0100 Subject: [PATCH 58/63] Removed "primary bound" viewlet This change displays an icon in the header table next to the fields that have the `primary_bound` set to `True`. --- bika/lims/browser/header_table.py | 7 ++++ bika/lims/browser/templates/header_table.pt | 32 +++++++++++++++++-- bika/lims/browser/viewlets/analysisrequest.py | 21 ------------ .../viewlets/templates/primary_ar_viewlet.pt | 21 +----------- bika/lims/content/analysisrequest.py | 2 +- 5 files changed, 38 insertions(+), 45 deletions(-) diff --git a/bika/lims/browser/header_table.py b/bika/lims/browser/header_table.py index da66866834..177ba54902 100644 --- a/bika/lims/browser/header_table.py +++ b/bika/lims/browser/header_table.py @@ -24,6 +24,7 @@ from bika.lims import logger from bika.lims.api.security import check_permission from bika.lims.browser import BrowserView +from bika.lims.interfaces import IAnalysisRequestWithPartitions from bika.lims.interfaces import IHeaderTableFieldRenderer from bika.lims.utils import t from plone.memoize import view as viewcache @@ -82,6 +83,12 @@ def __call__(self): self.context.plone_utils.addPortalMessage(message, "info") return self.template() + @viewcache.memoize + def is_primary_with_partitions(self): + """Check if the Sample is a primary with partitions + """ + return IAnalysisRequestWithPartitions.providedBy(self.context) + @viewcache.memoize def is_edit_allowed(self): """Check permission 'ModifyPortalContent' on the context diff --git a/bika/lims/browser/templates/header_table.pt b/bika/lims/browser/templates/header_table.pt index 22f48884db..d33e5ff89f 100644 --- a/bika/lims/browser/templates/header_table.pt +++ b/bika/lims/browser/templates/header_table.pt @@ -12,7 +12,8 @@ @@ -21,22 +22,34 @@ + + @@ -59,17 +73,28 @@ fieldName python:action['fieldName']; field python:context.Schema()[fieldName]; field_macro here/widgets/field/macros/edit; + editable python:mode=='edit'; + primary_bound python:getattr(field, 'primary_bound', False); widget python:field.widget; accessor python:field.getAccessor(context); errors python:{};"> diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index 3b18d8a45e..241c458d8a 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -63,27 +63,6 @@ def get_partitions(self): return partitions - def get_primary_bound_fields(self): - """Returns a list with the names of the fields the current user has - write priveleges and for which changes in primary sample will apply to - its descendants too - """ - bound = [] - schema = api.get_schema(self.context) - for field in schema.fields(): - if not hasattr(field, "primary_bound"): - continue - - if not check_permission(field.write_permission, self.context): - # Current user does not have permission to edit this field - continue - - if field.primary_bound: - # Change in this field will populate to partitions - bound.append(field.widget.label) - - return bound - class PartitionAnalysisRequestViewlet(ViewletBase): """ Current Analysis Request is a partition. Display the link to primary diff --git a/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt b/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt index 506766bb8e..7fb5df08c1 100644 --- a/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt +++ b/bika/lims/browser/viewlets/templates/primary_ar_viewlet.pt @@ -5,8 +5,7 @@
-
+
-
- -

- - Some changes might override partitions - -

-

- - Changes to any of the following fields will override the value in - underlying partitions for which the given field is editable: - - -

-
diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 90a37ee1a9..1c29c3b9fd 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -673,7 +673,7 @@ ReferenceField( 'Specification', required=0, - primary_bound=True, + primary_bound=True, # field changes propagate to partitions allowed_types='AnalysisSpec', relationship='AnalysisRequestAnalysisSpec', mode="rw", From 7b938f2637370e5972fd8047efc407e5ce9ad050 Mon Sep 17 00:00:00 2001 From: Ramon Bartl Date: Fri, 31 Jan 2020 15:14:58 +0100 Subject: [PATCH 59/63] Commit changes during the migration step to free some memory --- bika/lims/upgrade/v01_03_003.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/bika/lims/upgrade/v01_03_003.py b/bika/lims/upgrade/v01_03_003.py index 8bcb69aa94..8b9e32b21d 100644 --- a/bika/lims/upgrade/v01_03_003.py +++ b/bika/lims/upgrade/v01_03_003.py @@ -21,23 +21,21 @@ from collections import defaultdict from operator import itemgetter -from zope.interface import alsoProvides - +import transaction from bika.lims import api from bika.lims import logger from bika.lims.catalog import CATALOG_ANALYSIS_REQUEST_LISTING from bika.lims.catalog.bikasetup_catalog import SETUP_CATALOG from bika.lims.config import PROJECTNAME as product from bika.lims.interfaces import IAnalysisRequestWithPartitions -from bika.lims.setuphandlers import add_dexterity_setup_items from bika.lims.interfaces import ISubmitted from bika.lims.interfaces import IVerified +from bika.lims.setuphandlers import add_dexterity_setup_items from bika.lims.setuphandlers import setup_form_controller_actions from bika.lims.upgrade import upgradestep from bika.lims.upgrade.utils import UpgradeUtils from Products.Archetypes.config import UID_CATALOG - -from bika.lims.utils import dicts_to_dict +from zope.interface import alsoProvides version = "1.3.3" # Remember version number in metadata.xml and setup.py profile = "profile-{0}:default".format(product) @@ -570,6 +568,7 @@ def mark_samples_with_partitions(portal): if num and num % 100 == 0: logger.info("Marking samples with partitions: {}/{}" .format(num, total)) + transaction.commit() part = api.get_object(brain) parent = part.getParentAnalysisRequest() if not parent: @@ -602,7 +601,9 @@ def update_samples_result_ranges(portal): total = len(brains) for num, brain in enumerate(brains): if num and num % 1000 == 0: - logger.info("{}/{} samples processed".format(num, total)) + logger.info("{}/{} samples processed ...".format(num, total)) + transaction.commit() + logger.info("Changes commited") sample = api.get_object(brain) # Check if the ResultsRange field from sample contains values already From 7d94ebbfa0f167aff4a741ac2aced4ff8cf55531 Mon Sep 17 00:00:00 2001 From: Ramon Bartl Date: Fri, 31 Jan 2020 15:49:23 +0100 Subject: [PATCH 60/63] Minor cleanup and PEP8 --- bika/lims/browser/viewlets/dynamic_specs.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bika/lims/browser/viewlets/dynamic_specs.py b/bika/lims/browser/viewlets/dynamic_specs.py index df813b5715..72d252fc19 100644 --- a/bika/lims/browser/viewlets/dynamic_specs.py +++ b/bika/lims/browser/viewlets/dynamic_specs.py @@ -18,9 +18,8 @@ # Copyright 2018-2020 by it's authors. # Some rights reserved, see README and LICENSE. -from bika.lims import api -from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile from plone.app.layout.viewlets import ViewletBase +from Products.Five.browser.pagetemplatefile import ViewPageTemplateFile class DynamicSpecsViewlet(ViewletBase): @@ -37,7 +36,8 @@ class SampleDynamicSpecsViewlet(ViewletBase): might be overriden by the ranges provided in the xls file from the Dynamic Specification """ - template = ViewPageTemplateFile("templates/sample_dynamic_specs_viewlet.pt") + template = ViewPageTemplateFile( + "templates/sample_dynamic_specs_viewlet.pt") def get_dynamic_specification(self): """Returns the dynamic specification assigned to the Sample via From a8ed74f950c0d537e21135413a12dff49afa5e16 Mon Sep 17 00:00:00 2001 From: Ramon Bartl Date: Fri, 31 Jan 2020 16:45:02 +0100 Subject: [PATCH 61/63] Preserve the sample specification if no changes are done --- bika/lims/content/analysisrequest.py | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index 1c29c3b9fd..b694bb4ef2 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -1438,19 +1438,26 @@ def Description(self): descr = " ".join((self.getId(), self.aq_parent.Title())) return safe_unicode(descr).encode('utf-8') - def setSpecification(self, value): + def setSpecification(self, value, override=False): """Sets the Specifications and ResultRange values """ + current_spec = self.getRawSpecification() + changed = current_spec != value + + if changed and not override: + # preserve the current value + return + self.getField("Specification").set(self, value) - # Set the value for field ResultsRange, cause Specification is only used - # as a template: all the results range logic relies on ResultsRange - # field, so changes in setup's Specification object won't have effect to - # already created samples + # Set the value for field ResultsRange, cause Specification is only + # used as a template: all the results range logic relies on + # ResultsRange field, so changes in setup's Specification object won't + # have effect to already created samples spec = self.getSpecification() if spec: - # Update only results ranges if specs is not None, so results ranges - # manually set previously (e.g. via ManageAnalyses view) are + # Update only results ranges if specs is not None, so results + # ranges manually set previously (e.g. via ManageAnalyses view) are # preserved unless a new Specification overrides them self.setResultsRange(spec.getResultsRange(), recursive=False) From ae4c87b4dc30307e9bf967ae5129fc8f28c86ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 31 Jan 2020 17:16:29 +0100 Subject: [PATCH 62/63] Do not re-assign the Specification unless different value The aim is to preserve the result ranges manually set to analyses and to the Sample itself when the Save button is pressed in the Sample view while the user has not changed the value for Specification --- bika/lims/content/analysisrequest.py | 10 +++++----- bika/lims/tests/doctests/API_analysis.rst | 3 +++ .../tests/doctests/SpecificationAndResultsRanges.rst | 11 +++++++++-- 3 files changed, 17 insertions(+), 7 deletions(-) diff --git a/bika/lims/content/analysisrequest.py b/bika/lims/content/analysisrequest.py index b694bb4ef2..3cf0f25a02 100644 --- a/bika/lims/content/analysisrequest.py +++ b/bika/lims/content/analysisrequest.py @@ -1438,14 +1438,14 @@ def Description(self): descr = " ".join((self.getId(), self.aq_parent.Title())) return safe_unicode(descr).encode('utf-8') - def setSpecification(self, value, override=False): + def setSpecification(self, value): """Sets the Specifications and ResultRange values """ current_spec = self.getRawSpecification() - changed = current_spec != value - - if changed and not override: - # preserve the current value + if value and current_spec == api.get_uid(value): + # Specification has not changed, preserve the current value to + # prevent result ranges (both from Sample and from analyses) from + # being overriden return self.getField("Specification").set(self, value) diff --git a/bika/lims/tests/doctests/API_analysis.rst b/bika/lims/tests/doctests/API_analysis.rst index 84023b2f2a..d699a012b9 100644 --- a/bika/lims/tests/doctests/API_analysis.rst +++ b/bika/lims/tests/doctests/API_analysis.rst @@ -514,6 +514,7 @@ Set open interval for min and max from water specification We need to re-apply the Specification for the changes to take effect: + >>> ar.setSpecification(None) >>> ar.setSpecification(specification) First, get the analyses from slot 1 and sort them asc: @@ -546,6 +547,7 @@ Set left-open interval for min and max from water specification We need to re-apply the Specification for the changes to take effect: + >>> ar.setSpecification(None) >>> ar.setSpecification(specification) First, get the analyses from slot 1 and sort them asc: @@ -578,6 +580,7 @@ Set right-open interval for min and max from water specification We need to re-apply the Specification for the changes to take effect: + >>> ar.setSpecification(None) >>> ar.setSpecification(specification) First, get the analyses from slot 1 and sort them asc: diff --git a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst index 26885dc3ff..97c930c247 100644 --- a/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst +++ b/bika/lims/tests/doctests/SpecificationAndResultsRanges.rst @@ -169,12 +169,18 @@ effect to neither the Sample nor analyses: >>> (rr_sample_au.min, rr_sample_au.max) (10, 20) -We need to re-apply the Specification for the Sample's results range to update: +If we re-apply the Specification, nothing will change though, because its `uid` +is still the same: >>> sample.setSpecification(specification) + >>> specification.getResultsRange() == sample.getResultsRange() + False -And the ResultsRange value from Sample is updated accordingly: +But the ResultsRange value from Sample is updated accordingly if we set the +specification to `None` first: + >>> sample.setSpecification(None) + >>> sample.setSpecification(specification) >>> specification.getResultsRange() == sample.getResultsRange() True @@ -250,6 +256,7 @@ But analysis' result range has indeed changed: If we re-apply the Specification, the result range for `Zn`, as well as for the Sample, are reestablished: + >>> sample.setSpecification(None) >>> sample.setSpecification(specification) >>> specification.getResultsRange() == sample.getResultsRange() True From 6b83f3bda46381e475fa8e3aff599325bcc02fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jordi=20Puiggen=C3=A9?= Date: Fri, 31 Jan 2020 17:34:52 +0100 Subject: [PATCH 63/63] Remove unnecessary log.error --- bika/lims/browser/viewlets/analysisrequest.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/bika/lims/browser/viewlets/analysisrequest.py b/bika/lims/browser/viewlets/analysisrequest.py index 241c458d8a..3582801989 100644 --- a/bika/lims/browser/viewlets/analysisrequest.py +++ b/bika/lims/browser/viewlets/analysisrequest.py @@ -114,9 +114,7 @@ def is_results_ranges_out_of_date(self): specifications = sample.getSpecification() if not specifications: - # This should never happen - logger.error("No specifications, but results ranges set for {}" - .format(api.get_id(sample))) + # The specification was once assigned, but unassigned later return False spec_rr = specifications.getResultsRange()
+ + + + + + @@ -47,6 +60,7 @@
+ + + + + + @@ -77,6 +102,7 @@ +