diff --git a/CHANGES.rst b/CHANGES.rst index e5776a9dc4..f73345ef35 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -7,6 +7,7 @@ Changelog **Added** +- #1516 Consider analyses with result options or string in duplicate valid range - #1515 Moved Setup View into Core - #1506 Specification non-compliant viewlet in Sample - #1506 Sample results ranges out-of-date viewlet in Sample @@ -22,7 +23,6 @@ Changelog - #1483 Added Accredited symbol in Analyses listings - #1466 Support for "readonly" and "hidden" visibility modes in ReferenceWidget - **Changed** - #1513 Better Ajax Loader for Sample Add Form @@ -35,6 +35,7 @@ Changelog **Removed** +- #1516 Removed getResultsRange metadata from analysis_catalog - #1487 Dexterity Compatible Catalog Base Class - #1482 Remove `senaite.instruments` dependency for instrument import form - #1478 Remove AcquireFieldDefaults (was used for CCEmails field only) diff --git a/bika/lims/api/analysis.py b/bika/lims/api/analysis.py index e466e53600..dc30646de5 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 import IDuplicateAnalysis +from bika.lims.interfaces import ISubmitted from bika.lims.interfaces.analysis import IRequestAnalysis @@ -56,10 +58,41 @@ def is_out_of_range(brain_or_object, result=_marker): if result is _marker: result = api.safe_getattr(analysis, "getResult", None) - if not api.is_floatable(result): - # Result is empty/None or not a valid number + + if result in [None, '']: + # Empty result + return False, False + + if IDuplicateAnalysis.providedBy(analysis): + # Result range for duplicate analyses is calculated from the original + # result, applying a variation % in shoulders. If the analysis has + # result options enabled or string results enabled, system returns an + # empty result range for the duplicate: result must match %100 with the + # original result + original = analysis.getAnalysis() + original_result = original.getResult() + + # Does original analysis have a valid result? + if original_result in [None, '']: + return False, False + + # Does original result type matches with duplicate result type? + if api.is_floatable(result) != api.is_floatable(original_result): + return True, True + + # Does analysis has result options enabled or non-floatable? + if analysis.getResultOptions() or not api.is_floatable(original_result): + # Let's always assume the result is 'out from shoulders', cause we + # consider the shoulders are precisely the duplicate variation % + out_of_range = original_result != result + return out_of_range, out_of_range + + elif not api.is_floatable(result): + # A non-duplicate with non-floatable result. There is no chance to know + # if the result is out-of-range return False, False + # Convert result to a float result = api.to_float(result) # Note that routine analyses, duplicates and reference analyses all them @@ -159,6 +192,11 @@ def is_result_range_compliant(analysis): if not IRequestAnalysis.providedBy(analysis): return True + if IDuplicateAnalysis.providedBy(analysis): + # Does not make sense to apply compliance to a duplicate, cause its + # valid range depends on the result of the original analysis + return True + rr = analysis.getResultsRange() service_uid = rr.get("uid", None) if not api.is_uid(service_uid): diff --git a/bika/lims/browser/analyses/view.py b/bika/lims/browser/analyses/view.py index dcf2cab9bc..a74cb0fa7b 100644 --- a/bika/lims/browser/analyses/view.py +++ b/bika/lims/browser/analyses/view.py @@ -575,6 +575,8 @@ def folderitem(self, obj, item, index): self._folder_item_detection_limits(obj, item) # Fill Specifications self._folder_item_specifications(obj, item) + self._folder_item_out_of_range(obj, item) + self._folder_item_result_range_compliance(obj, item) # Fill Partition self._folder_item_partition(obj, item) # Fill Due Date and icon if late/overdue @@ -1024,17 +1026,18 @@ def _folder_item_detection_limits(self, analysis_brain, item): def _folder_item_specifications(self, analysis_brain, item): """Set the results range to the item passed in""" - # Everyone can see valid-ranges - item['Specification'] = '' - results_range = analysis_brain.getResultsRange - if not results_range: - return + analysis = self.get_object(analysis_brain) + results_range = analysis.getResultsRange() - # Display the specification interval - item["Specification"] = get_formatted_interval(results_range, "") + item["Specification"] = "" + if results_range: + item["Specification"] = get_formatted_interval(results_range, "") - # Show an icon if out of range - out_range, out_shoulders = is_out_of_range(analysis_brain) + def _folder_item_out_of_range(self, analysis_brain, item): + """Displays an icon if result is out of range + """ + analysis = self.get_object(analysis_brain) + out_range, out_shoulders = is_out_of_range(analysis) if out_range: msg = _("Result out of range") img = get_image("exclamation.png", title=msg) @@ -1043,17 +1046,25 @@ def _folder_item_specifications(self, analysis_brain, item): 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_result_range_compliance(self, analysis_brain, item): + """Displays an icon if the range is different from the results ranges + defined in the Sample + """ + if not IAnalysisRequest.providedBy(self.context): + return + + analysis = self.get_object(analysis_brain) + if is_result_range_compliant(analysis): + return + + # Non-compliant range, display an icon + 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/catalog/analysis_catalog.py b/bika/lims/catalog/analysis_catalog.py index 93f0f127e0..0636d61b3f 100644 --- a/bika/lims/catalog/analysis_catalog.py +++ b/bika/lims/catalog/analysis_catalog.py @@ -115,7 +115,6 @@ "getInstrumentEntryOfResults", "getAllowedInstrumentUIDs", "getInstrumentUID", - "getResultsRange", "getSampleTypeUID", "getClientOrderNumber", "getDateReceived", diff --git a/bika/lims/content/duplicateanalysis.py b/bika/lims/content/duplicateanalysis.py index 97467e6e47..4c13c365a9 100644 --- a/bika/lims/content/duplicateanalysis.py +++ b/bika/lims/content/duplicateanalysis.py @@ -28,7 +28,9 @@ from bika.lims.content.abstractroutineanalysis import schema from bika.lims.content.analysisspec import ResultsRangeDict from bika.lims.interfaces import IDuplicateAnalysis +from bika.lims.interfaces import ISubmitted from bika.lims.interfaces.analysis import IRequestAnalysis +from bika.lims import logger from bika.lims.workflow import in_state from bika.lims.workflow.analysis import STATE_RETRACTED, STATE_REJECTED from zope.interface import implements @@ -143,28 +145,41 @@ def getResultsRange(self): A Duplicate will be out of range if its result does not match with the result for the parent analysis plus the duplicate variation in % as the margin error. + + If the duplicate is from an analysis with result options and/or string + results enabled (with non-numeric value), returns an empty result range + :return: A dictionary with the keys min and max :rtype: dict """ - specs = ResultsRangeDict() - analysis = self.getAnalysis() - if not analysis: - return specs - - result = analysis.getResult() - if not api.is_floatable(result): - return specs - - specs.min = specs.max = result - result = api.to_float(result) - dup_variation = analysis.getDuplicateVariation() - dup_variation = api.to_float(dup_variation) + # Get the original analysis + original_analysis = self.getAnalysis() + if not original_analysis: + logger.warn("Orphan duplicate: {}".format(repr(self))) + return {} + + # Return empty if results option enabled (exact match expected) + if original_analysis.getResultOptions(): + return {} + + # Return empty if non-floatable (exact match expected) + original_result = original_analysis.getResult() + if not api.is_floatable(original_result): + return {} + + # Calculate the min/max based on duplicate variation % + specs = ResultsRangeDict(uid=self.getServiceUID()) + dup_variation = original_analysis.getDuplicateVariation() + dup_variation = api.to_float(dup_variation, default=0) if not dup_variation: + # We expect an exact match + specs.min = specs.max = original_result return specs - margin = abs(result) * (dup_variation / 100.0) - specs.min = str(result - margin) - specs.max = str(result + margin) + original_result = api.to_float(original_result) + margin = abs(original_result) * (dup_variation / 100.0) + specs.min = str(original_result - margin) + specs.max = str(original_result + margin) return specs diff --git a/bika/lims/content/instrument.py b/bika/lims/content/instrument.py index 4bbe16be2a..e100975e67 100644 --- a/bika/lims/content/instrument.py +++ b/bika/lims/content/instrument.py @@ -482,11 +482,12 @@ def isQCValid(self): "getReferenceAnalysesGroupID": group_id,} brains = api.search(query, CATALOG_ANALYSIS_LISTING) for brain in brains: - results_range = brain.getResultsRange + analysis = api.get_object(brain) + results_range = analysis.getResultsRange() if not results_range: continue # Is out of range? - out_of_range = is_out_of_range(brain)[0] + out_of_range = is_out_of_range(analysis)[0] if out_of_range: return False diff --git a/bika/lims/tests/doctests/DuplicateResultsRange.rst b/bika/lims/tests/doctests/DuplicateResultsRange.rst new file mode 100644 index 0000000000..0cf2ee4d8a --- /dev/null +++ b/bika/lims/tests/doctests/DuplicateResultsRange.rst @@ -0,0 +1,334 @@ +Duplicate results range +======================= + +The valid result range for a duplicate analysis is calculated by applying a +duplicate variation percentage to the result from the original analysis. If the +analysis has result options enabled or string results enabled, results from +both duplicate and original analysis must match 100%. + +Running this test from the buildout directory: + + bin/test test_textual_doctests -t DuplicateResultsRange + +Test Setup +---------- + +Needed imports: + + >>> 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.api.analysis import is_out_of_range + >>> from bika.lims.utils.analysisrequest import create_analysisrequest + >>> from bika.lims.workflow import doActionFor as do_action_for + +Functional Helpers: + + >>> def new_sample(services): + ... values = { + ... 'Client': client.UID(), + ... 'Contact': contact.UID(), + ... 'DateSampled': DateTime().strftime("%Y-%m-%d"), + ... 'SampleType': sampletype.UID()} + ... service_uids = map(api.get_uid, services) + ... ar = create_analysisrequest(client, request, values, service_uids) + ... transitioned = do_action_for(ar, "receive") + ... return ar + + >>> def new_worksheet(analyses): + ... analyses = [] + ... for num in range(num_analyses): + ... sample = new_sample(analyses) + ... analyses.extend(sample.getAnalyses(full_objects=True)) + ... worksheet = api.create(portal.worksheets, "Worksheet") + ... worksheet.addAnalyses(analyses) + ... return worksheet + +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) + >>> 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()) + >>> Au = api.create(setup.bika_analysisservices, "AnalysisService", title="Gold", Keyword="Au", Price="20", Category=category.UID()) + + +Duplicate of an analysis with numeric result +-------------------------------------------- + +Set the duplicate variation in percentage for `Cu`: + + >>> Cu.setDuplicateVariation("10") + >>> Cu.getDuplicateVariation() + '10.00' + +Create a Sample and receive: + + >>> sample = new_sample([Cu]) + +Create a worksheet and assign the analyses: + + >>> analyses = sample.getAnalyses(full_objects=True) + >>> worksheet = api.create(portal.worksheets, "Worksheet") + >>> worksheet.addAnalyses(analyses) + +Add a duplicate for analysis `Cu`: + + >>> worksheet.addDuplicateAnalyses(1) + [>> duplicate = worksheet.getDuplicateAnalyses()[0] + >>> duplicate.getAnalysis() + + + >>> duplicate.getResultsRange() + {} + +Set a result of 50 for the original analysis `Cu`: + + >>> cu = analyses[0] + >>> cu.setResult(50) + >>> duplicate.getAnalysis().getResult() + '50' + + >>> result_range = duplicate.getResultsRange() + >>> (result_range.min, result_range.max) + ('45.0', '55.0') + +We can set a result for the duplicate within the range: + + >>> duplicate.setResult(47) + >>> is_out_of_range(duplicate) + (False, False) + +Or an out-of-range result: + + >>> duplicate.setResult(42) + >>> is_out_of_range(duplicate) + (True, True) + +We can do same exercise, but the other way round. We can submit the result for +the duplicate first: + + >>> sample = new_sample([Cu]) + >>> cu = sample.getAnalyses(full_objects=True)[0] + >>> worksheet.addAnalyses([cu]) + +We add a duplicate for new analysis, that is located at slot number 3: + + >>> worksheet.addDuplicateAnalyses(src_slot=3) + [>> duplicate = worksheet.getDuplicateAnalyses() + >>> duplicate = filter(lambda dup: dup.getAnalysis() == cu, duplicate)[0] + >>> duplicate.getAnalysis() + + + >>> duplicate.getResultsRange() + {} + +We set the result for the duplicate first, but it does not have a valid +result range because the original analysis has no result yet: + + >>> duplicate.setResult(58) + >>> duplicate.getResultsRange() + {} + + >>> is_out_of_range(duplicate) + (False, False) + + >>> cu.setResult(50) + >>> result_range = duplicate.getResultsRange() + >>> (result_range.min, result_range.max) + ('45.0', '55.0') + + >>> is_out_of_range(duplicate) + (True, True) + + +Duplicate of an analysis with result options +-------------------------------------------- + +Let's add some results options to service `Fe`: + + >>> results_options = [ + ... {"ResultValue": "1", "ResultText": "Number 1"}, + ... {"ResultValue": "2", "ResultText": "Number 2"}, + ... {"ResultValue": "3", "ResultText": "Number 3"}] + >>> Fe.setResultOptions(results_options) + >>> Fe.getResultOptions() + [{'ResultValue': '1', 'ResultText': 'Number 1'}, {'ResultValue': '2', 'ResultText': 'Number 2'}, {'ResultValue': '3', 'ResultText': 'Number 3'}] + +Create a Sample and receive: + + >>> sample = new_sample([Fe]) + +Create a worksheet and assign the analyses: + + >>> analyses = sample.getAnalyses(full_objects=True) + >>> worksheet = api.create(portal.worksheets, "Worksheet") + >>> worksheet.addAnalyses(analyses) + +Add a duplicate for analysis `Fe`: + + >>> worksheet.addDuplicateAnalyses(1) + [>> duplicate = worksheet.getDuplicateAnalyses()[0] + >>> fe = duplicate.getAnalysis() + >>> fe + + + >>> duplicate.getResultsRange() + {} + +Set a result for original analysis: + + >>> fe.setResult(2) + >>> fe.getResult() + '2' + >>> fe.getFormattedResult() + 'Number 2' + +The result range for duplicate does not longer consider duplicate variation, +rather expects an exact result: + + >>> duplicate.getResultsRange() + {} + + >>> duplicate.setResult(1) + >>> duplicate.getResult() + '1' + >>> duplicate.getFormattedResult() + 'Number 1' + >>> duplicate.getResultsRange() + {} + >>> is_out_of_range(duplicate) + (True, True) + + >>> duplicate.setResult(2) + >>> duplicate.getResultsRange() + {} + >>> is_out_of_range(duplicate) + (False, False) + + >>> duplicate.setResult(3) + >>> duplicate.getResultsRange() + {} + >>> is_out_of_range(duplicate) + (True, True) + + +Duplicate of an analysis with string results enabled +---------------------------------------------------- + +Let's add make the analysis `Au` to accept string results: + + >>> Au.setStringResult(True) + +Create a Sample and receive: + + >>> sample = new_sample([Au]) + +Create a worksheet and assign the analyses: + + >>> analyses = sample.getAnalyses(full_objects=True) + >>> worksheet = api.create(portal.worksheets, "Worksheet") + >>> worksheet.addAnalyses(analyses) + +Add a duplicate for analysis `Au`: + + >>> worksheet.addDuplicateAnalyses(1) + [>> duplicate = worksheet.getDuplicateAnalyses()[0] + >>> au = duplicate.getAnalysis() + >>> au + + + >>> duplicate.getStringResult() + True + + >>> duplicate.getResultsRange() + {} + +Submit a string result for original analysis: + + >>> au.setResult("Positive") + >>> au.getResult() + 'Positive' + + >>> au.getFormattedResult() + 'Positive' + +The result range for duplicate does not longer consider duplicate variation, +rather expects an exact result: + + >>> duplicate.getResultsRange() + {} + + >>> duplicate.setResult("Negative") + >>> duplicate.getResult() + 'Negative' + >>> duplicate.getFormattedResult() + 'Negative' + >>> duplicate.getResultsRange() + {} + >>> is_out_of_range(duplicate) + (True, True) + + >>> duplicate.setResult("Positive") + >>> duplicate.getResultsRange() + {} + >>> is_out_of_range(duplicate) + (False, False) + +But when we submit a numeric result for an analysis with string result enabled, +the system will behave as if it was indeed, a numeric result: + + >>> Au.setDuplicateVariation("10") + >>> Au.getDuplicateVariation() + '10.00' + + >>> Au.getStringResult() + True + + >>> sample = new_sample([Au]) + >>> au = sample.getAnalyses(full_objects=True)[0] + >>> worksheet.addAnalyses([au]) + +We add a duplicate for new analysis, that is located at slot number 3: + + >>> worksheet.addDuplicateAnalyses(src_slot=3) + [>> duplicate = worksheet.getDuplicateAnalyses() + >>> duplicate = filter(lambda dup: dup.getAnalysis() == au, duplicate)[0] + >>> duplicate.getAnalysis() + + + >>> duplicate.getStringResult() + True + + >>> duplicate.getResultsRange() + {} + +And we set a numeric result: + + >>> au.setResult(50) + >>> results_range = duplicate.getResultsRange() + >>> (results_range.min, results_range.max) + ('45.0', '55.0') diff --git a/bika/lims/upgrade/v01_03_003.py b/bika/lims/upgrade/v01_03_003.py index 8b9e32b21d..8e62a7b119 100644 --- a/bika/lims/upgrade/v01_03_003.py +++ b/bika/lims/upgrade/v01_03_003.py @@ -24,6 +24,7 @@ import transaction from bika.lims import api from bika.lims import logger +from bika.lims.catalog import CATALOG_ANALYSIS_LISTING 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 @@ -229,6 +230,12 @@ ("bika_setup_catalog", "cancellation_state"), ("bika_setup_catalog", "getName"), ("bika_setup_catalog", "getServiceUID"), + + # Was only used in analyses listing, but it can lead to inconsistencies + # because there are some analyses (Duplicates) their result range depends + # on the result of an original analysis. Thus, better to remove the metadata + # and wake-up object than add additional reindexes, etc. everywhere + (CATALOG_ANALYSIS_LISTING, "getResultsRange") ] @@ -644,4 +651,3 @@ def update_analyses_results_range(sample): if analysis_rr: analysis = api.get_object(analysis) analysis.setResultsRange(analysis_rr) - analysis.reindexObject()