Skip to content

[Workflow] Add a line about the attribute type when persisted. #9156

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.

Sign up for GitHub

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
wants to merge 8 commits into from
Closed

[Workflow] Add a line about the attribute type when persisted. #9156

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
35d531a
fix(Attribute): fix on attribute type if persisted
Guikingone Jan 28, 2018
8bad73f
fix(storage): storage format
Guikingone Oct 19, 2018
41f5ea3
improvement(DB storage): improvement on type stored
Guikingone Dec 24, 2018
63a7df4
fix(type): fix on both class and Doctrine type
Guikingone Jan 2, 2019
664b5e3
fix(store): fix on store usage
Guikingone Jan 2, 2019
d1c0dd0
fix(marking): fix on marking sentence
Guikingone Jan 2, 2019
b9e3c0e
[FEAT] wording
Guikingone May 9, 2019
93afa79
fix(wording): fix on multiple phrases
Guikingone May 9, 2019
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
11 changes: 10 additions & 1 deletion workflow/usage.rst
View file
Open in desktop
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 "mark" this attribute as an array.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand what this means --> "Symfony mark this attribute as an array" Is this something that can only be understood if you use the Workflow component (which I don't) ... or is it supposed to be understood by all readers? Thanks!

Copy link
Member

@lyrixx lyrixx Jan 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

May be this is better:

By default symfony set the marking as an array.

as or with, I don't know

public $currentPlace;
public $title;
public $content;
Expand All @@ -155,7 +155,16 @@ like this:
The ``type`` (default value ``single_state``) and ``arguments`` (default value ``marking``)
Copy link
Contributor

@noniagriconomie noniagriconomie May 12, 2019 •
edited
Loading

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 persisted state type should be an ``array``,
Copy link
Member

Choose a reason for hiding this comment

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

This prhase needs some reword. It reads as "should be an array using Doctrine". Do you mean, a "PHP array" in the entity property and a "json" type in the @Column mapping?

using Doctrine, you can use ``json`` or ``json_array``.

If a ``single_state`` is used, the persisted state type could be a string
Copy link
Contributor

@noniagriconomie noniagriconomie Dec 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Or it could be also an integer, but you need to implement a MarkingStoreInterface to convert from string to int the place.

I see more often values of a $status property of object stored as int in the database, not string
Explaining it here could be very usefull to inform that we can do it easily IMO :)

cc @Guikingone

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback, I've added new lines about this case, can you review it and tell me if it's adapted? 🤔

Thanks 😊


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