This is a sub-issue of #1910606: Improve the configurations schemas for Views significantly.
Problem/motivation
#1866610: Introduce Kwalify-inspired schema format for configuration introduced some config schema coverage for views and #1910606: Improve the configurations schemas for Views significantly extended it, 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.
Create a configuration schema for missing view row.
Schema in place
Please refer core/modules/views/config/schema/views.row.schema.yml
Fields.php
Schema not yet in place
RssFields.php
EntityRow.php derivatives: CommentRow.php, NodeRow.php and UserRow.php
Steps to review
Check Steps to check on meta.
Comment | File | Size | Author |
---|---|---|---|
#24 | Screen Shot 2013-12-08 at 18.25.20.png | 53.74 KB | vijaycs85 |
#24 | 2019091-config-schema-views-row-24.patch | 2.82 KB | vijaycs85 |
#24 | 2019091-diff-20-24.txt | 2.19 KB | vijaycs85 |
#20 | 2019091-config-schema-views-row-19.patch | 2.56 KB | ianthomas_uk |
#20 | 2019091-15-19-interdiff.txt | 2.01 KB | ianthomas_uk |
Comments
Comment #1
manu4543 CreditAttribution: manu4543 commentedComment #2
manu4543 CreditAttribution: manu4543 commentedComment #3
littleindian CreditAttribution: littleindian commentedI have added configuration of EntityRow.php and RssFields.php in core/modules/views/config/schema/views.row.schema.yml
Comment #5
littleindian CreditAttribution: littleindian commentedPatch updated. Please review.
Comment #7
littleindian CreditAttribution: littleindian commented#5: views-row-configuration-schemas-for-views-row-2019091-4.patch queued for re-testing.
Comment #9
vijaycs85@ashwinikumar, the problem seems to be tab vs space.
Comment #11
vijaycs85Updating space issue...
Comment #12
ianthomas_ukThis config will not exist on a fresh install of Drupal.
The RssFields.php settings can be accessed by editing the feed display of a view, e.g. add /admin/structure/views/view/frontpage/edit/feed_1. Under FORMAT, ensure Format: is RSS Feed, and Show: is Fields. These options are then on the following settings form.
In my testing the options were saved but were not exported, so there may be a bug there.
Comment #12.0
vijaycs85Updated issue summary.
Comment #13
dawehner@ianthomas_uk Can you please verify whether this issue needs some work?
Comment #14
ianthomas_ukI can't reproduce the problem I was having before (I think I meant that my changes were showing in the UI, but had no effect on the config files).
In #11 the schema for views.row.rssfields reflected the types of data that would be returned in the RSS feed, but it should actually represent where this data should come from (so most of the settings are field names). I've attached an updated patch that corrects this.
I'm not sure how to EntityRow, as I couldn't work out how to create an appropriate view. I think it's intended as a base class and doesn't have any implementations in core. The schema's definitely wrong though, as it's based on the variables in init() instead of defineOptions().
Comment #15
vijaycs85Thanks for the update @ianthomas_uk. Seems EntityRow is a baseClass and it might not have it's own row. So removing it.
Comment #16
ianthomas_ukThanks @vijaycs85. I thought it should probably just be removed but needed a second opinion. I've updated the summary to reflect this.
I can confirm that #15 is my patch from #14 minus the changes for EntityRow.php.
If you review my changes for RssFields.php are we able to mark this RTBC between us, or do we need a third person?
Comment #17
dawehnerSo, does that mean that something like entity_row:node won't have properties, as it is automatically added via the derivative class?
Comment #18
ianthomas_ukThanks dawehner.
Looking at this again, the views module should provide the schema for the view_mode option, and each of the Comment and Node modules should provide the schema for the links option (which they define independently). The action module has an example of a module extending the schema of another.
To expose the configuration for NodeRow I set Format: HTML list, Show: Content.
Comment #19
ianthomas_ukHere's an improved patch: I've corrected the key for rss fields and added the EntityRow schemas. Node and comment seem to be working, but user is still showing as unknown in the config inspector and I'm not sure why. I've also spotted lots more classes that extend RowPluginBase that I think we need to provide schema for (listed in the summary).
Comment #20
ianthomas_ukThe patch files might help...
Comment #21
vijaycs85Right, this is currently blocked by #2152345: Change views row plugin key to make config schema clean
Comment #22
vijaycs85Comment #23
vijaycs85After the discussion between @ianthomas_uk, @dawehner and @vijaycs85, going to move forward with current implementation (i.e. views.row.entity:node) and will revisit once #2035345: Reconsider whether to use ':' as separator for derivative plugins is in...
Comment #24
vijaycs85Few updates and config inspector screenshot...
Comment #25
vijaycs85Will cover below row plugins in it's module specific issue:
Drupal\aggregator\Plugin\views\Rss
Drupal\comment\Plugin\views\row\Rss
Drupal\node\Plugin\views\row\Rss
Drupal\rest\Plugin\views\row\DataEntityRow
Drupal\rest\Plugin\views\row\DataFieldRow
Drupal\search\Plugin\views\row\SearchRow
Drupal\views_test_data\Plugin\views\row\RowTest
Comment #26
ianthomas_ukI can't RTBC this patch as I wrote lots of it, but I did want to mention a couple of things in vijaycs85's changes.
vijaycs85 explained that this matches the filename convention for schema files. I'm happy to follow the convention, but this is different to the pattern used to provide default config where the filename would start with the name of the module that you were extending and be in the module that is doing the extending, so it is only available if both are installed.
+1 for the comment, but it should be Node module.
Comment #27
dawehnerYeah let's mark this as work, though at the same time think about adding a follow up to support the usecase here for schema as well.
Comment #28
vijaycs85The patch on this issue has been updated as part of #2167623: Add test for all default configuration to ensure schema exists and is correct. As this issue doesn't have any test to confirm/validate the schema, making this change and closing this issue as duplicate of #2167623: Add test for all default configuration to ensure schema exists and is correct. The contributors of this issue (in commit message) is copied to #2167623: Add test for all default configuration to ensure schema exists and is correct.