As mentioned in #1648930-228: Introduce configuration schema and use for translation, we have to support multiple %parents to access data from different levels. However it is not working. Sample of not working is at https://gist.github.com/vijaycs85/5402151

Files: 
CommentFileSizeAuthor
#14 config_schema_parent-1972816-14.patch6.13 KByched
PASSED: [[SimpleTest]]: [MySQL] 57,851 pass(es).
[ View ]
#14 interdiff.txt796 bytesyched
#13 config_schema_parent-1972816-13.patch6.13 KByched
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]
#13 interdiff.txt5.02 KByched
#12 config_schema_parent-1972816-12.patch1.18 KByched
PASSED: [[SimpleTest]]: [MySQL] 57,900 pass(es).
[ View ]

Comments

vijaycs85’s picture

Title:%parent.%parent not working» %parent.%parent is not working
Gábor Hojtsy’s picture

I don't think we can call this a regression, we did not touch the parsing much since it was committed and this was not even in Drupal 7. Should be investigated nonetheless. I don't even know if this was supposed to work in the first place.

Gábor Hojtsy’s picture

Title:%parent.%parent is not working» %parent.%parent is not working in config schema dynamic type names
Issue tags:-sprint, -Configuration schema+regression
Gábor Hojtsy’s picture

Looks like one can cross-post with himself...

Gábor Hojtsy’s picture

Title:%parent.%parent is not working in config schema dynamic type names» Add support for %parent.%parent in config schema dynamic type names
Category:bug» feature

Looking at the committed patch for schemas (http://drupal.org/files/1866610.47.config-schema_0.patch), there is no documentation nor code to make the kind of things like %parent.%parent work. The replacement handling code would replace any instance of %parent with only the immediate parent, there is no concept of going further up in nesting levels.

Therefore I'd consider this a task or feature request, not a bug (also retitled for this). Maybe some discussion indicated this would be useful to add, but this was not added or expected in the initial patch. My understanding in #1953404: Add config schema to field and instance config entities is that we can at least temporarily avoid doing this and have a little more verbose schemas as a result.

yched’s picture

Category:feature» bug

we can at least temporarily avoid doing this and have a little more verbose schemas as a result

A little more verbose but also architecturally incorrect, since this forces schema snippets to redefine over and over again some schema properties about a structure that's actually enforced by the container structure.

So we have to trust 10s of schema snippets to properly expose a schema that's actually not their responsibility but the one of the container - only the container cannnot expose it itself because of syntactic limitations in the include mechanism.

This definitely qualifies as a bug IMO. A bug for which there is a known workaround, but still a bug.

vijaycs85’s picture

Status:Active» Needs review

Current implementation of %parent and %key only concern about immediate parents rather history. Also it saves the whole level-1 data in $replace[%parent]. so not sure how effective it would be to have $replace[%parent][%parent] with multiple set of data. Here is a work around for this problem: https://gist.github.com/vijaycs85/5433410

1. As per https://gist.github.com/vijaycs85/5433410#file-field-instance-default-ym... would have field_type in individual default value.
2. https://gist.github.com/vijaycs85/5433410#file-field-instance-default-ym... - Sequence will be part of main schema definition

flip side, would ask individual default value to provide field_type and same would happen for any other instance.

Gábor Hojtsy’s picture

I think we should be able to replace repeated instances of %parent with %parent0, %parent1, etc, and then traverse the config tree to build up the replacement array with parents?

vijaycs85’s picture

Status:Needs review» Needs work
Gábor Hojtsy’s picture

Issue tags:-sprint

It is a lie that we have people working on this.

jair’s picture

Status:Needs work» Active

Moving back to active, since there doesn't seem to be a patch yet.

yched’s picture

Status:Active» Needs review
StatusFileSize
new1.18 KB
PASSED: [[SimpleTest]]: [MySQL] 57,900 pass(es).
[ View ]

Patch "should" work.

Did not test it on an actual example yet, plus the %parent replacement logic doesn't seem to be covered by tests currently.
ConfigSchemaTest only checks "typed definitions" that still contain unreplaced [%parent.id] tokens. It still passes if I comment out the code that does the actual replacement in TypedConfigManager.

Hints welcome: how do you retrieve the fully resolved schema for a config file, so that you can you can check replacements were correct ?

yched’s picture

StatusFileSize
new5.02 KB
new6.13 KB
FAILED: [[SimpleTest]]: [MySQL] Setup environment: Test cancelled by admin prior to completion.
[ View ]

OK, figured it out.

Now with tests. I think we're good here.

yched’s picture

StatusFileSize
new796 bytes
new6.13 KB
PASSED: [[SimpleTest]]: [MySQL] 57,851 pass(es).
[ View ]

Of course, you only notice typos after posting the patch...

vijaycs85’s picture

Thanks @yched, patch in #14 looks good to me. RTBC from my side.

Gábor Hojtsy’s picture

Status:Needs review» Reviewed & tested by the community
Issue tags:+sprint

Woot, this looks amazing. I did not even expect this may be resolved without a huge refactoring. Wow :) Thanks!

catch’s picture

Status:Reviewed & tested by the community» Fixed

Looks good. Committed/pushed to 8.x, thanks!

Gábor Hojtsy’s picture

Issue tags:-sprint

Yay thanks!

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