joomla / joomla-cms

Home of the Joomla! Content Management System
https://www.joomla.org
GNU General Public License v2.0
4.77k stars 3.65k forks source link

[4.0][RFC] Scope of Workflow System #21485

Closed mbabker closed 6 years ago

mbabker commented 6 years ago

When I see references to a workflow system, I think of a State Machine or PHP libraries like https://github.com/winzou/state-machine and https://github.com/symfony/workflow. However it seems based on an arbitrary quick glance of the code added from that project (ignoring every other architectural issue with it), this is NOT a workflow system like those libraries or the high level capabilities they offer, but rather our definition of a workflow (and inherently what the Joomla\CMS\Workflow\Workflow base class is providing) relates to content publishing states and some arbitrary labels for what we now know as "published", "unpublished", and "trashed".

Please clarify the scope of this system and its design. Is this intended to be a proper workflow engine or is this ONLY suitable for content publishing workflows? If it is the latter, all documentation and API references should make clear this is the scope of the API.

bembelimen commented 6 years ago

Hello @mbabker , I'm not sure how to answer your question but I try my best.

In the current workflow we have two layers:

  1. we have in fact the same as this "state machine" and also the first glimps into the synfony workflow: you define states, you define transitions between states and you can add callbacks by adding your plugins to the plugin triggers. (The Joomla! way so to speak). Last but not least you can add permissions to transitions (who can execute the workflow).

On the other hand we have 2. "some arbitrary labels", so called conditions. When we were implementing the workflow we had a "big" problem: how do we define what happens with an item. Is it active/visible/published/online/released or is it inactive/invisible/unpublished/offline/blocked or is it trashed. So we have this 2nd layer. The condition defines what is the "behavior" of an item with status X.

So now how come this together?

We build this very generic state machine, which has first permissions. In the future it's very easy to move the current (e.g.) article permissions to the workflow states and become more and more the synfony workflow. So the possibility is there (already considered), but we didn't want to rebuild the whole permission process but go step by step. The state machine allows different "hooks" to link in.

On the other hand, we have the "problem", that we're not offer the framework only (as your linked libraries do), but we also have to offer a very generic way, that the user can create their workflow (without limitations). With the state-machine or Synfony workflow you (as developer) would know which states you're using and you can do a "hardlink" to actions. We have not this advantage. So we needed that condition thing to know, if the item is on or off in this state.

Archived?

And now to the frequently mentioned archive state from Joomla! 4. We removed it on purpose, because mapped on the condition concept it's just an item, that is "on" (with a special output => archived list). So we made this list generic (the user decides which items with which state will be shown) and moved archived to the condition "published". (As state itself it still exists). So the user can 1:1 create the same old structure with the "archived menu item", but now he can do more.

About the naming of the conditions (Published/Unpublished), that's a point we ofc can talk about...it was the first names which cames to our mind, but they can be changed to everything. Suggestions are welcome.

I hope, that answers your question, otherwise I try to be more concrete.

mbabker commented 6 years ago

States in a state machine aren't restricted to 3 arbitrary values that correspond to what we consider "publishing states". States can be anything you want them to be. The thing I'm having a hard time with is you're basically saying "everything in the state machine is either a published, unpublished, or trashed state with whatever label you want on it", and that's not how a state machine works at all. That works if you are creating a content workflow state machine (emphasis on content), because content itself has a few high level publishing states that represent the status of an item and inherently when and where it should be visible.

Take the order checkout workflow or state machine for one of the ecommerce platforms as an example, https://github.com/Sylius/Sylius/blob/master/src/Sylius/Bundle/CoreBundle/Resources/config/app/state_machine/sylius_order_checkout.yml. This state machine should not have to be mangled to fit into the Joomla\CMS\Workflow\Workflow definition of a workflow or state machine system. Yet, it does, because right now that class says "everything has to be published, unpublished, or trashed".

That's why I think this needs to be clear. Is the high level library API a true workflow or state machine system, or is the high level API by design only capable of supporting publishing workflows and not intended for or suitable for use in non-publishing related workflows?

bembelimen commented 6 years ago

You have the high level machine with this extra condition, which could be ignored. But perhaps it would be a good idea to decouple the condition? Then it would be clearer. (A shop would have all conditions on "published" and can ignore them).

To be honest, I'm not sure, if we should remove this condition now, because the idea was to use the articles as starting point but also use a concept, which can be extended. This does not mean, I'm not open for every improvement here, but then I need help in regards of manpower.

brianteeman commented 6 years ago

We removed it on purpose, because mapped on the condition concept it's just an item, that is "on" (with a special output => archived list)

It is much more than that as explained before many times. Which is why your change is an issue as explained before. At jab you said you understood and now you try to skip this major change back in. #disappointed.

bembelimen commented 6 years ago

Please don't lay words in my month. I wrote why we decided it "then", to explain the current status. I did not write anything about what will change...

bembelimen commented 6 years ago

I spoke with @HLeithner (member of the workflow team) about the condition problem and I think he found a nice solutions, we'll working on it.

brianteeman commented 6 years ago

I think the problem is that you dont fully understand what the current archive state is and how it works

alikon commented 6 years ago

At the current status this not a true State Machine, at least until you resolve the hardcoded condition imho

bembelimen commented 6 years ago

@alikon yes, as said above. But we decided to remove this limitation not in the future (as planned) but in the next days. So we'll have this "true state machine".

alikon commented 6 years ago

looking forward to see this happen