Problem/Motivation

Separating backend APIs from optional UIs is a core development principle of Drupal Commerce. However, by including data handling in UI modules, Commerce follows a nonstandard approach that presents both user and developer experience issues.

There are two established design patterns to UI provision in Drupal core and contrib:

  • Provide API and UI in a single module. Examples: Image module (core), Profile2.
  • Provide APIs and data handling in one module and UI in a dedicated UI module. Examples: Field and Field UI (core), Views and Views UI, Context and Context UI. In this second pattern, the UI module can be enabled if and when there is a need for UI handling of data objects, but disabling the UI module leaves the data and functionality fully intact.

These two patterns are explored in the core issue #937814: [meta] Smarter UI/API separation for modules.

In contrast to these two patterns, Commerce takes the API purity one step further: API modules should be free of even the assumption of a database layer for entitites. Hence commerce_product_ui owns $schema['commerce_product_type'] and commerce_tax_ui owns $schema['commerce_tax_type'].

However, this non-standard approach has several important drawbacks that may have been partially overlooked in the initial design decisions and that seem to conflict with other established Commerce development objectives:

    Pluggable UI modules. Commerce aims for UI modules to be optional and hence easily replaced by other solutions. But including schema definitions in the UI level interferes with this. E.g., if a site admin disables and uninstalls a default UI module to replace it with a different contrib one, her/his data disappear.
  • Strong exportables and distro support. Commerce aims to have strong support for exportables and distribution usage. But including schemas in the UI level interferes with established patterns and APIs for exportables, significantly complicating exportables and distribution usage.
  • Performance issues. Commerce priorizes performance. Part of the logic of standard UI/non-UI division is the option to improve site performance by disabling UI modules when they are not in use. But pushing schemas to the UI level means UI modules cannot be disabled without losing functionality. See catch's comments on performance issues related to UI modules.
  • Using core and contributed module APIs. Commerce aims to make the best use of available core and contributed module APIs, but the current UI approach doesn't take advantage of the well developed UI API available in Entity API (or, alternately, the export_ui available in Ctools). Switching to use the Entity API or Ctools APIs would require some refactoring, which might include addressing the location of schema info.

Additional issues with the inclusion of schemas in UI modules:

  • Unexpected outcomes for site administrators. Site administrators used to e.g., Views will be accustomed to being able to disable UI modules without losing data and functionality. For example, the Product UI description gives no cue that disabling it will break site functionality: "Exposes a default UI for Products through product edit forms and default Views."

Proposed resolution

Options for resolving this issue include:

  • Move schema information from UI to API modules. This is the simplest way to bring Commerce more in line with standard Drupal architecture.
  • Convert to Entity API (or Ctools) exportables UI and make this default in the API module. In this approach, we would remove the current UI modules for exportables and instead directly implement Entity API (or Ctools) APIs in the API modules. Profile2 is an example of a module that takes this approach. This apporach has the drawback of making pluggable UIs more difficult, as a particular UI would be bundled with the API.
  • Pull schema ownership to a separate set of modules. E.g., commerce_product_db or commerce_tax_db._ui would depend on _db modules, which would in turn depend on the api modules. This approach would retain the agnosticism of the API modules while not burdening UI modules with schema handling. However, it would further the proliferation of modules and complicate dependency chains.

Remaining tasks

  • Decide between approaches outlined in Proposed resolution, above.
  • Implement.

User interface changes

Would not directly produce UI changes but could facilitate e.g. #1319818: Exportables and Features integration for Drupal Commerce.

API changes

Schema definitions for storage of configuration-type entity or bundle data would be pulled from UI modules.

Comments

damien tournoud’s picture

damien tournoud commented

Thanks for the detailed analysis. A you mention, this is purely by design. The tables you noticed are for stuff creating from the UI. There are corresponding API-level hooks for both of them.

Same idea as: hook_node_info() vs. the {node_types} table. One is an API-level hook for defining node types, the other is the table in which are stored the node types created from the UI.

Because of this, the approach definitely doesn't actually go against two of the goals you mention: (1) Pluggable UI modules and (3) Performance issues.

Of course, it is slightly non-standard. At the time the base of Commerce was developed, CTools exportables were not yet ported to D7, or it would have likely be our go-to choice.

The problem here is that all the three possible scenarios have serious drawbacks:

  1. Moving the two info tables into the API modules and enabling ctools or Entity export: that means that the database table needs to be in an API module. Yuck. Moreover, the info hooks are also going to need to go through the exportability layer. Double yuck.
  2. Keep the info tables in the UI and make it exportable: that means that some types are going to be editable, other are not. Not worse then today, but yuck. Also, it is still not possible to disable the UI module.
  3. Add a middle module. Well, we are still going to have some types editable and other not.

All in all, I think the most future-proof approach would be to bite the bullet and just go with (1). We can probably implement a compatibility layer for the current info hook (basically by calling them in the API module in an implementation of a default hook).

joachim’s picture

joachim commented

> Same idea as: hook_node_info() vs. the {node_types} table. One is an API-level hook for defining node types, the other is the table in which are stored the node types created from the UI.

Though the thing is that Commerce Product doesn't actually follow this pattern.

If I declare a node type in hook_node_info(), then node module records this in the {node_types} table, along with the originating module and some combination of the 'locked' and 'custom' flags (I forget which). The database is the canonical place for getting a list of node types; the hook is a means for modules to get data into this table in a declarative way.

However, Commerce Product has this the other way round.

The UI module *only* is in charge of the table. It declares to hook_commerce_product_type_info() based on the contents of {commerce_product_type}. Other modules can also declare in the hook, and the central API gets data from the hook.

This causes problems when trying to get a round-trip working with http://drupal.org/project/commerce_features.

1. define the type in the UI: it lives in the table, the UI module declares it.
2. export to a feature: it now lives in code, the feature module tries to declare it and so does the UI.

I'm not entirely sure how Commerce Features gets round that, but I've seen it cause some awkward bugs.

> All in all, I think the most future-proof approach would be to bite the bullet and just go with (1).

In other words, switch to the same pattern as node types? That would certainly fix the problems I've seen :)

joachim’s picture

joachim commented
Issue summary: View changes

Add link to #937814.

bojanz’s picture

bojanz commented
Status: Active » Closed (outdated)