In building a bundle management UI for Fieldable Panels Panes (#1618308: User interface for managing bundles), I discovered a few places in the export-ui that could use some touch-ups. This is particularly in reference to the ctools_export_ui_process() function and ctools_export_ui() class:

  • More array unions in defining $plugin['menu']['items']. This will allow developers to make subtle updates, such as just changing the 'type' of a menu item without having to define the entire menu item. This can certainly also be done by overriding the hook_menu() method, but then those modifications will be unknown to the plugin definition and will not be reference-able later.
  • Array union in defining $plugin['redirect']. Currently, I cannot define new redirects or customize redirects because the export-ui process callback will clobber them.
  • Define redirect paths for delete/revert. To stay consistent and avoiding hard-coded assumptions.
  • Move the build of "allowed operation" links into its own method. This makes it easy to modify the "operation" links rendered, as well as cleans up the logic for removing inapplicable links.
  • Move the deletion of exportable items into its own method. This makes it possible to do post-deletion processes by overriding the method. This also uses the redirect() method, instead of hard-coding a drupal_goto().
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

helior’s picture

Status: Active » Needs review
FileSize
19 KB
BTMash’s picture

Status: Needs review » Needs work

I think the changes are quite nice. Below are a few nitpicks for consistency.

+++ b/plugins/export_ui/ctools_export_ui.class.php
@@ -1187,17 +1195,27 @@ class ctools_export_ui {
+   * Deletes exportable items from the database. Override this method if
+   * subsequent processing is needed.

You might be ok with just stating 'Deletes exportable items from the database'. I don't think stating when you should override this method.

+++ b/plugins/export_ui/ctools_export_ui.class.php
@@ -501,6 +463,52 @@ class ctools_export_ui {
+   * Build the operation links for a specific exportable item.

Should be 'Builds the operation links for a specific exportable item.'

Also, talking with you, there is an advanced help file for export ui. Since you are providing two new class functions, you might want to document some of the changes in there?

helior’s picture

Status: Needs work » Needs review
FileSize
18.94 KB

Patch is updated to accommodate for the nitpicks ;)

I took a look at the advanced help doc for export-ui. It was really intended to provide example code for a basic export ui plugin definition, and not to document class methods – it refers to reading the source file for that. Besides, this patch only has conservative updates that doesn't change the API, only makes it slightly more flexible.

merlinofchaos’s picture

Issue tags: +VDC

Tagging for VDC because we need to resolve this before we start converting export ui to D8 plugins and Configurables.

merlinofchaos’s picture

Committed and pushed in both 7.x and 8.x branches.

merlinofchaos’s picture

Status: Needs review » Fixed

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

Anonymous’s picture

Issue summary: View changes

Updated issue summary.