Plugins now use the 'context_definitions' key to define their contexts

All Rules Conditions will need to change this in their annotation. All contributed modules that integrate with Rules will also need to change this. Rules core will need to be changed to parse the new 'context_definition' annotation, as well as support the old 'context' annotation.

The @RulesAction annotation definition, which also uses 'context', should also probably be changed to be consistent. That should probably be handled in a separate issue which can be opened when the above is complete.

We have to wait to change this in Rules until core 8.6.x is no longer supported.

CommentFileSizeAuthor
#15 3030295-15-conditions-and-events-context-descriptions.patch39.93 KBtr
#13 3030295-13-conditions-and-events-context-descriptions.patch39.98 KBtr
#12 3030295-12-conditions-and-events-context-descriptions.patch39.26 KBtr
#11 3030295-11-conditions-and-events-context-descriptions.patch34.37 KBtr
#10 3030295-10-conditions-context-descriptions.patch26.96 KBtr
#9 3030295-9-conditions-context-descriptions.patch24.22 KBtr
#8 3030295-8-conditions-context-descriptions.patch12.63 KBtr

Comments

TR created an issue. See original summary.

tr’s picture

tr commented

Drupal 8.6.x will be supported until Drupal8.8.0 is released, which is scheduled for December 4, 2019

We can't make this change until Drupal 8.6.x becomes unsupported.

tr’s picture

tr commented
Status: Postponed » Active

We're a month away from being able to commit this, thus I'm re-opening this so we can work on a patch and get it tested and ready to commit once 8.8.0 is released.

jonathan1055’s picture

jonathan1055 commented

Would it be possible to change the title either in this issue or #3092085: 'context' deprecated in @Condition annotation because currently they are identical.

Edit: I did not initially see that this issue is for Rules and the other is for Rules Essentials.

tr’s picture

tr commented
Title: 'context' deprecated in @Condition annotation » 'context' deprecated in Rules @Condition annotation
Assigned: Unassigned » tr

OK. And I assigned it to myself, since I did most of the work yesterday (still trying to figure out why one of the tests fails). It's a huge patch (>1200 lines) as you might guess, because Context is everywhere in Rules.

jonathan1055’s picture

jonathan1055 commented
Parent issue: » #3089502: [meta] Rules deprecated code

Adding meta parent

tr’s picture

tr commented

I will post the patch for this AFTER core 8.8.0 is released - this is a big change and I'd like to get all the little ones that are also waiting committed first so I don't have to be constantly re-rolling this big patch.

tr’s picture

tr commented
Status: Active » Needs review
StatusFileSize
new 3030295-8-conditions-context-descriptions.patch12.63 KB

Let's do this incrementally, like we did with the @RulesAction context. Here is a simpleminded patch which just changes the annotations but doesn't change any other code. I'm posting this to see the extent of the test failures on the testbot.

Note, core 8.7+ has a BC layer for context_definitions so just changing the annotations shouldn't break anything.

tr’s picture

tr commented
StatusFileSize
new 3030295-9-conditions-context-descriptions.patch24.22 KB

Now with some code fixes, which SHOULD break something if I missed some changes.

tr’s picture

tr commented
StatusFileSize
new 3030295-10-conditions-context-descriptions.patch26.96 KB
tr’s picture

tr commented
StatusFileSize
new 3030295-11-conditions-and-events-context-descriptions.patch34.37 KB

Although the tests pass, deprecations are still triggered (as described in the change record). See #3089502-16: [meta] Rules deprecated code and following.

My guess is that these addition "context" keys are coming from our Event plugins, which don't use annotation but rather .yml files for discovery. Thus, I think we need to change the "context" key in our *.rules.events.yml files and Event plugin derivers.

Here is a new patch that combines #10 with changes to the event .yml to change context to context_definitions, and the corresponding changes to the event handling code.

tr’s picture

tr commented
StatusFileSize
new 3030295-12-conditions-and-events-context-descriptions.patch39.26 KB

Again.

tr’s picture

tr commented
StatusFileSize
new 3030295-13-conditions-and-events-context-descriptions.patch39.98 KB

And again ...

tr’s picture

tr commented

The patch breaks the autocomplete JavaScript. I will look into this.

tr’s picture

tr commented
StatusFileSize
new 3030295-15-conditions-and-events-context-descriptions.patch39.93 KB
jonathan1055’s picture

jonathan1055 commented
Status: Needs review » Reviewed & tested by the community

Tested this on Travis , all tests pass and there are no more 'Providing context definitions via the "context" key' deprecation warnings. This is RTBC I think.

  • TR committed 6a2d830 on 8.x-3.x 
    Issue #3030295 by TR: 'context' deprecated in Rules @Condition...
tr’s picture

tr commented
Status: Reviewed & tested by the community » Fixed

OK, committed.

jonathan1055’s picture

jonathan1055 commented

Just to let you know, the recent commits break other contrib modules which are providing Rules Events, RulesActions and Conditions - see #3101377: Argument 2 passed to Drupal\rules\Engine\RulesComponent::addContextDefinition() must implement interface Drupal\rules\Context\ContextDefinitionInterface

This is not a surprise, and I know Rules is allowed to do this as it is only on Alpha release status.

tr’s picture

tr commented

Yes, those changes will be in the release notes for the next alpha. I also updated all the documentation at the same time I made those commits.

One of the things I use D8 Rules Essentials for is to act as a prototypical Rules ecosystem contributed module, so I have to deal with (and thereby know about) all the side-effects of any core Rules changes.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.