10000 Workflow, document (best?) practices · Issue #9834 · symfony/symfony-docs · GitHub
[go: up one dir, main page]

Skip to content

Workflow, document (best?) practices #9834

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
atrauzzi opened this issue May 28, 2018 · 17 comments
Closed

Workflow, document (best?) practices #9834

atrauzzi opened this issue May 28, 2018 · 17 comments

Comments

@atrauzzi
Copy link
atrauzzi commented May 28, 2018

I've crawled over the ~4 pages of documentation on workflow from the symfony site which have been enough to get me to this point.

Unfortunately, the documentation I link to above focuses strictly on bootstrapping the component. There isn't as much as I'd like to see about any of the ways it its outputs can be used. I'm opening this ticket because while functionally I'm getting the output I desire, I'm in the phase now where I like to reflect on my usage of a new component and inquire about best practices.

I think my most specific thoughts/questions/facts are:

  • I am using the MultipleStateMarkingStore implementation
  • When the marking store calls my getState method, I'm currently assembling the state array based on various bits of information I infer from my domain graph.
    • Therefore, I do not serialize and store the entire state array because some states/places originate from outside my subject (related models).
    • Currently, my state transitions always transition from a collection of places, to a single place. This is because I only care about the one value and will regenerate the others from my domain in getState during future checks:
      new Transition(AddonSubscriptionTransition::INSTALL_FREE,
          [AddonSubscriptionState::INACTIVE, AddonSubscriptionState::FREE, AddonSubscriptionState::TRIAL_UNAVAILABLE],
          [AddonSubscriptionState::ACTIVE]),
      
      This kind of implies that I will only use a subject once per load.
    • When the marking store calls setState on my model, I'm only going to interpret the value to determine what data I need to write to the database.
  • So far, I've avoided using events because I feel comfortable interpreting the resultant setState call. But should I not be doing that and specifically focusing on using the events API of workflow to act on my data?

So, generally speaking, I'm looking for some anti-pattern advice, but I think some of this might do well to be codified somehow in the documentation so that people aren't inadvertently subverting the intended design of this very useful component!

I might update this description if I dream up better ways of explaining things, I hope you get the gist of what I'm after. 😄

@javiereguiluz
Copy link
Member

@atrauzzi thanks for helping us improve Symfony Docs. I can't help you here, but I'm pinging our main Workflow experts for their comments: @lyrixx and @HeahDude. Thanks!

@HeahDude
Copy link
Contributor
HeahDude commented May 30, 2018

Hello @atrauzzi, if you need a full control when writing and reading a state in your model, you should then use a custom MarkingStoreInterface implementation, to keep this logic outside of your model and bridge it in the Workflow component.

You would then need to replace the configuration by the following:

framework:
    workflows:
        some_entity_name:
            type: 'workflow'
            marking_store:
                service: App\Workflow\SomeEntityMarkingStore

@atrauzzi
Copy link
Author

@HeahDude - I don't mind using the current marking store and reading the array it provides, is there any reason I would want to use my own beyond that?

(Also, just to note: I'm not using this as part of a symfony project, so the YAML configs aren't really an option for me.)

@HeahDude
Copy link
Contributor

I don't mind using the current marking store and reading the array it provides, is there any reason I would want to use my own beyond that?

Yes, this separates concerns, so it follows best practices.
Your entity setter should stay simple, not handle the resulting logic of the Workflow component.
A custom MarkingStore could call many setters for example or one among others, and allows you to do whatever you want on your model instead of letting the property accessor call setMarking().

@lyrixx
Copy link
Member
lyrixx commented May 30, 2018

Hi.

About this transition:

new Transition(AddonSubscriptionTransition::INSTALL_FREE,
    [AddonSubscriptionState::INACTIVE, AddonSubscriptionState::FREE, AddonSubscriptionState::TRIAL_UNAVAILABLE],
    [AddonSubscriptionState::ACTIVE]),

Do you know you object should be in all three places (AddonSubscriptionState::INACTIVE, AddonSubscriptionState::FREE, AddonSubscriptionState::TRIAL_UNAVAILABLE) in order to pass through the transition ? If you are OK with that, it's perfect.


About the Custom Marking Store: Indeed it will be cleaner to use it. But you don't have to. But still ;)


Side node: You should never alter the state of your object directly.
If you call $workflow->can('A'), then if you do something will your model, another call to $workflow->can('A') should returns the same results. Always.

So be careful when you modify some information in your entity.


Finally, I don't have access to your code, but it looks like there is something strange here. Usually, you should not care about call to setState. the workflow manage it's own state. So it looks like you are using it but in a special/wrong way.

@atrauzzi
Copy link
Author
atrauzzi commented May 30, 2018

@lyrixx

  • Transition: Yes! That's totally what I expect and that the resultant state in the component will be ACTIVE.
  • To me, making a custom marking store isn't necessary. But more on that in a moment.
  • In my list of states, the only one that isn't altered directly is ACTIVE/INACTIVE. The remaining ones are generated from the current state of related models.

So that leaves the "strange" part. Unfortunately, the state I'm wanting to check on isn't summarized in any way. I'm feeding data into the state machine that comes from getState, but is effectively "read only" as far as the state machine is concerned.
All I'm really asking it for is "can I transition to ACTIVE/INACTIVE -- when these other things you don't control are true".

@noniagriconomie
Copy link
Contributor

Hi @atrauzzi

Is this issue still pending?

@atrauzzi
Copy link
Author

I'd say so, yeah. I think only half the story is presented in the current documentation. Effective, practical usage of workflow is not quite ev 8000 ident and all we're really shown currently are the core types.

@OskarStark
Copy link
Contributor

Can you name some „headlines“ you would want to read? This could maybe help us to get into the right direction and split this issue in smaller pull requests

@noniagriconomie
Copy link
Contributor

@atrauzzi do you mean you need/want more « real userland » code using workflow component inside the documentation?

Or you want to use it in a special way and you miss some docs? Also you can read the code doc classes or interfaces sometimes it helps

@atrauzzi
Copy link
Author
atrauzzi commented Nov 9, 2019

More userland 🙂

@noniagriconomie
Copy link
Contributor

it is maybe more some blog posts (symfony or other dev teams) then, to see more use cases of this component
also maybe you can ask for a feature being implemented in the symfony/demo repo :)

i think not it should be inside the doc

@atrauzzi
Copy link
Author

Hmm, I think the docs are still the right place. The first spot I looked for examples are here.

I really think there needs to be something to help people get started. Some kind of guidance. It's way too vague to just be given this base class and no idea of its lifecycle as it relates to one's own application.

I'm not asking for solutions on specific domain, but the nature by which specific domain code interacts with this component.

@milosa
Copy link
Contributor
milosa commented Dec 27, 2019

I've crawled over the ~4 pages of documentation on workflow from the symfony site which have been enough to get me to this point.

At first, and much to my frustration, I thought the documentation for Workflow was extremely inadequate.
But then I found that there are two documentation sections for Workflow:

The latter is much more detailed.
It took me a while to find the second one, since the first page was the first result in Google and I didn't expect there to be another doc section.

I really think they should be merged or, at least, have prominent links to one another.

@lyrixx
Copy link
Member
lyrixx commented J 8000 an 6, 2020

👍 I already reported that to the doc team. It's really hard to find all pages.

Would you mind creating a PR @milosa ?

javiereguiluz added a commit that referenced this issue Feb 26, 2020
…led doc (noniagriconomie)

This PR was merged into the 3.4 branch.

Discussion
----------

[Workflow] Add workflow documentation link to more detailed doc

Hi

Here is an attempt to better link the Workflow component bare + component with Symfony documentations

FYI the https://symfony.com/doc/master/workflow.html reference a "read more" page on the first paragraph
But as all components pages looks the same, I put this at the bottom of the page not on top

Related to #9834 (comment)

friendly ping @lyrixx

Commits
-------

4b1e7db Update workflow.rst
@javiereguiluz
Copy link
Member

I'm closing this old issue because we've made many changes in the Workflow component docs since the issue was opened. Thanks.

@atrauzzi
Copy link
Author

But did they address anything?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants
0