Follow-up from #2118245: [meta] Source classes for config entities.

Write a D6 source plugin + tests for the profile_fields table.

CommentFileSizeAuthor
#10 imp-created_source_profile_field-2121397-10.patch5.8 KBfastangel
#8 imp-created_source_profile_field-2121397-7.patch5.58 KBfastangel
#7 imp-created_source_profile_field-2121397-7.patch0 bytesfastangel
#5 imp-created_source_profile_field-2121397-5.patch5.58 KBfastangel
#3 imp-created_source_profile_field-2121397-3.patch5.57 KBfastangel
#1 imp-created_source_profile_field-2121397-1.patch4.44 KBfastangel

Comments

fastangel’s picture

fastangel commented
Status: Active » Needs work
StatusFileSize
new imp-created_source_profile_field-2121397-1.patch4.44 KB

I created a first version. Attach the patch.

I have a little question about test. Which should be the test cases?

chx’s picture

chx commented

We usually just cook up random things; the content doesn't matter much -- especially in a simple case like this. However, as the work progresses and we discover this or that source plugin actually needs a bit more logic it's better to have a simple test written already.

fastangel’s picture

fastangel commented
StatusFileSize
new imp-created_source_profile_field-2121397-3.patch5.57 KB

Sound good. I added a three simple cases that should be enough at the moment.

marvil07’s picture

marvil07 commented

Still some minor fixes to do:

  1. +++ b/lib/Drupal/migrate/Plugin/migrate/source/d6/ProfileField.php
@@ -0,0 +1,65 @@
+  function query() {

    adding visibility as public here 

  2. +++ b/tests/Drupal/migrate/Tests/D6ProfileFieldSourceTest.php
@@ -0,0 +1,115 @@
+      'type' => array(

    should be fid, right?

fastangel’s picture

fastangel commented
Status: Needs work » Needs review
StatusFileSize
new imp-created_source_profile_field-2121397-5.patch5.58 KB

Fixed. One little comment. I check the rest of classes and I saw that don't have public visibility. Should open a new issue to fix?

chx’s picture

chx commented

->select('profile_fields', 'pf')

+ $this->databaseContents['profilefield'] = $this->results;

Please fix the table name in the latter to be profile_fields. I hope the test didn't pass in this state...

fastangel’s picture

fastangel commented
StatusFileSize
new imp-created_source_profile_field-2121397-7.patch0 bytes

Ohh sorry. I attach new version.

fastangel’s picture

fastangel commented
StatusFileSize
new imp-created_source_profile_field-2121397-7.patch5.58 KB

Sorry.

marvil07’s picture

marvil07 commented
Issue summary: View changes
Status: Needs review » Needs work 
+++ b/lib/Drupal/migrate/Plugin/migrate/source/d6/ProfileField.php
@@ -0,0 +1,65 @@
+class ProfileField extends SqlBase {
+

Please inherit now from Drupal6SqlBase class.

Also, please implement RequirementsInterface, so the same check on profile values source is here, an example of use on Comment source.

fastangel’s picture

fastangel commented
Status: Needs work » Needs review
StatusFileSize
new imp-created_source_profile_field-2121397-10.patch5.8 KB

Updated

chx’s picture

chx commented
Status: Needs review » Fixed

I have committed this. Both of needs to improve here: I need to attend to your patches much faster, sorry for letting them linger. In turn, I would appreciate if you could run your tests before submitting them -- this one included two syntax errors.

fastangel’s picture

fastangel commented

Ok and sory about the bug of syntaxis. I forgot to execute test.

Status: Fixed » Closed (fixed)

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