The following bugs indicate some type of Entity exception, usually due to the field missing from the Entity. When doing debugging for the document content type, I enabled it and it failed to work properly. After reverting trusted contacts, it worked just fine.

https://drupal.org/node/1856348
https://drupal.org/node/1935064
https://drupal.org/node/1923172
https://drupal.org/node/2013006
https://drupal.org/node/2028313
https://drupal.org/node/2034605

I believe all of these issues could be resolved by simply reverting all the features. Since people tend to 'customize' their site a bit, we don't want to put this in hook_enable.

The reason we need to do that is because of code like this:
'commons_trusted_contacts.module: function--commons_trusted_contacts_system_info_alter'

  if ($file->name == 'commons_trusted_contacts') {
    // This field base for group_group is defined in Commons Groups.
    $info['features']['field_instance'][] = "user-user-group_group";
    foreach (node_type_get_types() as $bundle => $type) {
      $info['features']['field_instance'][] = "node-$bundle-og_user_group_ref";
    }
  }

This is the code that adds the og_user_group_ref field to all content types. Unfortunately it only runs when commons_trusted_contacts is being enabled or reverted. When commons_documents is being enabled, it does not trigger, and thus the field does not get added.

In summary, we probably need to make it very clear that:
1) Do not alter features within commons. If you want to make a change, do it in your own feature
2) Revert your features whenever enabling any commons modules.

Comments

RobKoberg’s picture

Can any of these types of errors be fixed by manually adding needed field through the web interface? Or by manually editing the DB. It might be useful to add that info to this issue.

For example, in the code block above, would adding the og_user_group_ref to the content type in question work too?

japerry’s picture

Features are a bit picky when they define a field_instance, so no -- you should not manually add the field to the content type, and never add fields via the database, as its prone to human failure, and you won't execute the hook system that drupal relies on to properly build fields.

The best thing to do is leave all commons features alone so that reverting them is not problematic.

japerry’s picture

Personally I think having these within the CTs wouldn't be a bad idea... although its a lot of content types. There might be a better way via system info, but we don't have many options during install to hook in globally to add fields (without them being overridden). That way we're only reverting specific modules that we know need to be reverted instead of taking a shot in the dark.

  /**
 * Implements hook_enable().
 * Make sure to revert commons_groups and commons_trusted_contacts
 * whenever we enable a CT module, otherwise the group field will not be added
 * to the content type.
 */
function commons_documents_enable() {
  $revert = array();
  if (module_exists('commons_groups')) {
    $revert['commons_groups'] = array('field_instance');
  }
  if (module_exists('commons_trusted_contacts')) {
    $revert['commons_trusted_contacts'] = array('field_instance');
  }
  if (!empty($revert)) {
    features_revert($revert);
  }
}
RobKoberg’s picture

From Error when adding content of type 'Commons Documents':

"By making the site builder manually revert, we allow them to do it on
their own time, after they've read the documentation and verify they
have no customizations."

The point is that there will be customizations. An example, someone adds a field to the group CT (which I now understand to be Content Type from the context in #3 :) ). In my case, I am told it is a must to have a field for a "group identifier" that teachers write on a board and student are able to find the group from that. Or perhaps a change is to a field label. I have a lot of customizations. I have created 10s of features to manage them as best I can, but it never works completely. And I might might miss something I should have added to a feature, so I lose my changes somewhere in the vastness of drupal. Or it is just broken and I have to reinstall from scratch and lose my changes somewhere in the vastness of drupal. At least I have my features, which you guys finally got me to learn much more than I knew before (which is good). I still don't know enough to be comfortable. Should I override field bases, field instances? What if that changes and breaks something vital?

Is it just assumed (required?!) no one will want to customize the defaults from the original install?

Your tracking down this issue (it is not really a bug) is a big step forward, but still...

Documentation:

I just checked out the Acquia site again. From: https://docs.acquia.com/commons/upgrade "ensure that all feature modules are in their default state" -- what if they are not? This has happened to me on clean installs of plain commons. Then copying over a later version on that clean install produces features that won't revert. When reviewing the overrides, they had the *exact* same values. I had only done a "drush fra" and not "drush --force -y fra", so maybe that would have changed my experience (perhaps add it to the docs).

I think someone (all!) of the Acquia team should try to create a commons site with some customizations, so core developers can see the problems and perhaps come up with solutions (for the documentation and here) that us less experienced developers might not know about.

There has to be the ability to customize.

behoppe333’s picture

I agree with robkoberg and add two cents from my non-developer perspective, all IMO:

  1. It takes only beginner/intermediate web tech skills to install a release of Drupal Commons and go to admin > modules.
  2. Most/all modules I have enabled from admin > modules are quite benign. Usually I see one of 3 things: (a) Desired functionality starts working, or (b) Module throws a warning that I have to configure it, or (c) I observe no change at all in my site and conclude that the module is too subtle for my non-developer mind to comprehend.
  3. All of the above 3 behaviors are friendly to people with beginner/intermediate web tech skills
  4. In contrast, enabling Commons Documents has the perverse effect of breaking the site until the site admin does some reconfiguring with technical skills much more advanced than required to enable most other pre-installed modules.
  5. Where is the documentation for enabling Documents? I can only find two kinds: (a) high-level descriptions which will not help me enable Documents module, and (b) advanced technical information such as this very thread, which I found only because I googled the error message thrown by my already broken site.

A non-programmer like me expects to need expert help to get SOLR going, but does not expect to need expert help to enable a pre-installed content type like Documents. The current behavior is going to frustrate and push away potential Commons site admins who (IMO) really do have sufficient technical skills but lack the persistence to surmount this sort of configuration hurdle.

ezra-g’s picture

Thanks, behoppe333 and RobKoberg.

To clarify, when japerry says not to customize the Commons features, what I think he meant to say was that it's a best practice to customize functionality without editing the underlying the Commons codebase. Indeed, ease and maintainability of customization is one key reason that people chose Commons (and even Drupal) over competing products.

In general, it's a best practice to store these customizations using alter hooks. I agree that we can do a better job of providing examples and documentation for these customizations. To help generate that documentation, I encourage you to file issues asking any questions you may have about how to implement specific customizations, or to continue to chat with us in IRC in the #drupal-commons channel. Your questions and feedback are very useful to helping us produce useful documentation.

I agree that the present issue with an exception error is a poor user experience and that the ultimate solution is to revert features components behind the scenes so that folks don't experience this error. Based on our experience with Feature management and specifically with reverting components as a result of a particular module being enabled, I think that the best solution for Commons 3.3 is to document the need to revert features, and work on a more automated solution for Commons 3.4. A main factor here is that we're now several months sine the last official Commons 3.x release, and there have been 2 security advisories (Organic Groups and Entity API) that were released this and last week, affecting Commons 3.2

RobKoberg’s picture

Part of my response included the upgrading an existing and in production instance of commons. The answer to many of the bug reports has been that it works on a clean install. I suppose fine for a brand new 3.3 install. What happens when some commons features don't revert. Are you testing on upgrading a 3.2 install? Which will apply going from 3.3 to 3.4.

I think 3.4 should focus on moving away from some/most of the configuration stored in features to an admin/commons where it will be easier to manage settings. Not sure how exactly that would be done, but you guys have done some pretty amazing drupal customizations in the code as is (not features).

ezra-g’s picture

I suppose fine for a brand new 3.3 install. What happens when some commons features don't revert. Are you testing on upgrading a 3.2 install?

We are. Are there specific issues you can point to that are reproducible when upgrading from 3.2 to the latest Commons dev snapshot and following the full upgrade instructions?

I think 3.4 should focus on moving away from some/most of the configuration stored in features to an admin/commons where it will be easier to manage settings.

Can you elaborate on what problem you'd solve by moving the UI for configuration from one place to another?

RobKoberg’s picture

I am starting with 3.3, but was concerned that testing fixes seems to rely on a clean install of 3.3. Good to know that you are also testing existing installs of past releases.

"Can you elaborate on what problem you'd solve by moving the UI for configuration from one place to another?"

More fine grained access to what is currently contained in a feature. Currently with a feature it is an all or nothing. And they don't always revert. It would also expose more of what is going on to GUI only users. Perhaps if config settings were extracted from features there would be less of a need to revert features.

ezra-g’s picture

Currently with a feature it is an all or nothing.

Actually, that's not totally accurate. You are able to revert features on a component basis. If a feature doesn't revert under a set of reproducible steps, we should generally address that as a bug.

I agree that we could do more to help simplify managing features. Have you checked out https://drupal.org/project/feature_set ? That's one example of a simplified feature-management UI.

RobKoberg’s picture

"Actually, that's not totally accurate. You are able to revert features
on a component basis."

But that is not what is recommend (required) for commons. The docs say and has been said here that you should (must?) ensure all features were reverted.

I had not seen feature_set, but from reading the description I don't think it addresses the issue.

Anyway, I will stop pestering so you can work on the release :)

japerry’s picture

Currently we require you to go through reverting all features upon upgrade, because many of them will show as overridden. But its not the whole feature that gets reverted, just the options that are overridden. So a feature with an overridden field_instance and a default view_view will only see the field_instance get re-pulled from code.

It is important that we emphasize that people not override or alter commons features because during database updates and module installs are frequently reverted to get new/updated features. In otherwords, commons features on their own aren't really GUI items or configurable.

However, the goal of Drupal and Commons is for end users to alter and extend the functionality via custom modules, and not override the code in commons. To that end, if there are views, fields, or other things your site needs -- bring it into a new module or use drupal's alter functions to override the configuration via your own code. This will allow our features to revert without conflicting with custom code.

ezra-g’s picture

Issue tags: -Commons 7.x-3.3 radar

Removing from the 3.3 radar based on discussion around documentation and post 3.3 automation.

RobKoberg’s picture

Perhaps for the future docs:

"It is important that we emphasize that people not override or alter commons features because during database updates and module installs are frequently reverted to get new/updated features. In otherwords, commons features on their own aren't really GUI items or configurable."

For the commons_groups, it defines a panelizer layout and configuration for the group CT Full page view. I want to add/remove/replace some some panes. I want group Notices back on the right, and I want my own group browsing widget. If this was a node variant, I could (I think) provide my own variant and disable the commons one, but the panelizer does not offer this option (I think, panelizer is new to me). So unless I want to completely disable commons_groups, I need to override.

Now, if I force reverting all features, that should handle whatever commons_groups needs(?). And then revert my feature that includes the override for the group CT panelizer all should be good(?).

Or how would you suggest adding/removing/replacing panes from the panelizer configuration?

I also use _alters in code. For the group form, I can do some of this in my mash_groups module, but some of it I have to do in a template because of the way commons moves its alter to the end. Should I be moving my alter to the end? Here is the one in the module:

function mash_groups_form_group_node_form_alter(&$form, &$form_state, $form_id) {
  global $user;
  // A teacher can only create moderated groups with private content
  if (in_array('teacher', $user->roles)) {
    $form['field_og_subscribe_settings'][LANGUAGE_NONE]['#default_value'] = 'approval';
    $form['field_og_subscribe_settings']['#access'] = FALSE;
    $form['field_og_access_default_value'][LANGUAGE_NONE]['#default_value'] = 1;
    $form['field_og_access_default_value']['#access'] = FALSE;
  }
}

And the one in the template.php:

    // Can't affect this in a module (see mash_groups, mash_groups_form_group_node_form_alter)
    // because commons forces their alter to the end.
    if ($form_id == 'group_node_form' && in_array('teacher', $user->roles)) {
      $form['privacy_settings']['#access'] = FALSE;
    }
behoppe333’s picture

This is a good enlightening discussion.

Japerry (or anyone) can you please help me, a non-programmer with beginner/intermediate Web tech knowledge, understand in a practical way what it means to override a feature? Put another way, when I go to the admin menu and change something (anything), how do I know if that change is overriding a feature?

A couple examples of my confusion:

  • With admin > people > permission, I have given "Content Moderators" rights to create Commons Events and edit their own events. I did that weeks ago, before I knew the Features Module existed. Now I can see that I have unintentionally overridden the Commons Events Feature by setting permissions in this way. How can I let non-admins create events without overriding the event feature?
  • With admin > config > rules, I have created a rule triggered by event registration that automatically registers my site's users for GoToWebinars when they click "Attend" on certain Commons Events. As far as I can tell, I have not overridden anything in the Events Feature by using Rules in this way. I am surprised by this. And I fear that a future release of Commons might expand the scope of the Events Feature so that my Rule suddenly becomes an override, after the fact.

Thanks for helping me understand when something I want to do in the admin UI will (or will not) override a Commons feature.

PS: Not sure if this last point belongs in this thread, hence the "PS": If I install RC3 from scratch, then disable "Events" link in main menu and save, then re-enable "Events" link in main menu and save again, the resulting site reports not only that Events Feature is overridden, but also that these features are overridden as well: Commons Follow, Commons Groups, and Commons Q&A. I am pretty confused about the boundaries of Commons Features and when my actions cross those boundaries or not.

japerry’s picture

behoppe333, your confusion is definitely understandable. Features, and especially how we use features, is a complex issue currently with drupal7. With drupal8, many of these problems we see today will go away.

In the meantime, the reason why enabling one feature may cause a bunch of others to be overridden is due to hooks being fired on all the other features. The most clear example here is the group_group field for commons_groups, and user-user-group_group field for trusted contacts. The code in the summary works like this:

For every time system_info_alter is ran (module is enabled), do the following for the commons_trusted_contacts info file:

  • Get the list of content types (CTs) from commons_groups_get_group_content_entity_types()
  • For every one of these content types make a new 'user-user-group_group' entry in the info file that corresponds to the content type.

The problem however, is now that you've enabled or changed a content type, there is a new field_instance called user-user-group_group for that content type, but it hasn't been created yet. This is where reverting is necessary, specifically on the field_instance section of the feature. When you revert the field_instance section, it will add the corresponding field that the info file was looking for, thus returning it to default.

For the rules example you set out, you won't be conflicting with commons, so you're fine there. As for adding fields to content types, you'll have to create a new module for your site, similar to commons_trusted_contacts, which contains the fields you want to set, and the entries in the info file. Unfortunately this setup is not for the faint of heart, and is not recommended for people who do not well understand how php, drupal, hooks, and features work.

behoppe333’s picture

Thanks japerry. I appreciate the info and support. I am curious what is the best way for me to do things like

  • In admin > structure > menus > main menu, disable "Events"
  • In admin > people > permissions > node, allow "Content Moderators" permission to create new Event content and edit own Event content

These sorts of things are likely to be regular tasks for me as I adapt Commons for users of my site. I would like to do those tasks so that upgrades to Commons are as easy as possible for me to integrate with my own customizations.

I think I can avoid adding fields to content types, but tweaking panels, views, menus, permissions, and access -- and perhaps creating roles -- will be essential for me to do in a way that is easy to maintain as Commons continues to evolve. Any tips about that?

thanks again

behoppe333’s picture

Category: bug » support

How can I tell in advance when copying a code patch or upgrade to the code of my Commons instance will cause my DB customizations to get erased?

I upgraded my Commons instances with 3.x-dev code (using drush make, and later zipped snapshots) 2-3 times a week for several weeks without losing any DB customizations, and so I was surprised when going from Aug 16’s RC4 to Aug 19’s 3.3 blew away all my custom panels (among other things).

I was expecting RC4->3.3 to cause less change, not more, than the dozens of Commons code upgrades I had done in June-July-August. I thought I was just putting a stamp of "approved and released" on the code I'd been testing for weeks.

I can see in #14 a partial answer to my question -- that panels are part of the Commons_Group feature. But I am desperately in need of clarification to my opening question.

I would also appreciate advice on how to upgrade Commons without losing my DB customizations. I realize there is a conflict between this question and #12:

It is important that we emphasize that people not override or alter commons features because during database updates and module installs are frequently reverted to get new/updated features. In other words, commons features on their own aren't really GUI items or configurable.

However, the goal of Drupal and Commons is for end users to alter and extend the functionality via custom modules, and not override the code in commons. To that end, if there are views, fields, or other things your site needs -- bring it into a new module or use drupal's alter functions to override the configuration via your own code. This will allow our features to revert without conflicting with custom code.

IMO the above policy is going to lead to a lot of frustrated Commons site admins (esp those who are daunted by creating modules) who either never upgrade Commons or abandon Commons so that they can configure and maintain a Drupal site that serves the needs of their audience.

behoppe333’s picture

I realize now that my panels did not get erased by changing code from rc4 to 3.3. Sorry for confusion on that.

My big questions remain, per #18

  • How to know when something in my DB is vulnerable to being changed by a feature upgrade
  • How to manage a Commons site for a demanding and diverse audience (i.e., different groups that want different custom panels etc) when module-building is not in my skill set
japerry’s picture

Status: Active » Fixed

For the most part, this is fixed, especially with documents on Acquia's website (in regards to upgrading)

If there are specific issues we can discuss them in new issues.

Status: Fixed » Closed (fixed)

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