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.
In D7, we could send variables as parameters or provided. Is it possible yet for D8? How can I use components now?
Comment | File | Size | Author |
---|---|---|---|
#27 | 2916331-27.patch | 18.67 KB | Megha_kundar |
#21 | interdiff_19-21.txt | 898 bytes | shubham.prakash |
#21 | 2916331-21.patch | 18.47 KB | shubham.prakash |
| |||
#19 | 2916331-19.patch | 18.51 KB | shubham.prakash |
#17 | interdiff_15-17.txt | 1.07 KB | shubham.prakash |
Comments
Comment #2
rlmumfordThis was a key feature in Drupal 7, I can't seem to find a mention of it in the roadmap?
Comment #3
rlmumfordComment #4
TR CreditAttribution: TR commentedYou have to start the test manually in this queue ...
Comment #5
rlmumfordUpdated for test fixes.
Comment #6
fagoThis looks like a good start, however I think we should add some test coverage for that!
Comment #7
rlmumfordNoticed another bug in Rules while working through this. It seems like different parts of the code assume different things about the 'context_definitions' and 'provided_context_definitions' properties in component config. In some places "provided_context_definitions" is assumed to be an array of names and "context_definitions" an array of all contexts associated with the component. Other places expect "context_definitions" to contain only those context_definitions required by the component and "provided_context_definitions" to contain the definitions of contexts that are provided by the component.
In the resulting confusion the code ended up such that RulesComponent::createFromConfiguration() and RulesComponent::getConfiguration() were not idempotent. (RulesComponentConfig::getProvidedContextDefinitions() and RulesComponent::createFromConfiguration() clearly expect 'provided_context_definitions' to be an array of definitions - RulesComponent::getConfiguration() would set 'provided_context_definitions' as an array of names)
In this patch, as well as exposing the feature requested in the UI, this bug has been fixed by making sure that "provided_context_definitions" is treated as an array of definitions where ever possible.
Comment #9
rlmumfordTest fixes.
Comment #11
rlmumfordSo switching to DI introduced more bugs.
Comment #12
fagothx, looks mostly good. However, I'm not sure the state changes should be done in execute() as usually we do the state setup in Drupal\rules\Engine\RulesComponent
I think this should be done in \Drupal\rules\Engine\RulesComponent::createFromConfiguration()
Comment #13
squibb CreditAttribution: squibb as a volunteer commentedTag is wrong... in #11 patch
'#title_disply'
has to be'#title_display'
Comment #14
TR CreditAttribution: TR commentedIn regards to #7, those decisions and changes were made in:
See #2664700: Expose Rules components as action plugin
And https://git.drupalcode.org/project/rules/commit/3a0d230
Reading those might shed some light on the issue or indicate other places that need to be fixed.
Comment #15
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commentedMade the changes after rerolling.
Comment #16
TR CreditAttribution: TR commentedThe two test fails are because of bad strings in the test case. The tests are looking for:
"There is no Rules component yet." and "Add rule"
But they should be looking for:
"There are no rules components yet." and "Add component"
Can you fix those, re-roll the patch and see if the testbot likes it?
Comment #17
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commentedMade the changes as suggested.
Comment #18
TR CreditAttribution: TR commentedAnd that fail is because in the tests all the field names need to start with
context_definitions[
instead ofcontext[
because of the changes introduced by #3030295: 'context' deprecated in Rules @Condition annotation and #3092087: Deprecate 'context' in Rules @RulesAction annotationComment #19
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commentedComment #20
TR CreditAttribution: TR commentedNote that the tests stop executing once they hit a failure, so what you are seeing is that fixing one thing will cause more of the test to run, potentially exposing other fails.
But we're near the end of that test now, so this might be the last one. The error is "Button with id|name|title|alt|value "context_data" not found." I think you should just be able to remove that pressButton() call because it was meant to switch to the data selector in the UI but that is not necessary or possible anymore after #2804941: Implement assignment restriction in plugin context was fixed.
Comment #21
shubham.prakash CreditAttribution: shubham.prakash at OpenSense Labs commentedRemoved pressButton() for context_data
Comment #22
TR CreditAttribution: TR commentedNice! Thank you! Tests now pass, so we can move on to evaluating the functionality ... I will try to post a thorough review in the next few days. This is an important patch that would be very nice to get finished and committed to Rules.
Comment #23
rlmumfordJust flagging that the patch no longer applies on alpha6
Comment #24
MegaChriz CreditAttribution: MegaChriz at WebCoo commentedThe patch indeed no longer applies, so it needs a reroll. Tagging with "Needs reroll". I removed the tag "Needs tests" as it seems to have tests already.
Comment #25
TR CreditAttribution: TR commentedThere are some major problems and omissions with the above patches.
I have put a lot of work into this and it's almost ready, but I haven't had time to work on it in the past few months. Assigning to myself. I'll try to finish this off soon.
Comment #26
dshields CreditAttribution: dshields commentedIt'd be great to get this working
Comment #27
Megha_kundar CreditAttribution: Megha_kundar at Specbee commentedComment #29
lukusJust adding that this would be a really useful piece of functionality.
Without this, the components have limited utility for my main use cases.
If there's anything I can do to help, please let me know.
Comment #30
rodmarasi CreditAttribution: rodmarasi commentedyes lukus. can you reroll a better patch?
Comment #31
rodmarasi CreditAttribution: rodmarasi commentedtest
Comment #32
marco5775 CreditAttribution: marco5775 commentedhello,
the Rules plugin is really important to configure my Drupal sites without programming.
Where are we with this "feature request"?
Thank you for everything you do ;-)
Comment #33
kopeboy CreditAttribution: kopeboy commentedSeconding this. I cannot do anything useful from the UI with Components right now if I cannot pass values from Rules nor calling the component from a View with VBO.
I can't code this but I would be very happy to sponsor your work to have this feature faster 🙏🏻
Comment #34
TR CreditAttribution: TR commentedThis fell off my radar last year. I still have the code, and if I recall correctly the only problem was that the ajax on the parameter add form wasn't working correctly. I'll see if I can reroll my patch and post it here as a first step.
Comment #35
TR CreditAttribution: TR commentedComment #36
kopeboy CreditAttribution: kopeboy commentedUp
Comment #37
alimbert CreditAttribution: alimbert commentedIs there an update on this?