Problem/Motivation

The form should not use overridden config, because this means the override will be saved to the database when submitting the form.

Steps to reproduce

Add this to settings.php

$config['simple_sitemap.settings']['base_url'] = 'https://' . $main_host;

Browse to /admin/config/search/simplesitemap/settings.
The overridden URL is displayed in the form field.

Proposed resolution

Use ConfigFactory::getEditable() to load the config.

Issue fork simple_sitemap-3359679

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

prudloff created an issue. See original summary.

prudloff opened merge request !66

prudloff’s picture

prudloff commented
Status: Active » Needs review
gbyte’s picture

gbyte
Location Berlin
commented
Version: 4.1.4 » 4.x-dev
Category: Bug report » Task
Status: Needs review » Postponed (maintainer needs more info)

Your reasoning makes sense but I wonder if you know of any other places (like core) known to do it your way? Also we'd have to scan the whole code base including submodules if this change is to be accepted.

prudloff’s picture

prudloff commented

It is explained here.

The core uses it for example in GDToolkit::buildConfigurationForm() or in ThemeExperimentalConfirmForm.

#2910353: Prevent saving config entities when configuration overrides are applied also talks about preventing saving the configuration when it contains overrides.

gbyte’s picture

gbyte
Location Berlin
commented
Status: Postponed (maintainer needs more info) » Needs work

This makes sense. I'm happy to merge this as soon as all forms are taken under account (including submodules). Feel free to grab it!

elber made their first commit to this issue’s fork.

elber’s picture

elber
Primary language Portuguese, Brazil
Location Brazil
commented
Status: Needs work » Needs review

Hi I just rebased please revise.

  • gbyte committed 8b4c37a2 on 4.x authored by prudloff 
    Issue #3359679 by prudloff, elber, gbyte: SettingsForm should not use...
gbyte’s picture

gbyte
Location Berlin
commented
Status: Needs review » Fixed

Commited to dev. Thank you!

Status: Fixed » Closed (fixed)

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

WalkingDexter closed merge request !66