This is a sub-issue of #1910624: [META] Introduce and complete configuration schemas in all of core.

Problem/motivation

#1866610: Introduce Kwalify-inspired schema format for configuration introduced some config schema coverage for views, but it is not complete. The changelog leads to (hopefully extensive) documentation on the format at http://drupal.org/node/1905070. While there are little cleanups planned for the format overall, the current format is a result of months of back and forths, so it should be perfectly fine to apply it more widely to core.

Proposed solution

Figure out the missing pieces that are not yet covered. Write schema file sections for them. Clean up / fix any issues in current schema.

Files: 
CommentFileSizeAuthor
#75 1910606-diff-71-73.txt4.25 KBvijaycs85
#73 1910606-config-schema-views-73.patch45.7 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 57,052 pass(es).
[ View ]
#71 1910606-config-schema-views-71.patch43.58 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 55,836 pass(es), 14 fail(s), and 2 exception(s).
[ View ]
#71 1910606-diff-66-71.txt2.65 KBvijaycs85
#66 1910606-config-schema-views-66.patch41.84 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 56,032 pass(es), 16 fail(s), and 2 exception(s).
[ View ]
#66 1910606-diff-63-66.txt4.73 KBvijaycs85
#63 1910606-config-schema-views-63.patch42.54 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 56,025 pass(es), 19 fail(s), and 2 exception(s).
[ View ]
#63 1910606-diff-59-63.txt2.68 KBvijaycs85
#59 1910606-config-schema-views-59.patch41.56 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 56,290 pass(es), 14 fail(s), and 2 exception(s).
[ View ]
#59 1910606-diff-53-39.txt7.5 KBvijaycs85
#53 1910606-config-schema-views-53.patch41.97 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 55,361 pass(es), 15 fail(s), and 2 exception(s).
[ View ]
#53 1910606-diff-49-53.txt25.75 KBvijaycs85
#49 1910606-config-schema-views-49.patch39.51 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 57,011 pass(es).
[ View ]
#49 1910606-diff-46-49.txt39.78 KBvijaycs85
#49 views-schema.png121.48 KBvijaycs85
#46 1910606-config-schema-views-46.patch37.79 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] 57,050 pass(es), 0 fail(s), and 2 exception(s).
[ View ]
#31 1910606-drupal-views-configuration-schema-31.patch30.39 KBsandipmkhairnar
PASSED: [[SimpleTest]]: [MySQL] 53,237 pass(es).
[ View ]
#25 drupal-views-configuration-schema-1910606-25.patch32.33 KBmavimo
PASSED: [[SimpleTest]]: [MySQL] 53,217 pass(es).
[ View ]
#5 1910606-5-complete-configuration-schema-for-views.patch4.88 KBmavimo
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1910606-5-complete-configuration-schema-for-views.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#1 drupal-1910606-1.patch4.68 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 49,464 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

dawehner’s picture

StatusFileSize
new4.68 KB
FAILED: [[SimpleTest]]: [MySQL] 49,464 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

This task is quite huge, but could be separated and help a lot to get new people involved.

You could for example split this task up into one per handler type, and one per core module integration.

Gábor Hojtsy’s picture

Status:Active» Needs review

Looks cool :)

Status:Needs review» Needs work

The last submitted patch, drupal-1910606-1.patch, failed testing.

tim.plunkett’s picture

Issue tags:+Configuration system, +VDC

Tagging

mavimo’s picture

StatusFileSize
new4.88 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1910606-5-complete-configuration-schema-for-views.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Added Drupal\views\Plugin\views\area\Result

dawehner’s picture

I'm wondering whether we should create a sandbox for that, as there will be a lot of files involved.

In general I would also suggest to split the actual schema files up if possible. @mavimo Maybe you have some ideas in your mind.

mavimo’s picture

@dawehner maybee we will split it as a list of files, one for plugin type (area, argument, default argument, ...) eg:

  • views.area.schema.yml
  • views.argument.schema.yml
  • views.argument_validator.schema.yml
  • ...

and add specific information for each plugin type... I'm working on "moving" options definition in the views.schema.yml file.. I can split it if can be more easy to mantain.

dawehner’s picture

I like that idea, so they are not too many files, but also not one big.

mavimo’s picture

Gábor Hojtsy’s picture

As per the current codebase (in Config/Schema/ScehamDiscovery), only one schema file per module will be recognized, the one exactly named after a module. Earlier we proposed a full separate directory where each "type" definition would have been its own file, and @effulgentsia suggested to merge into one file and place it alongside default conf files. I see collaboration is much easier on multiple files. The question is if you see it viable to merge it later into one file, or would it generally be better as separate files (and therefore needing code adjustments in Schema discovery and possibly going back to a separate dir for schema files).

dawehner’s picture

Good to know that this changed, thanks!
I'm okay with just having one file, but maybe other people would be confused by a maybe 1000 lines schema file.

mavimo’s picture

Maybee will be better using different files and runtime load all *.schema.yml files and "merge" the configuration. This will be usefull in all the modules that define a lot of data structre (eg views, but we can also immagine some other "complex modules" that can have a lot of files). Actually I create approx a dozen of schema files on views config folder with 900 LoC, but i think I'm just to half of work.

To have multiple schema file loading (and merging) just a small change is required in Config\Schema\SchemaDiscovery::loadSchema function, to load each file inside just one file.

Gábor Hojtsy’s picture

Yeah, right, so if there are multiple schema files, then the question of whether still storing them with the other config files, or their presence side by side is more confusing them it helps comes up. Because *.schema.yml files would be describing *.yml files that don't end in "schema". One exception in the directory seemed fine, but an arbitrary number might call for a separate directory. What do you think?

mavimo’s picture

A lot of core module not have/have only one schema file so there are a lot of file with:

module_1
|- config
|   |- schema
|       |-module_1.schema.yml
| ...

with only one file and this can "complicate" the structure, but whe need also consider that will be some module with complex schema outside core, so have a unique file can be "a problem" and a separate folder can help to separate the schema config without adding an exception in configuration file naming, so Ithink having a separate folder can be a semplification for "community contribution".

heyrocker’s picture

Somehow I had missed that we moved the schema files alongside all the other config, and I would really like to see that reversed as it is going to make the import process more complicated. We already have to ignore the manifest files in certain cases, I'd prefer not to add any more. I think a 'schema' directory underneath config makes the most sense. Should we just spawn a new issue for this and moving back to multiple files? Or one for each?

Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

Issue tags:+Configuration schema
Gábor Hojtsy’s picture

Gábor Hojtsy’s picture

All right schemas are now in their own directory and can be in any number of files. #1914366: Move all configuration schema files into a schema subdirectory is committed. So please go continue this work :)

There is also a config inspector module available to help you review your schemas and how they help expose the views data for form building (for translation to be in contrib) and identifying translatable pieces (in core). See more at http://drupal.org/node/1910624#comment-7088154 for tips.

mavimo’s picture

thx, I'll be back from holiday next week and can work on complete this task.

Gábor Hojtsy’s picture

I dont think anything should block this anymore. Any takers?

mavimo’s picture

@gabor I've worked on this issue on Drupal Code spreent this weekend, but not completed, I will push my upgrade on this week.

Gábor Hojtsy’s picture

Superb, thanks! I believe Views is the only place where text+format combinations exist in configuration in core(?), so this will help drive improvements to the schema system to support those cases too. Looking forward to your updates.

Gábor Hojtsy’s picture

@mavimo: I think this schema is the best stress-test for the schema system, and therefore highly important for Drupal 8's progress. If you can post your progress, others could help reviewing it and providing guidance. Thanks a lot for your continued work!

mavimo’s picture

StatusFileSize
new32.33 KB
PASSED: [[SimpleTest]]: [MySQL] 53,217 pass(es).
[ View ]

Update using multiple files in schema folder.

This contains only:

  • access
  • cache
  • display
  • exposed_form
  • handler
  • pager
  • schema
  • style

Other schema definition:

  • area
  • argument_default
  • argument
  • argument_validator
  • display_extender
  • field
  • filter
  • sort

need to be improved and tested before I can post a patch.

To test schema definition I suggest to use http://drupal.org/project/config_inspector module. To test it i create some views and check this options. NB: default views settings are not exported so we need to change all information you need to check.

There are some point need to be "improved", in sequence we need to create label with different information (Eg: Field group number %i).

dawehner’s picture

@gabor

Do you think it would be doable to get even this not 100% perfect patch in? Document each key and find a proper title seems to be a task which could be done over time?

Gábor Hojtsy’s picture

Status:Needs work» Needs review

I totally agree we should make progress without targeting a full schema coverage in the first commit. It is easier to get pieces reviewed. Especially that we allow for several files, we can get more issues going for specific parts. I'm not a good person to figure out AFAIS how to split up the views schema and we will need reviewers to help with making sure the structure is properly represented in the schema.

The proposed patch misses labels at many high level elements on the top level mappings. It is also missing quotes in several labels, even when multiple words are involved in the label. Just some of these:

+++ b/core/modules/views/config/schema/views.access.schema.ymlundefined
@@ -0,0 +1,17 @@
+views.access.*:
+  type: mapping
+
+views.access.none:
+  type: 'views.access.%'
+
+views.access.perm:
+  type: 'views.access.%'
+  label: Permission
+  mapping:
+    perm:
+      type: string
+      label: Permission
+
+views.access.role:
+  type: 'views.access.%'
+  label: Role

Labels should have quotes around them (even if single word within the schema). Missing labels on some items.

+++ b/core/modules/views/config/schema/views.sort.schema.ymlundefined
@@ -0,0 +1,18 @@
+views.sort.*:
+  type: mapping
+  mapping:
+    field:
+      type: string
+      label: Field name
+    id:
+      type: string
+      label: Field ID
+    order:
+      type: string
+      label: Order
+    table:
+      type: string
+      label: Table name
+    plugin_id:
+      type: string

Missing quotes.

mavimo’s picture

@gabor: you are right, YAML do not require to quote all string and I can't find a coding standard for schema element, I replace all string to have double quote. To all level label, I think this will not possibile/required. Some items are yust used as "base" for some other element (like abstract class) and liv only in other context (concrete class).

If you have some "suggestion" as coding standard i wil provide to change the already write (and write but not pushed) schema files.

Gábor Hojtsy’s picture

@mavimo: the Drupal config schema coding standards are documented at http://drupal.org/node/1905070#codestyle, definitely DO NOT use double quotes ever in your schema (or config data) yml files.

mavimo’s picture

Some improvement in sandbox, need more works before start to test and generate a new patch.

sandipmkhairnar’s picture

StatusFileSize
new30.39 KB
PASSED: [[SimpleTest]]: [MySQL] 53,237 pass(es).
[ View ]

Thansk @Gabor. As per the http://drupal.org/node/1905070#codestyle style use single quotes for label values and updated patch.

Gábor Hojtsy’s picture

Superb. It would be great to get some more experienced views people to review this one. Also trying this with http://drupal.org/project/config_translation, you should have a Translation operation in your views listing on each view and check how this is exposed as a translation user interface for your views. Exciting? :)

dawehner’s picture

What a huge amount of work done already! Here is just a review of the first bits. It's looking pretty well.

+++ b/core/modules/views/config/schema/views.access.schema.ymlundefined
@@ -0,0 +1,17 @@
+views.access.perm:
...
+views.access.role:

This two access plugins are defined by user module, so shouldn't the schema be defined there?

+++ b/core/modules/views/config/schema/views.area.schema.ymlundefined
@@ -0,0 +1,63 @@
+    content:
+      type: text
+      label: 'The shown text of the area'
+    format:
+      type: string
+      label: 'The filter format the content is in'

In D7 you always required the input format in order to translate such a text properly. Is this still required in Drupal 8? If yes, will this work automatically, or do we have to specify another key ?

+++ b/core/modules/views/config/schema/views.area.schema.ymlundefined
@@ -0,0 +1,63 @@
+      label: 'The title which will be overriden for the page'

Just wondering whether we have a status that describes whether we need a dot or not at the end of a label. Just in general, these long texts seems more like a description then a label?

+++ b/core/modules/views/config/schema/views.area.schema.ymlundefined
@@ -0,0 +1,63 @@
+    view_to_insert:
...
+    inherit_to_arguments:

Out of scope: Just in general, we could open an issue to name these variables better.

mavimo’s picture

@dawehner @gabor

+++ b/core/modules/views/config/schema/views.access.schema.ymlundefined
@@ -0,0 +1,17 @@
+views.access.perm:
...
+views.access.role:

This two access plugins are defined by user module, so shouldn't the schema be defined there?

You are right, now I have a doubt, what name I need to use?

  1. user.views.access.perm
  2. views.access.perm
  3. user.access.perm

The first is the most autoesplicative (main module / plugins definition ), but to have consistence all schema definition (views) will be change to:

  • views.views.area
  • views.views.pager.sql_base
  • ...

let me know your point of view.

dawehner’s picture

I would vote for user.views.access.*, so we basically match the schema of the namespaces. First the module, then the module which provides the plugin type, then the plugin type etc.f

mavimo’s picture

@dawehner so, also schemda definition in views will be changed to views.views.* ?

Gábor Hojtsy’s picture

@mavimo: the single most important point is that the schema keys would be unique, starting the views keys with views.* would be enough in itself for that if no other module would do that, no? I think you start to reach interesting territory when you reference a dynamic type for a plugin, and would say it's type would be 'views.[%parent.key]...' or somesuch, then it could be a views module provided type or a contrib provided type, right? So if such cases exist, then you need to use views. as a prefix for other module provided types. If the dynamic part (eg. [%parent.key]) is always prefixed with the providing module name, then it can be the first component, and you can forego the views prefix.

mavimo’s picture

@Gábor Hojtsy: there are two motivation to have views.views.*.
First is (and IMHO most important) is to have a consistent naming in schema definition.
Schema require only to be unique, right, so i can call it "wertyiop.display.*" but a user can't undestand what it mean. Having a "standard" naming in schema configuration will be better, and we can propose to be based on [module-name].[plugin-namespace], in views we can find plugins in:

  • $DRUPAL_ROOT/core/modules/views/lib/Drupal/views/Plugin

inside it we can find the following folder structure:

.
├── block
│   └── block
├── Core
│   └── Entity
├── Derivative
├── entity_reference
│   └── selection
└── views
    ├── access
    ├── area
    ├── ...
    ├── query
    ├── ...
    └── wizard

a good idea can be to have:

views.block.block.*
views.entity_reference.selection.*
views.views.area.*
...
views.views.query.*
...
views.views.wizard.*

The second point is what are you highlighted on dynamic reference, but i can think this can be less important (we can have views.views.[%parent.type] or views.[%parent.type]).

Gábor Hojtsy’s picture

Right, I think those are good arguments to use views.view.*. For the dynamic types, that is where the naming of plugins in the data and the naming of plugin types in the schema meet, so that will be the test to the applicability of the naming scheme to the use of plugins :)

mavimo’s picture

@Gábor Hojtsy I will update and push on sandbox..

Gábor Hojtsy’s picture

Issue tags:+sprint

Add on D8MI sprint for closer tracking.

dawehner’s picture

Issue tags:-sprint

Good points, I totally agree!

dawehner’s picture

Issue tags:+sprint
Gábor Hojtsy’s picture

Status:Needs review» Needs work

Any progress on this lately? It would be great to be able to cover more of views with config translation sooner than later :) Also, the schema writing for fields uncovered some missing pieces in schema capabilities (#1953404: Add config schema to field and instance config entities and related issues) so I assume pushing forward with this might uncover some as well, which would need API changes, so the sooner the better.

Gábor Hojtsy’s picture

@mavimo: looks like the last commit on http://drupal.org/node/1912550/commits was in March. Are you still around to continue this? Should we extract the state of this in patch form and get others on this? I think Views schemas are huge enough of an issue that we need to break this up and make progress as much as possible. We'll likely need API changes in the schema system (eg. text with input format to be a type), and we need to work through this to identify such things. Therefore it sounds like there is less than two months, if we want to make sure APIs can be changed as needed.

vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new37.79 KB
FAILED: [[SimpleTest]]: [MySQL] 57,050 pass(es), 0 fail(s), and 2 exception(s).
[ View ]

Work done in this patch:

1. updated main views.schema.yml
2. Added views.data_types.schema.yml
3. Updated views.cache.yml (complete for core)
4. Updated viwes.display.yml (complete for core)

TODO:
Content type change in below schemas:
views.access.schema.yml
views.area.schema.yml
views.argument.schema.yml
views.argument_default.schema.yml
views.argument_validator.schema.yml
views.display_extender.schema.yml
views.exposed_form.schema.yml
views.field.schema.yml
views.filter.schema.yml
views.handler.schema.yml
views.pager.schema.yml
views.sort.schema.yml
views.style.schema.yml

Once defined patten, we can have sub tasks to other modules to issue their part. Here is a alpha sample (haven't identified all yet) :) #1991260: Provide config schema to views components in node module

Status:Needs review» Needs work

The last submitted patch, 1910606-config-schema-views-46.patch, failed testing.

Gábor Hojtsy’s picture

The fails seem to be with sequence syntax:

Invalid argument supplied for foreach() Warning Sequence.php 23 Drupal\Core\Config\Schema\Sequence->parse()
Invalid argument supplied for foreach() Warning Sequence.php 23 Drupal\Core\Config\Schema\Sequence->parse()

Namely:

+++ b/core/modules/views/config/schema/views.filter.schema.ymlundefined
@@ -0,0 +1,172 @@
+views.filter.combine:
+  type: views.filter.string
+  mapping:
+    fields:
+      type: sequence
+      seqence:
+        - type: views.field
+
+# Views filter plugin: Drupal\views\Plugin\views\filter\Date
+views.filter.date:
+  type: views.filter.numeric
+  mapping:
+    fields:
+      type: sequence
+      seqence:
+        - type: views.field
+

Not lack of u in sequence at two places.

vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new121.48 KB
new39.78 KB
new39.51 KB
PASSED: [[SimpleTest]]: [MySQL] 57,011 pass(es).
[ View ]

Thanks for the review @Gábor Hojtsy. Fixing warning and few additional schemas for other plugins in #46's TODO. It looks amazing, Still lots pending though :)

Status:Needs review» Needs work
Issue tags:-Configuration system, -D8MI, -sprint, -language-config, -VDC, -Configuration schema

The last submitted patch, 1910606-config-schema-views-49.patch, failed testing.

vijaycs85’s picture

Status:Needs work» Needs review
Issue tags:+Configuration system, +D8MI, +sprint, +language-config, +VDC, +Configuration schema

#49: 1910606-config-schema-views-49.patch queued for re-testing.

Gábor Hojtsy’s picture

Thanks! It would be great to stop at a reasonable point (soon :) and let reviewers take hold. There are lots to review :) for example I noticed there are some checkboxes without labels on the screenshot.

vijaycs85’s picture

Issue tags:+views configuration schema
StatusFileSize
new25.75 KB
new41.97 KB
FAILED: [[SimpleTest]]: [MySQL] 55,361 pass(es), 15 fail(s), and 2 exception(s).
[ View ]

In this patch:
1. Updated labels for all schemas - couldn't find for few in views.fields.schama in front-end so updated as per Key. Other files should be sync with front end.
2. Moved module specific (taxonomy, user, node) plugin parts to their own issue.
3. Added new issue for other modules and issued Initial patch - http://drupal.org/project/issues/search/drupal?issue_tags=views%20config...

TODO / Yet to fix:
1. Group items are not working because of [%parent] not working inside sequence. Related issue: #1972816: Add support for %parent.%parent in config schema dynamic type names
2. PHP code field in argument_validation and default_argument as 'text' - however in schema text meaning translatable?
3. Test all parts of views and make sure all working fine.
4. Documentation on 'How to write schema for your view plugins'.

Status:Needs review» Needs work
Issue tags:-Configuration system, -D8MI, -sprint, -language-config, -VDC, -Configuration schema, -views configuration schema

The last submitted patch, 1910606-config-schema-views-53.patch, failed testing.

vijaycs85’s picture

Status:Needs work» Needs review

#53: 1910606-config-schema-views-53.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Configuration system, +D8MI, +sprint, +language-config, +VDC, +Configuration schema, +views configuration schema

The last submitted patch, 1910606-config-schema-views-53.patch, failed testing.

dawehner’s picture

I just went until maybe 45% but I got quite confused of views_sort_ vs. views.sort. ... i guess this is just some leftover code?

Here are some just general comments, I hope they are helpful.

+++ b/core/modules/views/config/schema/views.access.schema.ymlundefined
@@ -0,0 +1,20 @@
+views.access.none:
+  type: string

That's a bit odd, where is there a type string but no actual configuration ... , so do we actually it? Maybe this is here just for consistency?

+++ b/core/modules/views/config/schema/views.area.schema.ymlundefined
@@ -0,0 +1,58 @@
+views.area.*:
+  type: views_area
+  label: 'Default area'

Is there a reason why there is views.area.* but no views.access.* for example?

+++ b/core/modules/views/config/schema/views.area.schema.ymlundefined
@@ -0,0 +1,58 @@
+      label: 'The ID of the view which will be displayed'

Technical it's both the ID of the view + the display ID. I'm wondering whether we should try to be a bit more consistent with the labels of the ui.

+++ b/core/modules/views/config/schema/views.argument.schema.ymlundefined
@@ -0,0 +1,84 @@
+    path_case:
+      type: string
+      label: 'Case in path'

Is there a way to define: it's one of the following possible values? (it's a select list in the UI).

+++ b/core/modules/views/config/schema/views.argument.schema.ymlundefined
@@ -0,0 +1,84 @@
+views.argument.summary_options.default_summary:
+  type: mapping
+  label: 'Summary options'
+  mapping:
+    base_path:
+      type: string
+      label: 'Base path'
+    count:
+      type: boolean
+      label: 'Display record count with link'
+    override:
+      type: boolean
+      label: 'Override number of items to display'
+    items_per_page:
+      type: integer
+      label: 'Items to display'

Is that the proper way to define options which are defined on the base views argument? It just feels wrong because it has the potential to conflict with plugins.

+++ b/core/modules/views/config/schema/views.argument_default.schema.ymlundefined
@@ -0,0 +1,28 @@
+views.argument_default.php:
...
+    code:
+      type: text

Just mentioned before: the text is certainly not there to be translated ... that's actually a big security problem

+++ b/core/modules/views/config/schema/views.argument_default.schema.ymlundefined
@@ -0,0 +1,28 @@
+    index:
+      type: string

The index is actually a number, i guess we can use it then?

+++ b/core/modules/views/config/schema/views.argument_validator.schema.ymlundefined
@@ -0,0 +1,17 @@
+      type: text

Again ... text

+++ b/core/modules/views/config/schema/views.cache.schema.ymlundefined
@@ -0,0 +1,33 @@
+          label: 'The length of time raw query results should be cached.'

Just in general I'm wondering whether these labels should have a dot or not?

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+        - type: views.field.[table]_[field]

We do have plugin_id now in the views config, which points to the actual plugin we want to refer here. table_field most of the time doesn't map .

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    pager:
...
+        options:
...
+    exposed_form:
...
+          label: 'Options'

I guess there is a missing label on the pager/access etc.?

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+          type: views.pager.[%parent.type]

I just realized that the pieces defined below like views_pager_sql wouldn't map to this schema.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    other:
+      label: 'Other'

I don't think there is an option called "other".

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    sorts:
...
+    arguments:

What about grouping the different handler types?

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    query:
+      type: mapping
+      label: 'Query'
+      mapping:
+        type:
+          label: 'Query type'
+        options:
+          type: mapping
+          label: 'Query options'
+          mapping:
+            query_comment:
+              type: boolean
+              label: 'Query comment'

Oh we should make an extra issue to also set the plugin ID of the query plugin in the views config, so we can refer it from here. I mention this primarily because there are other options.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    defaults:
+      type: mapping
+      label: 'Defaults'
+      mapping:
+        style_plugin:
+          label: 'Style plugin'
+        style_options:
+          type: views.style.[%parent.style_plugin]
+        row_plugin:
+          label: 'Row plugin'
+        row_options:
+          type: views.style.[%parent.row_plugin]

Wait ... isn't defaults just a mapping of boolean values to mark that certain suboptions are overridden? I don't think we have to refer to actual instances of style plugins etc. Yeah maybe that's just a copy and paste problem.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    display_options:
+      type: mapping
+      label: 'Display options'
+      mapping:
+        exposed_block:
+          type: boolean
+          label: 'Exposed form in block'

Oh that's certainly also quite confusing. All this stuff from above (fields, style etc.) is stored below the display_options property in the config.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+views_sort:

Is there a reason why sort is not part of a separate file, i guess this would make sense.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    id:
+      type: string
+      label: 'ID'
+    table:
+      type: string
+      label: 'Table name'
+    field:
+      type: string
+      label: 'Field name'
+    relationship:
+      type: string
+      label: 'Relationship'
+    group_type:
+      type: string
+      label: 'Group type'
+    admin_label:
+      type: label
+      label: 'Administrative title'

This properties could all be documented once on a generic handler plugin schema and the rest just inherited. I see we have views_handler so why not use it :)

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+views_area:
...
+views_handler:

I guess we should better move this also to extra files?

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+      type: views.argument.summary_options.[%parent.summary.format]

I guess this should refer to views.style.[%parent.summary.format], because summaries are just a special style plugin.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+          label: 'Sort by'

That's the wrong label for number_of_records :)

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    validate_options:
+      type: views.style.[%parent.validate.type]

Should be views.argument_validator...

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    submit_button:
+      type: string
+      label: 'Submit button text'
...
+    reset_button_label:
+      type: string
+      label: 'Reset button label'
+    exposed_sorts_label:
+      type: string
+      label: 'Exposed sorts label'
...
+    sort_asc_label:
+      type: string
+      label: 'Ascending'

I guess all this options should be marked as translatable?

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+        alt:
+          type: string
+          label: 'Title text'
...
+        prefix:
+          type: string
+          label: 'Prefix text'
+        suffix:
+          type: string
+          label: 'Suffix text'
...
+        more_link_text:
+          type: string
+          label: 'More link label'

These ones should be translatable...

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+views_pager:
+  type: mapping
+  label: 'Pager'
+  mapping:
+    offset:
+      type: integer
+      label: 'Offset'

Not all views pager have an offset. For example the SqlPluginBase one defines one.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+          type: string
+          label: 'Next page link text'
...
+          type: string
+          label: 'Previous page link text'
...
+        items_per_page_label:
+          type: boolean
...
+        items_per_page_options_all_label:
+          type: string
+          label: 'All items label'
...
+        offset_label:
+          type: string
+          label: 'Offset label'

... These ones should be all translatable?

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+views_sort:

Ups, this has been defined above already, see comment before.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    value:
+      type: views.filter.[plugin_id]
+      label: 'Value'

I don't think we need this here. The reference views.filter.[plugin_id] should take care about all the inheritance of the schema.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+        label:
+          type: label
+          label: 'Label'
+        description:
+          type: label
+          label: 'Description'

... Translatable values.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+          label: 'User roles'

I think we need a better name for it.

Gábor Hojtsy’s picture

Quick answers to type related questions:

1. No, config schemas do not have an enum type where you can say which possible values are allowed (I don't think its feasible to introduce, we barely got in schemas as-is with fear from committers that they are too complex; they are definitely Drupalisms at their "best").

2. The text and label types have translatability pre-defined; it is suggested to use those types for translatable textual data (long and one-line respectively). While schemas *can* provide additional translatable types, best practice is to use the text and label types. You called out translatable values which were already labels, those should be fine as-is. Others which are "string" types should use label type instead.

vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new7.5 KB
new41.56 KB
FAILED: [[SimpleTest]]: [MySQL] 56,290 pass(es), 14 fail(s), and 2 exception(s).
[ View ]

Thanks for the review @dawehner and @Gábor Hojtsy. Please find updates on review comments:

1.

+++ b/core/modules/views/config/schema/views.access.schema.ymlundefined
@@ -0,0 +1,20 @@
+views.access.none:
+  type: string

A: We know that we won't get anything for options for type: none. However we save it as options: {}. If we set type as sequence, we have to provide some element :). However I made it sequence to avoid warning.

2.

+++ b/core/modules/views/config/schema/views.area.schema.ymlundefined
@@ -0,0 +1,58 @@
+views.area.*:
+  type: views_area
+  label: 'Default area'

Q: Is there a reason why there is views.area.* but no views.access.* for example?
A: Other then views, we never have this .* for things that we don't know. However views it is necessary as we can't add/expect to have schema for all [table]_[field] for example.

3.Technical it's both the ID of the view + the display ID. I'm wondering whether we should try to be a bit more consistent with the labels of the ui. - Updated label from UI.

4.

+++ b/core/modules/views/config/schema/views.argument.schema.ymlundefined
@@ -0,0 +1,84 @@
+    path_case:
+      type: string
+      label: 'Case in path'

Is there a way to define: it's one of the following possible values? (it's a select list in the UI). - We can do it with like type: prefix.[.type]

5.

+++ b/core/modules/views/config/schema/views.argument.schema.ymlundefined
@@ -0,0 +1,84 @@
+views.argument.summary_options.default_summary:
+  type: mapping
+  label: 'Summary options'
+  mapping:
+    base_path:
+      type: string
+      label: 'Base path'
+    count:
+      type: boolean
+      label: 'Display record count with link'
+    override:
+      type: boolean
+      label: 'Override number of items to display'
+    items_per_page:
+      type: integer
+      label: 'Items to display'

Is that the proper way to define options which are defined on the base views argument? It just feels wrong because it has the potential to conflict with plugins.
- Not sure what do you mean here...
6.

+++ b/core/modules/views/config/schema/views.argument_default.schema.ymlundefined
@@ -0,0 +1,28 @@
+views.argument_default.php:
...
+    code:
+      type: text

Just mentioned before: the text is certainly not there to be translated ... that's actually a big security problem - We can make it simply 'string' to avoid translation

7.

+++ b/core/modules/views/config/schema/views.argument_default.schema.ymlundefined
@@ -0,0 +1,28 @@
+    index:
+      type: string

Q: The index is actually a number, i guess we can use it then?
A: Updated as integer.
8.

+++ b/core/modules/views/config/schema/views.argument_validator.schema.ymlundefined
@@ -0,0 +1,17 @@
+      type: text

Again ... text
A: Updated as 'string'
9.

+++ b/core/modules/views/config/schema/views.cache.schema.ymlundefined
@@ -0,0 +1,33 @@
+          label: 'The length of time raw query results should be cached.'

Q: Just in general I'm wondering whether these labels should have a dot or not?
A: We just follow as close as UI.

10.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+        - type: views.field.[table]_[field]

Q: We do have plugin_id now in the views config, which points to the actual plugin we want to refer here. table_field most of the time doesn't map.
A: We don't have plugin in fields.[field] level. here is a sample:

      fields:
        title:
          id: title
          table: node
          field: title
          label: ''
          alter:
            alter_text: '0'
            make_link: '0'
            absolute: '0'
            trim: '0'
            word_boundary: '0'
            ellipsis: '0'
            strip_tags: '0'
            html: '0'
          hide_empty: '0'
          empty_zero: '0'
          link_to_node: '1'
          relationship: none
          group_type: group
          admin_label: ''
          exclude: '0'
          element_type: ''
          element_class: ''
          element_label_type: ''
          element_label_class: ''
          element_label_colon: '1'
          element_wrapper_type: ''
          element_wrapper_class: ''
          element_default_classes: '1'
          empty: ''
          hide_alter_empty: '1'

11.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    pager:
...
+        options:
...
+    exposed_form:
...
+          label: 'Options'

Q: I guess there is a missing label on the pager/access etc.?
A: Left it empty intentionally to allow calling schema to provide label. if we provide label here, it will override the child label.

12.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+          type: views.pager.[%parent.type]

Q: I just realized that the pieces defined below like views_pager_sql wouldn't map to this schema.
A: views_pager_sql is a data type to different types of pager defined in views.pager.schema.yml. May be I don't get the question here.

13.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    other:
+      label: 'Other'

Q: I don't think there is an option called "other".
A: Removed.

14.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    sorts:
...
+    arguments:

Q: What about grouping the different handler types?
A: the problem here is a) we don't have plugin_id b) sometime getting plugin_id getting value node_id where it is available.

15.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    query:
+      type: mapping
+      label: 'Query'
+      mapping:
+        type:
+          label: 'Query type'
+        options:
+          type: mapping
+          label: 'Query options'
+          mapping:
+            query_comment:
+              type: boolean
+              label: 'Query comment'

Q: Oh we should make an extra issue to also set the plugin ID of the query plugin in the views config, so we can refer it from here. I mention this primarily because there are other options.
A:OK, we need a issue for main view config?
16.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    defaults:
+      type: mapping
+      label: 'Defaults'
+      mapping:
+        style_plugin:
+          label: 'Style plugin'
+        style_options:
+          type: views.style.[%parent.style_plugin]
+        row_plugin:
+          label: 'Row plugin'
+        row_options:
+          type: views.style.[%parent.row_plugin]

Q: Wait ... isn't defaults just a mapping of boolean values to mark that certain suboptions are overridden? I don't think we have to refer to actual instances of style plugins etc. Yeah maybe that's just a copy and paste problem.
A: Yeah, it is replaced with boolean items, but not sure where exactly they are used in UI. So added my own label.
17.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    display_options:
+      type: mapping
+      label: 'Display options'
+      mapping:
+        exposed_block:
+          type: boolean
+          label: 'Exposed form in block'

Q: Oh that's certainly also quite confusing. All this stuff from above (fields, style etc.) is stored below the display_options property in the config.
A: Sorry for the confusion. We don't need display_options here. as you can see in views.schema.yml all defintions in views.display.schema.yml are not for views.view.[view]. they are for views.view.[view].default_options. so ideally the name should be views.display_options.page (instead of views.display.page). however we want to keep them simple. This is applicable for other parts as well.

18.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+views_sort:

Q: Is there a reason why sort is not part of a separate file, i guess this would make sense.
A: We want to have some common data types like the way we have in system.data_types.schema.yml so that contrib can implement with type: views_sort and add additional stuffs. Basically views provides list of content types...

19.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    id:
+      type: string
+      label: 'ID'
+    table:
+      type: string
+      label: 'Table name'
+    field:
+      type: string
+      label: 'Field name'
+    relationship:
+      type: string
+      label: 'Relationship'
+    group_type:
+      type: string
+      label: 'Group type'
+    admin_label:
+      type: label
+      label: 'Administrative title'

Q: This properties could all be documented once on a generic handler plugin schema and the rest just inherited. I see we have views_handler so why not use it :)
A: I can see we can use id, table, field. However they have different global label. however sort has it's own label.

20:

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+views_area:
...
+views_handler:

Q: I guess we should better move this also to extra files?
A: As mentioned in 18, this is data type. But we can move, if it is really necessary.

21.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+      type: views.argument.summary_options.[%parent.summary.format]

Q: I guess this should refer to views.style.[%parent.summary.format], because summaries are just a special style plugin.
A: not sure, they got different elements

views.argument.summary_options.default_summary:
  type: mapping
  label: 'Summary options'
  mapping:
    base_path:
      type: string
      label: 'Base path'
    count:
      type: boolean
      label: 'Display record count with link'
    override:
      type: boolean
      label: 'Override number of items to display'
    items_per_page:
      type: integer
      label: 'Items to display'

22.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+          label: 'Sort by'

Q: That's the wrong label for number_of_records :)
A: Need to fix the field name in config :). This is the label.

23.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    validate_options:
+      type: views.style.[%parent.validate.type]

Q: Should be views.argument_validator...
A: Updated.

24.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    submit_button:
+      type: string
+      label: 'Submit button text'
...
+    reset_button_label:
+      type: string
+      label: 'Reset button label'
+    exposed_sorts_label:
+      type: string
+      label: 'Exposed sorts label'
...
+    sort_asc_label:
+      type: string
+      label: 'Ascending'

Q: I guess all this options should be marked as translatable?
A: Yes, made them as type: label.
25.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+        alt:
+          type: string
+          label: 'Title text'
...
+        prefix:
+          type: string
+          label: 'Prefix text'
+        suffix:
+          type: string
+          label: 'Suffix text'
...
+        more_link_text:
+          type: string
+          label: 'More link label'

Q: These ones should be translatable...
A: Updated as label

27.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+views_pager:
+  type: mapping
+  label: 'Pager'
+  mapping:
+    offset:
+      type: integer
+      label: 'Offset'

Q: Not all views pager have an offset. For example the SqlPluginBase one defines one.
A: Will fix.
28.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+          type: string
+          label: 'Next page link text'
...
+          type: string
+          label: 'Previous page link text'
...
+        items_per_page_label:
+          type: boolean
...
+        items_per_page_options_all_label:
+          type: string
+          label: 'All items label'
...
+        offset_label:
+          type: string
+          label: 'Offset label'

Q: ... These ones should be all translatable?
A: changed as label
29.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+views_sort:

Q: Ups, this has been defined above already, see comment before.
A: sorry, merged them.
30.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+    value:
+      type: views.filter.[plugin_id]
+      label: 'Value'

Q: I don't think we need this here. The reference views.filter.[plugin_id] should take care about all the inheritance of the schema.
A: views.filter.[plugin_id] defined in views.filter.schema.yml is for value.

31.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+        label:
+          type: label
+          label: 'Label'
+        description:
+          type: label
+          label: 'Description'

Q: ... Translatable values.
A: Currently we don't add traslatable: true. We just specify as text/label and type label/text has trasnlatable: TRUE?

32.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,642 @@
+          label: 'User roles'

Q: I think we need a better name for it.
A: Mapping with UI label.

Status:Needs review» Needs work

The last submitted patch, 1910606-config-schema-views-59.patch, failed testing.

pbuyle’s picture

Here is what I tried to translate once I applied the patch in #59, with the missing items. I'm not sure if I was not able to translate the missing item because they are not properly defined in the schema files or if it is a issue with config_translation.

Title (of a display).

  • Title is translatable

Format

  • Grid/Table: Table summary are not translatable.

Fields (for one field)

  • Label is translatable
  • No Result Text is not translatable
  • Rewrite results is translatable

Filters (for one filter)

  • Label is translatable
  • Description is translatable
  • Administrative title is translatable

Sort criteria (for one sort criteria)

  • Label is not translatable
  • Administrative title is translatable

Page settings
I guess menu item title and description are translated via menu translation. They dot not appear in the view translation interface.

Gábor Hojtsy’s picture

I talked to @dawehner about this patch. He said it would be great to slice this up a bit and get some base stuff in and then improve. I'm not 100% sure he meant this as-is already a base step or needs more slicing. @dawehner: can you elaborate on that?

Also we need the test fail fixed. It indicates part of the schema is wrong and is not traversable. Slicing up the schema should also help with that I think.

Fatal error: Call to a member function getValue() on a non-object in /var/lib/drupaltestbot/sites/default/files/checkout/core/modules/locale/lib/Drupal/locale/Tests/LocaleConfigTranslationTest.php on line 87
FATAL Drupal\locale\Tests\LocaleConfigTranslationTest: test runner returned a non-zero error code (255).
vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new2.68 KB
new42.54 KB
FAILED: [[SimpleTest]]: [MySQL] 56,025 pass(es), 19 fail(s), and 2 exception(s).
[ View ]

Thanks for the review @mongolito404. Here is some updated on items in #61.
#61.1 Grid/Table: table summary
Fixed: updated 'string' to 'label

#61.2 No Result Text is not translatable
Not fixed: Have fixed for area_text_custom by adding a new schema file views.empty.schema.yml with area_text_custom. However not very sure how to make this global type to cover all possible options here. As we can see below, there is no global plugin/id/field :(

      empty:
        area_text_custom:
          id: area_text_custom
          table: views
          field: area_text_custom
          relationship: none
          group_type: group
          admin_label: 'admin for no content'
          empty: '1'
          content: 'no content text....'
          tokenize: '0'
          label: ''
          plugin_id: text
        node_listing_empty:
          admin_label: ''
          empty: '1'
          field: node_listing_empty
          group_type: group
          id: node_listing_empty
          label: ''
          relationship: none
          table: node
          plugin_id: node_listing_empty
        title:
          id: title
          table: views
          field: title
          relationship: none
          group_type: group
          admin_label: ''
          label: ''
          empty: '1'
          title: 'Welcome to [site:name]'
          plugin_id: title

#61.3 Label is not translatable
Fixed: added new file for expose settings of sort.

Status:Needs review» Needs work

The last submitted patch, 1910606-config-schema-views-63.patch, failed testing.

dawehner’s picture

Thanks for all the hard work. Here are some small comments.

Not fixed: Have fixed for area_text_custom by adding a new schema file views.empty.schema.yml with area_text_custom. However not very sure how to make this global type to cover all possible options here. As we can see below, there is no global plugin/id/field :(

We do have, see plugin_id

Answers/additional questions to your really detailed list (thanks!!)

1.

A: We know that we won't get anything for options for type: none. However we save it as options: {}. If we set type as sequence, we have to provide some element :). However I made it sequence to avoid warning.

What about adding this as feature to Drupal 8?

5.
See my comments below.

6.
Thanks for fixing it.

7.
Thanks

8.
Thanks

9.
I'm totally fine with that

10.
See below
If there are examples without the plugin_id, we should fix that

11.
Cool

15.
Opened an issue, see below

16.
They aren't shown in the UI at all, maybe just something like: "Style is not overridden"

17.
cool

31.

Currently we don't add traslatable: true. We just specify as text/label and type label/text has trasnlatable: TRUE?

Exactly.

+++ b/core/modules/views/config/schema/views.argument.schema.ymlundefined
@@ -0,0 +1,84 @@
+views.argument.summary_options.default_summary:

These options are defined in DefaultSummary so it should be more in views.style.default_summary

+++ b/core/modules/views/config/schema/views.cache.schema.ymlundefined
@@ -0,0 +1,33 @@
+views.cache.none:
+  type: mapping
+  lable: 'None'
+  mapping:
+    type:
+      type: string
+      label: 'Cache type'

The type should be consistent on all plugins shouldn't it?

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,631 @@
+        - type: views.field.[table]_[field]
...
+        - type: views.filter.[table]_[field]
...
+        - type: views.relationship.[table]_[field]

Should be views.$type.[plugin_id] ... as on the other types.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,631 @@
+    access:
+      type: mapping
+      label: 'Access'
+      mapping:
+        type:
+          type: string
+          label: 'Access type'
+        options:
+          type: views.access.[%parent.type]
+    cache:
+      type: views.cache.[type]
+      label: 'Caching'

Why is cache so different to access?

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,631 @@
+    query:

see #2006648: Add the query plugin ID in the views config

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,631 @@
+    defaults:
+      type: mapping
+      label: 'Defaults'
+      mapping:
+        style:
+          type: boolean
+          label: 'Style'
+        row:
+          type: boolean
+          label: 'Row'
+        pager:
+          type: boolean
+          label: 'Pager'
+        pager_options:
+          type: boolean
+          label: 'Pager options'
+        access:
+          type: boolean
+          label: 'Access'

I think there are quite some of the defaults missing.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,631 @@
+views_sort:

Let's add a short documentation to explain why this basic data types are defined here.

+++ b/core/modules/views/config/schema/views.display.schema.ymlundefined
@@ -0,0 +1,111 @@
+    block_caching:
+      label: 'Block caching'

This should also have a type

+++ b/core/modules/views/config/schema/views.display.schema.ymlundefined
index 0000000..2050a88
--- /dev/null

--- /dev/null
+++ b/core/modules/views/config/schema/views.empty.schema.ymlundefined

+++ b/core/modules/views/config/schema/views.empty.schema.ymlundefined
+++ b/core/modules/views/config/schema/views.empty.schema.ymlundefined
@@ -0,0 +1,5 @@

@@ -0,0 +1,5 @@
+# Schema for the views empty plugins.
+
+views.empty.area_text_custom:
+  type: views.area.text_custom
+  label: 'Unfiltered text'

That's confusing are views.area.text_custom should be enough.

+++ b/core/modules/views/config/schema/views.filter.schema.ymlundefined
index 0000000..da77e8d
--- /dev/null

--- /dev/null
+++ b/core/modules/views/config/schema/views.handler.schema.ymlundefined

+++ b/core/modules/views/config/schema/views.handler.schema.ymlundefined
@@ -0,0 +1 @@
+# Schema for the views handler plugins.

So let's skip this bit :)

+++ b/core/modules/views/config/schema/views.row.schema.ymlundefined
@@ -0,0 +1,21 @@
+    hide_empty:
+      type: string
+      label: 'Hide empty'

hide_empty is a boolean.

vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new4.73 KB
new41.84 KB
FAILED: [[SimpleTest]]: [MySQL] 56,032 pass(es), 16 fail(s), and 2 exception(s).
[ View ]

Thanks for the detailed review @dawehner. here is some updates on your review comments in #65.

1.

We do have, see plugin_id

as I mentioned in #63 (i.e. 61.2), we don't have them very global... plugin_id got text, node_listing_empty and title.

2.

These options are defined in DefaultSummary so it should be more in views.style.default_summary

- Updated
3.

+++ b/core/modules/views/config/schema/views.cache.schema.ymlundefined
@@ -0,0 +1,33 @@
+views.cache.none:
+  type: mapping
+  lable: 'None'
+  mapping:
+    type:
+      type: string
+      label: 'Cache type'

Q: The type should be consistent on all plugins shouldn't it?
A: I guess, type should match the data structure. anything consistent will be separate data_type ?

4.
Q: Should be views.$type.[plugin_id] ... as on the other types.
A: Updated.

5.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,631 @@
+    access:
+      type: mapping
+      label: 'Access'
+      mapping:
+        type:
+          type: string
+          label: 'Access type'
+        options:
+          type: views.access.[%parent.type]
+    cache:
+      type: views.cache.[type]
+      label: 'Caching'

Q: Why is cache so different to access?
A: for cache we are defining the whole part as type where as access, it is just options part. Basically, we keep common stuff in main type. In cache all change by cache type where as access.type is always there and access.options changes on various type.

6.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,631 @@
+    query:

Q: see #2006648: Add the query plugin ID in the views config
A: Thanks, will update query once plugin lands.

7.

+++ b/core/modules/views/config/schema/views.data_types.schema.ymlundefined
@@ -0,0 +1,631 @@
+    defaults:
+      type: mapping
+      label: 'Defaults'
+      mapping:
+        style:
+          type: boolean
+          label: 'Style'
+        row:
+          type: boolean
+          label: 'Row'
+        pager:
+          type: boolean
+          label: 'Pager'
+        pager_options:
+          type: boolean
+          label: 'Pager options'
+        access:
+          type: boolean
+          label: 'Access'

Q: I think there are quite some of the defaults missing.
A: Can we get some config with all possible defaults anyway? I tried to create a view and added whatever I can see there :)

8.

+views_sort:

Q: Let's add a short documentation to explain why this basic data types are defined here.
A: we stopped adding in-line comment as the definition says what it is about. however we explain about the file's purpose at the top

# Basic data types for views.

. Please let me know, if we need to update anything more there?

9.

+++ b/core/modules/views/config/schema/views.display.schema.ymlundefined
@@ -0,0 +1,111 @@
+    block_caching:
+      label: 'Block caching'

Q: This should also have a type
A: Updated

10.

+++ b/core/modules/views/config/schema/views.display.schema.ymlundefined
index 0000000..2050a88
--- /dev/null

--- /dev/null
+++ b/core/modules/views/config/schema/views.empty.schema.ymlundefined

+++ b/core/modules/views/config/schema/views.empty.schema.ymlundefined
+++ b/core/modules/views/config/schema/views.empty.schema.ymlundefined
@@ -0,0 +1,5 @@

@@ -0,0 +1,5 @@
+# Schema for the views empty plugins.
+
+views.empty.area_text_custom:
+  type: views.area.text_custom
+  label: 'Unfiltered text'

Q: That's confusing are views.area.text_custom should be enough.
A: Fixed (ref: https://gist.github.com/vijaycs85/5669850)

11.

+++ b/core/modules/views/config/schema/views.filter.schema.ymlundefined
index 0000000..da77e8d
--- /dev/null

--- /dev/null
+++ b/core/modules/views/config/schema/views.handler.schema.ymlundefined

+++ b/core/modules/views/config/schema/views.handler.schema.ymlundefined
@@ -0,0 +1 @@
+# Schema for the views handler plugins.

Q: So let's skip this bit :)
A: Removed this file.

12.

+++ b/core/modules/views/config/schema/views.row.schema.ymlundefined
@@ -0,0 +1,21 @@
+    hide_empty:
+      type: string
+      label: 'Hide empty'

Q: hide_empty is a boolean.
A: Updated.

Status:Needs review» Needs work
Issue tags:-Configuration system, -D8MI, -sprint, -language-config, -VDC, -Configuration schema, -views configuration schema

The last submitted patch, 1910606-config-schema-views-66.patch, failed testing.

vijaycs85’s picture

Status:Needs work» Needs review

#66: 1910606-config-schema-views-66.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Configuration system, +D8MI, +sprint, +language-config, +VDC, +Configuration schema, +views configuration schema

The last submitted patch, 1910606-config-schema-views-66.patch, failed testing.

dawehner’s picture

Q: Why is cache so different to access?
A: for cache we are defining the whole part as type where as access, it is just options part. Basically, we keep common stuff in main type. In cache all change by cache type where as access.type is always there and access.options changes on various type.

It would be cool if it would be consistent.

The default values of "defaults" is:

          'access' => TRUE,
          'cache' => TRUE,
          'query' => TRUE,
          'title' => TRUE,
          'css_class' => TRUE,

          'display_description' => FALSE,
          'use_ajax' => TRUE,
          'hide_attachment_summary' => TRUE,
          'show_admin_links' => TRUE,
          'pager' => TRUE,
          'use_more' => TRUE,
          'use_more_always' => TRUE,
          'use_more_text' => TRUE,
          'exposed_form' => TRUE,

          'link_display' => TRUE,
          'link_url' => '',
          'group_by' => TRUE,

          'style' => TRUE,
          'row' => TRUE,

          'header' => TRUE,
          'footer' => TRUE,
          'empty' => TRUE,

          'relationships' => TRUE,
          'fields' => TRUE,
          'sorts' => TRUE,
          'arguments' => TRUE,
          'filters' => TRUE,
          'filter_groups' => TRUE,

The proper way to find the defaults are the defineOptions() function in every class, for example DisplayPluginBase::defineOptions()

vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new2.65 KB
new43.58 KB
FAILED: [[SimpleTest]]: [MySQL] 55,836 pass(es), 14 fail(s), and 2 exception(s).
[ View ]

Updating default from #70

Status:Needs review» Needs work

The last submitted patch, 1910606-config-schema-views-71.patch, failed testing.

vijaycs85’s picture

Status:Needs work» Needs review
StatusFileSize
new45.7 KB
PASSED: [[SimpleTest]]: [MySQL] 57,052 pass(es).
[ View ]

Adding patch with some more fix.

Gábor Hojtsy’s picture

What was the problem? Can you provide an interdiff?

vijaycs85’s picture

StatusFileSize
new4.25 KB

Missed type in one element and had string instead of sequence. here is the interdiff...

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Let's get it in, which will allow us to work on all the other needed configuration schemas for the module specific integration code.

Gábor Hojtsy’s picture

Title:Complete configurations schema for Views» Improve the configurations schemas for Views significantly

Making the title more accurate. We'll still need followups for more things, a 45k patch in itself is big enough to make a big move forward and dawehner is on board with the direction.

alexpott’s picture

Status:Reviewed & tested by the community» Fixed
alexpott’s picture

Somehow this went missing...

Committed 9a42b9f and pushed to 8.x. Thanks!

Gábor Hojtsy’s picture

Ok there is now a huge army of issues to cover the rest of the views schema. Reviews welcome on https://drupal.org/project/issues/search/drupal?issue_tags=views%20confi...

YesCT’s picture

There were some typos. See follow-up #2021173: Fix typo: lable to label

Gábor Hojtsy’s picture

Issue tags:-sprint
Gábor Hojtsy’s picture

Remove sprint tag.

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

Anonymous’s picture

Issue summary:View changes

Add meta issue