Add Address name data migration rake task

[spaghetticode]

Description

After #3908 the table spree_addresses includes the name field, so it can be populated with historical data already present in the table by joining the content of firstname and lastname. Running rake solidus:migrations:migrate_address_names:up will take care of the job.

The task is arranged in steps that require user input in order to proceed:

• the task will exit no-op when no record with blank name is found;
• if records are found, the user is asked to confirm before proceeding;
• the user is now asked for the record batch size to be used. My tests showed that 100'000 records per batch should balance the necessity to avoid locking records for long and have the script complete without too much overhead;
• before migrating data, the user is asked to confirm once again.

For convenience, the Solidus upgrade task solidus:upgrade:three_point_zero has been added that needs to be run when upgrading to Solidus 3.0 (before deploying the 3.0 code) which runs this rake task.

Checklist:

• I have followed Pull Request guidelines
• I have added a detailed description into each commit message
• I have updated Guides and README accordingly to this change (if needed)
• I have added tests to cover this change (if needed)

[kennyadsl]

I don't get the upgrade strategy proposed. If this code is part of Solidus 3.0, how can we run the task before deploying it? Shouldn't this be part of 2.11?

[spaghetticode]

@kennyadsl yes it needs to be run in production when the app is still on 2.11, but as a part of the upgrade process to 3.0.

Given your question, I guess I did not understand properly these tasks upgrade nomenclature, my thinking was that, since the task needs to be run at most during the 3.0 upgrade, it should go under the 3.0 namespace.

[kennyadsl]

You run the task when upgrading from 2.10 to 2.11. This operation immediately makes the host app ready for 2.11, but also for 3.0 that will only contain removal of deprecated code at this point.

[kennyadsl]

So this means, if I got it correctly, that in 2.11 we'll have both name, first_name and last_name present in the database and the system always setting all these three fields all the times. The system works also with records that have only one version or the other (name vs first_name+last_name) and we have the rake task that fills all three for old records.

When users switch to Solidus 3, we remove usage of first_name and last_name, but the system should be ready to accept the change because all the old fields have been fixed.

Please let me know if you were thinking of another migration strategy.

[spaghetticode]

@kennyadsl yes exactly. The code that allows a 2.11 app to work with both firstname (as a real DB field) and firstname/lastname will be presented in another PR, which will be a prerequisite of this one (as a matter of fact, we don't have the name field in 2.11 yet).

Regarding the naming, I guess the problem comes from the fact that it's debatable whether a 3.0 upgrade task should be run before or after the actual upgrade. I would say that there may be cases when a task needs to be run before the upgrade is completed (like this one) and others when the task needs to (or can) be run right after the upgrade. We may use a more descriptive naming in order to clear up some confusion, for example rake solidus:upgrade:before:three_point_zero or rake solidus:upgrade:after:three_point_zero 🤔 .

[kennyadsl]

Got what you mean, I think that this has almost nothing to do with 3.0 though, which just removes the possibility to choose if combine names or not, forcing name to be used.

This change is just a continuation of the combined name feature and could live in 2.11 even for people that do not plan to upgrade.

[spaghetticode]

@kennyadsl I moved the address name migration task into the existing 2.11 upgrade task.

[kennyadsl]

Great job, left some comments/questions. We are so close!

[kennyadsl]

Can you please explain somewhere why we do need to not use transactional tests and cleanup database tables manually for these specs?

[spaghetticode]

Actually, we don't need this. This is leftover from the early stage of the development when the code also added the index on the column, which is not the case anymore. It can be safely removed 👍

[aldesantis]

@spaghetticode this looks good, although I'm also curious why we can't use transactional tests here.

Dismissing the approval, I saw that the specs are failing

[kennyadsl]

I saw the failures on the CI and I think it may be due to some ActiveRecord internal caching, which could be probably fixable using reset_column_information. Just a gut feeling, but let me know!

[spaghetticode]

@kennyadsl thanks for the hint, CI is green now 👍

[kennyadsl]

Thanks, Andrea!

[vokshirg]

I've been upgraded to solidus 3.0 but name fields are empty. Now I cant access firstname and lastname columns because of they are ignored. How could I update name now?

[kennyadsl]

@vokshirg did you run the update task on 2.11 before upgrading to 3.0?

In this discussion I outlined the suggested path for the upgrade.

[vokshirg]

Yes, but that is not possible to remember what was wrong, therefore I've wrote migration and that is helped. Thanks!

class FillNamesAtAdresses < ActiveRecord::Migration[6.0]
  def up_only
    ActiveRecord::Base.connection.execute("UPDATE spree_addresses SET name = CONCAT_WS(' ', firstname, lastname) WHERE name is NULL")
  end
end