🇳🇿New Zealand @xurizaemon

@willp24 it looks like the Migrate Orphans code on that site requires a change to account for the change in this module.

The change required is documented at https://www.drupal.org/node/3537201 →

This will require an issue in the Migrate Orphans project.

Oh no, Gitlab ratted me out! The "throwaway" MR on that issue repo can be ignored; I was using a recent issue as a test to identify where we tag the previous Git state on force push. Those last few updates from me (after the issue was fixed) are bogus and can be ignored :)

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

Ran into this myself, for me it reproduces when executing any migration that has data_parser=xml.

@heddn I think there is existing coverage for this, eg tests/src/Kernel/Plugin/migrate_plus/data_parser/XmlTest.php?

Testing locally, 6.0.x @ 46d05c256

www-data@da85fa35e6fa:/app$ phpunit -c /app/phpunit.xml web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/XmlTest.php
PHPUnit 9.6.23 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest
SEEEEEEE                                                            8 / 8 (100%)

Time: 00:09.863, Memory: 10.00 MB

There were 7 errors:

1) Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest::testCurrentUrl
TypeError: Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Xml::__construct(): Argument #4 ($file_system) must be of type Drupal\Core\File\FileSystemInterface, Drupal\migrate_plus\DataFetcherPluginManager given, called in /app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php on line 147

/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:105
/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:147
/app/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:21
/app/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:83
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:272
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:93
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

2) Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest::testReduceSingleValue
TypeError: Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Xml::__construct(): Argument #4 ($file_system) must be of type Drupal\Core\File\FileSystemInterface, Drupal\migrate_plus\DataFetcherPluginManager given, called in /app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php on line 147

/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:105
/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:147
/app/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:21
/app/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:83
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:272
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:112
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

3) Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest::testSingleValueWithAttributes
TypeError: Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Xml::__construct(): Argument #4 ($file_system) must be of type Drupal\Core\File\FileSystemInterface, Drupal\migrate_plus\DataFetcherPluginManager given, called in /app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php on line 147

/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:105
/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:147
/app/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:21
/app/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:83
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:272
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:138
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

4) Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest::testMultipleItems
TypeError: Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Xml::__construct(): Argument #4 ($file_system) must be of type Drupal\Core\File\FileSystemInterface, Drupal\migrate_plus\DataFetcherPluginManager given, called in /app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php on line 147

/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:105
/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:147
/app/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:21
/app/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:83
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:272
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:171
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

5) Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest::testPredicateMatch with data set "odd parity" ('/items/item[@parity="odd"]', array(array('Value 1'), array('Value 3')))
TypeError: Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Xml::__construct(): Argument #4 ($file_system) must be of type Drupal\Core\File\FileSystemInterface, Drupal\migrate_plus\DataFetcherPluginManager given, called in /app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php on line 147

/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:105
/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:147
/app/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:21
/app/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:83
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:272
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:206
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

6) Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest::testPredicateMatch with data set "even parity" ('/items/item[@parity="even"]', array(array('Value 2')))
TypeError: Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Xml::__construct(): Argument #4 ($file_system) must be of type Drupal\Core\File\FileSystemInterface, Drupal\migrate_plus\DataFetcherPluginManager given, called in /app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php on line 147

/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:105
/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:147
/app/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:21
/app/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:83
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:272
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:206
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

7) Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest::testPredicateMatch with data set "special attribute" ('/items/item[condition[@specia...rue"]]', array(array('Special value')))
TypeError: Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Xml::__construct(): Argument #4 ($file_system) must be of type Drupal\Core\File\FileSystemInterface, Drupal\migrate_plus\DataFetcherPluginManager given, called in /app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php on line 147

/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:105
/app/web/modules/contrib/migrate_plus/src/Plugin/migrate_plus/data_parser/Xml.php:147
/app/web/core/lib/Drupal/Core/Plugin/Factory/ContainerFactory.php:21
/app/web/core/lib/Drupal/Component/Plugin/PluginManagerBase.php:83
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:272
/app/web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/BaseXml.php:206
/app/vendor/phpunit/phpunit/src/Framework/TestResult.php:729

ERRORS!
Tests: 8, Assertions: 0, Errors: 7, Skipped: 1.

With !133 @ a9f469bf

www-data@da85fa35e6fa:/app$ phpunit -c /app/phpunit.xml web/modules/contrib/migrate_plus/tests/src/Kernel/Plugin/migrate_plus/data_parser/XmlTest.php
PHPUnit 9.6.23 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser\XmlTest
S.......                                                            8 / 8 (100%)

Time: 00:09.940, Memory: 10.00 MB

OK, but incomplete, skipped, or risky tests!
Tests: 8, Assertions: 10, Skipped: 1.

Unsilenced deprecation notices (7)

  7x: Creation of dynamic property Drupal\migrate_plus\Plugin\migrate_plus\data_parser\Xml::$fileSystem is deprecated
    3x in XmlTest::testPredicateMatch from Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser
    1x in XmlTest::testCurrentUrl from Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser
    1x in XmlTest::testReduceSingleValue from Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser
    1x in XmlTest::testSingleValueWithAttributes from Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser
    1x in XmlTest::testMultipleItems from Drupal\Tests\migrate_plus\Kernel\Plugin\migrate_plus\data_parser

Confirmed, MR resolves recently introduced fatal error when using this module on PHP 8.1. Appreciate the patch!

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

xurizaemon → created an issue.

Patch fixes the issue.

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

xurizaemon → created an issue.

Good point Michael. I feel I'm confused now from juggling changes (and git is giving me weird results / not showing changes from upstream?), so I'm going to leave this for tonight. Thanks!

Following up on my own question re branch for 6.1.x - FYI @heddn, the 6.1.0 and 6.1.1 releases are from the 6.0.x branch.

(Not meaning to nitpick, just alerting to something which was confusing me when I tried git operations relating to that branch.)

I believe this change also caused breakage on sites using Migrate Source UI, as the 6.0.x function signature is used there. Opened 🐛 Migrate Tools v6.1.x compatibility Active .

Quick workaround can be to downgrade to 6.0.x for sites using Migrate Source UI? That worked for 6.0.5 for me.

I see this MR (and issue) currently targets 6.0.x, but introduces a deprecation notice and fixes handling for 6.1.x - should the MR be targeting 6.1.x or 6.0.x?

Suggest that we update "Proposed resolution" so that the intent of this change is clearer?

Thanks for working on this!

xurizaemon → created an issue.

Linking 🐛 New parameters introduced in MigrateExecutable class constructor Active (likely intended link for last comment)

It was a dupe of 🐛 Declare PHP requirement >= 7.4 Closed: outdated anyway :)

Closing as a duplicate of 📌 Automated Drupal 11 compatibility fixes for jsonlog Active because the other has RTBC already

xurizaemon → created an issue.

Quick MR (for today) is a start point, not considered a finished solution :)

I expect tracing is a performance critical path, so we don't want to add too much code anyway?

Interested to hear thoughts!

xurizaemon → created an issue.

removing assignment from 2019

We observed issue with PDFs embedded on the "main" (www.example.org) URL when viewed from an admin-specific URL (eg admin.example.org, if you were viewing from a separate admin URL for use cases such as CloudFlare). The error message displayed was "Message: file origin does not match viewer's". I've added this to the issue description as Case 2.

Patch 10 from 🐛 Behind HTTPS Load Balancer, Renders HTTP load of JS to Browser Needs work makes the viewer JS a relative URL, removing the hostname. For some folks running into this issue, that may be enough to solve the display problem.

However, it seems like that patch won't address Case 1 in this issue description (remote PDF, via remote stream wrapper, on a different host to the viewed page).

Is this related to 🐛 Render using theme input and select instead of lists with links for checkboxes and dropdown Needs review changes?

The above comment was made via MR !297, and when viewed on the MR shows more context - that the specific change is the removal of the secondary selector , .js-facets-links from line 19 of js/checkbox-widget.js.

Rebased

Improved positioning at least via https://git.drupalcode.org/project/migrate_visualize/-/merge_requests/17

xurizaemon → created an issue.

The change record didn't clearly communicate the required fix here for me, and I overthought it.

Solution (I now believe!) is to add langcode: en to the default configuration yaml files in the module's config/install folder.

benjifisher → credited xurizaemon → .

xurizaemon → created an issue.

I haven't forgotten about this! I've been making some progress over on 📌 Connect pseudofield sources to use in pipeline Active which is adjacent.

I think if we pull the BPMN JS libraries in directly (as is done for Mermaid currently), we can then output the diagram in BPMN notation, and I don't expect that to take much refactor as we are adding a third output format (GraphViz, Mermaid, BPMN) to an implementation which currently delivers two.

I expect this would trigger renaming the `migrate_visualize/js` JS to `migrate_visualize/mermaid` and adding a similar JS to initialize BPMN.

That solves the "what might ECK / BPMN do" issue. If such an integration module for BPMN does appear, we can always switch to that later. For now, CDN inclusion is fine by me; this is a dev module.

Pseudofield edges are working now, but the chart isn't yet to my liking.

The following module-generated Mermaid works on mermaid.live, but doesn't in the module's render of the same output. I'm having trouble activating the Elk layout renderer.

---
# Generated with Migrate Visualize
# https://drupal.org/project/migrate_visualize
---
flowchart LR

subgraph Source[fa:fa-cloud-download Source]
  source:meta:plugin["plugin: url"]
  source:field:id["id"]
  source:field:title["title"]
  source:field:link["link"]
  source:field:updated["updated"]
  source:field:data["data"]
  source:field:content["content"]
  source:field:constants_subtype["constants/subtype"]
  source:field:__path_parts_3["@_path_parts/3"]
  source:field:__path_parts_4["@_path_parts/4"]
  source:field:__term_names["@_term_names"]
end

subgraph Destination[fa:fa-cloud-upload Destination]
  destination:meta:plugin["plugin: entity:node"]
  destination:field:created["created"]
  destination:field:changed["changed"]
  destination:field:title["title"]
  destination:field:type["type"]
  destination:field:field_remote_id["field_remote_id"]
  destination:field:field_data["field_data"]
  destination:field:field_subtype["field_subtype"]
  destination:field:_path_parts["_path_parts"]
  destination:field:_term_names["_term_names"]
  destination:field:field_tags["field_tags"]
  destination:field:field_url["field_url"]
end

subgraph Process[fa:fa-cloud-upload Process]
  process:field:created["created"]
  process:field:created:1["format_date"]
  process:field:changed["changed"]
  process:field:changed:1["format_date"]
  process:field:title["title"]
  process:field:title:0["get"]
  process:field:type["type"]
  process:field:type:0["default_value"]
  process:field:field_remote_id["field_remote_id"]
  process:field:field_remote_id:0["get"]
  process:field:field_data["field_data"]
  process:field:field_data:1["callback: json_encode"]
  process:field:field_subtype["field_subtype"]
  process:field:field_subtype:1["entity_generate"]
  process:field:_path_parts["_path_parts"]
  process:field:_path_parts:1["skip_on_empty"]
  process:field:_path_parts:2["explode"]
  process:field:_term_names["_term_names"]
  process:field:_term_names:0["get"]
  process:field:_term_names:1["callback: array_filter"]
  process:field:_term_names:2["callback: array_unique"]
  process:field:field_tags["field_tags"]
  process:field:field_tags:1["entity_generate"]
  process:field:field_url["field_url"]
  process:field:field_url:0["get"]
end

source:field:updated --> process:field:created:1
process:field:created:1 --> process:field:created
process:field:created --> destination:field:created
source:field:updated --> process:field:changed:1
process:field:changed:1 --> process:field:changed
process:field:changed --> destination:field:changed
source:field:title --> process:field:title:0
process:field:title:0 --> process:field:title
process:field:title --> destination:field:title
process:field:type:0 --> process:field:type
process:field:type --> destination:field:type
source:field:id --> process:field:field_remote_id:0
process:field:field_remote_id:0 --> process:field:field_remote_id
process:field:field_remote_id --> destination:field:field_remote_id
source:field:data --> process:field:field_data:1
process:field:field_data:1 --> process:field:field_data
process:field:field_data --> destination:field:field_data
source:field:constants_subtype --> process:field:field_subtype:1
process:field:field_subtype:1 --> process:field:field_subtype
process:field:field_subtype --> destination:field:field_subtype
source:field:link --> process:field:_path_parts:1
process:field:_path_parts:1 --> process:field:_path_parts:2
process:field:_path_parts:2 --> process:field:_path_parts
process:field:_path_parts --> destination:field:_path_parts
source:field:__path_parts_3 --> process:field:_term_names:0
source:field:__path_parts_4 --> process:field:_term_names:0
process:field:_term_names:0 --> process:field:_term_names:1
process:field:_term_names:1 --> process:field:_term_names:2
process:field:_term_names:2 --> process:field:_term_names
process:field:_term_names --> destination:field:_term_names
source:field:__term_names --> process:field:field_tags:1
process:field:field_tags:1 --> process:field:field_tags
process:field:field_tags --> destination:field:field_tags
source:field:link --> process:field:field_url:0
process:field:field_url:0 --> process:field:field_url
process:field:field_url --> destination:field:field_url

destination:field:_term_names --> source:field:__term_names

Mermaid.live rendering with Elk:

Migrate Visualize rendering without Elk:

Looks like I've broken the "copy source" JS with WIP also.

I intended to create the issue relating to the organisation from the shared account which owns node 1802754, but I was not able to create the node as that user, nor add a comment as that user giving approval. (Shared accounts have limited access to the issue queue.)

You may check with that shared account's email to verify, before making the change, to confirm this is OK.

xurizaemon → created an issue.

xurizaemon → created an issue.

Thanks so much! Your persistent effort is much appreciated :)

fubarhouse → credited xurizaemon → .

xurizaemon → created an issue. See original summary → .

fubarhouse → credited xurizaemon → .

jct321 → credited xurizaemon → .

xurizaemon → created an issue.

Thanks heaps Khaled! Nice fix.

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

Merged, setting back to Active for future updates.

Another user has said in Slack today that "Google Search Console reports an indexing issue because it does not support nested sitemap indexes". I've directed them towards this issue. Perhaps Google has recently changed behaviour; hopefully someone with a GSC account and a site big enough to hit this is able to provide more detail.

Hi @cosmicdreams! Apologies for missing this issue posted here until you raised the topic in Slack the other day :)

BPMN does look cool, and I am interested to have an explore of making that work within this module. (I am happy to support progress on that within this module, and I am also happy for that progress to happen in some other module. If this is the right place for it, great!

There is another issue raised at ✨ [Spike] Change to BPMN.iO Active - eventually one will be closed duplicate, but no rush. For the most part this module has been unmaintained the last few years ... I had the opportunity of a sprint at DrupalSouth today, so I got things a bit tidied up, intending to support BPMN output as a third option.

I don't (yet) believe we need to replace GraphViz? I implemented both GraphViz and MermaidJS in round one of this module, and it should function OK without GraphViz, falling back to MermaidJS only if there's no server support. So I'm changing the title to "Add BPMN output format" because if we add BPMN we will have three options for display.

At this stage, I believe that missing GraphViz should be handled in the UI of this module. If you're aware of an issue that occurs when the server lacks GraphViz support, please open a specific bug report for that, and I'm happy to look into it. Please include steps to reproduce and observed behaviour when doing so. See Drupal\migrate_visualize\Form::buildForm() source for more.

So far I like the idea of providing for BPMN, and I am glad that the existing (1.x) version provides for multiple outputs.

Even if it doesn't conflict with BPMN, I don't think it's the role of this (developer-targeted) module to depend on ECA. Currently the BPMN.io module depends on ECA (which was odd to see, but it appears the BPMN.io module functionality adds BPMN to ECA). Noting also that there's BPMN.io and bpmn-js. My concern here is that if Migrate Visualize depends on ECA, adding Migrate Visualize as part of migration development and review can have architectural impacts on the site in question.

I see there is also an existing issue at ✨ Consider replacing GraphViz Active which proposed investigating BPMN. (I'll update the title of that issue so it mentions BPMN!)

I plan to take a look at whether we can introduce BPMN's JS in a similar manner to how we introduce MermaidJS. I'd use a CDN for this if possible. There's an assumption here that the use of BPMN is similar to MermaidJS and GraphViz, ie that we're visualizing the migration, rather than implementing a visual configuration editor (which would be outside the scope of this module).

xurizaemon → created an issue.

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

xurizaemon → changed the visibility of the branch 2.0.x to active.

xurizaemon → changed the visibility of the branch 2.0.x to hidden.

xurizaemon → created an issue.

Thanks @restoismile for input. It's clearer for other collaborators to use the existing merge request rather than add a patch which I then need to compare to the MR. Drupal's Gitlab process allows you to add your own changes, and you'll then receive credit more properly too.

- https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... →

I believe the additional change you're proposing (restoring!) is in src/MenuBasedBreadcrumbBuilder.php:

-+        $raw_parameters = (array) $route_match->getRawParameters();
++        $raw_parameters = (array) $route_match->getRawParameters()->all();

I suspect the (array) there is redundant and can be removed if we use ->all()?

Would you please take another look and propose the change via the MR? That will help this issue to progress more easily.

If the issue already has a MR open, that should be preferred for collaboration.