Problem/Motivation

The d6_user migration lists d6_user_picture_file as an optional dependency. Unfortunately, d6_user uses the d6_user_picture process plugin to handle the user_picture column. If the d6_user_picture_file migration hasn't been run yet, this processor will result in a MigrateSkipRowException being thrown, and the user will not be imported.

Proposed Resolution

Two options leap to mind:

  1. Make d6_user_picture_file a required dependency of d6_user
  2. Make the d6_user_picture processor return NULL when the MigrateSkipRowException is thrown -- in other words, the picture will always be optional

I prefer option #1. Option #2 strikes me as the sledgehammer approach.

Remaining Tasks

  1. Decide on an approach to fix this
  2. Make a patch
  3. Get it committed

API Changes

None -- unless we go with the second approach, since that will change the way that the d6_user_picture processor is used.

UI Changes

None.

CommentFileSizeAuthor
#5 2484105-4.patch573 bytesphenaproxima

Comments

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Issue summary: View changes
benjy’s picture

benjy commented

I don't see why the picture migrations should be required to run the user migration, it requires at least 3 or 4 more migrations to run. I don't feel that strongly about it though if someone wants to change my mind.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

@benjy -- Ideally, user pictures should not be required in order to complete a user migration. But this bug is serious enough that it can actually prevent users from being imported. So I'm only proposing the hard dependency on user_picture as a stop-gap solution until we build something more robust.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

In the context of a full upgrade, you need to migrate the pictures, and they need to be migrated before the user migration to get attached. I agree it should be a hard dependency.

In the contrib custom Drupal-to-Drupal migration case, if in a given case you don't want the pictures, you can just remove the dependency.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Active » Needs review
StatusFileSize
new 2484105-4.patch573 bytes

After discussing with @mikeryan on IRC, I think the best and easiest solution here is to declare d6_user_picture_file as a hard dependency of d6_user, which is what the attached patch does. It's a lot harder to alter the stubbing stuff such that it won't cause users to be skipped during import.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Needs review » Postponed

This issue would also be resolved by #2487568: Process plugins should not be allowed to skip rows, which I personally think is a more elegant solution. So I'm postponing it for now, and if the other patch gets in, this will be wontfixed.

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Postponed » Closed (duplicate)

#2484405: User pictures do not need a dedicated process plugin addresses this on the heels of #2560671: The Migration process plugin should not skip rows.