Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
I'd like to be able to pop off items one-by-one from a token list (without using custom events).
Steps to reproduce
Proposed resolution
- Add an action "Remove item from list" that pops off the last item of a list and stores it as a new token. The action may provide an option to either pop from the end or the beginning of the list, or event may provide a key input that allows removing a certain delta / index.
- Add a condition that checks whether a list is empty (or just use the existing list count action, guess that one is already fine)
Remaining tasks
Discuss details, implement, test.
User interface changes
API changes
Data model changes
Issue fork eca-3300517
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
mxhThink I can just try realizing the suggested action.
Comment #3
mxhComment #4
mxhComment #6
mxhAdded two new actions for list operations, and a new condition plugin "Token: exists". Using these components, I'm able to iterate over a list without using custom events.
Had to refator the DataTransferObject a bit and also fixed a possible infinite loop for dependency calculation when creating a cyclic graph representing a loop in BPMN.
Comment #7
mxhI've found another possible action plugin that may be useful regarding list operations: Putting an entity onto a list. Will add that into this MR.
Comment #8
mxhComment #9
mxhThe only thing that could be cherry-picked for 1.0.x from this MR is the following commit:
https://git.drupalcode.org/project/eca/-/merge_requests/209/diffs?commit...
That includes the prevention of an infinite loop during dependency calculation.
All others are feature additions and therefore belong into 1.1.x.
Comment #10
jurgenhaasThis is great stuff, well done!
Just a couple of comments that need to be addressed/discussed.
Comment #11
mxhWill have a look
Comment #12
mxhSetting to review for continuation discussing the last note.
Comment #13
jurgenhaasAdded another comment to the discussion.
Comment #14
mxhComment #15
mxhComment #16
jurgenhaasI have now added test cases that should demonstrate the use case I wanted to describe. It's in fact a great idea to do that with tests instead, that should be much more clear.
Comment #17
mxhThat's great, thanks for that. Tests are green on my system, which looks like both your and my expectations are met by the current implementation - unless the test coverage is not complete yet (?)
I think you need to enable test runs for the 1.1.x branch.
Comment #18
jurgenhaasSorry, my bad. My test case was still incomplete. The intent was NOT to test for existence of
placeholder
but to test for the token name that[plaveholder]
represents. I've now added 2 more tests and one of them should assert False but does assert True.Regarding the test runs, I'll add them right away.
Comment #19
mxhSetting to needs work, now that a test is failing.
Let's look at the test that is currently failing:
The reason why this is failing, is because
is adding the string "mytoken3" as token data for token name "placeholder". The condition then says "Yes, it exists" because there is token data, in this case a DTO holding "mytoken3" as string representation value, and token replacement would replace "Thevalue: [placeholder]" to "Thevalue: mytoken3".
Comment #20
mxhWould changing the above mentioned line to
$this->tokenServices->addTokenData('placeholder', $this->tokenServices->getTokenData('mytoken3'));
do what you want to reflect here as a use case? As an alternative, you could also make use of an action that represents your case, like "Token: set value". But I currently doubt that the current test case is properly reflecting your targeted use case, as it's adding a string to the Token stack as described above, instead of representing another token name.
Comment #21
jurgenhaasI'll explain my use case in a second, but before that, I try again explaining why I feel the changed treatment of the value in
token_name
feels like the natural one. As a user, when I have to provide a value in a field that asks for a name, I can either provide a static string or I can provide a variable which contains the name. And that's what I think this field needs to do: if the given value is a token (i.e. the variable), then that token's string value is the name; otherwise the given string is the name. And once we have the name, we can then check if a token with that name exists.Now, talking about a use case: we have a model with lots of different events and processes, each of which working with different tokens like node, taxonomy term, comment, etc. All those tokens are named according to their entity type for better clarity and to avoid conflicts if one of the processes deals with more than one. Each "branch" in that somehow bigger model defines a token called
type
which declares the entity type it deals with, it's the same as the token name it deals with.Each of those "branches" calls the same single action at the end, which starts evaluating if user messages have to be produces and what type of message was required. This wants to test if that token is available, either the node, the term or the comment. We could check that with
[type]
.Comment #22
mxhReading #21, this looks like you expect the "token_name" input to support tokens. Currently, none of our plugins supports token replacement in that input, and the new condition in this MR would be the very first one supporting it.
I'd recommend to see this as a separate feature request, because for that we'd have to look at all other plugins making use of "token_name" and change the overall behavior to keep consistency accross all plugins. Cleary an issue on its own.
Comment #23
jurgenhaasAgreed, let's address this in #3302569: Add token support to fields "token_name". Removed my extra test, which should now allow all tests to pass again. Will merge this, when the tests completed successfully.
Comment #26
jurgenhaasMerged the MR into 1.1.x and cherry-picked the dependency calculation bug fix into 1.0.x