Skip to content

[workflow component] [RFC] Store a history of marking states in a field of the entity #28253

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
ChangePlaces opened this issue Aug 23, 2018 · 3 comments

Comments

@ChangePlaces
Copy link

ChangePlaces commented Aug 23, 2018

Description
I have a few workflows where I need to keep a log of the workflow places the entity has been through also including a timestamp and optional 'memo' - a short text detailing the state change. This is for three reasons;

  • to be able to display a timeline of states in the admin for that entity,
  • to be able to use the workflow to alert the admin to states that have not changed after a pre-determined amount of times (e.g. a cart has items in it but has not been checked out, items that need to be urgently dispatched as they've been sitting in the to_dispatch place for a while and are now overdue the three-day dispatch time window etc).
  • above all, to be machine readable and parsable easily via code.

config

I have come upon this solution for the workflow definition by allowing an additional config inside the workflow.yaml file:

       # the workflow config
        payflow:
            ....
            ....
            marking_history_store:
                # the column name of where to store the history
                history_property: state_history
                # an optional field which will be stored with the history and then cleared, ready for next change 
                memo_property: current_state_note

This additional configuration inside the workflow will essentially help set up a listener that stores a history of states for that entity inside the trail_property field which listens for the workflow's entered event. An optional memo_property allows the user to store a brief description of the state change if required (e.g. payment verified by daniel, state automatically changed, admin changed state etc)

entity

The entity will be set up like this (with getters / setters omitted):

    /**
     * a note for the current state
     * @var string
     * @ORM\Column(type="string", length=255, nullable=true)
     */
    private $currentStateNote;

    /**
     * an archive of the states been through along with note and timestamps
     * @var array
     */
    private $stateHistory;

output

Upon a state change, the state history is populated with an assoc array that contains the state the entity is changed to, including a timestamp, the froms and tos, the name of the transition used, and the value of the optional memo field if not null. This array is appended to the existing array value, and is keyed by workflow name allowing histories of multiple workflow states to be stored if necessary.

A dd of the state history field after a few state changes:

array:1 [▼
  "payflow" => array:3 [▼ // the name of the workflow
    0 => array:4 [▼  // an assoc array detailing a state change 
           "timestamp" => "2018-08-25 15:16:23"
      "marking" => array:1 [▼
       0 => "not_shipped"
      ]
      "transition" => "awaiting_shipment"
      "memo" => "updating test"
    ]
    1 => array:4 [▼
      "timestamp" => "2018-08-25 15:16:23"
      "marking=> array:1 [▼
        0 => "shipped"
      ]
      "transition" => "item_shipped"
      "memo" => ""
    ]
    2 => array:4 [▼
      "timestamp" => "2018-08-25 15:16:23"
      "marking" => array:1 [▼
        0 => "received"
      ]
      "transition" => "received_delivery_needs_checking"
      "memo" => ""
    ]
  ]
]

I'd like to ask for comments on:

  • does the config seem sensible? property names are good etc
  • does the output seem reasonable?
@ChangePlaces ChangePlaces changed the title An audit trail stored in a field of the entity for the workflow component [workflow component] An audit trail stored in a field of the entity Aug 23, 2018
@ChangePlaces ChangePlaces changed the title [workflow component] An audit trail stored in a field of the entity [workflow component] [RFC] Store a history of marking states in a field of the entity Aug 25, 2018
@ChangePlaces
Copy link
Author

Would like a little help with what tests to write!

@lyrixx
Copy link
Member

lyrixx commented Nov 13, 2018

Hello,

(I'm really sorry for this huge delay)

Thanks for your proposal and thanks for your first contribution 🎉 👏

I think we could take another approach thanks to this PR: #29146

  1. all the logic could live in the subject. $subject->setMarking() could append the new state in an history attributes
  2. thanks to the PR the memo property could by passed through the $context. It's more generic
  3. I could update [Workflow] Added a context to Workflow::apply() #29146 to add the $context to all events dispatched in the Workflow::apply method. Listeners could be able to mutate the $context. For exemple to automatically inject the current user.

While your idea is really interesting, I think your PR is too opinionated to fit the Core. More over your initial issue could be solved with few lines of code in your application.

Instead I would suggest to open a PR in the documentation repository to explain this. What do you think ?

@lyrixx
Copy link
Member

lyrixx commented Nov 19, 2018

I'm closing this issue. See #28266 (comment) for more details.

@lyrixx lyrixx closed this as completed Nov 19, 2018
lyrixx added a commit that referenced this issue Mar 6, 2019
…ixx)

This PR was merged into the 4.3-dev branch.

Discussion
----------

[Workflow] Added a context to `Workflow::apply()`

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #27925 (maybe #28253 and #28266)
| License       | MIT
| Doc PR        |

Commits
-------

7d5b7a3 [Workflow] Added a context to `Workflow::apply()`
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

3 participants