modules/user/user.install:562: update_variables_to_config('user.flood'

Thanks in advance for helping many hands to make light work!
See #2181257: [meta] Variables to config migration [d7] for instructions

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fastangel’s picture

Assigned: Unassigned » fastangel

working on this.

fastangel’s picture

Status: Active » Needs review

I can't find mapping for this. In d8 we have:

uid_only: false
ip_limit: 50
ip_window: 3600
user_limit: 5
user_window: 21600

But in d6 the table flood is into module system. And the variables to settings flood are managed with system module (That were implemented in other issues)

chx’s picture

Title: Variable to config: user.flood » Variable to config: user.flood [D7]
Status: Needs review » Postponed

Let me try to explain this once again: if you were to copy the problematic Drupal 7 variables then I would not need to open user.install , find the update function and that we talk about user_failed_login_user_limit. This is frustrating and wastes my time. You have this in front of you. Please save a little time for me. What we have in Drupal 8 is (almost) irrelevant in this case because we need to see what the variable is used for in D7 and then compare to D8.

In this specific case, if you run a search on user_failed_login_user_limit in D7 then you can find that it has no UI to set and it's read merely twice. It's not upgraded from D6, that's for sure. It is visible that it's used a configuration for flood control.

Searching for flood_is_allowed in D6 shows that only contact was flood controlled and user login was not. So this is D7 only.

eliza411’s picture

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

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

eliza411’s picture

Status: Postponed » Active
eliza411’s picture

Title: Variable to config: user.flood [D7] » Variable to config: user.flood [d7]
Issue summary: View changes
eliza411’s picture

eliza411’s picture

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

Assigned: Unassigned » joshtaylor

I am working on this.

joshtaylor’s picture

Status: Active » Needs review
FileSize
4.22 KB
benjy’s picture

Status: Needs review » Needs work
+++ b/core/modules/migrate_drupal/lib/Drupal/migrate_drupal/Tests/d7/MigrateUserFloodConfigTest.php
@@ -0,0 +1,55 @@
+    $this->assertIdentical($config->get('uid_only'), NULL);
+    $this->assertIdentical($config->get('ip_limit'), NULL);
+    $this->assertIdentical($config->get('ip_window'), NULL);
+    $this->assertIdentical($config->get('user_limit'), NULL);
+    $this->assertIdentical($config->get('user_window'), NULL);

Why would all these be null?

+++ b/core/modules/migrate_drupal/config/migrate.migration.d7_user_flood.yml
@@ -0,0 +1,18 @@
+  plugin: drupal6_variable

Shouldn't this be d6_variable?

joshtaylor’s picture

FileSize
1.63 KB
4.21 KB

Good catch - I've updated to d6_config as per the new standard.

I also can't get the tests to pass - for some reason after loading the dumps it thinks it is null.

This is also my first interdiff, let me know if it is incorrect.

benjy’s picture

  1. +++ b/core/modules/migrate_drupal/config/migrate.migration.d7_user_flood.yml
    @@ -0,0 +1,18 @@
    +  uid_only: user_failed_login_identifier_uid_only
    

    This key doesn't match above?

  2. +++ b/core/modules/migrate_drupal/lib/Drupal/migrate_drupal/Tests/Dump/Drupal7UserFlood.php
    @@ -0,0 +1,51 @@
    +      ->values(array(
    +        'name' => 'uid_only',
    +        'value' => "b:0;",
    +      ))
    +      ->values(array(
    +        'name' => 'ip_limit',
    +        'value' => "i:50;",
    +      ))
    +      ->values(array(
    +        'name' => 'ip_window',
    +        'value' => "i:3600;",
    +      ))
    +      ->values(array(
    +        'name' => 'user_limit',
    +        'value' => "i:5;",
    +      ))
    +      ->values(array(
    +        'name' => 'user_window',
    +        'value' => "i:21600;",
    +      ))
    +      ->execute();
    

    Ident is wrong on this.

joshtaylor’s picture

FileSize
3.98 KB
2.57 KB

Sorry for the delay on this - I have updated the files to mimic https://drupal.org/node/2130283 .

I've fixed the indentation levels, updated the YML for user_failed_login_identifier_uid_only, and all tests pass.

joshtaylor’s picture

Status: Needs work » Needs review
benjy’s picture

Status: Needs review » Fixed

Thanks. Committed to drupal7 c641847

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
Assigned: joshtaylor » Unassigned
Status: Closed (fixed) » Needs work

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

phenaproxima’s picture

Status: Needs work » Needs review
FileSize
9.67 KB

Rerolled against 8.0.x, with a test of the d7_user_flood migration.

phenaproxima’s picture

FileSize
9.64 KB

Deleted a bit of extraneous cruft from the test.

Status: Needs review » Needs work

The last submitted patch, 20: 2132221-20.patch, failed testing.

phenaproxima’s picture

Status: Needs work » Postponed
phenaproxima’s picture

Status: Postponed » Needs review
FileSize
9.58 KB
phenaproxima’s picture

phenaproxima’s picture

Status: Needs review » Closed (won't fix)