Make User module active config save format match the default yml file.

+++ b/core/profiles/standard/config/user.role.administrator.ymlundefined
@@ -1,4 +1,4 @@
-label: Administrator
-weight: 2
+label: 'Administrator'
+weight: '2'

I dont think single words get quotes in config.

Let's be sure to actually save the settings and compare the saved config with the default config.

patch coming.
Also, status is in the saved config.
See #1964254-10: Configuration schemas missing langcode and uuid at places

a.
minimal profile does have
admin_role: ''

so leaving that.

--
b.
but both minimal and standard have
register: visitors_admin_approval

so that should be changed.

--
c.
$ grep -R status_cancelled * | grep yml | grep -v ".swp"
core/modules/user/config/schema/user.schema.yml: status_cancelled:
core/modules/user/config/user.settings.yml: status_cancelled: '0'
sites/default/files/config_p8IrHKOjHxQP-h2IQnOMp-95ActxXLpbSQfsvUE6hKI/active/user.settings.yml: status_cancelled: '0'
[~/foo/drupal]
01:28 AM [YesCT] (drush-iq-make-user-module-active-config-1942178-#4)
562 $ grep -R status_canceled * | grep yml | grep -v ".swp"
core/modules/user/config/schema/user.schema.yml: status_canceled:
core/modules/user/config/user.mail.yml:status_canceled:
sites/default/files/config_p8IrHKOjHxQP-h2IQnOMp-95ActxXLpbSQfsvUE6hKI/active/user.mail.yml:status_canceled:
sites/default/files/config_p8IrHKOjHxQP-h2IQnOMp-95ActxXLpbSQfsvUE6hKI/active/user.settings.yml: status_canceled: '0'

we should fix canceled vs cancelled
wc says we should use canceled

left the ones in t()'s and comments as that was out of scope I thought, and could use a follow-up.

this change to the update:

/**
 * Moves account settings from variable to config.
 *
 * @ingroup config_upgrade
 */
function user_update_8004() 

might be out of scope.. if so, a separate issue can be opened, but it's the only place
user_mail_status_cancelled_notify is. Does this mean we dont have a test for upgrades? Probably.
core/modules/user/user.install: 'user_mail_status_cancelled_notify' => 'notify.status_cancelled',

--
d.
why is the active config saving all newlines with \r too?

$ diff ./core/modules/user/config/user.mail.yml sites/default/files/config_DocumzykiE6AOl6bPoPGjrDg5FVGEk0g_85eliFXKX0/active/user.mail.yml 
2c2
<   body: "[user:name],\n\nA request to cancel your account has been made at [site:name].\n\nYou may now cancel your account on [site:url-brief] by clicking this link or copying and pasting it into your browser:\n\n[user:cancel-url]\n\nNOTE: The cancellation of your account is not reversible.\n\nThis link expires in one day and nothing will happen if it is not used.\n\n--  [site:name] team"
---
>   body: "[user:name],\r\n\r\nA request to cancel your account has been made at [site:name].\r\n\r\nYou may now cancel your account on [site:url-brief] by clicking this link or copying and pasting it into your browser:\r\n\r\n[user:cancel-url]\r\n\r\nNOTE: The cancellation of your account is not reversible.\r\n\r\nThis link expires in one day and nothing will happen if it is not used.\r\n\r\n--  [site:name] team"

I did not make the active saved config match the default config for user.mail.yml
but that might need a issue opened if we dont want the config saved like that.

So it seems like we should be leaving the quotes off the single words

Got it -- I see it says "Use single quotes for label values even if they are one word for consistency" on https://drupal.org/node/1905070. Any idea why there are not matching single quotes in the active config directory yml file?

I've added the patch #10 back as #13 to (hopefully!) avoid confusion

Looks good.

Patch no longer applies.

Rerolled.

NM, there are still fuzz issues.

[edit] I took a second look, seems to work with the latest HEAD now.

@vijaycs85 and I double checked, and that link to the standard about single quotes around single words...
is to the schema recommendations. This issue is about a config (not a schema).

further in that doc it says:

Note: The regular configuration data .yml file style dictates you only use single quotes when more than one word is used because the .yml serialization will do that as a standard practice, so this standard makes diff-ing simpler for changing configuration. However the schema recommendations above differ from that, because schema files are always hand-written and using quotes around label values all the time is better for consistency.

this is confusing.. even for us who worked on schemas. :) But... it is that way to try and make it easier for contrib module maintainers.

so... we can check this by resaving the config settings and diffing the default file with the one generated in the active config

This one adds single quotes to the mentioned:
Files need to be fixed
user.role.anonymous.yml
user.role.authenticated.yml

And also
entity.view_mode.user.full.yml

And removes them from a single word here:
views.view.user_admin_people.yml

I checked by running diff on the files in the user module config vs my files/config/active and looking at the changes.

Run from files/config/active: diff . ../../../../../core/modules/user/config/

Updated issue summary.

This one also adds single quotes to integers (to match config output).

Re-roll.

23: 1942178-23.patch queued for re-testing.

23: 1942178-23.patch queued for re-testing.

What did you do to find those new updates?

Problem:

Applying Patch: https://drupal.org/files/issues/1942178-config-schema-user-28.patch
error: patch failed: core/modules/user/config/entity.view_mode.user.full.yml:1
error: core/modules/user/config/entity.view_mode.user.full.yml: patch does not apply
error: patch failed: core/modules/user/config/user.role.anonymous.yml:1
error: core/modules/user/config/user.role.anonymous.yml: patch does not apply
error: patch failed: core/modules/user/config/user.role.authenticated.yml:1
error: core/modules/user/config/user.role.authenticated.yml: patch does not apply
error: patch failed: core/modules/user/config/views.view.user_admin_people.yml:47
error: core/modules/user/config/views.view.user_admin_people.yml: patch does not apply

This patch is very old and does not apply anymore.
Let me change it manually.

New patch for review

Claiming this issue.

* Updated the patch for current HEAD
* Updated the user_admin_people View to match the schema, so it should pass tests.

Had a look at the code and also reproduced the export, everything is conform.

1. +++ b/core/modules/user/config/install/user.role.authenticated.yml
   index 6ab676d..99bf69e 100644
   --- a/core/modules/user/config/install/views.view.user_admin_people.yml
   
   --- a/core/modules/user/config/install/views.view.user_admin_people.yml
   +++ b/core/modules/user/config/install/views.view.user_admin_people.yml
   

   All of the changes to this file are incorrect. The way to test this is to enable views and view_ui. Then go and add the admin people view and resave it. Then export your configuration and compare the resulting YAML file to the one in the default.

2. +++ b/core/modules/user/config/install/views.view.user_admin_people.yml
   @@ -299,7 +299,7 @@ display:
   -          not: '0'
   +          not: 'false'
   

   This change is not necessary

3. +++ b/core/modules/user/config/install/views.view.user_admin_people.yml
   @@ -614,12 +614,12 @@ display:
   -            user_bulk_form: '0'
   -            name: '0'
   -            status: '0'
   -            rid: '0'
   -            created: '0'
   -            access: '0'
   +            user_bulk_form: 'false'
   +            name: 'false'
   +            status: 'false'
   +            rid: 'false'
   +            created: 'false'
   +            access: 'false'
   

   This change is incorrect '0' is the unselected value.

4. +++ b/core/modules/user/config/install/views.view.user_admin_people.yml
   @@ -696,8 +696,8 @@ display:
   -              anonymous: '0'
   -              administrator: '0'
   +              anonymous: 'false'
   +              administrator: 'false'
   
   @@ -737,8 +737,8 @@ display:
   -              anonymous: '0'
   -              administrator: '0'
   +              anonymous: 'false'
   +              administrator: 'false'
   
   @@ -778,8 +778,8 @@ display:
   -              anonymous: '0'
   -              administrator: '0'
   +              anonymous: 'false'
   +              administrator: 'false'
   
   @@ -819,8 +819,8 @@ display:
   -              anonymous: '0'
   -              administrator: '0'
   +              anonymous: 'false'
   +              administrator: 'false'
   

   Not necessary

5. +++ b/core/modules/user/config/install/views.view.user_admin_people.yml
   @@ -941,7 +941,7 @@ display:
   -        context: ''
   +        context: 'false'
   

   Not necessary

Thanks for the fast review, @alexpott . I tried making the changes manually b/c I thought it would be faster and teach me more about the new standards... my bad. Patch recreated with the automated view export, against current tip of 8.x branch... turns out that the View is actually identical to what's already in 8.x. Just the changes to the schema file.

Agh attached the wrong version of that patchfile and interdiff. It's fine except that it includes an extra whitespace and UUID in the View. Here's the proper version.

actually, given that this is so simple, the funtional elements are already reviewed, and the busted parts are removed from the patch, I'll just send this right back to you as reviewed. Hope that's in order.

Actually the views.view.user_admin_people.yml is not quite correct :) it is missing a dependency in one of it's plugins. If you export the view and compare it you'll see.

n.b. we do not include default UUIDs in default config

@alexpott I think you might have been looking at the wrong version of the patch, which I uploaded by accident in comment 43.

Please check the one in comment 44, which doesn't modify the View at all. The version in 8.x perfectly matched my export (apart from the UUID of course).

@ohthehugemanatee - yep I exported all the config and compared to default... so you're right the views are looking good but the roles also need

status: true
dependencies: {  }

added at the bottom.

Re-exported the roles, removed the permissions from them by hand because they aren't in the original... I guess they get added by a different module. Thanks for your patience with me!

So, the test failed because dependencies isn't defined as an array type in the schema.

1) should that be explicitly defined in our schema, or is that a universal element?
2) does that fit into this ticket anyway?

Not RTBC if tests are failing. :)

Yes, this works. Followed instructions by alexpott in #42.

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 0116e11 and pushed to 8.0.x. Thanks!

Cleaning up drupalcampfi tags.