Problem/Motivation

In the Drupal 7 contrib migrate module, there was a "system-of-record" concept. By default migrating into an existing content item completely replaces it with source data (system-of-record == SOURCE). In D7, setting the system-of-record to DESTINATION allowed you to selectively import only particular fields from the source, leaving other fields (which may have been manually modified on the destination side) unmolested. APIs for the system-of-record support (getSystemOfRecord(), setSystemOfRecord()) got carried forward into D8 but were never actually used - instead, we implemented an overwrite_properties property on destination plugins which achieves the same goal more cleanly. The vestigial system-of-record stuff should be removed.

Proposed resolution

  1. Remove getSystemOfRecord() and setSystemOfRecord() from MigrationInterface and Migration.
  2. Remove the SOURCE and DESTINATION constants from MigrationInterface.
  3. Remove the protected $systemOfRecord property from Migration.

Remaining tasks

Do it.

User interface changes

N/A

API changes

getSystemOfRecord() and setSystemOfRecord() removed from the interface. Since these are never actually used anywhere, and it's highly likely anyone is implementing MigrationInterface other than via the existing Migration plugin class, the likely impact of removing them is zero.

Data model changes

None.

Comments

mikeryan created an issue. See original summary.

shashikant_chauhan’s picture

Assigned: Unassigned » shashikant_chauhan
Status: Active » Needs review
StatusFileSize
new2.82 KB

Adding patch.

naveenvalecha’s picture

Issue tags: +Needs change record

This needs CR

shashikant_chauhan’s picture

I have added the Change record. Kindly review.

phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs change record +Dublin2016, +Migrate BC break

Change record updated to mention the closest Drupal 8 equivalent to the system-of-record stuff.

This is most definitely a BC break, though, since we're changing MigrationInterface and any custom implementation of it will now break. Marking as such, but otherwise this is RTBC in my opinion. We'll have to discuss with the committers if it's kosher to make a change like this now, but since Migrate is still experimental we may have a shot.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 2: 2801851-2.patch, failed testing.

phenaproxima’s picture

Issue tags: +Needs reroll
phenaproxima’s picture

Version: 8.2.x-dev » 8.3.x-dev

This is not RC-eligible, so it should be rerolled against 8.3.x.

yogeshmpawar’s picture

Status: Needs work » Needs review
StatusFileSize
new2.82 KB

I have rerolled the patch against 8.3.x

yogeshmpawar’s picture

Issue tags: -Needs reroll
phenaproxima’s picture

Status: Needs review » Reviewed & tested by the community

Looks great!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed
Issue tags: +rc eligible

Committed and pushed 6416f45 to 8.3.x and ea5c39e to 8.2.x. Thanks!

Added to 8.2.0 since migrate is experimental.

  • alexpott committed 6416f45 on 8.3.x
    Issue #2801851 by shashikant_chauhan, Yogesh Pawar, phenaproxima,...

  • alexpott committed ea5c39e on 8.2.x
    Issue #2801851 by shashikant_chauhan, Yogesh Pawar, phenaproxima,...

Status: Fixed » Closed (fixed)

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