Problem/Motivation

As per #2183983: Find hidden configuration schema issues, several contact module tests would fail if we turn on full schema checking. Here is a patch to expose those errors and fix them. A subset of issues there.

Proposed resolution

Find and fix the issues.

Remaining tasks

Fix them.

User interface changes

None.

API changes

None except fixed schemas.

Files: 
CommentFileSizeAuthor
#12 2379683-contact-schema-fix-12.patch5.02 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,737 pass(es). View
#5 2379683-contact-schema-fix-5.patch5.02 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2379683-contact-schema-fix-5.patch. Unable to apply patch. See the log in the details link for more information. View
#5 interdiff.txt1.49 KBGábor Hojtsy
#3 interdiff.txt864 bytesGábor Hojtsy
#3 2379683-contact-schema-fix.patch3.53 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,414 pass(es), 8 fail(s), and 0 exception(s). View
#1 2379683-config-schemas.patch2.68 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,410 pass(es), 9 fail(s), and 1 exception(s). View

Comments

Gábor Hojtsy’s picture

FileSize
2.68 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,410 pass(es), 9 fail(s), and 1 exception(s). View

Status: Needs review » Needs work

The last submitted patch, 1: 2379683-config-schemas.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
3.53 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,414 pass(es), 8 fail(s), and 0 exception(s). View
864 bytes

Two plugin IDs were missing from the test view in views.view.test_contact_link.

Status: Needs review » Needs work

The last submitted patch, 3: 2379683-contact-schema-fix.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
1.49 KB
5.02 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2379683-contact-schema-fix-5.patch. Unable to apply patch. See the log in the details link for more information. View

The remaining fails were due to account settings forms side-effects.

1. The account settings form uses and saves to site.settings a mail_notification key which was missing.
2. The user notification for account cancellation is spelled "status_canceled" in the code. The default config and the schema spelled it with two L's ("status_cancelled"), which obviously does not match.

This should make tests pass, at least the config storage test passes fine locally.

vijaycs85’s picture

Status: Needs review » Reviewed & tested by the community

Changes looks good.

+++ b/core/modules/system/config/schema/system.schema.yml
@@ -38,6 +38,9 @@ system.site:
+    mail_notification:

+++ b/core/modules/user/config/install/user.settings.yml
@@ -6,7 +6,7 @@ notify:
+  status_canceled: false

I guess only changes related to this issue is those two elements.

+++ b/core/modules/contact/src/Tests/ContactPersonalTest.php
@@ -19,6 +19,15 @@
+  protected $strictConfigSchema = TRUE;

is it from core test which works already?

Gábor Hojtsy’s picture

@vijaycs85: yes, if you grep for status_cancalled you only find these two and status_canceled will lead to the actual code dealing with it :) as for the testing variable, yeah existing tests are using this and the more places we introduce it the less it is possible to make mistakes -- we can eventually default it to TRUE in core tests if all schemas are good :)

vijaycs85’s picture

existing tests are using this and the more places we introduce it the less it is possible to make mistakes -- we can eventually default it to TRUE in core tests if all schemas are good :)

sounds good.

Gábor Hojtsy’s picture

Title: Fix configuration schema issues in contact module » Fix configuration schema issues in contact (indirectly user and system) modules

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 5: 2379683-contact-schema-fix-5.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
5.02 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,737 pass(es). View

Reroll due to #2316909: Revisit all built-in test/default views configuration in core. Applied with offsets. Back to RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

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 26f8d71 and pushed to 8.0.x. Thanks!

  • alexpott committed 26f8d71 on 8.0.x
    Issue #2379683 by Gábor Hojtsy: Fix configuration schema issues in...

Status: Fixed » Closed (fixed)

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