Follow-up from #1653026: [META] Use properly typed values in module configuration.

Problem/Motivation

All integers, Booleans, and even octal numbers in config object files are converted to strings.

Proposed resolution

#1653026: [META] Use properly typed values in module configuration has fixed core, so no need to convert all data types to string anymore.

Remaining tasks

search.routing.yml ('TRUE' to TRUE)
entity.view_mode.node.search_index.yml
entity.view_mode.node.search_result.yml
search.settings.yml
search_embedded_form.settings.yml
search_extra_type.settings.yml

User interface changes

NO

API changes

NO

Parent: #1653026: [META] Use properly typed values in module configuration

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhodgdon’s picture

Status: Active » Postponed

There is a note at the top of the meta-issue that says:
WARNING: DO NOT work on below sub-tasks until #2106459: Core config has everything as string
@chx got a better option to cover most of the sub tasks as a single issue

So I think this issue needs to be postponed for now.

mr.baileys’s picture

Status: Postponed » Needs review
Issue tags: +Novice

Since #2106459: Core config has everything as string has landed, I assume this issue is no longer postponed?

Attached is a patch that removes the force-casting to string form search module's configuration files. Note that this excludes search.routing.yml (I assume this is not a configuration file, and removing the quotes there causes a fatal error: A fatal error occurred: Routing requirement for "_search_access" must be a string.)

jhodgdon’s picture

Title: Make sure all YML files in Search module has no type-casting to string. » Make sure all YML files in Search module have no type-casting to string.
Status: Needs review » Needs work

The patch is missing?

mr.baileys’s picture

Status: Needs work » Needs review
FileSize
1.37 KB

Oops, sorry. Here it is.

jhodgdon’s picture

Status: Needs review » Needs work

This doesn't look problematic, and I verified it covers all the changes we need in core/modules/search/config.

However, there are a couple of test classes in core/modules/search/test that have config, and are not covered.

mr.baileys’s picture

Thanks for the review, jhodgdon!

However, there are a couple of test classes in core/modules/search/test that have config, and are not covered.

The only config file under core/modules/search/tests that contained non-strings cast to string is core/modules/search/tests/modules/search_embedded_form/config/search_embedded_form.settings.yml I think, and it is included in the patch in #4. Are there other config files under core/modules/search I missed?

jhodgdon’s picture

Issue tags: -Novice, -YAML

That looks like it. I just saw that one and added the comment without looking to see if there were more. Thanks!

jhodgdon’s picture

Status: Needs work » Needs review

#4: 21005983-2-search-yml.patch queued for re-testing.

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: +Novice, +YAML

I just applied the patch above and confirmed there are no remaining files under core/modules/search that need to be fixed (at least, I think that's it).

I am not sure what that test failure was - random glitch? This patch should be fine.

Adding tags back that I deleted somehow in an earlier comment.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 21005983-2-search-yml.patch, failed testing.

jhodgdon’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.45 KB

This is a straight reroll. The above patch applied fine with fuzz.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/search/config/entity.view_mode.node.search_index.yml
@@ -1,6 +1,6 @@
+status: 0
+cache: 1

+++ b/core/modules/search/config/entity.view_mode.node.search_result.yml
@@ -1,6 +1,6 @@
+status: 0
+cache: 1

These are booleans so we should use the values true and false here. See entity.view_mode.comment.full.yml

vijaycs85’s picture

updating as per #12. However committing #2096373: Provide config schema for View modes and running @chx script (in #2106459-7: Core config has everything as string) solves this problem globally on all view modes.

a_thakur’s picture

Status: Needs work » Needs review
FileSize
1010 bytes

Updated patch as per comment #12

vijaycs85’s picture

Sorry for the cross post @a_thakur, but your patch missing

+++ b/core/modules/search/tests/modules/search_embedded_form/config/search_embedded_form.settings.yml
@@ -1,2 +1,2 @@
-submitted: '12'
+submitted: 12
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Agreed. The patch in ****#13**** -- not #14 -- is good to go. Thanks!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed c8b0a63 and pushed to 8.x. Thanks!

alexpott’s picture

Issue summary: View changes

updating files

Status: Fixed » Closed (fixed)

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

a_thakur’s picture

Issue summary: View changes