Including Registration Types entity in Features fails to set all Registration Type properties when feature is enabled on another installation

Attached is a patch to fix!

A couple notes:

• This also affects the "Import registration type" form, not just Features. It'll fail to import the 'held_expire' and 'held_expire_state' there as well.
• I'm not entirely sure how the 'Export' functionality works without any special code around it like the code I'm adding in this patch. It seems like that would need it too! So, there might be some other mechanism to fix this in the same way that export is magically working, but I couldn't find it.

This patch works as expected. Without the patch, a feature with held_expire and held_expire_state set is overridden. With the patch, the feature is in its default state.

I'm not entirely sure how the 'Export' functionality works without any special code around it like the code I'm adding in this patch. It seems like that would need it too! So, there might be some other mechanism to fix this in the same way that export is magically working, but I couldn't find it.

Your code actually fixes both. When I go to the Export page for the registration type without the patch, I see

{
  "name" : "event_reg",
  "label" : "Registration",
  "locked" : "0",
  "default_state" : null,
  "data" : null,
  "weight" : "0"
}

And when I go to the page with the patch applied, I see

{
  "name" : "event_reg",
  "label" : "Registration",
  "locked" : "0",
  "default_state" : null,
  "weight" : "0",
  "held_expire" : "1",
  "held_expire_state" : "canceled"
}

Export was working for me before my patch. I think you might have seen it failing because you were trying it on a Registration Type that was not imported from Features correctly.

Here's steps to show it working:

1. Using Registration 1.4 with out my patch
2. Create a new Registration type manually in the admin UI
3. Click the "Export" link next to it
4. It has the "held_expire" and "held_expire_state" in the JSON at the top-level

I would have expected that since those fields are actually inside the $type->data array, that the export would have shown them there, like:

{
  "name" : "event_reg",
  "label" : "Registration",
  "locked" : "0",
  "default_state" : null,
  "weight" : "0",
  "data" : {
    "held_expire" : "1",
    "held_expire_state" : "canceled"
  }
}

... and that the controller would need to override the export() method to move them to the top-level. But it appears to put them at the top-level already via some other mechanism?

Anyway, since this patch fixes the bug, it's definitely better than nothing. But if some other mechanism is making export work, then it might be better/simpler to use that same mechanism to allow import to work, if only we could figure what was doing it. :-)

When I run drush rr --debug

I get the following:

Undefined property: RegistrationType::$held_expire registration_type.controller.inc:13 [468.87 sec, 218.55 MB] [notice]
Undefined property: RegistrationType::$held_expire_state registration_type.controller.inc:13 [468.87 sec, 218.55 [notice]

I am not seeing the same error on my installation. Can you provide more details of your setup?

Here's my drush output:

$ drush rr -d
Bootstrap to phase 0. [0 sec, 1.89 MB] [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drush() [0.01 sec, 2 MB] [bootstrap]
Cache MISS cid: 6.6-dev-alias-path--ca77b61602a5e545016b5003d805cd8a [0.01 sec, 2.01 MB] [debug]
Cache SET cid: 6.6-dev-alias-path--ca77b61602a5e545016b5003d805cd8a [0.05 sec, 4 MB] [debug]
Cache MISS cid: 6.6-dev-commandfiles-0-97ddad0c854e2b16e7328e4fc5a50018 [0.06 sec, 2.04 MB] [debug]
Cache SET cid: 6.6-dev-commandfiles-0-97ddad0c854e2b16e7328e4fc5a50018 [0.07 sec, 2.07 MB] [debug]
Bootstrap to phase 0. [0.12 sec, 7.97 MB] [bootstrap]
Bootstrap to phase 0. [0.15 sec, 7.97 MB] [bootstrap]
Found command: registry-rebuild (commandfile=registry_rebuild) [0.15 sec, 7.97 MB] [bootstrap]
Calling hook drush_registry_rebuild [0.22 sec, 8.11 MB] [debug]
Bootstrap to phase 4. [0.22 sec, 8.11 MB] [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drupal_root() [0.23 sec, 9.08 MB] [bootstrap]
Initialized Drupal 7.38 root directory at /sites/test [0.23 sec, 9.08 MB] [notice]
Drush bootstrap phase : _drush_bootstrap_drupal_site() [0.23 sec, 9.09 MB] [bootstrap]
Initialized Drupal site Sites at sites/default [0.23 sec, 9.09 MB] [notice]
Cache MISS cid: 6.6-dev-install_profile-66ecfeb9791a023150773849f1550c5d [0.24 sec, 9.1 MB] [debug]
Cache MISS cid: 6.6-dev-commandfiles-2-d49b2b23fa795040f8c524a616765199 [0.24 sec, 9.1 MB] [debug]
Cache SET cid: 6.6-dev-commandfiles-2-d49b2b23fa795040f8c524a616765199 [0.29 sec, 9.11 MB] [debug]
Drush bootstrap phase : _drush_bootstrap_drupal_configuration() [0.31 sec, 10.35 MB] [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drupal_database() [0.31 sec, 10.36 MB] [bootstrap]
Successfully connected to the Drupal database. [0.31 sec, 10.36 MB] [bootstrap]
This DRUSH_MAJOR_VERSION is: 6 [0.33 sec, 13.85 MB] [notice]
Internal Drush cache cleared with drush_cache_clear_drush (1). [0.34 sec, 13.85 MB] [notice]
Bootstrap caches have been cleared in DRUSH_BOOTSTRAP_DRUPAL_DATABASE. [0.34 sec, 14.61 MB] [notice]
We are on the DRUSH_BOOTSTRAP_DRUPAL_DATABASE level still. [0.34 sec, 14.61 MB] [notice]
The registry has been rebuilt via registry_rebuild (A). [0.71 sec, 15.67 MB] [success]
Internal Drush cache cleared with drush_cache_clear_drush (2). [0.71 sec, 15.67 MB] [notice]
Bootstrapping to DRUPAL_BOOTSTRAP_FULL. [0.71 sec, 15.67 MB] [notice]
Bootstrap to phase 5. [0.71 sec, 15.67 MB] [bootstrap]
Drush bootstrap phase : _drush_bootstrap_drupal_full() [0.71 sec, 15.67 MB] [bootstrap]
Cache MISS cid: 6.6-dev-install_profile-66ecfeb9791a023150773849f1550c5d [0.9 sec, 37.93 MB] [debug]
Cache SET cid: 6.6-dev-install_profile-66ecfeb9791a023150773849f1550c5d [0.9 sec, 37.93 MB] [debug]
Cache HIT cid: 6.6-dev-install_profile-66ecfeb9791a023150773849f1550c5d [0.9 sec, 37.93 MB] [debug]
Cache MISS cid: 6.6-dev-commandfiles-5-2b5d500022837cd76f7175d65b469003 [0.9 sec, 37.98 MB] [debug]
Cache SET cid: 6.6-dev-commandfiles-5-2b5d500022837cd76f7175d65b469003 [0.94 sec, 37.98 MB] [debug]
All caches have been cleared with drush_registry_rebuild_cc_all. [10.13 sec, 111.62 MB] [success]
The registry has been rebuilt via drush_registry_rebuild_cc_all (B). [10.13 sec, 111.62 MB][success]
There were 853 files in the registry before and 853 files now. [10.14 sec, 112.34 MB] [notice]
All caches have been cleared with drush_registry_rebuild_cc_all. [14.31 sec, 110.21 MB] [success]
All registry rebuilds have been completed. [14.31 sec, 110.21 MB] [success]
Returned from hook drush_registry_rebuild [14.31 sec, 108.77 MB] [debug]
Command dispatch complete [14.31 sec, 108.74 MB] [notice]
Timer Cum (sec) Count Avg (msec)
page 14.001 1 14001.31
Peak memory usage was 128.35 MB [14.31 sec, 108.73 MB]

I'm assuming the difference is configuring php.ini to show more/less notices.

Undefined property: RegistrationType::$held_expire registration_type.controller.inc:13

Well, the warning is true! However, the code depends on reading/writing those undefined properties all over the place, not just in the code that this patch adds. We could fix it by declaring those properties in this patch, but we didn't create the problem.

David,

I've created a new issue for the PHP Notice so that it doesn't block review of your work here. Thanks for finding it.

https://www.drupal.org/node/2543280

Overriding the save method this way doesn't have any impact on exports or features, at least not intentionally. The root cause is improper structure of the exports, which is taken care of in this commit, f8f4b20.

Thanks for flagging the issue.