Problem/Motivation

Can't get last_imported timestamps recorded in the map table. Tried using $migration->setTrackLastImported(TRUE) in my source plugin but nothing gets recorded.

Proposed resolution

Fix Drupal\migrate\Plugin\Migration::getTrackLastImported() so it actually returns something. Might as well fix Migration::getDestinationIds() while we're there. :)

CommentFileSizeAuthor
#2 migrate-last-imported-timestamp-707484-1.patch599 bytesmilesw
#7 2723123-7-complete.patch2.03 KBagoradesign
#7 2723123-7-tests.patch1.44 KBagoradesign

Comments

milesw created an issue. See original summary.

milesw’s picture

milesw commented
Status: Active » Needs review
StatusFileSize
new migrate-last-imported-timestamp-707484-1.patch599 bytes

Patch to fix return values.

milesw’s picture

milesw commented

FYI, this was introduced March 30, 2016 in #2694009: MigrationInterface doesn't include get()

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Version: 8.1.1 » 8.1.x-dev
Issue tags: +Needs tests

Ouch - how did that get in? Answer - lack of tests, let's add them.

benjy’s picture

benjy commented
Status: Needs review » Needs work

NW for tests

agoradesign’s picture

agoradesign commented
Assigned: Unassigned » agoradesign

will write the test cases

agoradesign’s picture

agoradesign commented
Assigned: agoradesign » Unassigned
Status: Needs work » Needs review
StatusFileSize
new 2723123-7-tests.patch1.44 KB
new 2723123-7-complete.patch2.03 KB

So, here are the tests. As I've only added the tests, there's no interdiff necessary

btw, the migrate test structure seems to be a little bit messy, unless there's a clear intention for this:

There are 3 classes with @coversDefaultClass \Drupal\migrate\Plugin\Migration annotation:

  • \Drupal\KernelTests\KernelTestBase\MigrationTest
  • \Drupal\KernelTests\KernelTestBase\Plugin\MigrationTest
  • \Drupal\Tests\migrate\Unit\MigrationTest

At least the first two kernel tests should be combined into a single class... But that shouldn't be part of this issue now...

The last submitted patch, 7: 2723123-7-tests.patch, failed testing.

benjy’s picture

benjy commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Thanks, RTBC.

Lets create a follow-up to merge the first two classes. The second one is the process plugin so that's fine.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

Committed fb1c0f3 and pushed to 8.1.x and 8.2.x. Thanks!

  • alexpott committed 9bb1c36 on 8.2.x 
    Issue #2723123 by agoradesign, milesw: Last imported timestamps not set...

  • alexpott committed fb1c0f3 on 8.1.x 
    Issue #2723123 by agoradesign, milesw: Last imported timestamps not set...

Status: Fixed » Closed (fixed)

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