The search settings at admin/config/search/settings need to be converted to use the new configuration system.

Tasks

  • Create a default config file and add it to the search module.
  • Convert the admin UI in search.admin.inc to read to/write from the appropriate config.
  • Convert any code that currently accesses the variables used to use the config system.
  • Write an upgrade path from D7 -> D8 to migrate existing data to the new system and drop the appropriate variables.

I am tagging this config novice but it is actually a bit more complex than the other novice issues. There are a lot more settings and its more far reaching, but if you're feeling brave go for it!

Files: 
CommentFileSizeAuthor
#72 1496510-72.search-cmi.patch25.98 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 37,218 pass(es).
[ View ]
#72 65-72-interdiff.txt7.93 KBalexpott
#65 interdiff-59-65.txt21.5 KBalexpott
#65 1496510-65.search_cmi.patch26.13 KBalexpott
PASSED: [[SimpleTest]]: [MySQL] 37,216 pass(es).
[ View ]
#59 1496510-59.patch25.61 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 37,115 pass(es).
[ View ]
#56 1496510-57.patch25.65 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 37,294 pass(es).
[ View ]
#53 1496510-53.patch25.74 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 37,296 pass(es).
[ View ]
#51 1496510-51.patch25.65 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 37,264 pass(es).
[ View ]
#44 1496510_44_search_cmi.patch21.62 KBcosmicdreams
FAILED: [[SimpleTest]]: [MySQL] 36,261 pass(es), 7 fail(s), and 35 exception(s).
[ View ]
#42 1496510_42_search_cmi.patch22.23 KBcosmicdreams
PASSED: [[SimpleTest]]: [MySQL] 36,601 pass(es).
[ View ]
#40 1496510_40_search_cmi.patch20 KBcosmicdreams
FAILED: [[SimpleTest]]: [MySQL] 36,613 pass(es), 2 fail(s), and 0 exception(s).
[ View ]
#38 1496510_38_search_cmi.patch20 KBcosmicdreams
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/search/search.test.
[ View ]
#33 1496510-33.patch22.24 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 36,177 pass(es).
[ View ]
#30 1496510-30.patch22.71 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 35,923 pass(es).
[ View ]
#28 1496510-28.patch22.71 KBswentel
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1496510-28.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#26 1496510-26.patch22.71 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 35,919 pass(es).
[ View ]
#24 search-cmi.patch22.24 KBmarcingy
FAILED: [[SimpleTest]]: [MySQL] 35,868 pass(es), 9 fail(s), and 4 exception(s).
[ View ]
#20 search-cmi.patch22.17 KBmarcingy
FAILED: [[SimpleTest]]: [MySQL] 35,845 pass(es), 22 fail(s), and 4 exception(s).
[ View ]
#17 search-1496510-17.patch21.1 KBmarcingy
FAILED: [[SimpleTest]]: [MySQL] 35,429 pass(es), 496 fail(s), and 2,778 exception(s).
[ View ]
#13 search-1496510-13.patch20.98 KBjvns
FAILED: [[SimpleTest]]: [MySQL] 35,534 pass(es), 41 fail(s), and 104 exception(s).
[ View ]
#11 search-1496510-11.patch20.35 KBjvns
FAILED: [[SimpleTest]]: [MySQL] 35,601 pass(es), 40 fail(s), and 24 exception(s).
[ View ]
#10 search-1496510-8.patch19.48 KBjvns
FAILED: [[SimpleTest]]: [MySQL] 35,084 pass(es), 553 fail(s), and 2,742 exception(s).
[ View ]
#8 search-1496510-8.patch19.48 KBjvns
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch search-1496510-8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#6 search-1496510-6.patch1.3 KBjvns
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch search-1496510-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#2 search-1496510-1.patch20.46 KBjvns
FAILED: [[SimpleTest]]: [MySQL] 35,256 pass(es), 482 fail(s), and 24 exception(s).
[ View ]

Comments

Assigned:Unassigned» jvns

Status:Active» Needs review
StatusFileSize
new20.46 KB
FAILED: [[SimpleTest]]: [MySQL] 35,256 pass(es), 482 fail(s), and 24 exception(s).
[ View ]

Here's a first stab at this.

Status:Needs review» Needs work

The last submitted patch, search-1496510-1.patch, failed testing.

There is some missing white space

<?php
$limit
= (int)config('search.settings')->get('cron_limit');
?>

should be (This happens in number of places)

<?php
$limit
= (int) config('search.settings')->get('cron_limit');
?>

You can remove the extra white space that you have in your xml eg

<overlap_cjk> TRUE </overlap_cjk>

should be

<overlap_cjk>TRUE</overlap_cjk>

Also each sub tag should be idented 2 characters

eg

<config>
  <minimum_word_size>3</minimum_word_size>
</config>

You can't have 2 keys with the same name

+<active_modules> node </active_modules>
+<active_modules> user </active_modules>

Instead we need something like

<active_modules>
  <node_module>node</node_module>
  <user_module>user</user_module>
</active_modules>

There are some layout issue in the update hook

<?php
$variable_names
= array(
+           
'minimum_word_size' => 'minimum_word_size',
+           
'overlap_cjk' => 'overlap_cjk',
+           
'sample_search_force_keywords' => 'sample_force_keywords',
+           
'search_active_modules' => 'active_modules',
+           
'search_and_or_limit' => 'and_or_limit',
+           
'search_cron_limit' => 'cron_limit',
+           
'search_default_module' => 'default_module',
+           
'search_embedded_form_submitted' => 'embedded_form_submitted',
+           
'search_tag_weights' => 'tag_weights',
+    );
?>

Status:Needs work» Needs review

Okay, fixed the whitespace issues. Haven't done anything about the issues with the upgrade path yet, but let's see what this does.

StatusFileSize
new1.3 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch search-1496510-6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

should actually attach a patch.

Status:Needs review» Needs work

The last submitted patch, search-1496510-6.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new19.48 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch search-1496510-8.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Oops. This one should apply.

The latest patch seems to be missing the xml file but all the white space problems look like they are fixed and changes seem good. Leaving for needs to review for the bot but I am assuming the lack of the config file is going to use failures.

StatusFileSize
new19.48 KB
FAILED: [[SimpleTest]]: [MySQL] 35,084 pass(es), 553 fail(s), and 2,742 exception(s).
[ View ]

Uploading again to see if it gets picked up by the test bot this time

StatusFileSize
new20.35 KB
FAILED: [[SimpleTest]]: [MySQL] 35,601 pass(es), 40 fail(s), and 24 exception(s).
[ View ]

With the config file

Status:Needs review» Needs work

The last submitted patch, search-1496510-11.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new20.98 KB
FAILED: [[SimpleTest]]: [MySQL] 35,534 pass(es), 41 fail(s), and 104 exception(s).
[ View ]

Fixed up the search tests some.

Status:Needs review» Needs work
Issue tags:-Configuration system, -Config novice

The last submitted patch, search-1496510-13.patch, failed testing.

Status:Needs work» Needs review

#13: search-1496510-13.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+Configuration system, +Config novice

The last submitted patch, search-1496510-13.patch, failed testing.

StatusFileSize
new21.1 KB
FAILED: [[SimpleTest]]: [MySQL] 35,429 pass(es), 496 fail(s), and 2,778 exception(s).
[ View ]

Converting to new helper update hook.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, search-1496510-17.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new22.17 KB
FAILED: [[SimpleTest]]: [MySQL] 35,845 pass(es), 22 fail(s), and 4 exception(s).
[ View ]

Hopefully fixes some issues

Status:Needs review» Needs work

The last submitted patch, search-cmi.patch, failed testing.

Assigned:jvns» Unassigned

+++ b/core/modules/search/config/search.settings.xmlundefined
@@ -0,0 +1,28 @@
+ <user>user</user>

Whitespace

+++ b/core/modules/search/search.admin.incundefined
@@ -50,13 +50,18 @@ function _search_get_module_names() {
+  $active_modules = config('search.settings')->get('active_modules');
+  if (!empty($active_modules)) {
+    foreach (config('search.settings')->get('active_modules') as $module) {

You can reuse $active_modules variable again instead of calling config() again.

+++ b/core/modules/search/search.admin.incundefined
@@ -128,10 +134,13 @@ function search_admin_settings($form) {
+  $active_modules = $config->get('active_modules');
+  if (!empty($active_modules)) {
+    foreach ($config->get('active_modules') as $module) {

Reuse $active_modules instead of calling with $config again.

+++ b/core/modules/search/search.admin.incundefined
@@ -156,13 +165,18 @@ function search_admin_settings_validate($form, &$form_state) {
+ $config->set('overlap_cjk', $form_state['values']['overlap_cjk']);

Whitespace

+++ b/core/modules/search/search.admin.incundefined
@@ -171,9 +185,15 @@ function search_admin_settings_submit($form, &$form_state) {
+   $enabled_modules[$module] = $module;
+ }
+ $config->set('active_modules', $enabled_modules);

Whitespace issues

-29 days to next Drupal core point release.

Status:Needs work» Needs review
StatusFileSize
new22.24 KB
FAILED: [[SimpleTest]]: [MySQL] 35,868 pass(es), 9 fail(s), and 4 exception(s).
[ View ]

New patch fixing comments above and fixing up some tests. Still having an issue with the config folder not being created as part of the upgrade test, which is the main blocker to last test passing as far as I can tell.

Giving to the bot to see where this leaves us.

Status:Needs review» Needs work

The last submitted patch, search-cmi.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new22.71 KB
PASSED: [[SimpleTest]]: [MySQL] 35,919 pass(es).
[ View ]

This will make the tests pass.

It needed a change it config.inc to make sure the simpletest directory existed. The same fix is also used in #1496458: Convert maintenance mode settings to configuration system, so as this or the other is committed (with that fix or another way than the current one), this or the other patch needs to be updated.

Status:Needs review» Needs work

Ah that makes sense but shouldn't we be using

<?php
file_prepare_directory
($path, FILE_CREATE_DIRECTORY);
?>

instead of direct calls to file_exists and mkdir.

Status:Needs work» Needs review
StatusFileSize
new22.71 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1496510-28.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Absolutely, good catch!

Status:Needs review» Needs work

The last submitted patch, 1496510-28.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new22.71 KB
PASSED: [[SimpleTest]]: [MySQL] 35,923 pass(es).
[ View ]

Meh, that's what you getting trying to manually update a patch :)

Looks good. Tempted to mark this rtbc. But if maintenance gets committed first this needs a reroll.

Status:Needs review» Needs work

#1517138: SimpleTest does not create configuration folder causing test failures has now been committed to get round the folder issue this likely needs a re-roll.

Status:Needs work» Needs review
StatusFileSize
new22.24 KB
PASSED: [[SimpleTest]]: [MySQL] 36,177 pass(es).
[ View ]

New patch

Status:Needs review» Reviewed & tested by the community

Reroll is just minus the test folder element so happy to RTBC.

Status:Reviewed & tested by the community» Needs work

+function search_update_8000() {
+  config_install_default_config('search');

This makes me uncomfortable. What happens if default config for search changes during the Drupal 8 lifecycle? Seems like it should explicitly lay out the default configuration as it is now, install that, then it'll be guaranteed to be the same regardless of when any given 8.x site gets upgraded.

Status:Needs work» Reviewed & tested by the community

The problem if we do that is then when things change, we have to update it in two places instead of just in the config file, and things are pretty likely to be changing at this point (we're already talking about, for instance, changing the file format.) Until such a time as we are supporting head-to-head upgrades, I don't really consider this a problem.

Status:Reviewed & tested by the community» Needs work

One thing I only just realized about this patch is that it removes the 'search_' prefix from all the variable names. I am not sure how I feel about this. We have not been doing it on the other conversion patches, and I would like to keep it consistent. We can attack changing the variable names in a followup if we want.

Status:Needs work» Needs review
StatusFileSize
new20 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/search/search.test.
[ View ]

This patch rerolls the previous patch and includes the edits called for in #37. Also, I applied a few coding standards that have been pointed out to me in the past. Namely use the fluid syntax when possible on store the config object in a variable whenever that variable can be reused.

Status:Needs review» Needs work

The last submitted patch, 1496510_38_search_cmi.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new20 KB
FAILED: [[SimpleTest]]: [MySQL] 36,613 pass(es), 2 fail(s), and 0 exception(s).
[ View ]

Found my mistake, new patch.

Status:Needs review» Needs work

The last submitted patch, 1496510_40_search_cmi.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new22.23 KB
PASSED: [[SimpleTest]]: [MySQL] 36,601 pass(es).
[ View ]

Fixed the fails by discovering that one of the config gets was using the wrong key name. Also changed the way the incrementing was coded to be absolutely sure that the number of submits is incrementing.

Status:Needs review» Needs work

I think an htaccess diff has crept into the patch.

Also I think we can make a minor code style improvement:

<?php
$active_modules = $config->get('search_active_modules');
+  if (!empty(
$active_modules)) {
+    foreach (
$active_modules as $module) {
?>

should be

<?php
$active_modules = $config->get('search_active_modules');
+  foreach (
$active_modules as $module) {
?>

Basically checking if $active_modules is empty is unnecessary... There are two instances of this. To prove the point, there is other code in this patch the makes the assumption the $config->get('search_active_modules') will return an array - such as:

<?php
+  foreach (config('search.settings')->get('search_active_modules') as $module) {
?>

Status:Needs work» Needs review
StatusFileSize
new21.62 KB
FAILED: [[SimpleTest]]: [MySQL] 36,261 pass(es), 7 fail(s), and 35 exception(s).
[ View ]

Thanks Alex, this patch fixes that.

Status:Needs review» Needs work

The last submitted patch, 1496510_44_search_cmi.patch, failed testing.

Now that you do this:

-  foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
+  foreach (config('search.settings')->get('search_active_modules') as $module) {

You can't use $active_modules here....

   // Per module settings
-  foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
-    $added_form = module_invoke($module, 'search_admin');
-    if (is_array($added_form)) {
-      $form = array_merge($form, $added_form);
+  if (!empty($active_modules)) {
+    foreach ($active_modules as $module) {
+      $added_form = module_invoke($module, 'search_admin');
+      if (is_array($added_form)) {
+        $form = array_merge($form, $added_form);
+      }
     }
   }

I suggest you make the following change:

   // Per module settings
-  foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
-    $added_form = module_invoke($module, 'search_admin');
-    if (is_array($added_form)) {
-      $form = array_merge($form, $added_form);
+  foreach (config('search.settings')->get('search_active_modules') as $module) {
+    $added_form = module_invoke($module, 'search_admin');
+    if (is_array($added_form)) {
+      $form = array_merge($form, $added_form);
+    }
    }

Two things need to happen to this patch now:

- It need to be updated to YAML instead of XML
- The naming needs to be adjusted to match the conventions defined at http://drupal.org/node/1667896

Assigned:Unassigned» aspilicious

Assigned:aspilicious» Unassigned

Unassign not enough time available

Assigned:Unassigned» swentel

Taking

Status:Needs work» Needs review
StatusFileSize
new25.65 KB
PASSED: [[SimpleTest]]: [MySQL] 37,264 pass(es).
[ View ]

Patch attached. config file is now in YAML and converted the variable names, let's hope this turns green!

Talked this through first a bit with alexpott on IRC and at first we thought splitting these up in search.settings and search.indexing. However, that turned out to be a little awkwardly and overcomplicated, especially in the admin form.

+++ b/core/modules/search/lib/Drupal/search/Tests/SearchEmbedFormTest.phpundefined
@@ -43,7 +43,7 @@ class SearchEmbedFormTest extends SearchTestBase {
+    config('search.settings')->set('search_embedded_form_submitted', $this->submit_count)->save();

search_embed_form_submitted is kinda verbose. And don't we need a default value? (or is it not needed in this test)

+++ b/core/modules/search/search.admin.incundefined
@@ -50,10 +50,12 @@ function _search_get_module_names() {
function search_admin_settings($form) {

Don't we need to pass $form_state (for config_settings_form) ?

-13 days to next Drupal core point release.

StatusFileSize
new25.74 KB
PASSED: [[SimpleTest]]: [MySQL] 37,296 pass(es).
[ View ]

New patch with $form_state variable

The default value of search_embed_form_submitted is set in the test itself, no need to have a default value for that for core itself as it's not used.

do we no longer need to modify the search_update_8000 function? I still find the variables we're modifying elsewhere there.

I think I might need to catch up with the documentation on this.

Status:Needs review» Needs work

This looks good to me.

+++ b/core/modules/search/lib/Drupal/search/Tests/SearchEmbedFormTest.phpundefined
@@ -67,7 +67,7 @@ class SearchEmbedFormTest extends SearchTestBase {
-    $count = variable_get('search_embedded_form_submitted', 0);
+    $count = config('search.settings')->get('search_embedded_form_submitted');

I don't see these in the search.settings. Also, these appear to be test related search variables so do we want them there? Third, should we drop the 'search_' prefix?

+++ b/core/modules/search/lib/Drupal/search/Tests/SearchKeywordsConditionsTest.phpundefined
@@ -26,8 +26,7 @@ class SearchKeywordsConditionsTest extends SearchTestBase {
     // Test with all search modules enabled.
-    variable_set('search_active_modules', array('node' => 'node', 'user' => 'user', 'search_extra_type' => 'search_extra_type'));
-    menu_router_rebuild();
+    config('search.settings')->set('active_modules', array('node' => 'node', 'user' => 'user', 'search_extra_type' => 'search_extra_type'))->save();    menu_router_rebuild();

Missing return before menu_router_rebuild().

Status:Needs work» Needs review
StatusFileSize
new25.65 KB
PASSED: [[SimpleTest]]: [MySQL] 37,294 pass(es).
[ View ]

New patch. Fixed the return. Dropped the 'search_' prefix as well.
I changed search.settings to search.testing as the embedded variable is indeed only used in testing.
However, really strictly speaking, I think 'embedded_form_submitted' a state, thus not belonging to the config system, so should we wait untill #1175054: Add a storage (API) for persistent non-configuration state gets in ?

Issue tags:+API change

All config system conversions are API changes, so tagging as such.

Status:Needs review» Needs work

From another cmi issue

Additionally, update_variables_to_config() "installs" (merges) the default configuration of a module already (if appropriate), so this call to config_install_default_config() should be removed.

Status:Needs work» Needs review
StatusFileSize
new25.61 KB
PASSED: [[SimpleTest]]: [MySQL] 37,115 pass(es).
[ View ]

New patch without update_variables_to_config()

Is that correct? Since the update_variables_to_config() is in an update hook it seems appropriate.

Sorry, patch is without the config_install_default_config() function :)

That's what I mean. I think that the patch needs the update_variables_to_config function. I've seen that function in other update hooks.

However, your patch went green. so........

Status:Needs review» Needs work

Thanks for working on this! :)

+++ b/core/modules/search/config/search.settings.yml
@@ -0,0 +1,21 @@
+minimum_word_size: 3
+overlap_cjk: 1
+cron_limit: 100
+tag_weights:
+    h1: 25
+    h2: 18
+    h3: 15
+    h4: 14
+    h5: 9
+    h6: 6
+    u: 3
+    b: 3
+    i: 3
+    strong: 3
+    em: 3
+    a: 10
+active_modules:
+    node: node
+    user: user
+and_or_limit: 7
+default_module: 'node'

I wonder whether there is some room for improving the keys? It looks like a good chunk of them belongs to indexing, and another chunk to searching.

So how about this?

index:
    minimum_word_size: '3'
    overlap_cjk: '1'
    cron_limit: '100'
    tag_weights:
        h1: '25'
        ...
search:
    active_modules:
        - node
        - user
    and_or_limit: '7'
    default_module: node

Would that make sense?

Please also note that all non-strings need to be enclosed in single-quotes. All strings are not quoted.

+++ b/core/modules/search/config/search.settings.yml
@@ -0,0 +1,21 @@
+active_modules:
+    node: node
+    user: user
+++ b/core/modules/search/lib/Drupal/search/Tests/SearchKeywordsConditionsTest.php
@@ -26,7 +26,7 @@ class SearchKeywordsConditionsTest extends SearchTestBase {
+    config('search.settings')->set('active_modules', array('node' => 'node', 'user' => 'user', 'search_extra_type' => 'search_extra_type'))->save();
+++ b/core/modules/search/lib/Drupal/search/Tests/SearchPageOverrideTest.php
@@ -29,7 +29,7 @@ class SearchPageOverrideTest extends SearchTestBase {
+    config('search.settings')->set('active_modules', array('node' => 'node', 'user' => 'user', 'search_extra_type' => 'search_extra_type'))->save();
+++ b/core/modules/search/search.admin.inc
@@ -49,11 +49,13 @@ function _search_get_module_names() {
-  foreach (variable_get('search_active_modules', array('node', 'user')) as $module) {
+  foreach (config('search.settings')->get('active_modules') as $module) {

It looks like this should be an indexed array instead of an associative/hashmap (and actually has been that way before); i.e.,

active_modules:
    - node
    - user

+++ b/core/modules/search/search.admin.inc
@@ -49,11 +49,13 @@ function _search_get_module_names() {
+function search_admin_settings($form, $form_state) {

&$form_state needs to be taken by reference.

+++ b/core/modules/search/search.admin.inc
@@ -156,24 +158,38 @@ function search_admin_settings_validate($form, &$form_state) {
+  if ($config->get('cron_limit') != $form_state['values']['cron_limit']) {
+    $config->set('cron_limit', $form_state['values']['cron_limit']);
+  }
+  if ($config->get('default_module') != $form_state['values']['default_module']) {
+    $config->set('default_module', $form_state['values']['default_module']);
+  }

For these two, we do not have to check the existing value and can just simply set the new value.

+++ b/core/modules/search/search.api.php
@@ -69,7 +69,7 @@ function sample_search_conditions_callback($keys) {
-  if ($force_keys = variable_get('sample_search_force_keywords', '')) {
+  if ($force_keys = config('search.settings')->get('sample_search_force_keywords')) {

The idea of this API hook example code is to demonstrate a module that integrates with Search module, so the sample config object actually belongs to sample_search.module; i.e.:

config('sample_search.settings')->get('force_keywords')

+++ b/core/modules/search/tests/modules/search_embedded_form/search_embedded_form.module
@@ -30,7 +30,7 @@ function search_embedded_form_menu() {
+  $count = config('search.testing')->get('embedded_form_submitted');
@@ -56,8 +56,10 @@ function search_embedded_form_form($form, &$form_state) {
+  $search_config = config('search.testing');

The config object name needs to be in search_embedded_form's namespace; i.e.:

config('search_embedded_form.settings')->get('submitted')

Assigned:swentel» Unassigned

I'm off for a few days - so if anyone else wants to re-roll with sun's remarks, feel free! If it's not done by wednesday, I'll take it back then!

Status:Needs work» Needs review
StatusFileSize
new26.13 KB
PASSED: [[SimpleTest]]: [MySQL] 37,216 pass(es).
[ View ]
new21.5 KB

Made changes proposed in #63 and did a couple of code tidy ups.

re: search_embedded_form.module and it's config value "submitted"... I think this really is state but in absence of the state system and the fact that this is a test module I've converted it anyway.

Status:Needs review» Needs work
Issue tags:-API change, -Configuration system, -Config novice

The last submitted patch, 1496510-65.search_cmi.patch, failed testing.

Status:Needs work» Needs review
Issue tags:+API change, +Configuration system, +Config novice

#65: 1496510-65.search_cmi.patch queued for re-testing.

Personally this looks to verbose
$limit_combinations = config('search.settings')->get('search.and_or_limit');

I understand the reason for grouping stuff in "index" but why group again in search. Everything not grouped is part of "search" by default. No need to create an extra layer.

Looking at this again I agree with @aspilicious - the yml file should look like this:

active_modules:
    - node
    - user
and_or_limit: '7'
default_module: node
index:
    cron_limit: '100'
    overlap_cjk: '1'
    minimum_word_size: '3'
    tag_weights:
        h1: '25'
        h2: '18'
        h3: '15'
        h4: '14'
        h5: '9'
        h6: '6'
        u: '3'
        b: '3'
        i: '3'
        strong: '3'
        em: '3'
        a: '10'

Do I need to reroll this patch with the changes is #69?

@cosmicdreams: The answer to such questions is always "yes." :-D

StatusFileSize
new7.93 KB
new25.98 KB
PASSED: [[SimpleTest]]: [MySQL] 37,218 pass(es).
[ View ]

Rerolled patch with changes suggested by @aspilicious

Status:Needs review» Reviewed & tested by the community

AWESOME, friends! Thank you so much for getting this done! :)

Status:Reviewed & tested by the community» Fixed

Rock! So great to have another one of these off our plate! :D

Committed and pushed to 8.x. Thanks!

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