Follow-up to #2419479: Remove 'foreign keys' from field base

It's always override by schema so not needed as part of field base.

Comments

heddn’s picture

Status: Closed (fixed) » Active
Related issues: +#2102265: Features integration

This causes errors for multifield, because the nested fields do not exist yet on features revert.

heddn’s picture

Status: Active » Needs review
StatusFileSize
new960 bytes
mustanggb’s picture

Status: Needs review » Reviewed & tested by the community

Doesn't break anything for me and fixes things that weren't working before.

mpotter’s picture

Status: Reviewed & tested by the community » Needs work

So, the problem with this patch is it causes all existing field base features to be marked as overridden.

This is different from #2419479: Remove 'foreign keys' from field base which was done in response to a change in core that was causing overrides.

We need a different solution to this that is backwards compatible and doesn't cause overrides. People shouldn't need to re-export features for something minor like this, and it's an issue for distributions that would need to be updated to re-export features.

heddn’s picture

Is there a function in features I could call to check if the indexes already exist in a features export? Then only remove the indexes if they don't already exist.

mpotter’s picture

There was also some functionality added recently to add an "ignore" list of keys, so maybe just adding indexes to that list would fix it. Look at the bottom of features.module for the implementation of the current ignore hook.

heddn’s picture

StatusFileSize
new326 bytes

No interdiff, as this is an entirely different approach than #2.

mpotter’s picture

Status: Needs work » Needs review

Set the status so this gets tested via the testbot

osopolar’s picture

Patch from #7 works for me (applied together with patch from multifield-issue #2102265: Features integration, comment #32).

brunodbo’s picture

The patch from #7, together with patch #32 from #2102265: Features integration, work for me as well. I'm now able to deploy multifields from local to staging.

However, the feature that contains the base multifield and base subfields ends up being overridden, no matter how many times I try to revert. It was in 'Needs review' state at first, and then after reverting, it went into eternal 'Overridden' mode.

Here's the part the feature reports as being overridden:

'indexes' => array(
  'field_unit_length_value' => array(
    0 => 'field_unit_length_value',
  ),
  'id' => array(	
    0 => 'id',
  ),
),

I suspect that it has something to do with this issue, but not 100% sure. Any ideas how to get the feature to revert?

mustanggb’s picture

StatusFileSize
new854 bytes

#7 didn't work for me for the reason described in #10, however this one does, had to fix a bug in _features_remove_ignores() as well.

heddn’s picture

Can you provide an interdiff?

badrange’s picture

Patch in #11 worked for me. Thanks MustangGB!

  • mpotter committed 7c288b1 on 7.x-2.x authored by MustangGB
    Issue #2506877 by heddn, MustangGB: Remove 'indexes' from field base
    
mpotter’s picture

Status: Needs review » Fixed

This is looking good now. Especially the bug fix to _features_remove_ignores(). I added an additional unset($value) which is just best practice after any foreach using &. Committed to 7c288b1.

Status: Fixed » Closed (fixed)

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