Support from Acquia helps fund testing for Drupal Acquia logo

Comments

chr.fritsch created an issue. See original summary.

bojanz’s picture

Not yet, patches welcome.

svendecabooter’s picture

Migration of Drupal 7 field types doesn't work yet. See #2631736: Cckfield Plugins must distinguish core versions.
I'm following up on that core issue, to be able to write the D7 addressfield to D8 address migration.

Some DB fields from D7 are no longer available in D8, according to my first investigation:
* sub_administrative_area
* sub_premise
* first_name
* last_name
* data

Should these just be skipped in the migration? Any thoughts?

hass’s picture

Huh??? I'm using first and lastname everywhere. Why is this missing now? Just having a fullname does not fit in most cases. Address module need to add this fields back than.

I also use the first/lastname tokens in realname and lastnames only in many views. Publishing the firstname is often not allowed.

bojanz’s picture

sub_administrative_area and sub_premise were never actually exposed in the UI by addressfield, and they would not be migrated.

Thanks for keeping us up to date, svendecabooter. I know webflo made some efforts in this area as well, you should ping him on IRC.

The new data model follows Google's address library used by Chrome and Android. It's been very carefully designed.
Google's address library makes an explicit note of sub_administrative area not being there because it's not used for addressing.
I made a similar comment in the library:

Doesn't include the sub-administrative area (United States: county, Italy: province, Great Britain: county) because it is not required for addressing purposes.

sub_premise would map to a potential address_line3 in the new model: #2482969: Add address line 3.
As I've said in that issue, I'm not opposed to adding it, but have no data on when it's supposed to be used (and Google doesn't support it).

As for first_name and last_name, we are once again following Google's lead, which means only using a single line for names.
Using first_name and last_name is something which only works in a very limited number of western countries, and we have no data / resources to try and get it right. See http://www.kalzumeus.com/2010/06/17/falsehoods-programmers-believe-about... for reference. Using a single line is a widely recommended best practise, and matches actual addressing needs (no address label has separate fields for different parts of the name).

hass mentions a use case where it's important to know the individual components of a name, but that's not an addressing use case, thus out of scope for this module.

hass’s picture

I don't know what google does there, but they seems to be wrong as *often*. This is a US company and it has mostly no idea about enterprise requirements or things outside of US.

If I start asking a user for his firstname and lastname and that ask again to fill the address field with a single string containing the same data they will not fill the fields. This is just worse usability.

Well I'm using addressfield not for commerce module. I use it to collect address data and full contact information.

It is the job of this module to maintain the full data in a re-usable way. A single field is a heavy regression. I can no longer send personal newsletters with "Dear Mr. Foo...", cannot use just the lastname wherever I need it. On address stickers I may only write one firstname letter and the full lastname. I cannot use the tokens in any email template and so on. You are also not able to place firstname and lastname in tables to create pdf invoice files.

Combining both names to one field is super failure and we learned this in several years of business.

Google is not the middle of this world.

bojanz’s picture

@hass
Reread #5, and read the link I gave. Name will always stay a single field.

The point of Address module is to collect the address. The name in that case is the recipient. You are free to disable the recipient field in the field settings, use a more complex name module (or custom fields) on your Contact entity for newsletter and other non-addressing purposes.

hass’s picture

Never read such a bullshit in my life like

40. Someone born into slavery in the Sudan, a woman born in rural China, an American baby recovered after being born into a toilet, a feral child, an amnesiac, etc, etc.

Slaves without names are not using the internet, american babys from a toilet get a name when they grow up and so on.

We will not deliver anything to people without a name nor do we make contracts with them. They can also enter just a dot in such a field if these unrealistic scenarios really need to be covered.

kevinquillen’s picture

Drupal 7 to Drupal 8 seems to migrate field types now, except non-core / complex ones like Address (and geofield).

How can we best implement support to provide a migration path under the Migrate framework?

As for first name, last name, there are instances where this is used. In such examples, I allowed customers to create multiple mailing addresses for their Drupal Commerce account, in others, it was used to collect and store an address in a CRM style collection of leads in Drupal 7. This is probably similar to hass' setup, because mailers/labels etc were created based on this information.

There has to be some support or help for those folks, or they simply won't be able to migrate. The latter isn't very acceptable, I would be willing to bet a lot of people are using the name fields.

bojanz’s picture

We need to write a MigrateCckField plugin. It would map the addressfield to address column mapping, and any name joining logic.
Also documented in commerce_migrate: #2724933: Migrate Addressfield -> Address.

mikeryan’s picture

Note that in the general case (the original question here) you can migrate data into an address field now by mapping each column:

  'field_address/country_code':
    plugin: default_value
    default_value: US
  'field_address/langcode':
    plugin: default_value
    default_value: en
  'field_address/address_line1': streetAddress
  'field_address/locality': city
  # This has the form US-IN.
  'field_address/administrative_area':
    plugin: concat
    delimiter: -
    source:
      - '@field_address/country_code'
      - state
'field_address/postal_code': zip

Drupal sources, though, will need that field plugin to be automatically handled by the upgrade tools.

bojanz’s picture

miiimooo’s picture

I have tested this and it's working great. Often there are country names instead of two letter codes in the source data.

So I have written a little migrate process plugin that maps country names to two letter codes using the address modules own country list.

This is how it can be used:

  field_address/country_code:
    plugin: lookup_country
    source: country

Patch attached.

miiimooo’s picture

Status: Active » Needs review
hass’s picture

Status: Needs review » Needs work

Patch seems incomplete.

miiimooo’s picture

Hmm strange. The patch is working for me when I use it in composer patches. Any idea what's wrong with it?

hass’s picture

I do not see the migration code in reprository and your patch... how are you running a migration without migrate code!? You only shared a patch with your country conversion plugin... the migration_templates folder is completly missing.

miiimooo’s picture

Indeed it's just a plugin that can be used in a migration. It doesn't provide any migration (between versions or for content import) itself. But I think it could be useful for someone else who wants to import/migrate country address data. Should I open a separate issue for this patch?

hass’s picture

No, it should be part of the patch here.

sgurlt’s picture

Subscribing! Do I see it correctly that it is currently not possible to migrate data into addressfields using the latest dev version?

Just saw https://www.drupal.org/node/2594631#comment-11322879 :-)
Maybe someone else runs into my problem as well and this helps. It is important to map country_code and langcode, if those are not mapped it is not working.

hussainweb’s picture

Status: Needs work » Needs review
FileSize
2.45 KB

The patch in #13 is useful if you want to write a custom migration but it is not really necessary for an upgrade. As such, I am not including that process plugin in this patch, which purely handles an upgrade.

Right now, this patch depends on a core patch at #2787639-5: MigrateCckFieldPluginManager mixes up its behaviour for creating and loading definitions. With this patch, I am able to migrate the address field itself, the instance, and the widget configuration, but the formatter configuration does not get migrated due to another bug (which I will raise in the core issue queue).

hussainweb’s picture

More notes for the patch in #21.

The source field contains 13 properties while destination contains 10. In the source, name_line, first_name, and last_name are combined to a single property and sub_administrative_area and sub_premise are removed entirely. In the destination, sorting_code is a new property and I just set it to empty string. For the name, I first check name_line and if it is empty, I concatenate first_name and last_name and set that to the new name property.

hussainweb’s picture

I found an existing issue which causes a problem with the formatters at #2726803: Field formatters with names different than their field type can not be migrated.

hass’s picture

hussainweb’s picture

Cool, so whichever goes in first, we can make changes in the other issue. Since this depends on a core issue (or two), I am guessing the other one would have gone in by then.

chriscalip’s picture

In reply to patch #21
I tried this patch; installed patch in address module, updated my config yaml migration map to use this via

field_asset_address:
  plugin: d7_cck_address
  source: field_cap_address

I get error message :

✝ vagrant@multi-vlad ✝ ../vlad_aux/c5-dev/docroot (dev)
$ drush mi node_locale2asset --limit=1
exception 'Drupal\Component\Plugin\Exception\PluginNotFoundException' with message 'The "d7_cck_address" plugin does not exist.' in [error]
/var/www/site/vlad_aux/c5-dev/docroot/core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php:52
Stack trace:
#0 /var/www/site/vlad_aux/c5-dev/docroot/core/lib/Drupal/Component/Plugin/Discovery/DiscoveryCachedTrait.php(25):
Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition(Array, 'd7_cck_address', true)
#1 /var/www/site/vlad_aux/c5-dev/docroot/core/modules/migrate/src/Plugin/MigratePluginManager.php(56):
Drupal\Core\Plugin\DefaultPluginManager->getDefinition('d7_cck_address')
#2 /var/www/site/vlad_aux/c5-dev/docroot/core/modules/migrate/src/Plugin/Migration.php(377):
Drupal\migrate\Plugin\MigratePluginManager->createInstance('d7_cck_address', Array, Object(Drupal\migrate\Plugin\Migration))
#3 /var/www/site/vlad_aux/c5-dev/docroot/core/modules/migrate/src/MigrateExecutable.php(361):
Drupal\migrate\Plugin\Migration->getProcessPlugins(NULL)
#4 /var/www/site/vlad_aux/c5-dev/docroot/core/modules/migrate/src/MigrateExecutable.php(217):
Drupal\migrate\MigrateExecutable->processRow(Object(Drupal\migrate\Row))
#5 [internal function]: Drupal\migrate\MigrateExecutable->import()
#6 phar:///usr/local/share/drush/drush/includes/drush.inc(720): call_user_func_array(Array, Array)
#7 phar:///usr/local/share/drush/drush/includes/drush.inc(711): drush_call_user_func_array(Array, Array)
#8 /var/www/site/vlad_aux/c5-dev/docroot/modules/contrib/migrate_tools/migrate_tools.drush.inc(275): drush_op(Array)
#9 [internal function]: _drush_migrate_tools_execute_migration(Object(Drupal\migrate\Plugin\Migration), 'node_locale2ass...', Array)
#10 /var/www/site/vlad_aux/c5-dev/docroot/modules/contrib/migrate_tools/migrate_tools.drush.inc(241): array_walk(Array,
'_drush_migrate_...', Array)
#11 [internal function]: drush_migrate_tools_migrate_import('node_locale2ass...')
#12 phar:///usr/local/share/drush/drush/includes/command.inc(366): call_user_func_array('drush_migrate_t...', Array)
#13 phar:///usr/local/share/drush/drush/includes/command.inc(217): _drush_invoke_hooks(Array, Array)
#14 [internal function]: drush_command('node_locale2ass...')
#15 phar:///usr/local/share/drush/drush/includes/command.inc(185): call_user_func_array('drush_command', Array)
#16 phar:///usr/local/share/drush/drush/lib/Drush/Boot/BaseBoot.php(67): drush_dispatch(Array)
#17 phar:///usr/local/share/drush/drush/includes/preflight.inc(66): Drush\Boot\BaseBoot->bootstrap_and_dispatch()
#18 phar:///usr/local/share/drush/drush/includes/startup.inc(325): drush_main()
#19 phar:///usr/local/share/drush/drush/drush(114): drush_startup(Array)
#20 /usr/local/share/drush/drush(10): require('phar:///usr/loc...')
#21 {main}
chriscalip’s picture

In response to comment #26 for the purposes of testing patch #21

Needed to change migration map config yaml to

  field_asset_address:
    plugin: address_value
    source: field_cap_address
chriscalip’s picture

Attached patch fixes bug in patch #21 and adds in api change from d7 to d8 in the administrative_area column
for example on US values the d7 value for administrative_area_column is IN while d8 value is US-IN

FROM

      'country_code' => $value['country'],
      'administrative_area' => $value['administrative_area'],
      'locality' => $value['locality'],

TO bugfix version :

      'country_code' => $value['country'],
      'administrative_area' => $value['country'] . '-' . $value['administrative_area'],
      'locality' => $value['locality'],
hussainweb’s picture

@chriscalip, thank you for the fixes. Please include interdiffs in the future as they help in reviewing the changes.

As for your problem in #26 and #27, I take it that you were writing a custom migration. It's great to hear that the patch works for custom mappings even though it was designed for a Drupal upgrade process.

hussainweb’s picture

Also, the core issue that stopped the migration from working has been committed to 8.1.x, 8.2.x, and 8.3.x - #2787639-14: MigrateCckFieldPluginManager mixes up its behaviour for creating and loading definitions

hussainweb’s picture

FileSize
927 bytes
2.58 KB

I have been using the patch in #28 and it works great. There is just a small thing: The administrative area is optional and if it is not set, the migrated administrative area is weird, e.g. US- or IN-. This change fixes that.

bojanz’s picture

Status: Needs review » Needs work

I have just committed the name issue, which has removed the recipient column and added family_name, given_name, additional_name.
Map family_name to last_name and given_name to first_name.

The subdivisions are now also stored without the prefixes you saw previously ('CA' instead of 'US-CA'.'

chriscalip’s picture

Status: Needs work » Needs review
FileSize
1.71 KB
2.69 KB

Attached is a corresponding follow through on recent address-rc1 api changes on given_name, family_name, and administrative_area.

chriscalip’s picture

typo fix at patch 33; relavant interdiff source is still patch 31.

bojanz’s picture

Patch looks good.

Only question:

+/**
+ * @MigrateProcessPlugin(
+ *   id = "address_value"
+ * )
+ */

Why is this plugin called "address_value" and not just "address"? Is there a convention we're following?
I looked at core but found no examples, perhaps contrib has some?

chriscalip’s picture

@bojanz
Right no convention in play; the choice of "address_value" is purely subjective.
You can see a list of migrate.process here at https://www.drupal.org/node/2129651 or doing drupal console drupal plugin:debug migrate.process

  • bojanz committed 6be088e on 8.x-1.x authored by hussainweb
    Issue #2594631 by chriscalip, hussainweb, miiimooo: Migration...
bojanz’s picture

Status: Needs review » Fixed

Looked into this, looks like the plugins are usually named after the source. Which makes sense, since the process plugin is addressfield specific, for example. So, renamed both plugins to "addressfield" and committed. Thanks, everyone!

Status: Fixed » Closed (fixed)

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

vaccinemedia’s picture

@miiimooo would it be possible to write a state_lookup plugin too? I'm having to import data with full state names rather than the two letter codes.

chriscalip’s picture

@vaccinemedia
Here's how you deal with it.

  'field_asset_address/country_code':
    plugin: default_value
    default_value: 'US'
  'field_asset_address/administrative_area':
    plugin: cap_state_to_abbrev
    source: State
  'field_asset_address/locality': City
  'field_asset_address/postal_code': Zip
  'field_asset_address/address_line1': Address 1
  'field_asset_address/address_line2': Address 2

Create a simple custom plugin that takes in full state name then convert to two letter state code.

gpsloco’s picture

Discussion is kind of old, I am trying to import data into an address field, too. And the term "field_asset_address" do I use this literally or is this just a placeholder for my address field, in the mysql database: user__field_adresse?

vpa24’s picture

This is my config migrate but it's not work. Please help me!!!

id: d7_field_airport_address
label: 'Custom migrate field airport address'
source:
  plugin: addressfield 
process:
  field_address_country_code:
    plugin: default_value
    default_value: 'US'
  field_address_administrative_area:
    plugin: cap_state_to_abbrev
    source: State
  field_address_locality: City
  field_address_postal_code: Zip
  field_address_address_line1: Address 1
  field_address_address_line2: Address 2
destination:
  default_bundle: airport
migration_dependencies: {}

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "addressfield" plugin does not exist.

PieterDC’s picture

@vpa24 I guess you want to migrate address information for airports, where airport is a node type and the address information is in a node field.
Then your source plugin should not be addressfield but d7_node

For the process part, you'll probably want something more like what @chriscalip wrote before. Important part there are the slashes which indicate subparts of the address field instead of multiple destination fields.
Also, https://drupal.stackexchange.com/questions/208908/migrate-address-field can be helpful for you.

vpa24’s picture

id: address_field
label: 'Custom migrate field address'
source:
  plugin: d7_node 
process:
  'field_address/country_code':
    plugin: default_value
    default_value: US
  'field_address/langcode':
    plugin: default_value
    default_value: en
  'field_address/address_line1': streetAddress
  'field_address/locality': city
  # This has the form US-IN.
  'field_address/administrative_area':
    plugin: concat
    delimiter: -
    source:
      - '@field_address/country_code'
      - state
  'field_address/postal_code': zip
migration_dependencies:
  required: {}
  optional: {}

TypeError: Argument 2 passed to Drupal\migrate\Plugin\MigrateDestinationPluginManager::createInstance() must be of the type array, null given

PieterDC’s picture

The error you're hitting is that your migration is missing destination configuration.
I can recommend reading https://www.drupal.org/docs/8/api/migrate-api/migrating-drupal-7-page-ar...
Your comment at #43 contained a destination...

Besides, those questions are a bit out of scope for this issue.
There are other options for getting help with such things. A lot of them are listed here https://www.drupal.org/support

vpa24’s picture

id: address_field
label: 'Custom migrate field airport address'
source:
  plugin: d7_node 
process:
  field_address_country_code:
    plugin: default_value
    default_value: 'US'
  field_address_administrative_area:
    plugin: concat
    delimiter: -
    source:
      - '@field_address/country_code'
      - state
  field_address_locality: field_address_locality
  field_address_postal_code: field_address_postal_code
  field_address_address_line1: field_address_thoroughfare 1
destination:
  plugin: 'entity:node'
  default_bundle: airport
migration_dependencies: {}

1048 Integrity constraint violation: 1048 Column 'title' cannot be null: INSERT INTO {node_field_data}

scotwith1t’s picture

For nodes, title is required, so be sure to provide a source for that as well...

kevinquillen’s picture

Yes, title is required for node destination migrations.