-
-
Notifications
You must be signed in to change notification settings - Fork 96
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
Entity & Field plugins #155
Comments
Initial discussion about this happened a long time ago on the extension-side at jhedstrom/drupalextension#337 |
This sounds great. I'm not sure if it would be the same plugin system or not, but I'd like to split the individual drivers up into collections of plugins so they aren't these singular monolithic classes (I'm not sure of how that would work in practice). |
Entity and field plugins should serve to remove about 12+ methods from the
drivers, so they're a step on this process of taming the monolith. I have
wondered if the drivers should be collections of services, with the entity
and field plugin managers as two of those services.
…On 29 December 2017 at 00:40, Jonathan Hedstrom ***@***.***> wrote:
This sounds great. I'm not sure if it would be the same plugin system or
not, but I'd like to split the individual drivers up into collections of
plugins so they aren't these singular monolithic classes (I'm not sure of
how that would work in practice).
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#155 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ADWYQe2Nz5QT7cGi43CjhZVoipyWXSINks5tFDUSgaJpZM4ROwR5>
.
|
I've completed initial development of an entity and field plugin/wrapper system, see PR #157. BenefitsCustomisation: users can add special handling for new field or entity types, or override handling of standard types. Simplicity: we can get rid of all entity-type specific methods in the driver and extension. We can even throw away the whole hook system in the extension. Labels: identifying entity types, bundles, fields and referenced entities by labels rather than machine names is (more) possible and customisable. Cleaning up: we can specify additional tear down instructions for particular entity types. OverviewIn a nutshell if you want to create a new Drupal entity using the driver what now happens is:
From the point of view of the end-consumer, it's dead easy: you ask the driver for a new entity wrapper and then interact with that wrapper as if it were a native D8 entity. In the future the Drupal extension only has to know about that one method getNewEntity, we deprecate all the other entity-type-specific create & delete methods. Behind the scenes it's necessarily complex, and there are many possible implementation questions. Here I'll raise some general matters: StatusThis definitely needs testing on existing test suites as it's changing fundamental APIs and it would be easy to have missed a hidden consequence somewhere. There's a lot that's rough and debatable in the implementation, but it's fully backwards compatible and has extensive test coverage. Therefore I believe it's OK to commit as it, warts and all, and then tackle the specific problems in more manageable follow-ups. However, I can't spend mountains more time on this, and have hit some challenges that need other people's skills, so it might not be me completing those follow-ups though I'll be reviewing for sure. What belongs in Driver vs ExtensionCurrently the EntityReference field handler expands labels into id's, a feature that is enormously helpful for BDD. I've expanded that approach further, allowing labels instead of ids as widely as possible. In #116 @pfrenssen suggested that the current behavior of the EntityReferenceHandler is an abuse of the handler for a Behat use case, and that the role of the Driver is only to provide a version-agnostic Drupal API. If this is the case, then potentially we might want to keep all natural language parsing out of the Driver. In this case the Extension could have its own wrappers extending the driver's wrappers a little, and its own plugins adding to the driver's plugins and doing more aggressive processing of field inputs. There is some added complication, but not much. Discovery of project pluginsIdeally you could put plugins in your project Behat code folder, the one autoloaded based on what's specified in behat.yml: something like features/bootstrap/plugin/DriverEntity/ I've opened up an argument called $projectPluginRoot which is passed around amongst the driver, wrapper, pluginmanagers and plugins in order to facilitate this. But my small attempt to tickle the plugin managers into instantiating a plugin from this folder failed, probably because I don't understand the relationship between namespaces, plugin discovery, and composer autoloading. D6, D7, and DefaultPluginManagerDrupal 8's plugin component is available as a drupal-independent composer dependency, so we can use it with D6 & D7. However, I cut a corner and extended the DriverPluginMangerBase from Drupal 8's DefaultPluginManager. To get things working for D7 this need to be reworked to not have this dependency. I have created plugins for D8 (which is easy) but not D7 and D6. Creating the needed plugins is really pretty easy if you understand D7, which I do not. It's largely a matter of copying code from a current driver method into a new plugin class. Travis & test runningNow we have lots of nice kernel tests, it would be great if travis could run them. run-tests.sh won't discover tests using the @group annotation outside of drupal core and modules. Maybe php unit will help. Tickling Travis is not something I'm very good at so I haven't attempted this. There's currently a hardcoded path in DriverKernelTestTrait that will need addressing. validateDrupal in userCreateThere was a call to validateDrupal() in the driver's userCreate(). |
I'm most of the way there with my Christmas present: an initial rough implementation of
I'm thinking this should be for version 2.0; do you have a timeline for that or other things on the roadmap for it?
The text was updated successfully, but these errors were encountered: