Skip to content
Closed
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion workflow/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ like this:

class BlogPost
{
// This property is used by the marking store
// This property is used by the marking store, by default, Symfony set the marking as an array.
public $currentPlace;
public $title;
public $content;
Expand All @@ -155,7 +155,19 @@ like this:
The ``type`` (default value ``single_state``) and ``arguments`` (default value ``marking``)
Copy link
Contributor

@noniagriconomie noniagriconomie May 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Guikingone For 4.3+ this need to be removed no?
As single/multiple state are deprecated/removed in favor of method
This PR must be rewrite for method marking store IMO

attributes of the ``marking_store`` option are optional. If omitted, their default values
will be used.

.. note::

If a ``multiple_state`` is used, the attribute should be an ``array``,
using Doctrine, you can use ``json``.

If a ``single_state`` is used, the persisted state type should be a string,
if you need to store an int (due to DB limitations and/or project constraints),
be aware that you need to implement :class:`Symfony\\Component\\Workflow\\MarkingStore\\MarkingStoreInterface`
in order to transform the int back to something understandable by Workflow.

If something else is used, you're in charge of choosing the best storage format.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the previous tip related to MarkingStoreInterface apply in this case too? If yes, we must merge these two phrases.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@javiereguiluz yes it's apply to. So it should be merged

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Guikingone could you merge theses phrase? Thanks.

After that, IMHO, we will be OK to merge it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improved, to be checked 🙂


With this workflow named ``blog_publishing``, you can get help to decide
what actions are allowed on a blog post::

Expand Down