Complete configuration schemas for user module [#1912258]

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 user module, 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.

Schema in place
user.mail.yml
user.settings.yml
user.flood.yml
user roles

Schema not yet in place

Comment	File	Size	Author
#13	1912258-config_schema_for_user-13.patch	4.82 KB	vijaycs85
#13
#13	1912258-diff-11-13.txt	497 bytes	vijaycs85
#11	1912258-config_schema_for_user-11.patch	4.73 KB	vijaycs85
#11
#11	1912258-diff-9-11.txt	808 bytes	vijaycs85
#9	1912258-config_schema_for_user-9.patch	4.63 KB	vijaycs85
#9
#9	1912258-diff-7-9.txt	2.88 KB	vijaycs85
#7	1912258-config_schema_for_user-7.patch	4.63 KB	vijaycs85
#7
#3	1912258-3-config_schema_for_user.patch	914 bytes	adammalone
#3
#1	1912258-1-config_schema_for_user.patch	1.14 KB	adammalone
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

adammalone

English

Sydney

CreditAttribution: adammalone commented 9 February 2013 at 00:14

File	Size
1912258-1-config_schema_for_user.patch	1.14 KB

Here's a patch that adds in the additional schemas.

Comment #2

adammalone

English

Sydney

CreditAttribution: adammalone commented 9 February 2013 at 00:15

Status:

Active

» Needs review

Comment #3

adammalone

English

Sydney

CreditAttribution: adammalone commented 9 February 2013 at 03:56

File	Size
1912258-3-config_schema_for_user.patch	914 bytes

Actually... this patch would adhere more to kwalify formats since user roles should be user.role.*

Comment #4

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 February 2013 at 14:04

Issue tags:

+Configuration system, +D8MI, +language-config, +Configuration schema

Tagging up.

Comment #5

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 14 February 2013 at 15:35

Status:

Needs review

» Needs work

+++ b/core/modules/user/config/user.schema.ymlundefined
@@ -73,3 +73,33 @@ user.settings:
+  mapping:
+    "id": "Role ID"
+    "label": "Role Label"

No type on these?

Also are these the actual labels on the UI? Title cased? Drupal does not usually use title cased naming on the UI. Same true for the other part added (as well as the header "User Roles").

Comment #6

adammalone

English

Sydney

CreditAttribution: adammalone commented 14 February 2013 at 20:36

Assigned:

Unassigned

» adammalone

I'll have another go over these. I wasn't aware that the label/id had to be linked to that in the UI.

Comment #7

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 24 February 2013 at 12:23

Status:

Needs work

» Needs review

File	Size
1912258-config_schema_for_user-7.patch	4.63 KB

Adding additional schemas with code style fixes.

Comment #8

guy_schneerson CreditAttribution: guy_schneerson commented 4 March 2013 at 17:35

Status:

Needs review

» Needs work

Removed - not applicable

Comment #9

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 4 March 2013 at 05:10

File	Size
1912258-diff-7-9.txt	2.88 KB
1912258-config_schema_for_user-9.patch	4.63 KB

error: core/modules/user/config/user.schema.yml: No such file or directory

doesn't sound like patch in #7.

Fixed few labels as per UI.

Comment #10

guy_schneerson CreditAttribution: guy_schneerson commented 4 March 2013 at 17:47

Sorry vijaycs85 you are right, I removed #8 and Tested your latest patch in #9 and found only two small Issues:

register uses an "enum" key and as far as I know we have no support for enumerations at this point and the configuration form should handle it. I think all thats needed is to remove the enum line
register_pending_approval_admin is missing in the "user.mail:" section

Comment #11

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 4 March 2013 at 18:41

Status:

Needs work

» Needs review

File	Size
1912258-diff-9-11.txt	808 bytes
1912258-config_schema_for_user-11.patch	4.73 KB

Thank you @guy_schneerson. Updated patch to accommodate changes mentioned in #10. Good catch :)

Comment #12

guy_schneerson CreditAttribution: guy_schneerson commented 6 March 2013 at 10:12

Status:

Needs review

» Needs work

Great job @vijaycs85, the interdiff makes it real easy to review.

Not sure if I missed it before or if it was added recently but we are also missing password_strength from the user.settings.yml (this was recently added to allow disabling of the password strength indicator) and is a boolean.

Sorry if i missed it before - setting status to "needs a tiny bit more work :)"

Comment #13

vijaycs85

London, UK

CreditAttribution: vijaycs85 commented 6 March 2013 at 13:37

Status:

Needs work

» Needs review

File	Size
1912258-diff-11-13.txt	497 bytes
1912258-config_schema_for_user-13.patch	4.82 KB

Thanks again @guy_schneerson. added password_strength.

Comment #14

guy_schneerson CreditAttribution: guy_schneerson commented 6 March 2013 at 15:57

Status:

Needs review

» Reviewed & tested by the community

Looks great @vijaycs85

Patch applied cleanly
All settings are accounted for and are of the correct type in the user.schema.yml
The configuration inspector tool produces forms with all settings and labels

Also updated the issue description and moved moved user.flood.yml & user roles into the "Schema in place" section as you have implemented those.

Not sure if anyone else needs to review but setting it to "Reviewed & tested by the community" so sett it back if you think it needs more review.