Support from Acquia helps fund testing for Drupal Acquia logo

Comments

hosef’s picture

Title: Variable to config: language.types [d7] » Variable to config: search.settings [d7]
Status: Active » Needs work
Issue tags: +Needs tests
FileSize
870 bytes

Attached is the YAML for this issue, from the patch in #2382117: Migration Files for Drupal 7 Variables.
Test(s) (and maybe a dump file) still need to be written.

miguelc303’s picture

Added organization support to Anexus IT

jcost’s picture

Project: IMP » Drupal core
Version: » 8.0.x-dev
Component: Code » migration system

Will need to be submitted again to Core since moving from sandbox.

phenaproxima’s picture

phenaproxima’s picture

phenaproxima’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
2.32 KB

Updated for HEAD, and wrote a test.

quietone’s picture

quietone’s picture

And moved to the search module.

phenaproxima’s picture

Status: Needs review » Needs work

There is disparity between the D6 and D7 versions of this migration; the D6 one selects several variables which are never mentioned in the process pipeline. That's a WTF and should be addressed before this gets committed.

quietone’s picture

The differences your've spotted are nicely outlined in the know issues documentation, specifically
Search Settings. According to that D7 contains 2 variables, search_tag_weights and search_and_or_limit, that are not in D6 or D8.

However, looking at D8 search settings it seems that 'search_and_or_limit' is now 'and_or_limit' and 'search_tag_weights' is now 'tag_weights'.

Since this issue is clearly marked D7, I'll make a new issue for the change to d6_search_settings.yml.

quietone’s picture

Status: Needs work » Needs review
FileSize
4.43 KB
3.2 KB

Added migration for 'search_and_or_limit' and 'search_tag_weights' and tests.

mikeryan’s picture

Status: Needs review » Needs work

I do think it's helpful for long-term maintenance to keep the D6 and D7 tests as in-sync as possible - the only differences should really be inherent differences between D6 and D7.

  1. +++ b/core/modules/search/migration_templates/d7_search_settings.yml
    @@ -0,0 +1,28 @@
    +dependencies:
    +  module:
    +    - migrate_drupal
    +    - search
    

    The module dependencies are unnecessary and should be removed.

  2. +++ b/core/modules/search/src/Tests/Migrate/d7/MigrateSearchSettingsTest.php
    @@ -0,0 +1,58 @@
    + * Contains \Drupal\search\Tests\d7\MigrateSearchSettingsTest.
    

    Discrepancy from D6, where this is called MigrateSearchConfigsTest.

  3. +++ b/core/modules/search/src/Tests/Migrate/d7/MigrateSearchSettingsTest.php
    @@ -0,0 +1,58 @@
    +    $this->installConfig(static::$modules);
    

    Why is installConfig necessary here but not in the D6 test?

  4. +++ b/core/modules/search/src/Tests/Migrate/d7/MigrateSearchSettingsTest.php
    @@ -0,0 +1,58 @@
    +  public function testMigration() {
    

    Discrepancy with D6, where this is testSearchSettings().

  5. +++ b/core/modules/search/src/Tests/Migrate/d7/MigrateSearchSettingsTest.php
    @@ -0,0 +1,58 @@
    +    $config = \Drupal::config('search.settings')->get();
    

    Can use $this->config here (see the D6 test).

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
5.19 KB
4.36 KB

All fixed.

mikeryan’s picture

Status: Needs review » Reviewed & tested by the community

The last submitted patch, 1: search_settings-2409447-1.patch, failed testing.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed a07b164 on 8.0.x
    Issue #2409447 by quietone, phenaproxima, hosef, mikeryan: Variable to...

Status: Fixed » Closed (fixed)

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