Problem/Motivation
Provide a condition plugin system and unify before incompatible contrib APIs (CTools + Rules)
Proposed resolution
The implementation adds a condition system below \Drupal\Core\Condition, whereas general aspects factored out into \Drupal\Core\Executable. This can serve as common base for conditions and action plugins. This does not include "processors" yet, which are plugins that process data before it is passed to individual executables - think of token running token replacements. This is in particular necessary for actions, so it would be better flushed separately. Action API refactoring will happen at #1846172: Replace the actions API.
Follow-ups tasks
- Make sure configuration related methods end-up in its own interface. This methods already exists for blocks, so it's out of scope for this issue. Related: #1764380: Merge PluginSettingsInterface into ConfigurableInterface
- Allow grouping of provided conditions for UI purposes, e.g. to allow selecting them from a select with opt-groups. Should be done plugin-generic.
- Add a way for specifying access to plugins, such that available plugins can be filtered out based upon user access. Should be done plugin-generic.
- #1846172: Replace the actions API
- #1921540: Create a User Role Condition
- #1921544: Create a Current Path Condition
- #1921546: Create a PHP Filter Condition
- #1921550: Create a Language check Condition
- #1921568: Create a generic Add Condition FormInterface component
- #1921572: Create a generic Edit Condition FormInterface component
- #1932810: Add entity bundles condition plugin for entities with bundles
User interface changes
None. Conditions come with the possibility to provide a simple configuration form for their configuration options, which is to be embedded by the caller. Some usage of that can be seen at #1912452: Condition Group User Interface.
API changes
Well, it adds a new condition API :-) No changes to existing APIs.
Original report by EclipseGC
Blocks have a notion of visibility right now that we need to continue supporting, however the existing solution is completely ignorant of any sort of context, and needs to largely be rebuilt. As a replacement for this, a Condition Plugin type needs to be created, along with supporting condition plugins. The first pass of this could utilize data-source-less plugins that can rely on things like the request object (for example, determining which page your on to allow for blocks to appear on blog/* or something similar).
CTools' Access plugins are a virtually identical to what we want here. Rules & Context modules have a very similar analog. All 3 of these patterns are virtually interchangeable, and core has many use cases for this same tool. Blocks & Layouts absolutely needs this as well.
Eclipse
Comment | File | Size | Author |
---|---|---|---|
#72 | condition-api.patch | 50.29 KB | EclipseGc |
#72 | condition-interdiff.txt | 6.8 KB | EclipseGc |
#69 | condition-api.patch | 49.34 KB | EclipseGc |
#69 | condition-interdiff.txt | 3.34 KB | EclipseGc |
#65 | condition-api.patch | 48.99 KB | EclipseGc |
Comments
Comment #1
fagoGreat! Dumping my thoughts about how this should work here:
Obviously, conditions (or actions) should be plugins. Then, I think the definition should look somehow as it does for Rules now, in particular it needs to describe the required parameter. E.g. this is how it looks in Rules now:
I guess most of it will be covered by plugins already, so parameters really is what we need to take care of. Ideally, this should work inline with the way we describe entity properties. See #1696640: Implement API to unify entity properties and fields.
Comment #2
EclipseGc CreditAttribution: EclipseGc commentedHow to:
Comment #3
amateescu CreditAttribution: amateescu commentedThe attached patch provides the initial structure for this plugin and an example of how a condition will be implemented.
I'm posting it in this state because I'll be away for at least one week and maybe someone wants to take it over :)
Comment #4
klonos...coming from #1211396: [META] Getting started with Rules 8.x
Comment #5
jide CreditAttribution: jide commentedNice to see this first stab.
I know the implementation is just an example, but I'm worried by the way this is implemented.
Will we implement all kinds of little plugins providing precise cases like "node is of type" ?
Couldn't we imagine using a more generic approach like :
entity -> of type "node" -> whose bundle is "blog" ?
And we could go even further :
entity -> of type "node" -> whose bundle is "blog" -> and has a property "title" which contains "This is a test" etc.
Obviously, that's more work on the UI side, but it's worth it IMHO.
I've always thought that a lot of Drupal paradigms around these definitions have something wrong when we have to declare in code things we already know through APIs.
Comment #6
jide CreditAttribution: jide commentedAhem... I realize @fago mentionned the forthcoming Entity Property API and how the Condition Plugin System should play nice with it, but I'm still curious about how all of you see this on the plugins implementation side, and how generic this could eventually be.
Comment #7
Gábor HojtsyIf this is made pluggable / reusable elsewhere, it could become a really useful system for things like #1787942: Allow assigning layouts to pages.
Comment #8
EclipseGc CreditAttribution: EclipseGc commentedThat is 100% the intent. I already have a plugin UI in the #1535868: Convert all blocks into plugins patch with the intent of reusing it here and allowing for very generic condition groups to be used across the entire system.
Eclipse
Comment #9
jide CreditAttribution: jide commentedGreat !
Comment #10
EclipseGc CreditAttribution: EclipseGc commentedTagging
Comment #11
mitchell CreditAttribution: mitchell commentedI searched for a library that provides type-checking and found Underscore.php. I think it makes sense to use this, because TypeData API doesn't provide this required functionality.
The library doesn't add support for 'node is of type' but it has many operations that are useful in conditions and condition groups, like isDate, isString, isEmpty, isNull, ..., union, intersection, and difference, and more. For the proposal, see #1803734: Add Underscore.php to Core or another fluent array manipulation wrapper library, and for a Rules use case, see #1809470: Use Underscore.php for data and list operations.
---
The "AND" operation in #5, when implemented in condition groups, could be built similar to Rules' evaluation engine or using Underscore's intersection (logical conjunction). "OR" would be union or (logical disjunction). (Edit: linked to set-theoretic functions.)
In Rules, condition groups are called, "components", which I'm hoping that despite the difficulty in arranging to call these plugin-instances "components", it will considered. IMHO, it could transform Drupal's 'site builder' interface paradigm into that of an 'application composer', where components are built and streamed together within the admin UIs.
I find the prospect of using Plugin UI and Entity Forms (with ConfigEntity) very interesting as a foundation, and I'm sure others who have worked on meta-module UIs in D7 would agree. Perhaps with Backbone.js and jQuery UI, we could more easily build graphical interfaces, as well.
----
Also, beyond the obviously interest from a Rules perspective, the condition plugins are also being mentioned as having uses in additional contexts. Perhaps not far from #7, is an idea I've had about using Rules + Conditional Rules, or something like it, when entities are built to create "complex entities" & "dynamic fields". For example: 'on user profile edit form view, if request-method is [x], and user is of group [y], then add field [z] to form [a], otherwise use default value of [v].'
chx mentioned in his blog at http://www.drupal4hu.com/node/338 that "Back in Paris bojanz had this crazy plan of unifying the condition interfaces of DBTNG, EFQ and Views." Perhaps with #1763974: Convert entity type info into plugins and 'if plugins are compiled to dependency injection's cache', this could all work at bootstrap.
With those ideas in mind, I asked about plugins being able to have routes in #1801570: DX: Replace hook_route_info() with YAML files and RouteBuildEvent. I think having these conditions' results as executables on knowable URLs would help with building something like Rules in D8. We could then leverage the Kernel as an evaluation framework instead of porting Rules' state and data evaluation engine.
Comment #12
fmizzell CreditAttribution: fmizzell commented@mitchell I am not very familiar with underscore.php/js, but I am not sure that I understand what "TypeData API doesn't provide [type checking]" means. We have classes that represent our primitives, so we could easily require that data of a given type is used.
function operation(Integer $x, Integer $y){}
But to have a general conditions system, I guess that we need the Type Data API to support comparison operators, so we could simply do
So, lets say that we have a condition "node is of type article", our general coditions system will do the following:
I guess the point is that we don't want to have to create a plugin for every type of question that we want to ask the system, but instead we want that magic method/subsystem that knows how to retrieve the necessary info from our entities or from any other complex data that is using the property api system to expose the structure of its data, and maybe an extension to the typed data api to allow for comparisons. So it sounds to me like we need a way to store the definitions of "conditions", but not necessarily what the current code does: allow the retrieval of code that was written representing a condition.
Comment #13
fagosee #1846172: Replace the actions API
Comment #14
EclipseGc CreditAttribution: EclipseGc commentedOK, this patch provides what I think is likely to be a good starting point here. It provides a UI, depends on #1886616: Multistep Form Wizard and has a working API at its core.
Next steps:
We need to separate the UI from the API. I'd like to keep the plugin type in the conditions module. We just need to come up with a logical way of removing the UI and putting it in a follow up patch.
Only node_type has been converted, and we likely should stay with just that on this first run through.
Conditions should be unit testable, so we need some unit tests on the current plugin.
Eclipse
Comment #15
podarok#14 looks good
Comment #16
xjmLet's add those tests. Also, there are some whitespace issues. Leaving at NR though for architectural review. Hopefully @sun will approve of my use of his tag. ;)
Comment #17
EclipseGc CreditAttribution: EclipseGc commentedI didn't realize that was the spell to summon him. Duly noted.
Eclipse
Comment #18
sunComment #19
effulgentsia CreditAttribution: effulgentsia commentedComment #20
EclipseGc CreditAttribution: EclipseGc commentedOk, no tests, but this is the basic api. I think I managed to remove all the UI components. I'll post those in a follow up.
This provides a node_type condition implementation. The form validate and submit components are all there but won't be usable until the UI is in. This patch depends on (and slightly modifies) #1896076: Contextual Plugins and supporting code. I'm not 100% sure of that modification as of yet and am waiting for feedback from the TypedData experts, but for the sake of this patch in its current state, it works.
If you had an existing node and wanted to validate that that node was an article, the condition api could be invoked as follows:
There are a number of other ways this same condition could be invoked, but this is the clearest and easiest to read.
Eclipse
Comment #22
EclipseGc CreditAttribution: EclipseGc commentedThis is still dependent upon #1896076: Contextual Plugins and supporting code (which I'm hoping is committed very very soon). Added in execute tests for node_type condition. I need to add summary tests and then we need to expand the docs on processors and probably add one of those (or fago needs to concede that we can add it later, I have no opinion since I've not used it, so I need him to weigh in here and give guidance).
These patches aren't going to work until #1896076: Contextual Plugins and supporting code lands, so if you want to play with this stuff you need to be applying that first.
This is up to date with that patch as of comment #56.
Eclipse
Comment #23
EclipseGc CreditAttribution: EclipseGc commentedAdded some summary tests and got this patch working against comment #59 in #1896076: Contextual Plugins and supporting code
Eclipse
Comment #25
fagoShould be "NodeTypeCondition" for making sense of the class name alone.
Architecturally, let's discuss some points and figure out to deal with them. Not necessarily in this patch, but just to have a plan:
We previously had something as the following pseudo-code:
I guess we'd have to extend the context classes for supporting selectors?
This is what we had for processors:
I think we should implement the token processor as first core-example.
We need form generation to be pluggable/overridable such that Rules can squeeze all in its features afterwards.
Also, do we need a conditions module or shouldn't it be just an (core) API component?
We should base validation upon typed-data validation and do form-validation based upon that. So we can validate any provide plugin-configuration is correct. This could also be the base of a sanity-check to verify configurations are still valid after module enable/disable.
Let's discuss :-)
Comment #26
fagoNote: working on it :)
Comment #27
fagoShould we enable processors to work generally with contexts or tie them executables only? I guess there is no reason to tie them to executables only, so what about moving it to Drupal\Core\Plugin\ContextProcessor ? -> they really just process context values.
Comment #28
fagook, discussed quite a while with EclipseGC on IRC. We figured out already quite some details, we need to discuss form-generation more though.
Here is a first interdiff which takes care of moving execution control to the managers. I'll take a stab on validation afterwards.
Note: I've started using Git for rolling this patches, and just added it to my entity-api sandbox:
http://drupal.org/sandbox/fago/1497344
Branches:
condition-base (8.x base)
condition-fago
context (depending patch)
Feel free to use condition-yourname branches there for any changes.
Comment #29
fagook, so finally here is working patch with validation re-worked based upon symfony validator.
Other changes:
- It turned out the 'bundles' parameter did not use the context yet. I fixed that, what resulted in problems as typed data validation is not yet able to properly validate that due to missing generic list-classes and choice validation. Turned out fixing that is too much to incorporate here, so moved that to its own issue: #1913334: Support choice based validation
- So, for being able to make any interim context definition that validates for the bundles context, I've introduced an "any" data type which validates with anything. Having such a data available is generally a good idea imo. For adding a generic list type that would allow us to do a more specific context definition here I've added #1913328: Provide general list and map classes.
- I've added @throws documentation to the methods throwing exceptions, however it turned out it is pretty nasty having them all throwing exceptions when a value should be pre-configured during configuration time. For now I've re-worked and documented getContextValues() to not throw exceptions so you can use that during configuration time. However, we still miss a way to initially set a value without firing validation, what would be needed in extractFormValues(). We could support getting empty (NULL) context objects if they are not set and just throw the exception in getContextValue() ? Or we remove auto-validation from the plugin-base setContextValue()?
Note: This probably needs an update in UI code to use extractFormValues() and trigger form errors for validation violations from validate(). They do not contain correct property paths yet (code has a todo), I think this can be taken care of as a follow-up.
Comment #30
EclipseGc CreditAttribution: EclipseGc commented#23: condition-api.patch queued for re-testing.
Comment #31
podaroka small typo negaeted.
Comment #32
fagook, after letting this settle a while, I think we should do the following:
- Have $context wrapper instances even if the context is not set, but have a NULL value in this case. This would be inline with what typed data (and PHP) does, NULL means not set. However, having the objects available always results in simpler code and allows us to do the following:
- Use $context wrapper for accessing context values without any auto-magic exceptions or validations - but have simple API methods for every operation, i.e. get, set, validate. Those don't throw exceptions if validation fails or the context is unset, so we can use this API safely during configuration-time of the plugin.
- Have all the auto-magic exceptions and validation logic in $plugin context methods like $plugin->getContextValue() and setContextValue(), such that you can simply rely on them during execution time and everything unexpected throws an exception.
Makes sense?
Comment #33
fagoNext, I gave the context mapping case some thought also. What about this:
Comment #34
fagook, talked through some details and differences of our understanding with EclipseGC on skype. Conceptually, Rules works different of what EclipseGc has in mind in that it treats all parameters as the same while EclipseGc wants to clearly separate values stemming from context and configuration.
In order to move on here we agreed upon:
For validation we'd agreed upon leaving plugin-level configuration validation out for now. This is something Rules 2 already does and so will continue to do in Drupal 8, but maybe this could be covered via config-entity validation already.
This, together with the already discussed possibilities to
always Rules to continue its job in contrib, but based upon *the same* condition system. That way we would reach the ultimate goal to avoid ending up with to separate conditions API which developers have to choose from.
It would probably
After writing this I end up seeing a potentially problematic issue we did not discuss during the call: Processors. If conditions and actions written end-up having to care about stuff like token replacements theirself, Rules won't be able to apply it's pluggable processors without having stuff like that done twice. I guess we could implement a processor for tokens that gets configured via the regular plugin configuration?
Also, I still hope/think that the result of this ends up being usable for actions also, such that we can reproduce the same API for actions: #1846172: Replace the actions API
Comment #35
EclipseGc CreditAttribution: EclipseGc commentedas Fago mentioned, we had a long conversation today. This patch does not contain everything we talked about and is just an incomplete hybridization of my code and his. I will continue this tomorrow pushing towards the details that fago laid out above. I'm doing this for myself so that I know all the changes that he's made and why I adopted them.
In this patch a lot of code was moved around. I abandoned the condition module in favor of Drupal\Core\Condition which is probably the appropriate thing to do here. I embraced a few things fago has done like the ExecutableManagerInterface and moving execute() up into the manager. The primary reason for this is to that system can take over these high profile methods in certain situations.
Yet to embrace:
Moving the validation of context around. I'm pretty much on board with fago at this point. What we specced was that constructor injection will not validate, and method injection will validate. We both essentially get what we want this way. I'm still reading through his code on this and will need to do a little additional hybridization in order to meet the goals set forth here.
Need to document the configuration's data type definition in the annotation. This shouldn't add significantly to most condition annotations and most only have one or two fields. This may get more interesting in more complicated scenarios. I think this deserves a bit more discussion in the long term, but in the short term it makes sense to flesh it out and see where this goes.
I didn't include every little bit of everything in fago's previous patches. This doesn't mean I'm rejecting it, it just means I'm trying to make sure I understand everything, and there are definitely some pieces of code that we would both agree don't belong any longer. I'm just trying to make sure I weed that stuff out appropriately. Again this is in a transitional state.
The interdiff is from my patch in #23.
Eclipse
Comment #37
fagoThis should be called formValidate() so it clearly differs from validation not tied to forms which usually is called validate() already, e.g. we'll have $entity->validate() which is not form validation.
Yes! Good move, thanks.
So what about that?
Comment #38
sunMeh, as predicted... I already mentioned in #1913084-20: Introduce a Form interface for building, validating, and submitting forms that the new FormInterface should use
buildForm()
,validateForm()
, andsubmitForm()
method names, so that they are self-descriptive and do not clash with otherbuild()
andvalidate()
method names. The ExecutableInterface here should really just re-use the FormInterface.Comment #39
EclipseGc CreditAttribution: EclipseGc commented@sun
I've used the FormInterface pretty heavily thus far and I have some reservations about plugins in general extending it. They may be unwarranted, but I'd like to experiment some more before we just make that a blanket suggestion. Still renaming the method names is probably appropriate in this case. I'll play a little more and see where this goes.
Odd that we got so many failures. They all look random to me except for the actual node type condition test, and that passes locally for me so... weird.
Eclipse
Comment #40
fagoAfter thinking more about whether Rules could work without switching input modes and so configuration metadata, I think it would work in some simple cases like some optional boolean options of an action/condition. However, there are use-cases where Rules needs it: For example the "Send an e-mail" action: Usually message subject or message text are pre-configured by the users (and include tokens), but Rules allows you to switch input modes and pass in another e-mail text from your context, e.g. from some message-template entities maintained outside of rules. Having this possibility without the need of creating another action for that is really was makes Rules so flexible and powerful.
So no, we won't get away without that.
@EclipseGc: So in the above example, the message mail text being pre-configured but having token replacements / processors, would that be context or configuration for you?
Comment #41
EclipseGc CreditAttribution: EclipseGc commentedIt's configuration, but there's nothing preventing you from injecting configuration at run time. In fact you are very welcome and encouraged to do so if that fits for your use case. You don't HAVE to configure at creation time, and you can override existing configuration at any point before execution, so it's really just a matter of which method you use to do it. Since your example is saying "something outside rules can invoke the rule and then override its configuration" I'm 100% ok with saying plugins can handle that today. as an example, assuming rules is ultimately a configuration entity, and that entity has an actions property that contains preconfigured plugins you want to fire:
The plugin's now updated, so when the rule fires, the email message will be what the implementation specified. I assume there'd be a $rules->execute() or something at some point that checks conditions and fires actions. Long as this happens first, you should be golden.
Eclipse
Comment #42
EclipseGc CreditAttribution: EclipseGc commentedAlrighty, this is a LOT of changes, but I think we're actually closing in on the mark now. This allows for opt-in documentation of the configuration values in the plugin definition. The node_type plugin can't use it yet since it requires a list and I haven't seen any plugin in typed data that handles that. In scenarios such as these validation of the configuration value will automatically pass.
Configuration validation is done the same way context validation is. This will only happen when using the setConfig() method. Constructor injected configuration and contexts will bypass validation completely. Setter injection will cause validation to be run before the context/config key specified is set.
I removed all processor stuff. It's completely not fleshed out yet and it has wider implications for any plugin that might want to use context processing. I think we can totally justify doing it in a follow up task.
I've provided two interdiffs: one from the patch in 35 and another from the patch in 23 (just to see how this has changed). This adopts all of fago's changes that moved the automated validation from the Context classes to the ContextAwarePluginBase classes. I think this is pretty close to where we need to be to satisfy everyone's concerns. I don't have testing coverage on a few things in this yet, but in general it should work. I am completely baffled by why the patch in 35 failed, it passes locally so... w/e I guess. We can track that down next.
Eclipse
Comment #43
EclipseGc CreditAttribution: EclipseGc commentedComment #44
EclipseGc CreditAttribution: EclipseGc commentedOH! this main patch is also dependent upon the patch in #38 of #1913084: Introduce a Form interface for building, validating, and submitting forms
Comment #46
EclipseGc CreditAttribution: EclipseGc commentedNo surprise there since it depends on another patch.
Eclipse
Comment #47
sunIt's unusual to bump a feature request to critical, but it is my sincere belief that the lack of these condition plugins will only cause us to duct-tape and doctor around architectural limitations for the coming 1-2 years until release.
The condition plugins in combination with the recently committed context plugins will allow us to solve architectural challenges that would be very hard to solve otherwise. I'm primarily thinking of #1839516: Introduce entity operation providers and similar issues. That may seem to be a far stretch based on this issue's perspective, but from what I can tell, these issues are complementary to each other from an architectural perspective.
I think we absolutely need this facility in order to make (more) sense of D8.
Comment #48
jthorson CreditAttribution: jthorson commentedBot is re-testing #42 now ... the patch apply message was a bot failure.
Comment #50
andypost2 broken tests needs re-roll
Comment #51
EclipseGc CreditAttribution: EclipseGc commentedIncluded fago's updates to the context tests I missed. Also one patch is with the form interface and the other is without. hopefully the one with will pass all tests now.
Eclipse
Comment #52
EclipseGc CreditAttribution: EclipseGc commentedComment #53
fagoI worked over the patch and mostly concentrated on documentation and ensuring everything makes sense. I found a few glitches on the way which the attached patch corrects, see attached interdiff.
Fortunately the FormInterface patch got committed meanwhile, so attaching just the patch + the interdiff.
Comment #55
EclipseGc CreditAttribution: EclipseGc commented#51: condition-api.patch queued for re-testing.
Comment #56
fagoPatch applies for me to latest HEAD, thus re-uploading. Maybe the test-bot has some sort of caching and misses the latest changes?
Comment #57
fagoI've added an issue summary, please review/correct.
Comment #57.0
fagoUpdated issue summary.
Comment #59
fago"file_put_contents(): Only 0 of 93 bytes written, possibly out of free disk space", grml looks like test-bot node is broken.
Comment #60
fago#56: d8_condition.patch queued for re-testing.
Comment #61
EclipseGc CreditAttribution: EclipseGc commentedI've not added in fago's changes here yet. This just adds buildForm/validateForm/submitForm coverage and uses the NodeType as the example. Digging through fago's changes now. The interdiff is from my last patch.
Eclipse
Comment #62
EclipseGc CreditAttribution: EclipseGc commentedis not ist, that's a Diablo II rune. ;-)
Comment #63
fagoContains \Drupal..
implements hook.
Missing docs.
Implements interface docs.
Comment #64
fagoActually "ist" is German for "is" - damn. ;-)
Comment #65
EclipseGc CreditAttribution: EclipseGc commentedthis patch includes all of fago's changes in the previous interdiff he posted, I fixed the minor typo I mentioned in the previous comment, and added and corrected documentation to all my form test coverage. Much of fago's previous patch includes some common sense changes to the context aware plugin system, and since conditions is the first plugin to make use of that, I generally support doing most of this here.
I think we're pretty close at this point. Feedback very very welcome.
Interdiff is from my last patch and will include much of fago's previous interdiff.
Eclipse
Comment #66
EclipseGc CreditAttribution: EclipseGc commentedif this passes, my next patch will include fago's "Contains" doc issue, otherwise I got the others.
Eclipse
Comment #67
fagohm, an interdiff to my patch would have helped me more. Anyway, I looked at the whole patch again and stumbled over that:
executable plugin exception class
I missed removing this @see before. $configuration is protected and so does not matter to the API consumer at all.
Anyway no reason to hold this up on this on this minor documentation fixes.
I think this is overall ready and I'd call it RTBC from my side given tests come back green.
Comment #68
EclipseGc CreditAttribution: EclipseGc commentedI have one more pass to make on this that adds in the configured form execution test so I'll get those changes in it. It'd be good to have that test, still it won't be any architectural change over what's here, just an extra test and cleanup.
Eclipse
Comment #69
EclipseGc CreditAttribution: EclipseGc commentedok, what I hope is the last patch. Adds in the test I missed previously and fixes fago's doc issues above.
Eclipse
Comment #70
EclipseGc CreditAttribution: EclipseGc commentedand per fago's rtbc, I'd like to suggest that extends to this patch since there aren't any architectural changes, and all I did was add a new test and fix some docs.
Eclipse
Comment #71
effulgentsia CreditAttribution: effulgentsia commentedSome of this might be follow up material or invalid feedback, so not changing status, but here's my thoughts upon reading this patch for the first time. Overall, great work! Very much excited about seeing this land.
ExecutableInterface doesn't include validate(), so doc is wrong. Beyond that, I don't see why we want validate() on ContextAwarePluginBase. It doesn't look like the patch makes use of it. It seems out of scope for this issue: can we defer it to a follow up?
As above, this seems odd for ContextAwarePluginInterface.
Maybe I'm missing something, but the entire Drupal\Core\Executable package seems like over-abstraction to me. What are use cases for it other than Condition?
This seems useful to push down to ContextAwarePluginBase.
There are so many todos here that this just doesn't seem baked yet. Can we remove all of this and deal with it in a follow up?
Doc is wrong.
Comment #72
EclipseGc CreditAttribution: EclipseGc commentedDiscussed this at length with effulgentsia on skype. I tried to justify a number of these things and overall he seemed ok to file follow ups, however the validate() method on ContextAwareInterface didn't make sense as is because it's disconnected from the unvalidated __construct that was on ExecutablePluginBase. As such I moved that construct to the Core and Component ContextAwarePluginBase classes, and renamed validate() to validateContexts(). I also fixed the docs issue effulgentsia found and fixed one I also found.
Eclipse
Comment #73
effulgentsia CreditAttribution: effulgentsia commentedI filed #1920816: Factor getConfig(), setConfig(), getConfigDefinition(), and getConfigDefinitions() out of ExecutablePluginBase and #1920822: Decouple ExecutableInterface from Conditions and FormInterface for the two things I still think are problematic, but those will need input from fago, and it's very late night / early morning in Europe now, and I don't think they should hold up this really valuable addition to Drupal (see #47). It'll be great to get block visibility and actions API using this in core, and Panels, Context, Rules, etc. all reusing this from contrib. Therefore, RTBC, assuming bot is happy.
Comment #74
webchickI'm re-categorizing this as a major task since it seems to block a major task, according to #47.
Kris took a good half hour to walk me through this patch on IRC. While I can't guarantee that I understand every line of it, most of it was pretty straight-forward. This also blocks other stuff and has the effulgentsia seal of approval! Ergo...
Committed and pushed to 8.x. Thanks!
Back to critical task + active for the change notice.
Comment #74.0
webchickUpdated issue summary.
Comment #75
podaroktags due to #1846172-17: Replace the actions API
Comment #76
fagowoohoo - thanks everyone! I responded to the created issues.
Comment #77
jibranchange record is still pending.
Comment #77.0
jibranUpdated issue summary. Added follow-up
Comment #77.1
EclipseGc CreditAttribution: EclipseGc commentedadding follow up condition plugins.
Comment #78
jibranAs NodeType condition is created here. So I created #1932810: Add entity bundles condition plugin for entities with bundles which is related and possible followup.
Comment #79
podaroktag
Comment #80
xjmThere's still no change record draft here. :)
Comment #81
EclipseGc CreditAttribution: EclipseGc commentedChange notice is here: http://drupal.org/node/1961370
Comment #82
EclipseGc CreditAttribution: EclipseGc commentedComment #83
ParisLiakos CreditAttribution: ParisLiakos commentedrestoring category and removing tag
Comment #85
fagoFYI: Opened #1976758: Plugins miss metadata about configuration
Comment #85.0
fagoadding form component follow ups
Comment #85.1
jibranAdded 1932810 to summary.
Comment #86
josephcheekComment #87
AnybodyI'd like to use the condition plugin system for other entity types than blocks. To be precise I'd like to add ot to a node type (header image) to determine when a view (filter) shell show that node as header image in a slider.
Is there any documentation how to use that side of the condition plugin? I found out how to add custom conditions but nothing about that part yet. I could imagine to create a condition field for that as general module solution with a view filter evaluating it agains the current page context.
Can someone help with a link or code example?
Comment #88
larowlan@Anybody https://www.previousnext.com.au/blog/using-drupal-8-condition-plugins-api may help