Allow Product migration to set a specific revision_uid

What's your use case here? I don't think it's a bug that we capture whoever actively initiated the creation of a revision, but I'm open to the change.

Hmm, interesting. How do you manage authentication for the incoming API request?

I have encountered this problem with a slightly more common use case: Drush migration execution.

If you run a migration via Drush and don't set the --user flag (which normally you don't need to do), Commerce creates new variants with a revision_uid of 0, and in the UI, if you look at a variant's revision history, the first revision is labeled as "Anonymous". This appears because the commerce_product controller hard-codes revision_uid from $user->uid, which is not set in Drush migrations unless you invoke the --user flag.

It would seem that at a minimum, the commerce_product controller should check whether $user->uid is set, and if not default to 1 (admin) to avoid the creation of anonymous revisions. Is this something that would be considered a bug, rather than a feature request?

Taking it to the next level, I could see wanting to have the ability to set the revision_uid on a per-product basis during migrations, especially if you have different users managing different parts of your catalog, and you'd like to preserve that relationship from the former commerce site to the new one during a migration. Otherwise, all products start off with the same uid (either the user who runs the migration through the UI or the uid set by --user with Drush). I believe the patch from yaworsk would handle this use case. However, it would probably be good to add an additional condition that sets revision_uid to 1 if both $product->revision_uid and $user->uid are not set, to ensure that userless (anonymous) revisions are never created.

I'll also note that there is precedence for being able to set revision_uid during migration for nodes:
https://www.drupal.org/node/1349696#revision_uid

Is there anything special about products that they should NOT be able to have revision_uid set to something other than $user->uid, unlike nodes?

At the suggestion of @jerdavis I looked deeper into how updating revision_uid during node migrations works. It turns out that this is done as a secondary process in MigrateDestinationNode::insert where the node revision is initially created as usual (hard-coded values based on the current user session), but then the revision uid (and timestamp, if desired) is updated by overwriting the original value in the node_revision table. It seems clunky and extra work, but it has the benefit of not modifying the basic node creation method (as is proposed in the first patch for this issue).

To clarify: node.module's node_save() function behaves similar to the Commerce Product controller in that both hard-code the session user's uid when creating a revision. The precedent that I previously mentioned was to allow revision_uid to be overwritten during a migration, as part of the migration codebase; it is not something inherent to the node CRUD codebase.

I've proposed an alternate solution that follows the MigrateDestinationNode::insert example. It adds the ability to overwrite the default revision uid during product migrations as part of the MigrateDestinationCommerceProduct class. It may not solve @yaworsk's problem (unless @yaworsk uses Commerce Migration for product importing), but it would solve my problem, and it would make it easier to avoid confusion about the source of and fix for anonymous revisions during drush migrations. My patch updates MigrateDestinationCommerceProduct to define an insert() method which first executes the parent::insert() method, and then follows up with a check for revision_uid from the migration and updates the commerce_product_revision table in the same way as in the MigrateDestinationNode::insert method.

(Please excuse my terminology if it's not 100% correct -- I'm rather new to OOP and especially Drupal PHP OOP.)

Since this is a change to Commerce Migrate, rather than to Drupal Commerce, I suspect this needs to get moved from where it is, but I'm not sure what the protocol is. Is this different enough that a new issue needs to be created?

Finally, I suspect that there is a better way to check for and set the revision id than with $return[1], but I couldn't figure anything out that worked. I'm open to suggestions.

Forgot to update the issue status when submitting my patch.

Can I please get some help figuring out why my patch failed testing? This is my first attempt at a patch, so there is still a lot I have to learn (both technically and procedurally).

Is the problem as simple as the fact that my patch is to Commerce Migrate, not Commerce, and the testing system is trying to apply the patch to the wrong branch of code? If so, what is the solution, moving the issue to the Commerce Migrate project? Is that something I can do myself by updating the issue metadata, or does protocol leave that sort of change to either the original issue submitter (yaworsk) or someone with more experience? Should I create a new issue under Commerce Migrate and reference this one?

Hey Dale, yep, it's just failing beacuse the patch is to the other project. You should have the ability to move this issue to another project by updating the autocomplete textfield for "Project" to Commerce Migrate. Congrats on the first patch! Way to dive in. : )

(fyi, I've left the issue unmoved so you could see how it works. Note you may need to update the selected project version when you move issues between projects.)

Thanks, rszrama! I got busy with another project and forgot that this was still waiting for me to follow up on. I'll get to that next and see what happens.

Moving this issue from the Drupal Commerce project to Commerce Migrate because submitted patch falls within Commerce Migrate's codebase.

Summary of the issue so far:
When migrating products into Drupal Commerce, it is impossible to set a revision_uid to anything other than the uid of the user running the migration. The ability to set revision_uid would be useful in migrations that wish to preserve previous revision history from the original commerce site. The ability to set revision_uid during migration exists for basic node migrations (see MigrateDestinationNode's insert() method). I've submitted a patch that replicates this behavior, using the same method as applied to nodes, but for Commerce product migrations.

I'm not sure if moving the issue will automatically re-test the failed patch, so I'm re-uploading the original patch in order to ensure that it is tested with this update.

Two new questions:

1. What does it take for a patch to get into the test queue? When moving the issue to this project, I set the status to "needs review" which I thought would automatically put it in the queue, but after 2 days without it being tested, I am starting to wonder whether something else needs to happen for it to be tested.

2. In my current project, the above patch works for setting revision_uid to what I want it, but the field "revision_uid" is not defined for the product destination. When I go to view the migration in the UI I see the message:

'"revision_uid" was used as destination field in "" mapping but is not in list of destination fields'

What would it take to include revision_uid as an available destination field? When you edit a product variant in the UI, at least in Commerce Kickstart, there is both an "Edit" and a "Revisions" tab, and the Revisions tab includes a "Created By" field (which is what is updated by setting revision_uid), so revisions are a default and inherent part of product variants, so why would revision_uid not appear as an available destination field? The list of available destination fields appears to come from the Commerce Product definition, not anything in Commerce Migrate.

Ahh, in this case, I'm guessing Commerce Migrate does not have a testbot setup, so it's likely just going to need manual review.

Linking to #2555065: Can't map to product_id and update existing products which implements same method. I'd like to get tests added first for these.