🇮🇹Italy @reinchek

Removes a typo

MR was merged into 2.1.0, a new major release.
I've also wrote the documentation part about your ResultsEvent.

Adds the documentation about the "ResultsEvent" event, introduced by 2.1.0 major release.

Hi @ricovandevin, i merged all things into 2.1.0, a new major release.
I've also wrote the documentation part about your ResultsEvent.

Here a summary:
---
ResultsEvent for Data Manipulation

Starting from version 2.1.0, the Migrate Source GraphQL module introduces the ResultsEvent, allowing other modules to modify the results obtained from a GraphQL query during migration.
Key Features

- getResults(): Retrieves the current query results.
- setResults(array $results): Allows modification of these results.

Usage Example
Modules can subscribe to the ResultsEvent to manipulate the migration data:

use Drupal\migrate_source_graphql\Event\ResultsEvent;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

class MyModuleSubscriber implements EventSubscriberInterface {
  public static function getSubscribedEvents() {
    return [ResultsEvent::class => 'onResultsEvent'];
  }

  public function onResultsEvent(ResultsEvent $event) {
    $results = $event->getResults();
    // Custom modifications to results.
    $results['custom_field'] = 'custom_value';
    $event->setResults($results);
  }
}

The GraphQL plugin now dispatches this event, enabling custom data handling during migrations.

Thanks

Hi @ricovandevin, thanks for your contribution!

Recently i worked on the new documentation, including all about *-dev merged features (read more: https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... → ).

I will merge your MR in 2.x-dev and i think that the module is ready for a new stable release 2.0.8 but not before adding the instructions about the ResultsEvent (let me know if you want to do it by your self modifying the documentation page or if you can't; then i'll do it as soon as possible).

Thanks very much!

Better description of the data_key and % placeholder.

restructuring of documentation to include new features and to disambiguate them

Hi hansfn, thanks for your help i really appreciate it!

I am in the process of restructuring the documentation to include new features that are on the dev branch and will soon be brought to a new stable version. However, I find your suggestions very helpful, so I endorse them regardless of the evolutions I am making to the structure.

Thanks

I tested the specific case installing 'drupal/storybook:^1.0@beta'.
Now works.
I set the issue's status to Fixed. If something doesn't add up, feel free to comment below. Thanks!

reinchek → changed the visibility of the branch origin/3447165-support-symfonydom-crawler6.4 to hidden.

Hi @mortona2k,
Thank you for the report. I've changed the minimum symfony/dom-crawler version to ^6. Now there should be no conflict with storybook dependencies.
I've to check better how to organize the three dependencies but for now I think it can be okay.

Thanks.

reinchek → made their first commit to this issue’s fork.

Completing the documentation.

Merged into 2.x (dev) branch.

Completing the documentation.

I am completing the documentation

Fixed and merged in 2.x (dev) branch.

Hi,

i've merged https://git.drupalcode.org/project/migrate_source_graphql/-/merge_reques... inside 2.x (dev) branch.
Now, dev also contains the following features:
- PROPERTY_SEPARATOR logic
- possibility to use the '%' placeholder to get all array's items
- migrate_source_graphql_test sub-module which setup some Kernel tests (runs some basic migration cases): needs work.

Thanks.

Hi @bander2, thanks for your contribution.
I've merged, as you can see, the code into 2.x-dev branch.

I'm collecting there all new small features, pending the appropriate documentation...

Thanks.

reinchek → made their first commit to this issue’s fork.

Great! thank you!

I made a stupid error, on this. So ignore my proposal. I apologise.

I am closing the task because I opened it for the wrong form, I apologise.

reinchek → created an issue.

Hi, related to this topic it's needed to change the way as the `NodeTypeConditionCreator` creates the plugin instance.
It's needed to change the removed "node_type" with new "entity_bundle:node".

I made a little patch that will fixes this issue.

reinchek → made their first commit to this issue’s fork.

Sorry for the shitpatch... but i'm doing these changes while working...
I've added in this new patch a fix.

PS: I've hided bad patches and leaved visible only the correct one.

Sorry for the shitpatch... but i'm doing these changes while working... I've added in this new patch a fix.

PS: I've hided bad patches and leaved visible only the correct one.

Hi @jlstrecker,
according to this issue " Selecting date causes page reload when AJAX is enabled 🐛 Selecting date causes page reload when AJAX is enabled Active " i've made a patch (which only applies if your, @jlstrecker, is already applied) to allow the module to perform a page refresh via ajax.
This also requires some modifications to be made to the drupal/facets module like as i proposed with this patch:
Making 'helper functions' (to do ajax submissions) globally usable ✨ Making 'helper functions' (to do ajax submissions) globally usable Needs work .

Nino.

I've made i mistake forgotting to say that this patch works only if the patch " #3273136 - Selecting date clears other facet values when using AJAX → " was previous applied.

I've rewrited the patch removing some console.log.

So we can call globally the function like in this way:

  Drupal.behaviors.facetsViewsAjax.updateFacetsView(href, view_dom_id, ajaxPath);

reinchek → created an issue.

Hi, i'm having same issue.
As you said the problem is caused by:
window.location.href = facetUrl.replace("__date_range_min__", min).replace("__date_range_max__", max);
causing the full page reload.

For now i've resolved (look at the attached patch) this by calling, inside the autoSubmit() function, the function updateFacetsView() of `Drupal.behaviors.facetsViewsAjax` library. But to do this it's also needed to move the "Helper funciton" updateFacetsView() inside the Drupal.behaviors.facetsViewsAjax object.

For the sake of completeness, I enclose the changes made within `facets/js/facets-views-ajax.js` (drupal/facets:1.6.0):

diff --git a/modules/contrib/facets/js/facets-views-ajax.js b/modules/contrib/facets/js/facets-views-ajax.js
--- a/modules/contrib/facets/js/facets-views-ajax.js
+++ b/modules/contrib/facets/js/facets-views-ajax.js	(date 1695895680257)
@@ -17,6 +17,7 @@
    */
   Drupal.behaviors.facetsViewsAjax = {
     attach: function (context, settings) {
+      var helpers = this;

       // Loop through all facets.
       $.each(settings.facets_views_ajax, function (facetId, facetSettings) {
@@ -38,10 +39,10 @@
         }

         // Update view on summary block click.
-        if (updateFacetsSummaryBlock() && (facetId === 'facets_summary_ajax')) {
+        if (helpers.updateFacetsSummaryBlock() && (facetId === 'facets_summary_ajax')) {
           $('[data-drupal-facets-summary-id=' + facetSettings.facets_summary_id + ']').find('a').once('facets_summary_ajax_link').click(function (e) {
             e.preventDefault();
-            updateFacetsView($(this).attr('href'), current_dom_id, view_path);
+            helpers.updateFacetsView($(this).attr('href'), current_dom_id, view_path);
             // Remove clicked element, ajax callback will update the content.
             $(this).parents('li').remove();
           });
@@ -54,112 +55,126 @@
               $(facet_item).on('facets_filter.facets', function (event, url) {
                 $('.js-facets-widget').trigger('facets_filtering');

-                updateFacetsView(url, current_dom_id, view_path);
+                helpers.updateFacetsView(url, current_dom_id, view_path);
               });
             }
           });

         }
       });
-    }
-  };
+    },

-  // Helper function to update views output & Ajax facets.
-  var updateFacetsView = function (href, current_dom_id, view_path) {
-    // Refresh view.
-    var views_parameters = Drupal.Views.parseQueryString(href);
-    var views_arguments = Drupal.Views.parseViewArgs(href, 'search');
-    var views_settings = $.extend(
-        {},
-        Drupal.views.instances['views_dom_id:' + current_dom_id].settings,
-        views_arguments,
-        views_parameters
-    );
+    // Helper function to update views output & Ajax facets.
+    updateFacetsView: function (href, current_dom_id, view_path) {
+      // Refresh view.
+      var views_parameters = Drupal.Views.parseQueryString(href);
+      var views_arguments = Drupal.Views.parseViewArgs(href, 'search');
+      var views_settings = $.extend(
+          {},
+          Drupal.views.instances['views_dom_id:' + current_dom_id].settings,
+          views_arguments,
+          views_parameters
+      );

-    // Update View.
-    var views_ajax_settings = Drupal.views.instances['views_dom_id:' + current_dom_id].element_settings;
-    views_ajax_settings.submit = views_settings;
-    views_ajax_settings.url = view_path + '?q=' + href;
+      // Update View.
+      var views_ajax_settings = Drupal.views.instances['views_dom_id:' + current_dom_id].element_settings;
+      views_ajax_settings.submit = views_settings;
+      views_ajax_settings.url = view_path + '?q=' + href;

-    Drupal.ajax(views_ajax_settings).execute();
+      Drupal.ajax(views_ajax_settings).execute();

-    // Update url.
-    window.historyInitiated = true;
-    window.history.pushState(null, document.title, href);
+      // Update url.
+      window.historyInitiated = true;
+      window.history.pushState(null, document.title, href);

-    // ToDo: Update views+facets with ajax on history back.
-    // For now we will reload the full page.
-    window.addEventListener("popstate", function (e) {
-      if (window.historyInitiated) {
-        window.location.reload();
-      }
-    });
+      // ToDo: Update views+facets with ajax on history back.
+      // For now we will reload the full page.
+      window.addEventListener("popstate", function (e) {
+        if (window.historyInitiated) {
+          window.location.reload();
+        }
+      });

-    // Refresh facets blocks.
-    updateFacetsBlocks(href);
-  }
+      // Refresh facets blocks.
+      this.updateFacetsBlocks(href);
+    },

-  // Helper function, updates facet blocks.
-  var updateFacetsBlocks = function (href) {
-    var settings = drupalSettings;
-    var facets_blocks = facetsBlocks();
+    // Helper function, updates facet blocks.
+    updateFacetsBlocks: function (href) {
+      var settings = drupalSettings;
+      var facets_blocks = this.facetsBlocks();

-    // Remove All Range Input Form Facet Blocks from being updated.
-    if(settings.facets && settings.facets.rangeInput) {
-      $.each(settings.facets.rangeInput, function (index, value){
-        delete facets_blocks[value.facetId];
-      });
-    }
+      // Remove All Range Input Form Facet Blocks from being updated.
+      if(settings.facets && settings.facets.rangeInput) {
+        $.each(settings.facets.rangeInput, function (index, value){
+          delete facets_blocks[value.facetId];
+        });
+      }

-    // Update facet blocks.
-    var facet_settings = {
-      url: Drupal.url('facets-block-ajax'),
-      submit: {
-        facet_link: href,
-        facets_blocks: facets_blocks
-      }
-    };
+      // Update facet blocks.
+      var facet_settings = {
+        url: Drupal.url('facets-block-ajax'),
+        submit: {
+          facet_link: href,
+          facets_blocks: facets_blocks
+        }
+      };

-    // Update facets summary block.
-    if (updateFacetsSummaryBlock()) {
-      facet_settings.submit.update_summary_block = true;
-      facet_settings.submit.facet_summary_plugin_id = $('[data-drupal-facets-summary-id=' + settings.facets_views_ajax.facets_summary_ajax.facets_summary_id + ']').data('drupal-facets-summary-plugin-id');
-      facet_settings.submit.facet_summary_wrapper_id = settings.facets_views_ajax.facets_summary_ajax.facets_summary_id;
-    }
+      // Update facets summary block.
+      if (this.updateFacetsSummaryBlock()) {
+        facet_settings.submit.update_summary_block = true;
+        facet_settings.submit.facet_summary_plugin_id = $('[data-drupal-facets-summary-id=' + settings.facets_views_ajax.facets_summary_ajax.facets_summary_id + ']').data('drupal-facets-summary-plugin-id');
+        facet_settings.submit.facet_summary_wrapper_id = settings.facets_views_ajax.facets_summary_ajax.facets_summary_id;
+      }

-    Drupal.ajax(facet_settings).execute();
-  };
+      Drupal.ajax(facet_settings).execute();
+    },

-  // Helper function to determine if we should update the summary block.
-  // Returns true or false.
-  var updateFacetsSummaryBlock = function () {
-    var settings = drupalSettings;
-    var update_summary = false;
+    // Helper function to determine if we should update the summary block.
+    // Returns true or false.
+    updateFacetsSummaryBlock: function () {
+      var settings = drupalSettings;
+      var update_summary = false;

-    if (settings.facets_views_ajax.facets_summary_ajax) {
-      update_summary = true;
-    }
+      if (settings.facets_views_ajax.facets_summary_ajax) {
+        update_summary = true;
+      }

-    return update_summary;
-  };
+      return update_summary;
+    },
+

-  // Helper function, return facet blocks.
-  var facetsBlocks = function () {
-    // Get all ajax facets blocks from the current page.
-    var facets_blocks = {};
+    // Helper function, return facet blocks.
+    facetsBlocks: function () {
+      // Get all ajax facets blocks from the current page.
+      var facets_blocks = {};

-    $('.block-facets-ajax').each(function (index) {
-      var block_id_start = 'js-facet-block-id-';
-      var block_id = $.map($(this).attr('class').split(' '), function (v, i) {
-        if (v.indexOf(block_id_start) > -1) {
-          return v.slice(block_id_start.length, v.length);
-        }
-      }).join();
-      var block_selector = '#' + $(this).attr('id');
-      facets_blocks[block_id] = block_selector;
-    });
+      $('.block-facets-ajax').each(function (index) {
+        var block_id_start = 'js-facet-block-id-';
+        var block_id = $.map($(this).attr('class').split(' '), function (v, i) {
+          if (v.indexOf(block_id_start) > -1) {
+            return v.slice(block_id_start.length, v.length);
+          }
+        }).join();
+        var block_selector = '#' + $(this).attr('id');
+        facets_blocks[block_id] = block_selector;
+      });

-    return facets_blocks;
+      return facets_blocks;
+    },
+
+    // Helper function to add exposed form data to facets url
+    addExposedFiltersToFacetsUrl: function (href, view_name, view_display_id) {
+      var $exposed_form = $('form#views-exposed-form-' + view_name.replace(/_/g, '-') + '-' + view_display_id.replace(/_/g, '-'));
+
+      var params = Drupal.Views.parseQueryString(href);
+
+      $.each($exposed_form.serializeArray(), function () {
+        params[this.name] = this.value;
+      });
+
+      return href.split('?')[0] + '?' + $.param(params);
+    }
   };

   /**
@@ -188,26 +203,13 @@
       });

       if (reload) {
-        href = addExposedFiltersToFacetsUrl(href, options.extraData.view_name, options.extraData.view_display_id);
-        updateFacetsBlocks(href);
+        href = this.addExposedFiltersToFacetsUrl(href, options.extraData.view_name, options.extraData.view_display_id);
+        this.updateFacetsBlocks(href);
       }
     }

     // Call the original Drupal method with the right context.
     beforeSend.apply(this, arguments);
-  }
-
-  // Helper function to add exposed form data to facets url
-  var addExposedFiltersToFacetsUrl = function (href, view_name, view_display_id) {
-    var $exposed_form = $('form#views-exposed-form-' + view_name.replace(/_/g, '-') + '-' + view_display_id.replace(/_/g, '-'));
-
-    var params = Drupal.Views.parseQueryString(href);
-
-    $.each($exposed_form.serializeArray(), function () {
-      params[this.name] = this.value;
-    });
-
-    return href.split('?')[0] + '?' + $.param(params);
   };

 })(jQuery, Drupal);

reinchek → created an issue.

Hi @ricovandevin,
thanks again for your constant activity on the module!

I would like to embed also this feature but i think that we need to work on a exphansive documentation. I've already created a doc page:
https://www.drupal.org/docs/extending-drupal/contributed-modules/contrib... →
But need to be completed of all module's features.

I will take some more time to review it and possibly integrate it into the branch, accompanying the change, however, with documentation.

Thanks,
let me know what do you thinks about.

Hi @marcoliver,
I fully agree with the proposed changes. Indeed, thank you for taking the trouble to look and fix those unclean portions of code!

@ricovandevin, thanks!
You can find all in this MR:
- https://git.drupalcode.org/project/migrate_source_graphql/-/merge_requests/10

The MR was created on top of current (2.x) dev branch. Inside you can find:
- PROPERTY_SEPARATOR logic
- possibility to use the '%' placeholder to get all array's items
- migrate_source_graphql_test sub-module which setup some Kernel tests (runs some basic migration cases): needs work.

Thanks again

Move to deleted

Added link to module's home page.

Hi @ricovandevin, i've merged your MR inside a new @dev branch (you can found it there).
Just to avoid to create too much minimum module's versions.
So, in this way we can collect more features in a single release :)

Thanks again. I'm extending your data_key PROPERTY_SEPARATOR logic introducing "%" (ex: `data_key: data/%/user` ) symbol that means: maps all data array's values getting for each the user object. For easy understanding, the code:

      const DATA_KEY_INDEXES = '%';
      // ...
      // ...
      $getIndexes = false;
      foreach ($propertyPath as $path) {
        if ($path === self::DATA_KEY_INDEXES) {
          $getIndexes = true;
          continue; // goto the next path item, with $getIndexes eq true.
        }
        if ($getIndexes) {
          $results = array_map(function ($item) use ($path) {
            return $item->{$path};
          }, array_values($results));
          $getIndexes = false;
        } else {
          $results = $results->{$path} ?? [];
        }
      }

so if we has a response from graphql server like this:

$data => array (
    0 =>
        (object) array(
            'user' =>
                (object) array(
                    'id' => '6',
                    'username' => 'Leopoldo_Corkery',
                    'name' => 'Mrs. Dennis Schulist',
                    'email' => 'Karley_Dach@jasper.info',
                ),
        ),
);

The results will be remapped as:

$results => array (
  0 => 
  (object) array(
     'id' => '6',
     'username' => 'Leopoldo_Corkery',
     'name' => 'Mrs. Dennis Schulist',
     'email' => 'Karley_Dach@jasper.info',
  ),
)

This could provide more flexibility to the module.
What do you think about?
Soon i will push the code in a new MR.

Hi @ricovandevin, thanks!
I reviewed and locally tested your MR. It works well!
I would like to take advantage of this upcoming release to also add some module's tests. At least the kernel-type one; If I have the time, however, I implement unit ones as well.

I try to do this tomorrow.
If I find that I cannot do this I will proceed with the merge of your MR.

Thanks ;)

New features inside v. 2.0.7 → .

@ricovandevin thanks you too.

Sorry because i avoided to paste other code changes (to adapt, correctly the buildQueryRecursive method) but the intent was just for show you a possible (i tried as you done and can say...) working solution.

About the "fields" property (as i wroted in "The migration" paragraph of the module's "documentation"):

The only difference between the GraphQL query and the YAML transposition is the mandatory property fields, the usefulness of which is solely a matter of choice for the developer.

So, we could do some semantics changes to the code to remove the priority "fields" property role. But, yes, i'm agreed with you, for now, let it out.

I also agree with data_key solution. 👌

I've tested and then merged the PR. I've also added a new doc section (you can read in module's home "From 2.0.6 version (issue-3381675)").

Thanks again ;)

Edit:
the above code breaks simple query parsing like as the following one:

  query:
    posts:
      fields:
        -
          data:
            - id
            - title
            - body

This code, instead, will work with anyway:

  private function buildQueryRecursive(string $queryName, array $query, ?Query &$qlQuery = null) {
    $arguments = NULL;
    $keys = array_keys($query);

    if (in_array('fields', $keys)) {
      // $queryName has "fields"
      $fields = $query['fields'];
      if (in_array('arguments', $keys)) {
        // $queryName has "arguments"
        $arguments = $query['arguments'];
      }

      $subQlQuery = $this->buildQuery($queryName, $fields, $arguments);
      // If $qlQuery is null means that we found 'fields' || 'arguments' at the first recursive operation...
      if (is_null($qlQuery)) {
        $qlQuery = $subQlQuery;
      } else {
        $qlQuery->setSelectionSet([$subQlQuery]);
      }
    } else {
      if (is_null($qlQuery)) {
        $qlQuery = new Query($queryName);
        $subQlQuery = $qlQuery;
      } else {
        $subQlQuery = (new Query($queryName));
        $qlQuery->setSelectionSet([$subQlQuery]);
      }

      return $this->buildQueryRecursive(array_key_first($query), reset($query), $subQlQuery);
    }
  }

Hi @ricovandevin,
the first thing (maybe simple) that comes to my mind is to embed some recursive logic to \Plugin\migrate\source\GraphQL::getGenerator() buildQuery() instead to call directly $this->buildQuery() we could call $this->buildQueryRecursive (but it would works only for first "arguments" match) that will builds the query recursively.
Somethings like this:

  private function buildQueryRecursive(string $queryName, array $query, ?Query &$qlQuery = null) {
    $arguments = NULL;
    $keys = array_keys($query);

    if (in_array('fields', $keys)) {
      // $queryName has "fields"
      $fields = $query['fields'];
      if (in_array('arguments', $keys)) {
        // $queryName has "arguments"
        $arguments = $query['arguments'];
      }

      $subQlQuery = $this->buildQuery($queryName, $fields, $arguments);
      $qlQuery->setSelectionSet([$subQlQuery]);
    } else {
      if (is_null($qlQuery)) {
        $qlQuery = new Query($queryName);
        $subQlQuery = $qlQuery;
      } else {
        $subQlQuery = (new Query($queryName));
        $qlQuery->setSelectionSet([$subQlQuery]);
      }

      return $this->buildQueryRec(array_key_first($query), reset($query), $subQlQuery);
    }
  }

So, using this code, i tried to launch your migration (just for obtains compiled query) and the query generated was as follows:

query {
  search {
    listings(
      locale: nl_BE
      metadata: { size: 400 }
      publication: { changedDateFrom: "2023-08-12T00:00:00.000Z" }
    ) {
      metadata {
        currentPage
        totalPages
      }
      listings {
        id
        details {
          vehicle {
            classification {
              make {
                formatted
              }
              model {
                formatted
              }
              modelVersionInput
            }
            numberOfDoors
          }
        }
      }
    }
  }
}

Maybe this could be the way, what do you think?

Message in a bottle:
I've created a new release 2.0.5 → that includes:

• #3381426 - String arguments should not be JSON encoded
• #3369542 📌 Automated Drupal 10 compatibility fixes Fixed - Automated Drupal 10 compatibility fixes

Message in a bottle:
I've created a new release 2.0.5 → that includes:

• #3381426 - String arguments should not be JSON encoded
• #3369542 📌 Automated Drupal 10 compatibility fixes Fixed - Automated Drupal 10 compatibility fixes

reinchek → made their first commit to this issue’s fork.

Hi @ricovandevin, i've tried the use case (argument as string and not object/array) and your code works good.
I'm merging right now the MR.

Thanks.

Hi @ricovandevin, thanks for your contribution!
Can you attach an example case just to better understand the issue?

Anyway, reviewing the MR's code i guess that, clarified the mechanism, i will merge as soon as possible.

Thanks to you! 💪

Hi @bander2,
I have performed further tests and can confirm that it is now working correctly.
I have released everything in the new version 2.0.4.
Thank you.

Hi @bander2, i've made a patch 3341694_string_arguments_not_possible.patch 3341694_string_arguments_not_possible.patch →
The patch includes:
- Fix for string arguments
- Multiple arguments now are supported
- Errors are now shown directly as drush output without -vvv needs.

I will include this patch for the 2.0.4 version on which i'm currently working.
Thank's, let me know how is it.

Hi @bander2, i've switched on your same fresh install, and yes. I've the same your error.
I'm working on a patch.
Which I will release as soon as possible.
Thank you.

Hi @bander2, i will check again and let you know soon.

Hi @bander2, i ran a quick check and fortunately there's no bug.
The problem lies in your configuration. The data_key property shold be specified under `source:` and not before. You can also see this from the documentation on the module's home page.
As you can see by using the following migration:

id: migrate_morty
label: Migrate Morty to Articles
migration_tags: null
source:
  plugin: graphql
  endpoint: https://rickandmortyapi.com/graphql
  data_key: results
  query:
    characters:
      arguments:
        filter:
          name: "Morty"
      fields:
        - results:
            - id
            - name
  ids:
    id:
      type: string
process:
  title: name

destination:
  plugin: 'entity:node'
  default_bundle: article

I got the desired result:

I close the issue, thank you.

Thank you @bander2 for the preliminary analysis you provided me.
I will try to understand it better and propose a solution as soon as possible.

+1 Vue.js
-1 Recat.js