Conflict whithin 'Image style' and 'field' configurations [#2048699]

Steps to reproduce

Drupal clean install
Enable configuration and configuration UI modules
Create user-defined image style (e.g. 'featured', effects don't matter)
Go to the article's 'manage display' section, view mode 'default', set image style 'featured' to the field_image
Go to the Configuration Manager's 'Not tracking' page, select 'article' content type (with all optionals and dependencies) and press 'start tracking'
Ensure that all tracked confs are 'In Sync' and both our field_image and image style 'featured' are tracked
Go to the 'Synchronize' section and press 'Synchronize configurations'.

Expected result: nothing changes
Actual result: field_image's formatter has 'Original image' as preset

Guessed problem

After digging into this great (no jokes) module I found out something.
It happens because the field's config import is processing before the image style's config. Import of image styles consist of two phases: delete image style and create image style. But image_style_delete() invokes hook_image_style_delete() which is implemented by Image module. There Image module just removes this "deleted" image style from all its own field formatters.

Proposed resolution

I noticed that it doesn't happen when you import configs using 'Import Datastore to Activestore' button if option 'Process component dependencies.' is enabled.

'Synchronize' button calls ConfigurationManagement::importToActiveStore($to_track, FALSE, FALSE, TRUE);

I tried to change it to ConfigurationManagement::importToActiveStore($to_track, TRUE, TRUE, TRUE); and it works like a charm now.

However I'm don't think that it is the best way. For example I can include only image style to the tracked.inc (without tracking field_image) but it will be very strange if synchronization of image style will break field settings that even not tracked at all...

Comment	File	Size	Author
#8	sync_checkboxes_and_image_style_save-2048699-8.patch	6.68 KB	rylowry@gmail.com
#8
#7	sync_checkboxes_and_image_style_save-2048699-7.patch	6.65 KB	rylowry@gmail.com
#7
#2	sync_checkboxes_and_image_style_save-2048699-2.patch	3.13 KB	rylowry@gmail.com
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

rylowry@gmail.com CreditAttribution: rylowry@gmail.com commented 31 July 2013 at 20:24

I noticed this, too. I think the synchronize page needs 2 checkboxes added to process dependencies and optionals, that way you can have a bit of control when you are using the sync page. I've had cases where it is absolutely necessary to add dependencies BEFORE a particular component when syncing.

Comment #2

rylowry@gmail.com CreditAttribution: rylowry@gmail.com commented 1 August 2013 at 20:22

File	Size
sync_checkboxes_and_image_style_save-2048699-2.patch	3.13 KB

This patch adds checkboxes for dependencies and optionals to the sync page. It also reworks the ImageStyleConfiguration::saveToActiveStore() method. If an image style already exists, it leaves the style in place, and recreates the style's effects. Since no delete occurs, the fields should stay as they are.

Comment #3

xumepadismal CreditAttribution: xumepadismal commented 6 August 2013 at 20:54

Status:

Active

» Needs review

Ok, the patch looks good and it works for me.
Maybe someone else can test it so we can set the status to RTBC.

Comment #4

risenhoover CreditAttribution: risenhoover commented 6 August 2013 at 21:57

My results from attempting the patch. I have the alpha build + the performance patch + the "status does not refresh for variables" ( https://drupal.org/node/1956910 ) patches.

wilton@dev:~/test-drupal/sites/all/modules/contrib/configuration$ patch < sync_checkboxes_and_image_style_save-2048699-2.patch
can't find file to patch at input line 5
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/lib/Drupal/configuration/Config/ImageStyleConfiguration.php b/lib/Drupal/configuration/Config/ImageStyleConfiguration.php
|index 1041b3d..e120890 100644
|--- a/lib/Drupal/configuration/Config/ImageStyleConfiguration.php
|+++ b/lib/Drupal/configuration/Config/ImageStyleConfiguration.php
--------------------------
File to patch: lib/Drupal/configuration/Config/ImageStyleConfiguration.php
patching file lib/Drupal/configuration/Config/ImageStyleConfiguration.php
can't find file to patch at input line 39
Perhaps you should have used the -p or --strip option?
The text leading up to this was:
--------------------------
|diff --git a/ui/configuration_ui.admin.inc b/ui/configuration_ui.admin.inc
|index df07f8a..6e18d97 100644
|--- a/ui/configuration_ui.admin.inc
|+++ b/ui/configuration_ui.admin.inc
--------------------------
File to patch: ui/configuration_ui.admin.inc
patching file ui/configuration_ui.admin.inc
Hunk #1 FAILED at 589.
Hunk #2 FAILED at 718.
2 out of 2 hunks FAILED -- saving rejects to file ui/configuration_ui.admin.inc.rej

Comment #5

risenhoover CreditAttribution: risenhoover commented 6 August 2013 at 22:11

Ok so although the patch didn't work, I went through the patch file manually and made the updates to my source. I verified that the extra fields (dependencies / optionals) appeared. Great! I left them checked, and clicked to Synchronize.

It reported that some mcrypt-related fields were created, then failed with an error. In the logs, I have the following message. This is consistent with other error messages that I get when attempting to do a sync. The fields that are reported in the message do not actually exist in the target database, it's new functionality that I'm trying to migrate over, whether it's field_encrypt here or a standard data field like field_data_account_number.

TYPE php
DATE Tuesday, August 6, 2013 - 15:07
USER wilton
LOCATION http://test.xxx.com/admin/config/system/configuration/sync
REFERRER http://test.xxxx.com/admin/config/system/configuration/sync
MESSAGE DatabaseSchemaObjectExistsException: Table field_encrypt already exists. in DatabaseSchema->createTable() (line 657 of /var/www/vhosts/test.xxxx.com/drupal-git/includes/database/schema.inc).
SEVERITY error

Comment #6

2 November 2013 at 20:34

Status:

Needs review

» Needs work

The last submitted patch, 2: sync_checkboxes_and_image_style_save-2048699-2.patch, failed testing.

Comment #7

rylowry@gmail.com CreditAttribution: rylowry@gmail.com commented 16 November 2013 at 04:00

Issue summary:

View changes

File	Size
sync_checkboxes_and_image_style_save-2048699-7.patch	6.65 KB

Comment #8

rylowry@gmail.com CreditAttribution: rylowry@gmail.com commented 16 November 2013 at 04:30

File	Size
sync_checkboxes_and_image_style_save-2048699-8.patch	6.68 KB

This patch tries to fix the short comings of the last and pass tests. It uses the image style's storage field to try to make some better decisions about what to do when writing from the datastore to the activestore.