Problem

Quite a few configuration overrides in Open Social load the current configuration and include that configuration in the overrides array. This causes the entire configuration object to be marked as overwritten which can lead to problems if someone else wants to do the same.

It's also unnecessary because configuration overrides are performed using a deep merge which means that only the final leaf of the overrides object is used for the override.

Solution

Remove loading of configuration that's put into the returned $overrides array for all Override classes.

A quick search sees this pattern in the following classes but a more thorough search may be required.

modules/custom

  • SocialFilePrivateFieldsConfigOverride
  • SocialFilePrivateTExtEditorConfigOverride
  • SocialLanguageConfigOverride

modules/features

  • SocialEmbedConfigOverride
  • SocialProfileFieldsOverride
  • SocialMentionsConfigOverride
  • SocialTaggingOverrides
  • SocialGroupSelectorWidgetConfigOverride
  • SocialEventTypeConfigOverride
  • SocialEmbedEditorConfigOverride
  • SocialLikeConfigOverride
  • SocialPostPhotoConfigOverride
  • SocialEventManagersConfigOverride
  • SocialLandingPageConfigOverride
  • SocialBookConfigOverride

An example commit where we already made these changes: https://github.com/goalgorilla/open_social/pull/789/commits/cbf58984f94e...

An example (merged) pull request that made these changes:
https://github.com/goalgorilla/open_social/pull/850

Tasks

  • Create an issue for each class/module to keep individual changes small and easily reviewable. This also makes it possible for discussion around 1 class that may be special to not block all the others. Make those issues a child issue of this issue and update the list above to link to it.
  • Perform a more extensive search than a quick grep for config.*get in *Override* files
  • Fix all the issues

Comments

Kingdutch created an issue. See original summary.

kingdutch’s picture

Issue summary: View changes
Issue tags: +Novice

Add examples of commits/PRs that already tackled those issues.

bramtenhove’s picture

Version: 8.x-1.x-dev » 8.x-3.x-dev

Updated the version to 3.x. Still something that can be picked up.

Any help is greatly appreciated.

nginex’s picture

Issue tags: +LutskGCW19