Commerce Kickstart Feature 2.x changes the display field settings to hidden

Tagging for kanban board: http://contribkanban.com/#/board/commerce_kickstart

Able to reproduce. Adjusted field instance display settings on Teaser to move from "hidden" to "visible". On upgrade to HEAD the field display settings reverted.

This might also be changing field base configurations.

In testing I have tried

Non-Demo, Interactive update

1. Checkout tag 7.x-2.22
2. Install non-demo store
3. Customize Product Displays' teaser display.
4. Update the site via update.php

Results: not reproducing. Feature shows up as overridden as expected. Strongarm for attached extras [from product entity] and Instance [from displaying product catalog].

However, if I repeat the same steps but use "drush updb" instead of visiting update.php it is reproduced.

You're right. It is either way. I reviewed the rebuild code for field_base and field_instance. I think the only way to fix this is to implement the "locking" feature provided by Features to prevent rebuild from happening in lite_product and product field_base and field_instance. Testing this now.

PATCH! Lockin' those fields DOWN.

Tests: https://travis-ci.org/commerceguys/commerce_kickstart/builds/71491448

Argh. Features is sure doing what it's meant to do ;) preserve configuration. I think locks need to be put on all components of the Feature's we're expecting people to customize, then. I'll update the patch.

Updated patch to lock variables. I think this should be fine - field base, field instance, variable.

Just realized this won't work. We need to also set this on hook_install() to lock those components.

Here we go! This sets it on install, too. If tests pass, going to commit.

https://travis-ci.org/commerceguys/commerce_kickstart/builds/71515592

:/ I'm not sure how #16 could break, when the difference from #11 is setting the lock after rebuild.

I'm updating the tests to import a customized site DB and ensure customizations stick, so we have less manual testing to do.

OK! Here is a patch which includes a modified 7.x-2.22 install with Behat tests to verify the changes. It also adds a Travis CI environment to test by importing the database.

It's passing locally for me.

Ok.. so Field Base is screwing up. Once I get the regression environment to run properly on TravisCI I'll extend the tests for all of these cases.

It's the last one in the matrix, but it should run properly this time: https://travis-ci.org/mglaman/commerce_kickstart/jobs/71603162

Tests passed! https://travis-ci.org/commerceguys/commerce_kickstart/jobs/71603190

The following was checked

• Field display on Teaser - hide ones displayed by default, show ones that weren't
• Change content type's name and description
• Mark it was "unpublished" at first
• Disable comments

Need to update the SQL dump for some field_base stuff, and ensure field display weight.

Here is the latest patch, and here is a snippet of the Behat test which checks field base changes and field instance changes, along with strongarm (variable.)

Link: https://github.com/mglaman/commerce_kickstart/blob/features-rebuild-madn...

@no-demo @api
Feature: Customizations to non-demo store stay
  In order to customize my site
  As a site builder
  I need Commerce Kickstart to respect my changes

  Background:
    Given I am logged in as a user with the "administrator" role

  Scenario: My changes to Product Display teaser view mode stay
    When I am on "/admin/structure/types/manage/product-display/display/teaser"

    # Verify custom visible.
    Then the "edit-fields-productsku-type" field should contain "visible"
    Then the "edit-fields-producttitle-field-type" field should contain "visible"
    Then the "edit-fields-productcommerce-price-type" field should contain "visible"

    # Verify hidden.
    Then the "edit-fields-body-type" field should contain "hidden"
    Then the "edit-fields-title-field-type" field should contain "hidden"
    Then the "edit-fields-productfield-images-type" field should contain "hidden"

  Scenario: Changes to the Product Display type stay
    When I am on "/admin/structure/types/manage/product-display"
    Then the "edit-name" field should contain "Product display (altered)"
    Then the "edit-description" field should contain "This is the description of the product display"
    Then the "edit-node-options-status" field should contain ""
    Then the "edit-comment--2" field should contain "0"

  Scenario: Changes to the Product Display body field instance
    When I am on "/admin/structure/types/manage/product-display/fields/body"
    Then the "edit-instance-label" field should contain "Description"
    Then the "edit-instance-required" field should contain "1"
    Then the "edit-instance-description" field should contain "Description of the product"
    Then the "edit-instance-widget-settings-rows" field should contain "10"

  Scenario: Product category kept field widget change
    When I am on "/admin/structure/types/manage/product-display/fields/field_product_category/widget-type"
    Then the "edit-widget-type" field should contain "taxonomy_autocomplete"

  Scenario: Product variation field display settings did not change
    When I am on "/admin/commerce/config/product-variation-types/product/display"
    Then I should see "Image style: Large (480x480)"

@howdytom can you specify what types in particular? This patch is covering field and variable information for commerce_kickstart_product and lite_product, so the Product Display type and demo node types, along with matching product types.

:) That explains it, then. This is only targeting product display node types, when it should be any Feature which provides node types, field instances, etc.

I'll have a new patch incoming

Trying something new.

Added a function to mark the entire Feature as locked. It runs on install, after rebuild, now. And there's an update hook to set it. This should preserve customization across the board, but need to make sure upgrades/install runs right.

/**
 * Locks a Feature to prevent automatic rebuilding.
 *
 * This is causing a loss of data for customized sites. We need Features to
 * just provide our base configuration.
 *
 * @see features_feature_is_locked()
 *
 * @param string $module
 *    The Feature to lock.
 */
function commerce_kickstart_lock_feature($module) {
  $locked = variable_get('features_feature_locked', array());

  // Mark all components as locked.
  $locked[$module]['_all'] = TRUE;

  variable_set('features_feature_locked', $locked);
}

Test run: https://travis-ci.org/mglaman/commerce_kickstart/builds/71672401

So I think this issue has been tackled the incorrect way. I think the proper solution will be to explore Features Override. Features is technically just doing it's job, but we need to allow customization on top of it, which is the purpose of the Features Override module.

I think the Features Override route is best path. However I discovered Features doesn't really pick up alters for the previous API, where fields were in a single config. Here is a patch which adds support for Features Overrides created on Features API 1 for field exports (pre 7.x-2.24 CK2.)

This way users can export their overrides and upgrade while keep their changes, and able to re-export into Features API 2.

This is cleaner than toying with component locks and pushing for better practices when customizing a distribution.

Before committing I want to create a sample Feature which implements some overrides, and during tests it becomes enabled and changes verified.

Test verifying usage of Features Override for keeping customizations: https://travis-ci.org/mglaman/commerce_kickstart/builds/71716533

Tests pass! Features Overrides is being tested on a 7.x-2.23 upgrade to HEAD.