Problem/Motivation

We need an upgrade path for the core dblog module.

Remaining Tasks

It's not necessary to migrate all watchdog messages. But any variables maintained by dblog will need to be moved into configuration.

Contributors

@hosef and @-enzo- wrote the original migration; @bdmiaggio updated it to match HEAD and provided a test.

git commit -m 'Issue #2500499 by bdimaggio, -enzo-, hosef: Upgrade path for dblog 7.x'

Comments

bdimaggio’s picture

Assigned: Unassigned » bdimaggio
bdimaggio’s picture

Status: Active » Needs review
StatusFileSize
new2.26 KB

OK, here's the migration yml and test.

bdimaggio’s picture

StatusFileSize
new2.21 KB

A little cleanup on the test...

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Looks perfect to me. Thank you, @bdimaggio!

bdimaggio’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new2.21 KB

I noticed that I wasn't using my self::$modules when enabling the module in setUp(). That's fixed now.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Seems legit.

phenaproxima’s picture

Issue summary: View changes

Adding credit for @hosef, who authored the original migration in #2409439: Variable to config: dblog.settings [d7].

bdimaggio’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new2.21 KB

Last time: I realized that I had copied and pasted used bold originality to name my test testBookSettings rather than testDblogSettings, which this patch fixes.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community
StatusFileSize
new2.21 KB
new503 bytes

Changed the test group (apparently spaces matter). Otherwise, this still looks great.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work

This needs to use a migration template now - see https://www.drupal.org/node/2527568

phenaproxima’s picture

Status: Needs work » Needs review
StatusFileSize
new2.11 KB
new1.34 KB

Done.

phenaproxima’s picture

Issue summary: View changes

Added @-enzo- as a contributor, since the migration is based on his work in #2382117: Migration Files for Drupal 7 Variables.

benjy’s picture

  1. +++ b/core/modules/migrate_drupal/src/Tests/d7/MigrateDblogConfigsTest.php
    @@ -0,0 +1,53 @@
    +  public static $modules = array('dblog');
    

    Can hardly believe you #phenaproxima didn't use the new array syntax ;)

  2. +++ b/core/modules/migrate_drupal/src/Tests/d7/MigrateDblogConfigsTest.php
    @@ -0,0 +1,53 @@
    +    $this->installConfig(self::$modules);
    

    static::

phenaproxima’s picture

StatusFileSize
new2.1 KB
new911 bytes

Corrected :)

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

@benjy gave me his blessing on IRC to re-RTBC this.

phenaproxima’s picture

xjm’s picture

Title: Upgrade path for dblog 7.x » Migration path for dblog 7.x
phenaproxima’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new1.9 KB
new3.11 KB
chx’s picture

assertConfigSchema ? Why does this test do that and no other does ? Do we need to add it to others or do we need to remove from her or is dblog special? I asked alexpott about how much the schema is enforced

The value types are enforced. Tt does not enforce the structure other than throwing an error if something is array and it is not supposed to be - or vice versa

so I do not think we need to assertConfigSchema here.

phenaproxima’s picture

StatusFileSize
new1.73 KB
new909 bytes

Fixed.

chx’s picture

Priority: Minor » Normal
Status: Needs review » Reviewed & tested by the community

Looks good.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Migrate is not subject to beta evaluation. Committed 251ca21 and pushed to 8.0.x. Thanks!

  • alexpott committed 251ca21 on 8.0.x
    Issue #2500499 by phenaproxima, bdimaggio: Migration path for dblog 7.x
    

Status: Fixed » Closed (fixed)

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