Variable to config: statistics.settings [d7]

Added organization support to Anexus IT

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

Added a test and removed the migration settings for the popular block, since they don't seem to live in the statistics.settings config object anymore.

Re-rolled. No interdiff due to conflicts with the {variable} dump.

Six variables as source and only three has process and test. statistics_flush_accesslog_timer is an integer (the default integer) in D6 but a string in D7 and somehow it doesn't blow up here? What happened to the famous config schema? At least assertIdentical please.

Re-rolled, but also changed -- thanks to @chx I noticed that a few statistics-related variables belong in a different config object, so I wrote a separate migration and test for that. I also noticed that the statistics.settings migration is identical in D6 and D7, so I condensed them into a single one (the test of which uses the Drupal 6 source data).

Unfortunately, I can't provide an interdiff due to conflicts with the variable.php dump file.

On second thought...looks like D6 maintains the same exact set of Statistics-related variables that D7 does. So I've generalized both Statistics migrations.

Hm. Well, this one, at least, passes on my local machine.

sharing++

Ok awesome, onward!

Committed and pushed to 8.0.x. Thanks!

This apparently broke #2541800-32: Some config do not inherit from config_object, so locale_system_set_config_langcodes() results in schema errors, so rolling this one back.

Reroll.

I updated the d6 dumps not the d7 dumps and missed that. Sigh.
There will still be a failure with the schema.

Enabled statistics and the popular block and D8 and discovered that this is saving to the wrong configuration. It should be saving to an array, settings, in block.block.popular.content not block.settings.statistics_popular_block. Fixed that up and the test now fails on ConfigSchemaChecker , 'No schema for block.block.popularcontent'. Fine, that makes sense. Unfortunately, installing the block config doesn't install block.block.popularcontent.

Looked at the block migration and see that the statistics block is not migrated in that test. So no quick answer there.

How do we handle this case?

Put a langcode entry in the schema and the tests passed locally. But, not convinced this is correct, don't really know enough about the config system. And puzzled by the relevance of the comment in StatisticsReportsTest.php that " statistics.module doesn't use node entities, prevent the node language from being added to the options.".

Forgot to build the dump files.

Simply a reroll.

This patch has actually 2 parts, one is migrating statistics variables and the other is migrating the statistics block variables. The first part is straight forward and working fine but the second isn't. It will require more work and an better understanding of block migrations than I have. It makes better sense to me to move that statistics block work to a new issue. At minimum a new issue title will better identify what need to be done.

The attached patch removes everything related to the statistics block variables.

1. +++ b/core/modules/statistics/config/schema/statistics.schema.yml
   @@ -25,6 +25,9 @@ block.settings.statistics_popular_block:
   +    langcode:
   +      type: string
   +      label: 'Language code'
   

   This belongs with the separated-out block issue, no? If it belongs in a migration patch at all...

2. +++ b/core/modules/statistics/src/Tests/Migrate/MigrateStatisticsConfigsTest.php
   @@ -2,10 +2,10 @@
    use Drupal\migrate_drupal\Tests\d6\MigrateDrupal6TestBase;
   

   This is just going to run for D6, we should also test on D7 (i.e., keep the d6 test and make a dupe-except-for-the-base-class in the d7 directory).

1. Wow that was an oversight
2. D7 test file created.

Thx, mikeryan.

The last patch looks good but I note that previous patches had changes to the dumps, did something change there?

The changes to the dumps were about the statistics block settings. That work was moved to #2613870: Variable to config: statistics block settings [d7] because they have a different config destination. Looks like that happened in #38.

rerolled

fixed

Sorry guys, don't understand this issue.

On my local all tests passed

Console command: php core/scripts/run-tests.sh --color --concurrency 2 --verbose --sqlite ~/tmpfs/drupal/test.sqlite --url http://d8.lc/ --class '\Drupal\migrate_drupal_ui\Tests\d7\MigrateUpgrade7Test'

Please, check results in attached file

Code which generated this issue: $this->assertText(t('Congratulations, you upgraded Drupal!'));

@vprocessor, thanks for working on this. I don't know why that test is failing, maybe the testbot had a hiccup.

Just two items,

1. +++ b/core/modules/statistics/src/Tests/Migrate/d7/MigrateStatisticsConfigsTest.php
   @@ -0,0 +1,46 @@
   +namespace Drupal\statistics\Tests\Migrate\d7;
   

   This should be in tests/src/Kernel/Migrated/d7, similar to d6/MigrateStatisticsConfigtsTest.php

2. +++ b/core/modules/statistics/src/Tests/Migrate/d7/MigrateStatisticsConfigsTest.php
   @@ -0,0 +1,46 @@
   +/**
   + * @file
   + * Contains \Drupal\statistics\Tests\Migrate\d7\MigrateStatisticsConfigsTest.
   + */
   

   This isn't needed. See the d6/MigrateStatisticsConfigtsTest.php for an example.

@vprocessor - did you mean to upload a new patch responding to @quietone's comments?

One of those issues where the interdiff is harder to create than the patch.

Addressing comments in #58.

I was sure I had uploaded the interdiff. Anyway, here it is...

I requeued the patch for testing and it still passes.

Seems pretty straightforward.

Committed and pushed 3fff17d to 8.3.x and 5817330 to 8.2.x. Thanks!