Beta phase evaluation

Reference: https://www.drupal.org/core/beta-changes
Issue category Bug because the entity conversion option is misleadingly named.
Disruption Disruption is minimal, core only uses this option once in a real life scenario and once in a test.

Problem/Motivation

It is possible to use the use_current_language upcasting option to opt out of override-free upcasting on admin paths. For example #2137595-26: 'Create @name' page title uses override-free configuration (eg. not localized) instead of the overridden configuration (eg. localized) used that to load an entity type properly in a possibly admin context.

Proposed resolution

Rename this option to with_config_overrides or something along those lines.

Remaining tasks

Commit.

User interface changes

None.

API changes

The incorrectly named use_current_language option is renamed to with_config_overrides.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Status: Active » Needs review
FileSize
4.23 KB

Proposed patch using "with_overrides" language.

Status: Needs review » Needs work

The last submitted patch, 1: 2405939-with-overrides-param.patch, failed testing.

Gábor Hojtsy’s picture

Version: 8.1.x-dev » 8.0.x-dev

Status: Needs work » Needs review
hass’s picture

Status: Needs review » Needs work

Placing a boolean in quotes looks wrong to me. We have boolean type for variables in .yml files.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
4.23 KB
1.11 KB

system.routing.yml and other routing files are full of

  requirements:
    _access: 'TRUE'

It is true that some things use TRUE (without quotes) even in system.routing.yml, there is no real consistency. It does not actually matter from the code if its a string or a bool, if its 'TRUE' or TRUE, it evaluates to true either way.

hass’s picture

I was not aware of this. I only remembered when I defined types in schema files of my modules I started to run into issues when I tried to save a wrong type.

Gábor Hojtsy’s picture

@hass: config files saved using config schema will enforce types, routing.yml are developer written and does not enforce types for values programatically.

hass’s picture

That is very inconsistent. DX fail.

hass’s picture

jhodgdon’s picture

Patch looks good to me.

We will need to have a change record and beta evaluation.

Also, adding link to original issue where this parameter was added. As a note there was no change record there.

Gábor Hojtsy’s picture

Issue summary: View changes

Added change notice at https://www.drupal.org/node/2407035 (draft), added beta evaluation to issue summary. @jhodgdon: what do you think? :)

jhodgdon’s picture

Looks good, thakns! I think the change notice should mention the previous name of the parameter, so I added a short note. This would allow someone whose contrib module using the old option suddenly broke to search and find this change notice.

Gábor Hojtsy’s picture

Issue tags: +SprintWeekend2015Queue

@jhodgdon: thanks. Looks like all this needs is a review confident to RTBC :)

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

I have verified that this patch changes all current Core uses of 'use_current_language' to 'with_overrides'. I think the new parameter name is clearer than the old. Tests pass. There's a beta evaluation and a change notice.

Let's do it!

Gábor Hojtsy’s picture

Assigned: Unassigned » alexpott

Assigning to @alexpott as #2392319: Config objects (but not config entities) should by default be immutable might necessitate changing the terminology here.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

Let's go with with_config_overrides. I don't think aligning to the getEditable() terminology from #2392319: Config objects (but not config entities) should by default be immutable is relevant - since entities (including config entities) are mutable.

Gábor Hojtsy’s picture

Issue summary: View changes
Status: Needs work » Reviewed & tested by the community
FileSize
4.7 KB
3.58 KB

Rolled with that name. Updated issue summary and change notice.

DevElCuy’s picture

DevElCuy’s picture

Issue tags: +SprintWeekend2015Queue

Removed SprintWeekend2015Queue by mistake.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Yep use_current_language is a bad name when this relates to config overrides. Committed cc9d0f9 and pushed to 8.0.x. Thanks!

  • alexpott committed cc9d0f9 on 8.0.x
    Issue #2405939 by Gábor Hojtsy, olli: use_current_language upcasting...
Gábor Hojtsy’s picture

Issue tags: -sprint

Status: Fixed » Closed (fixed)

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