Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fastangel’s picture

Assigned: Unassigned » fastangel

working on this.

fastangel’s picture

FileSize
18.66 MB

I think that we need to postponed this to D7. The variables in D8 are:

session:
  parameter: language
url:
  source: path_prefix
  prefixes:
    en: ''
  domains:
    en: ''

You can enable multiple system detection and order with a weight (See image attached). But In D6 we have only a variable (language_negotiation see https://api.drupal.org/api/drupal/includes%21locale.inc/function/locale_...). Then I think that we can't map :(.

What do you think?

fastangel’s picture

Title: Variable to config: language.negotiation » Variable to config: language.negotiation [d7]
Status: Active » Postponed

I talked with chx in irc and we move to postponed.

eliza411’s picture

Assigned: fastangel » Unassigned

Moving to the core queue to consolidate issues now that we're doing all the work there.

eliza411’s picture

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

Moving to the core queue to consolidate issues now that we're doing all the work there.

eliza411’s picture

Issue summary: View changes
Status: Postponed » Active
eliza411’s picture

Project: Drupal core » IMP
Version: 8.x-dev »
Component: migration system » Code
Parent issue: #2125745: [meta] Variables to config migration [D6] » #2181257: [meta] Variables to config migration [d7]
penyaskito’s picture

alanburke’s picture

Status: Postponed » Active

Blocking issues now unblocked

michaellenahan’s picture

FileSize
3.67 KB

I wasn't able to test this locally, I got a wierd recursion error. I think that's probably just my setup though.

David Hernández’s picture

Status: Active » Needs work

Just applied the patch and run the test locally. Here's the output:

---- Drupal\migrate_drupal\Tests\d7\MigrateLanguageNegotiationTest ----


Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Pass      Other      MigrateTestBase.p   49 Drupal\migrate\Tests\MigrateTestBas
    Enabled modules: locale, migrate_drupal, migrate
Fail      Other      MigrateLanguageNe   49 Drupal\migrate_drupal\Tests\d7\Migr
    Value NULL is identical to value false.
Fail      Other      MigrateLanguageNe   50 Drupal\migrate_drupal\Tests\d7\Migr
    Value NULL is identical to value false.
michaellenahan’s picture

FileSize
3.71 KB

The last patch was not good. Here's another try.

I have a strange problem.

In the test runner I get a failure at line 50.

Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
Pass      Other      MigrateTestBase.p   49 Drupal\migrate\Tests\MigrateTestBas
    Enabled modules: locale, migrate_drupal, migrate
Pass      Other      MigrateLanguageNe   49 Drupal\migrate_drupal\Tests\d7\Migr
    Value 'language' is identical to value 'language'.
Fail      Other      MigrateLanguageNe   50 Drupal\migrate_drupal\Tests\d7\Migr
    Value 'path_prefix' is identical to value false.

It turns out that url.source is set to path_prefix.

This is the literal value of url.source in /core/modules/language/config/language.negotiation.yml

I don't understand why this value isn't being set to boolean FALSE as a result of the test.

michaellenahan’s picture

FileSize
3.67 KB

I've removed the debugging line from the end.

michaellenahan’s picture

Status: Needs work » Needs review
David Hernández’s picture

There were a couple issues. One on the dump, where the second variable wasn't being created. The other one is the name of the language module (no longer locale) on the test. I've also fixed the test to use values more similar to the real environment.

benjy’s picture

Status: Needs review » Fixed

Committed to 8.x. I fixed some indenting and moved the migration into the install folder on the way in.

Status: Fixed » Closed (fixed)

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

jcost’s picture

Project: IMP » Drupal core
Version: » 8.0.x-dev
Component: Code » migration system
Status: Closed (fixed) » Needs work

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

phenaproxima’s picture

Needs to be updated and merged into the parent issue.

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
3.36 KB

Updated.

quietone’s picture

Moved to language module

quietone’s picture

Reroll. Since interdiff fails and the changes are minimal, I've included a diff

phenaproxima’s picture

Status: Needs review » Needs work

Except for these two things, looks great.

  1. +++ b/core/modules/language/migration_templates/d7_language_negotiation_settings.yml
    @@ -0,0 +1,19 @@
    +dependencies:
    +  module:
    +    - language
    +    - migrate_drupal
    

    This needs to be removed; config dependencies are not allowed in migration templates.

  2. +++ b/core/modules/language/src/Tests/Migrate/d7/MigrateLanguageNegotiationSettingsTest.php
    @@ -0,0 +1,42 @@
    +use Drupal\migrate_drupal\Tests\d7\MigrateDrupal7TestBase;
    +/**
    

    Nit: Need an extra line between the use statement and the doc comment.

quietone’s picture

Status: Needs work » Needs review
FileSize
3.03 KB
979 bytes

Both fixed.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Noice. Pre-emptive RTBC assuming testbot passes.

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 8.0.x. Thanks!

  • webchick committed 5da22c7 on 8.0.x
    Issue #2130307 by quietone, michaellenahan, phenaproxima, David...

Status: Fixed » Closed (fixed)

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

penyaskito’s picture

I'm not sure if this is enough, I think we need language_types config too. I will create another issue for that, now looking from d6 perspective but d7 should follow.

quietone’s picture

@penyaskito, language_types is at Variable to config: language.types [d7].

penyaskito’s picture

This is not migrating language prefixes. Created #2625148: [PP-1] Migrate language prefixes and domains to language.negotiation [d7] for that.