slight brush up on wordpress migrate section.
cool thank you ryumaou. i'm going to block in late Jan or early Feb another sprint on this module for a few days so anything else you can nail down helps a ton.
Regardless of the 'nice to have' like xml check, unit tests and certain code standard flags, I think we need to be assured that the migration is working correctly for most use cases.
Forgive my silly question but is "document" like PDF files and things of that nature? In other words if someone has added a plugin that allows the PDF uploads in the media library, this would cover importing those. I'm a little more familiar with the built in audio content type.
regarding checking the xml file see #3248042: Add XML Lint to check the WordPress exports → xml lint could be added to composer but i figured that would intensify requirements. as well as ✨ WP import xml entities & security considerations Active for a note on other considerations.
regarding testing media types I suppose this would require a new test site altogether with the types of assets in question.
if you think we can subdivide out any portion of the patch as well (in other words if you think some standalone portion would actually be RTBC as it is), let me know. Thanks for all this!
excellent stuff abarrio. do you think it is RTBC yet? I don't have a test xml with audio attachment to test.
Thanks so much abarrio for rebasing. Any ideas what we can do to fix the "Access to an undefined property" stuff flagged above? That seems like the biggest flagged snag currently.
Maybe this needs to be added - https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Plugi...
It could be that the title of the tags are longer than the taxonomy entity title field permits? I think this would work for trimming the tag name.
Note we do not have substr as a process here: https://git.drupalcode.org/project/wordpress_migrate/-/blob/8.x-3.x/migr...
the max taxonomy tag title length might be 255 chars https://www.drupal.org/forum/support/post-installation/2009-10-22/overri... →
could help determine if this is the case: https://www.drupal.org/project/title_length →
programmatic checker idea here: https://drupal.stackexchange.com/questions/107758/how-do-i-find-the-max-...
@ryumaou yes if you can test it it might help with 🐛 Tags and categories not migrating Active as well. Sorry I haven't had more time to hack on this one lately (or yours). I also wonder if the issue is similar to #2955482: Pingbacks are imported as comments - nodes with random names are created → which puzzled me from way back.
Regarding catching exceptions which I think is generally a good idea and best practice. Just adding some links here to refer to later, re best practices and how other modules are doing it, what is around in the migrate subsystem and so on.
I had some old WIP patches on a couple issues in the project to add these - linked below. The API for exceptions and watchdog logging changed after 10.1.x so it is good we are switching to 10.2.x minimum.
- On branches 9.x to 11.x: https://api.drupal.org/api/drupal/namespace/Drupal%21migrate%21Plugin%21...
- badplugindefinitionexception: https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Plugi...
- https://api.drupal.org/api/drupal/core%21modules%21migrate%21src%21Migra...
- https://api.drupal.org/api/drupal/namespace/Drupal%21migrate%21Exception... includes EntityValidationException and RequirementsException
- watchdog_exception was deprecated https://www.drupal.org/node/2932520 → . A number of classes that were using watchdog_exception() now explicitly inject a logger to be able to pass to \Drupal\Core\Utility\Error::logException()
- example implementation: https://git.drupalcode.org/project/webform/-/merge_requests/406/diffs
- https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Utility%2...
Code for adding log entries should now be like:
<?php
use \Drupal\Core\Utility\Error;
$logger = \Drupal::logger('modulename');
Error::logException($logger, $exception);
?>- Exceptions also discussed in: 💬 XML (XHR) invalid characters issue Postponed: needs info within wordpress_migrate. another test I made WIP: 🐛 Duplicative generated migration group causes uncaught error at review stage EntityStorageException Needs work . using exceptions with the pluggable interface - some ideas: ✨ Support contrib wordpress data / custom data | Yoast SEO Needs review . duplicate user exception: #3123393: Users are duplicated (Integrity constraint violation) Duplicate entry → . some notes here similar: 🐛 Better exception handling for if username does not exist Active
- Nice patch for showing lines of errors: https://www.drupal.org/files/issues/2020-07-30/2969551-25.patch → - via #2969551: Migrate messages from caught exceptions need file and line details →
- Way to do badplugindefinitionexception: #2908282: Throw exception for source plugins without a source_module property → see patch https://www.drupal.org/files/issues/2908282-83.patch →
- Main doc page about exceptions in coding standards: https://www.drupal.org/node/608166 →
- Old forum thread, adding this because it uses "throwable" as a catchall - https://www.drupal.org/forum/support/module-development-and-code-questio... →
- Example of how it was patched for 10.1+ this year in webform: 📌 [Drupal:10.1.x] watchdog_exception() is deprecated in drupal:10.1.0 and is removed from drupal:11.0.0 Fixed
Thanks all!
Thanks for this!!
thank you!
hongpong → made their first commit to this issue’s fork.
hongpong → made their first commit to this issue’s fork.
thank you for checking on this!
Thanks for filing this. Yeah let's get the xml posted to test against .. if it doesn't allow the upload I can be emailed as well ` hongpong @ hongpong.com ` - if you like I can add it to that outside Repo for test XML files I made as well. Thanks!
Might want to check this out as well for debugging - https://www.drupal.org/project/migrate_devel →
Thanks for these insights! My idea for a plugin API version is not based on embedded data in the WXR files. It is just a way to indicate if we have internally changed the processing within this module of posts, authors, comments significantly. And in turn it would also let the custom developers write another plugin manager which could accept different API version numbers for their own custom imports - this would let them run separate 'lanes' of imports within this open ended manager framework that we can implement.
thanks baltowen very good stuff. Regarding the equals signs, there are some silly rules in phpcs that already exist. From today's HEAD (not your patch). I
Drupal.Formatting.MultipleStatementAlignment.NotSame
Drupal.Formatting.MultipleStatementAlignment.Incorrect
197 | ERROR | [x] Equals sign not aligned correctly; expected 1 space but
| | found 16 spaces
| | (Drupal.Formatting.MultipleStatementAlignment.Incorrect)
261 | ERROR | [x] Equals sign not aligned with surrounding assignments;
| | expected 21 spaces but found 1 space
| | (Drupal.Formatting.MultipleStatementAlignment.NotSame)
267 | ERROR | [x] Equals sign not aligned with surrounding assignments;
| | expected 27 spaces but found 1 space
| | (Drupal.Formatting.MultipleStatementAlignment.NotSame)
I think the alignments are very silly for them to impose on us. phpcs.xml.dist (or maybe without .dist) could perhaps override this in Drupal flavored GitlabCI within this project?
Using statements like https://git.drupalcode.org/project/drupal/blob/8.8.x/core/phpcs.xml.dist
<rule ref="Drupal.Arrays.Array">
<!-- Sniff for these errors: CommaLastItem -->
<exclude name="Drupal.Arrays.Array.ArrayClosingIndentation"/>
<exclude name="Drupal.Arrays.Array.ArrayIndentation"/>
<exclude name="Drupal.Arrays.Array.LongLineDeclaration"/>
</rule>
I think looking ahead we want to make sure these are caught not just in the UI wizard but also in the migration generator which could be later triggered from drush (after drush 12 is implemented!). In other words malformed elements could be coming in that UI measures cannot prevent. This was flagged by mikeryan on one of the oldest issues. #2742283: Validate configuration in WordPressMigrationGenerator::createMigrations → ..
to be fair this issue could be postponed until we have drush working again. ✨ Implement Drush 12+ on wordpress_migrate Active . I think adding exceptions, like in the WIP here, is still a good idea when entities are being stored and similar.
wordpress_migrate Alpha7 is released, thank you everyone! Release note: https://www.drupal.org/project/wordpress_migrate/releases/8.x-3.0-alpha7 →
I had a couple ideas to add in the mix. Hopefully this is not excessive. I think if we can future proof this, by adding a couple parameters to the plugin loader, and WordpressMigrationExtensionPluginInterface.php and / or src/Plugin/WordpressMigrationExtensionPluginBase.php. Additional possible arguments for extension plugins:
$api_version (int), $extra_args (array), $suggested_weight (int). The purpose of $extra_args would be a catch all that could provide special directives outside of other configuration objects, for now just an empty array. (this is kind of common in WordPress already.) $suggested_weight could be used to direct the sorting of these plugins in their execution order, which right now seems to be arbitrary. (e.g. higher weight executes last, or whatever. If the weights match then sort by name?)
In the properties of WordpressMigrationExtensionPluginManager.php it could be added, static int for min_api_version, max_api_version and a static function, isApiVersionSupported(int) that returns true if the int is within the supported range. (This would let other modules poll to make sure that they are compatible.)
Thus if the attempted plugin loading is coded with a higher API number then this WordpressMigrationExtensionPluginManager will not load it, or it can throw an exception if it tries to execute. This could also help people who are making custom migrations privately, they could set up their own API number ranges for internal use. (This seems like a cleaner solution than using the Semver of our module to enforce this kind of thing).
Second idea is to add a preprocessor plugin manager that works similarly to this plugin manager. It could be triggered at the end of SourceSelectForm.php: submitForm, and similarly could load the entire uploaded file to a "WordpressMigrationPreprocessorExtensionPlugin" (perhaps selected from another manager wizard page) before the process moves on to AuthorForm.php . I figure this would be the best way to add an XML linter or other security measures ( #3248042: Add XML Lint to check the WordPress exports → ). It could look for encoded XML entities or other oddball or security issues. The plugin manager could also have static int properties for min_preprocessor_api_version, and max_preprocessor_api_version, and a static bool function, isPreprocessorApiVersionSupported(int).
The point with the API versions is that the execution of the plugins processes could change in the future and this would give the reliability of linking an API number to known implementations without locking us into it, and with a minimum API number we can safely jettison old APIs years down the road. I feel like the people making custom extensions for private projects would also benefit from some ideas like this. Stop me if this seems unnecessary or cumbersome but I think it could work well for everyone.
hongpong → created an issue.
hongpong → created an issue.
hongpong → created an issue.
added note re getting pluggable nature refactor on this ✨ Support contrib wordpress data / custom data | Yoast SEO Needs review
thanks everyone for collaborating on this thing and particularly ressa for revamping the documentation entirely. it was a huge volunteer effort. if nobody has any errors to flag i should tag the alpha7 release today or tomorrow . I have a decent release note already drafted. Excellent work everyone and thank you!!!
change to describe how sandboxes are superseded and lead them to the new docs
add links to "using gitlab to contribute to drupal" which has additional info
fixing these links that were broken and clarifying some things
i may have messed up the commits in the MR just now - however the MR should be up to date with 8.x-3.x HEAD and your patch as well .. i think we have all our pre alpha7 commits in now? thanks for all this.
user duplicates and migration group duplicates have been fixed in other issues so this issue is shrinking away...
postponing as we need reproducible XML data to test against.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it. There is now a WordPress SQL module we have it linked on the front of this module.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Closing this as we move on from 7.x. Feel free to open again if anyone works on it.
Some upstream changes in wordpress_migrate_ui/src/Form/ImageSelectForm.php - the merge conflict is only there, basically got similar modernization by other folks. I'm not actively going to tangle with this one more for now, but if someone else wants to hammer away on it, go ahead.
Okay I have been able to finish the cherrypicking of several items from this issue into 8.x-3.x, and then rebased the MR to match the new 8.x-3.x, which has scaled down the size of the rest of the patch at least a good ways. Thanks for bearing with me, I won't do any more cherry picking from here. Excellent work everyone!!
Something in the IDE flagged the attempt to add TypedDataManager dependency injection so I held that back. Now going to update other MRs. Thanks everyone!!
Thank you everyone, another nasty bug squashed. baltowen, dinarcon, lobodakyrylo, uridrupal, splash112!!
I was able to get one 'updating existing users is not allowed' message to print by running the loremipsum XML twice. it ignores all of them but it only gives one message, in my case at
/admin/structure/migrate/manage/my_wordpress7lorem/migrations/my_7loremwordpress_authors/messages
under the 'messages' tab for authors.
ollie.medhurst Informational my_7loremwordpress_authors:_skip_existing_user: Updating existing users is not allowed.
I think we can commit this for now and make note that, the authors skipped, do not each generate a message.
Ok that worked great, thank you baltowen, it is committed on 8.x-3.x. I might go back to add more coverage for exceptions but for now let's just keep this simple.
Is there a "Get push access" button on this page for you? that's what they tell me on slack rn.
Anyone can click the "Get push access" button on the issue to get push access to all branches in the issue fork. (its kind-of covered as a bullet on https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... → )
( doesn't say → ). I will pull this in though.
We are not going to roll this into the Alpha7 release but hopefully we will be close for Alpha8. Thank you for going deep into it!!
Thanks baltowen that definitely helps! It would be better to prevent this from even happening but I think it would still improve matters to have it in there. If there is a quick way to add migration group naming form field validator function, then the submission would be blocked from proceeding
It is possible in the future that the wizard would not be used, (eg drush) so its good to have additional checks added.
hongpong → created an issue.
i figured a fancy approach for functional test: would be to pull at least one of the XML from my test repo, https://gitlab.com/HongPong/wordpress-test-imports , download that in the test runner, try to move through the form stages, then run the imports, then do some kind of spot checking that it imported correctly.
Truthfully I am not an advanced Drupal CI developer, but I thought I could use steps like this to pull this off https://gist.github.com/edutrul/9d04d7742545dbedd1a36f7b17632b7a
https://git.drupalcode.org/project/dtt/-/commit/964cd7009fc9f0fa2a994505... - altho that is selenium i am not sure if functional test file upload fields on forms work this way
updated with latest issues
hongpong → created an issue.
We are getting there, hoping to have this rolled in after the next tagged release. Thank you everyone for continuing to test and refine this.
Thank you baltowen I was also wondering about that one and not sure if it is a string but, if it works it is probably fine.
I might cherry pick some of the smaller changes out of this patch to apply before the larger patch on D11 so we can get some of it into the next tagged release this week. I will also try and get some more testing. We are nearly there folks!
This kind of thing is still popping up periodically. One of the migrate modules tends to print these a little often. one of the yml files may be triggering this but it is not obvious which one.
resolved on 🐛 Undefined array key "migration" in findMigrationDependencies() Active
not going this direction with the module i believe.
I attached a bunch more XML files to the test repo. https://gitlab.com/HongPong/wordpress-test-imports
includes a11y, Japanese, Gutenberg, and some very light Yoast SEO content.
it is set now!
I made an XML file with a little bit of yoast SEO data on a few posts in it here: https://gitlab.com/HongPong/wordpress-test-imports/-/blob/master/wp-impo...
Please feel free to use that. Is from today's versions of WordPress and Yoast SEO.
Having trouble getting the git push to the MR to work but anyway here is the patch with a few adjustments noted above. Awesome stuff thank you!
Oh this is really looking look great. Thank you for this elegant workaround for D9. I think we can just tag the final D9 release before applying your patch. I can test it on D10 and D9 tomorrow as well.
Hoping we can get a fix on duplicate users ( #3123393: Users are duplicated (Integrity constraint violation) Duplicate entry → ) before tagging the next release.
on SourceSelectForm.php . the IDE doesn't like !empty but I think 'isset' is a little better.
if (isset($constraint_definitions['FileExtension'])) {
Also I was able to get a TypedDataManager in the __construct() here to replace the \Drupal notice in getFileValidators.
on ImportWizard.php $tempstore_id needs a parameter type.
Thank you for working through this as I reshuffle a bunch of stuff and try to get nearby code fixes in there. I think we can freeze for a day or 2 unless someone has a quick fix for broken users. Really appreciate all this work.
Thank you dinarcon ressa baltowen. I would say we could open another issue about refactoring the migration entities - whether that is a deriver, placeholder values, base migrations or similar. However not sure how it should be titled.
I swear, I will get the merge request interface correctly for the commit messages someday (facepalm)!
I would note that I still get this issue, but I think it is not because of this issue: #3123393: Users are duplicated (Integrity constraint violation) Duplicate entry → .
still getting stuff like this
SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry 'themereviewteam-en' for key 'user__name': INSERT INTO "users_field_data" ("uid", "langcode", "preferred_langcode", "preferred_admin_langcode", "name", "pass", "mail", "timezone", "status", "created", "changed", "access", "login", "init", "default_langcode") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3, :db_insert_placeholder_4, :db_insert_placeholder_5, :db_insert_placeholder_6, :db_insert_placeholder_7, :db_insert_placeholder_8, :db_insert_placeholder_9, :db_insert_placeholder_10, :db_insert_placeholder_11, :db_insert_placeholder_12, :db_insert_placeholder_13, :db_insert_placeholder_14); Array ( [:db_insert_placeholder_0] => 6 [:db_insert_placeholder_1] => en [:db_insert_placeholder_2] => en [:db_insert_placeholder_3] => [:db_insert_placeholder_4] => themereviewteam [:db_insert_placeholder_5] => [:db_insert_placeholder_6] => themereviewteam@gmail.com [:db_insert_placeholder_7] => America/New_York [:db_insert_placeholder_8] => 1 [:db_insert_placeholder_9] => 1732512346 [:db_insert_placeholder_10] => 1732512346 [:db_insert_placeholder_11] => 0 [:db_insert_placeholder_12] => 0 [:db_insert_placeholder_13] => [:db_insert_placeholder_14] => 1 ) (/var/www/html/web/core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php:817)
The readme is updated significantly (including all contributors to date added), the pathauto / path alias function is extracted to a new function processUrlPathAliases. Thanks everyone.