xispa
commented
3 weeks ago Thanks for this fix and providing a test for it! I just thought that it would be great to get an information displayed for the user to confirm the action before deactivation. But we'll do this in another PR.
I thought about this as well, but I think this is not how this whole thing should work. I think best would be to not update profiles on service inactivation, but make the system "active/inactive"-aware when required (e.g. when a profile is assigned on sample creation or edition). This could be partially achieved by changing the signature of the profile's getter to getServices(self, active_only=True) and revisit the setter to not remove inactive uids unless explicitly provided. This approach entails the following benefits:
- no need to update/recatalog other objects via subscriber on service deactivation
- service can be deactivated/reactivated and user won't need to manually reassign services to profiles later
- the developer becomes responsible of the inclusion/exclusion of services depending on the use case
- less code
Same principle would apply for SampleTemplate
This is for another PR though. I wanted this PR to only restore the functionality that was already there before #2492
ramonski
Description of the issue/feature this PR addresses
This Pull Request fixes a side-effect of https://github.com/senaite/senaite.core/pull/2492 , so that inactivated services were no longer removed from existing panels on inactivation. Amongst other undesired effects, this was causing user was able to add inactive services on sample creation through the selection of analysis profiles with those services assigned.
Current behavior before PR
Inactive services are added on sample creation/edition when a profile with inactive services is selected
Desired behavior after PR is merged
Inactive services are added on sample creation/edition when a profile with inactive services is selected
-- I confirm I have tested this PR thoroughly and coded it according to PEP8 and Plone's Python styleguide standards.