Remove deprecation warnings. #353

rockfruit · 2017-11-04T15:37:43Z

I refactored some usages of deprecated functions that were used,
and I removed a lot of deprecated functions entirely.

I also removed the @deprecated warnings from the workflow functions
in content/* as I know we will remember to remove them when we merge
the workflow branch!

@deprecated

I refactored some usages of deprecated functions that were used, and I removed a lot of deprecated functions entirely. I also removed the @deprecated warnings from the workflow functions in content/* as I know we will remember to remove them when we merge the workflow branch!

rockfruit · 2017-11-05T07:14:57Z

issue #341

xispa

Don't be afraid of too many comments. Three important things:

comment re sortable_title in bika/lims/browser/worksheet/workflow.py Remove deprecation warnings. #353 (comment)
comment re bika/lims/browser/referenceanalysis.py Remove deprecation warnings. #353 (comment)
Since you've changed the indexes from analysis catalog, this PR requires an upgrade step. ;)

xispa · 2017-11-06T22:24:04Z

bika/lims/browser/late_analyses.py

@@ -43,8 +43,8 @@ def __init__(self, context, request):

        self.columns = {'Analysis': {'title': _('Analysis'),
                                     'index': 'sortable_title'},
-                        'RequestID': {'title': _('Request ID'),
-                                      'index': 'getRequestID'},
+                        'getRequestID': {'title': _('Request ID'),


Heads up!! this is a column name!. Don't think is necessary to change it.

xispa · 2017-11-06T22:24:14Z

bika/lims/browser/late_analyses.py

@@ -59,7 +59,7 @@ def __init__(self, context, request):
             'title': _('All'),
             'contentFilter':{},
             'columns':['Analysis',
-                        'RequestID',
+                        'getRequestID',


Column name!

xispa · 2017-11-06T22:24:33Z

bika/lims/browser/late_analyses.py

-            items[x]['RequestID'] = ''
-            items[x]['replace']['RequestID'] = "<a href='%s'>%s</a>" % \
+            items[x]['getRequestID'] = ''
+            items[x]['replace']['getRequestID'] = "<a href='%s'>%s</a>" % \


Column name in both cases!

xispa · 2017-11-06T22:32:54Z

bika/lims/browser/analysisrequest/manage_results.py

@@ -37,7 +37,7 @@ def __call__(self):
                                     getPointOfCapture=poc,
                                     sort_on='title',
                                     show_categories=show_cats,
-                                     getAnalysisRequestUID=ar.UID())
+                                     getRequestUID=ar.UID())


Wait, getRequestUID is used in AnalysesView as a content filter key-value: https://github.com/senaite/bika.lims/blob/senaite-integration/bika/lims/browser/analyses.py#L45
But there is no index getRequestUID in bika_analysis_catalog. Keep the old one getAnalysisRequestUID instead: https://github.com/senaite/bika.lims/blob/senaite-integration/bika/lims/catalog/analysis_catalog.py#L26

Please note there is an index getRequestID though (note ID not UID): https://github.com/senaite/bika.lims/blob/senaite-integration/bika/lims/catalog/analysis_catalog.py#L36 .... ummm.... mental note for future @xispa we need to avoid missunderstandings here and better to only have two indexes with the same pattern getRequestUID + getRequestID instead of getAnalysisRequestUID + getRequestID .... confusing story

Forget about this... I've seen now you already changed the index names in catalog.

xispa · 2017-11-06T22:33:23Z

bika/lims/browser/analysisrequest/publish.py

@@ -1265,7 +1264,7 @@ def _analyses_data(self, ar, analysis_states=None):
        showhidden = self.isHiddenAnalysesVisible()

        catalog = get_tool(CATALOG_ANALYSIS_LISTING)
-        brains = catalog({'getAnalysisRequestUID': ar.UID(),
+        brains = catalog({'getRequestUID': ar.UID(),


There is no index getRequestUID in bika_analysis_catalog. Keep the old one getAnalysisRequestUID instead: https://github.com/senaite/bika.lims/blob/senaite-integration/bika/lims/catalog/analysis_catalog.py#L26

Please note there is an index getRequestID though (note ID not UID): https://github.com/senaite/bika.lims/blob/senaite-integration/bika/lims/catalog/analysis_catalog.py#L36 .... ummm.... mental note for future @xispa we need to avoid missunderstandings here and better to only have two indexes with the same pattern getRequestUID + getRequestID instead of getAnalysisRequestUID + getRequestID .... confusing story

Forget about this... I've seen now you already changed the index names in catalog.

xispa · 2017-11-06T22:56:52Z

bika/lims/browser/referenceanalysis.py

-                    item['ar_id'] = an.aq_parent.getRequestID()
+                    item['ar'] = an.getRequest()
+                    item['ar_url'] = an.getRequest().absolute_url()
+                    item['ar_id'] = an.getRequest().getId()


I think would be better like this:

if IRequestAnalysis.providedBy(an): ar = an.getRequest() item['ar'] = ar item['ar_url'] = ar.absolute_url() item['ar_id'] = ar.getId()

Otherwise, duplicate analyses will not be considered

xispa · 2017-11-06T22:57:44Z

bika/lims/browser/samplinground/analysisrequests.py

@@ -30,7 +30,7 @@ def __init__(self, context, request):
                                  'toggle': True},
            'samplingRoundTemplate': {'title': _('Sampling Round Template'),
                                      'toggle': True},
-            'getRequestID': {'title': _('Request ID'),
+            'getId': {'title': _('Request ID'),


A column name again... but seems ok to me

xispa · 2017-11-06T23:04:21Z

bika/lims/browser/worksheet/ajax.py

            elif analysis.portal_type == 'ReferenceAnalysis':
                if review_state not in attachable_states:
                    continue
                parent = analysis.aq_parent.Title()
+            else:
+                raise RuntimeError("Requires Duplicate/Analysis/Reference")


Is ok, but now you are here... all this loop full of ifelses can be simplified a lot:

for analysis in analyses: review_state = workflow.getCurrentState(analysis) if review_state not in attachable_states: continue parent = analysis.getParentTitle() ...

xispa · 2017-11-06T23:10:13Z

bika/lims/browser/worksheet/views/add_analyses.py

-                'replace_url': 'getAnalysisRequestURL',
-                'index': 'getRequestID'},
+                'attr': 'getId',
+                'replace_url': 'getRequestURL',


I see getAnalysisRequetURL is labelled as deprecated here https://github.com/senaite/bika.lims/blob/senaite-integration/bika/lims/content/abstractroutineanalysis.py#L338 in favour of getRequestURL https://github.com/senaite/bika.lims/blob/senaite-integration/bika/lims/content/abstractroutineanalysis.py#L174 , but in the catalog, the metadata column is still getAnalysisRequestURL https://github.com/senaite/bika.lims/blob/senaite-integration/bika/lims/catalog/analysis_catalog.py#L65 . As per the same reason discussed above (re UID vs ID), I think that indeed, is better to be coherent everywhere, so I'd use getRequestURL instead of getAnalysisRequestURL in the catalog

Forget about this... I've seen now you already changed the index names in catalog.

xispa · 2017-11-06T23:15:53Z

bika/lims/browser/worksheet/workflow.py

@@ -109,7 +109,7 @@ def __call__(self):
                # consistent with the order of the ARs
                catalog = get_tool(CATALOG_ANALYSIS_LISTING)
                brains = catalog({'UID': analysis_uids,
-                                 'sort_on': 'getRequestID'})
+                                 'sort_on': 'sortable_title'})


Wrong. this should be getRequestID. Otherwise, the manage_results view from worksheet will appear wrong, like described in #302 . We only sort analyses here for adding them in the worksheet, not for displaying them in a list.
The reason is that we need the Analyses to be sorted in AR blocks (cause there is some logic there that mix them otherwise). Also AR slots need to be sorted with the same natural order as were registered.

(this way it includes duplicates)

…heet

rockfruit added 3 commits November 4, 2017 17:37

Update changelog

d5e82be

Remove deprecated instrument.getMethodUID()

334d860

rockfruit force-pushed the remove-deprecation-warnings branch from 7166608 to 334d860 Compare November 5, 2017 07:05

xispa requested changes Nov 6, 2017

View reviewed changes

xispa added Improvement 🔧 P4: Nice To Have labels Nov 10, 2017

rockfruit added 3 commits November 14, 2017 05:10

use IRequestAnalysis to discover Analyses with associated ARs.

8dbdc74

(this way it includes duplicates)

use getRequestID for initial sort of analyses when assigning to works…

622b5c4

…heet

Merge branch 'senaite-integration' into remove-deprecation-warnings

5f481e8

rockfruit merged commit 788cced into senaite:senaite-integration Nov 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove deprecation warnings. #353

Remove deprecation warnings. #353

rockfruit commented Nov 4, 2017

rockfruit commented Nov 5, 2017

xispa left a comment •

edited

Loading

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

xispa Nov 6, 2017

Remove deprecation warnings. #353

Remove deprecation warnings. #353

Conversation

rockfruit commented Nov 4, 2017

rockfruit commented Nov 5, 2017

xispa left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xispa left a comment •

edited

Loading