[Bug] Transition Method Issues

[SchrodingersGat]

Problem description

Plugins which register a "TransitionMethod" do not need to be enabled for that method to be active. The transitionmethod is applied even if the plugin is disabled.

Steps to Reproduce

1. Activate the SampleTransitionPlugin sample plugin - and then disable it.
2. Try to mark a ReturnOrder as "complete"
3. Cannot "complete" the ReturnOrder - as the provided ReturnChangeHandler prevents it

Expected behaviour

A plugin which provides this functionality must be enabled to do so!

Discussion

This issue raises some other points about how "Transition Methods" are implemented which I am not happy about.

Activation

Plugins should NOT be able to have any effect on the system if they are not activated.

Registration

To register a transition method, we simply loop through all available classes which inherit from the TransitionMethod class. This bypasses any plugin activation checks, and allows any code on the system to inject custom state transition behavior without observability.

Collection

Transition methods are only collected at process startup, in InvenTree/apps.py. So, if we fix the issues above, we also need to ensure that we re-collect the methods whenever we reload the plugin registry.

Mixin

I think that we should refactor this and adda new mixin class for plugins and collect transition methods by explicitly iterating through active plugins which implement that mixin type.

This is:

a) In-line with how we implement other plugin functionality
b) Safer as it validates plugin active status first

Notification Method Registration

I see that the same issues exist with NotificationMethod - so we will have to fix problems there too..

This is a critical issue we need to address for 1.0 - and will be a breaking change for plugins which implement these features currently.

Target Goals

I think we should:

• Refactor the TransitionMethod and NotificationMethod classes to be plugin mixins
• Load them with the "normal" plugin mixin loading strategy

[matmair]

Plugin activation checks are trivial to bypass, they just give a false sense of security. The second you install a plugin you have to have 100% trust in it as long as it stays on the filesysten

[matmair]

At least I and the default notification plugin provide more than 1 method per plugin so the normal mixin route is not really feasible

[SchrodingersGat]

Either we could allow the "mixin" plugin class to return a set of multiple methods, perhaps?

Or a single "handler" could have logic for handling multiple transition types...

Either way, not insurmountable