Comments

fastangel’s picture

Status: Active » Needs work
StatusFileSize
new1.72 KB

I created a first version. I attach the patch. And some questions:
1.- Profile module is a optional module. Should we check if the table exist or check in table system if module is enable?
2.- Should we check in functions fields the name of profile fields to add the info?

Regards.

chx’s picture

I am so sorry, my reply didn't get posted :/

What's timezone_name? I just removed that.

1. Do we need a "requirements" plugin type? Or is this a source-y method? Good question!

2. fields() is not horribly important at this point, leave it.

I have a question too -- while I havent looked at profile in like five years, wasn't there some serialization going on here or there?

fastangel’s picture

StatusFileSize
new1.07 KB

Sorry I don't know because I changed timezone XD I attach a new patch with this change removed.

I wait a little discussion for the first question for continue working here.

About your last question. I have installed a drupal 6 for do test and check the information. I thought the same but checked the database and we don't have any serialize data.

marvil07’s picture

+++ b/lib/Drupal/migrate/Plugin/migrate/source/d6/User.php
@@ -54,4 +54,25 @@ class User extends SqlBase {
+    $ = array();

remove, right? ;-)

1. since source's should be kind of low level it is about deciding what to do: migrate data of installed/disabled modules of prefer status module, I guess trusting on the module status would be better. I would say that it is a good idea to add an intermediary ancestor between SqlBase and the real sources, so we can add some extra methods there(maybe even three: DrupalSqlBase/D6SqlBase/D7SqlBase), i.e. isModuleEnabled($module) , @chx, what do you think?

2. please leave a todo there so we do not forget please

About serialization, yes it's a little tricky to find but there is one profile field that needs (un)serialization: date ;-), so it's an extra conditional on the retrieval loop(see d6's _profile_field_serialize()).

fastangel’s picture

StatusFileSize
new1.52 KB

Ok removed :
$ = array.
Added todo comment to fields function.
Added special case for date field.

NOTE: waiting about implement a moduleEnable method or not.

fastangel’s picture

Status: Needs work » Needs review
fastangel’s picture

Issue summary: View changes

Updated issue summary.

marvil07’s picture

Issue summary: View changes
Status: Needs review » Needs work
+++ b/lib/Drupal/migrate/Plugin/migrate/source/d6/User.php
@@ -54,4 +55,30 @@ class User extends SqlBase {
+  function prepareRow(Row $row, $keep = TRUE) {
+    // Find profile values for this row.

Can we just make a join instead of 1+N queries?

About isModuleEnabled, it's now implemented at Drupal6SqlBase::moduleExists(), so you want to call that at the start of prepareRow().

About fields, it's about making another query on fields, I will definitely suggest to do that on another method with a local static cache.

fastangel’s picture

Status: Needs work » Needs review
StatusFileSize
new1.53 KB

Updated with the last change. About your last comment. Do you say create a new method for example fieldProfile or similar?

chx’s picture

I would say add requirements() and then later we can add an interface which this will already implement.

chx’s picture

Apparently RequirementsInterface has been added and you implemented it in the other issue. Please do here as well. Thanks!

fastangel’s picture

StatusFileSize
new1.96 KB

I added the change of your last comment. About implement requirement function. Do you think that we need to implement?. I think that we need to check profile in prepare row because a migration of user can be without profile then profile isn't a requirements. Is right?

chx’s picture

Title: Expand 6.x User plugin to include Profile » Add a test to the 6.x User plugin to cover Profile
Status: Needs review » Active

Ah, yes, you are completely right, it's not a requirement. I have committed this, now let's add a test for it. Thanks!