Problem/Motivation
We use payment types for bundles, so payment types can add fields to store additional data. Payment types' configuration is saved to the database as well, which means that is an alternative way to store payment-specific data. This might not be entirely obvious to developers.
On top of that it's become impossible to enforce synchronization between payments' bundles and payment types since #2134761: Convert plugins on payment entities to fields was fixed, unless we add a check to PaymentTypeItem
.
Proposed resolution
Remove bundles. Payment types can still store payment-specific data through their configuration (forms).
Remaining tasks
Remove the bundles.
User interface changes
Fields can only be added for all payments and no longer to specific bundles.
API changes
Fields can only be added for all payments and no longer to specific bundles.
Comments
Comment #1
XanoOne DX drawback of this is that creators of payment types will lose the advantage of getting automatic Views integration. Just like when writing payment method, payment status, or line item plugins, they will have to write custom integration.
Comment #2
andrewbelcher CreditAttribution: andrewbelcher at FreelyGive commentedI think I'm +1 to this. Adding a payment type to store information isn't an ideal solution.
Perhaps the views scenario could be solved by including a viewsData() method on the payment method where you can define the types and then Payment can make use of this to expose the information in the right way to views? I think that would result in a DX improvement. If developers aren't bothered about exposing data then they just leave it to the default implementation which exposes nothing?
Comment #3
XanoViews integration is a separate issue, and can always be done by implementing
hook_views_data()
. What I meant in #1 is that fields have automatic Views integration out of the box. If we store data on payment entities directly (like we do with payment methods), we lose this, and we'd have to add custom code to add it back, which is possible, but also bad for DX.Marking this as postponed, as we cannot change this until the next major version. The discussion can continue, though.
Comment #4
BerdirIt is worth nothing that https://github.com/fago/entity/pull/49 is a PR to add this concept as a standard feature in the entity.module and that exists because commerce uses this too.
So at least the "this is uncommon and not how core does it" argument for having plugins as bundles is kind of invalidated with that :)
I know that people dislike having bundles that are not config entities but I think this actually shows a valid use case for why we support that.