senaite / senaite.core

Enterprise Open Source Laboratory System (LIMS)
https://senaite.com
GNU General Public License v2.0
254 stars 145 forks source link

Rollback ARAnalysesField's getRaw behavior #2603

Closed xispa closed 3 months ago

xispa commented 3 months ago

Description of the issue/feature this PR addresses

getRaw function for ARAnalysesField was introduced with https://github.com/senaite/senaite.core/pull/2593 , but there are too many side-effects.

Keeping a list of UIDs in-sync in this case is challenging, cause we need to keep that list up-to-date in event subscribers (on creation and on removal). And this might entail issues (e.g. you never know which subscriber is called first). Reason is that, except on sample creation and on manage analyses, we always create analyses programmatically (e.g by using create_analysis. For instance, when we create a retest.

Making getRaw to rely on instance.objectValues('Analysis') instead of relying on an object attribute could be considered as an alternative. However, objectValues wakes up all the contained objects. A query against the catalog will be faster, cause we get brains instead. In such scenario, there is no benefit compared to what we had before https://github.com/senaite/senaite.core/pull/2593 . Likewise, having an inconsistency between the values returned by getRaw and get is also not worth.

Therefore, this Pull Request reverts back the attribute Analyses that was handled by ARAnalysesField and although it keeps the getRaw function, it is now consistent with the brains returned by default by get.

Current behavior before PR

Too many side-effects because of the need of keeping Analyses attribute up-to-date

Desired behavior after PR is merged

Analyses UIDs are no longer kept in an object attribute. getRaw is consistent with getAnalyses

-- I confirm I have tested this PR thoroughly and coded it according to PEP8 and Plone's Python styleguide standards.