🇨🇦Canada @dylan donkersgoed

I also encountered a problem related to this and codebymikey's solution fixed it for me. I don't think a process plugin would've worked for my use case.

Essentially I have several migrations that would conditionally set the node or user ID (e.g. because the node might already have translations or revisions which I think is a fairly common use case).

So, e.g., I most recently ran into the issue with a user migration that looked like this:

uuid: 337cbd24-5039-4a30-b646-1c2032f68163
langcode: en
status: true
dependencies: {  }
id: upgrade_d7_user
class: Drupal\user\Plugin\migrate\User
field_plugin_method: null
cck_plugin_method: null
migration_tags:
  - 'Drupal 7'
  - Content
migration_group: migrate_drupal_7
label: 'User accounts'
source:
  plugin: hh_migration_d7_user_with_group
process:
  # A subset of users already exist, target their UIDs if they are found.
  uid:
    - plugin: skip_on_value
      source: uid
      method: process
      not_equals: true
      value:
        - 1
        - 14347
        - 14927
        - 16473
        - 18850
    - plugin: static_map
      map:
        1: 1
        14347: 6
        14927: 4
        16473: 5
        18850: 3
      default_value: null
  name:
    -
      plugin: get
      source: name
    -
      plugin: skip_on_value
      method: row
      value:
  pass:
    -
      plugin: get
      source: pass
  mail:
    -
      plugin: get
      source: mail
  created:
    -
      plugin: get
      source: created
  access:
    -
      plugin: get
      source: access
  login:
    -
      plugin: get
      source: login
  status:
    -
      plugin: get
      source: status
  timezone:
    -
      plugin: get
      source: timezone
  langcode:
    -
      plugin: user_langcode
      source: entity_language
      fallback_to_site_default: false
  preferred_langcode:
    -
      plugin: user_langcode
      source: language
      fallback_to_site_default: true
  preferred_admin_langcode:
    -
      plugin: user_langcode
      source: language
      fallback_to_site_default: true
  init:
    -
      plugin: get
      source: init
  roles:
    - plugin: static_map
      source: roles
      map:
        authenticated user: employee
        administrator: administrator
      default_value: employee
  user_picture:
    -
      plugin: default_value
      source: picture
      default_value: null
    -
      plugin: migration_lookup
      migration: upgrade_d7_file
  groups:
    - plugin: static_map
      source: og_gid
      map:
        6093: 3
        18179: 4
      default_value: 1
    - plugin: default_value
      default_value: 1
  field_about_me:
    -
      plugin: get
      source: field_about_me
  field_contact_me:
    -
      plugin: get
      source: field_contact_me
  field_first_name:
    -
      plugin: get
      source: field_first_name
  field_last_name:
    -
      plugin: get
      source: field_last_name
  field_photo:
    -
      plugin: sub_process
      source: field_photo
      process:
        target_id:
          plugin: migration_lookup
          migration: upgrade_d7_file
          source: fid
        alt: alt
        title: title
        width: width
        height: height
destination:
  plugin: 'entity:user'
migration_dependencies:
  optional:
    - upgrade_d7_file

The important section is the uid bit. It will assign the UID for certain users only if the user already exists in Drupal so the migration will map it to the existing user. This is needed to make sure all the user relationships are preserved in revision history etc. even for the users that were already manually created. I've run into the problem in several other places though for migrations that handle node revisions/translations.

The problem was when I ran a migration with the --update flag I would get numerous errors like:

[error] SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '775cbb48-ed5d-4fb0-aa15-09b315905fe7' for key 'user_field__uuid__value': INSERT INTO "users" ("uuid", "langcode") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1); Array
(
[:db_insert_placeholder_0] => 775cbb48-ed5d-4fb0-aa15-09b315905fe7
[:db_insert_placeholder_1] => en
)

this happened when the uid was configured in the migration even if a process plugin skipped processing that field and no value was set. I think because the migration would somehow end up trying to do a mix of updating the already existing entity based on the existing destination IDs and creating a new one because the uid was not set.

Removing the empty destination property seems to have fixed the issue, but there is no way to do that without codebymikey's patch.

Uploaded a new patch with the latest MR changes for beta6.

Attaching a patch file for the Drupal 11 version from https://www.drupal.org/project/sqlsrv/issues/3509478 🐛 Module doesn't work with Drupal 11 Active .

dylan donkersgoed → created an issue. See original summary → .

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

Attaching a patch for use with composer.

dylan donkersgoed → created an issue.

Uploading a patch file with cristian100's changes above for use with composer.

Uploaded another patch which expands this for dropdown facets as well.

Link facets are still just links which makes sense to me, but I think the same principle could apply just making them lazy-loaded links that a bot wouldn't pick up unless it was looking for them. And of course any contrib/custom facets aren't affected.

Attaching a static file for the patch above.

I encountered an issue with this patch preventing hook_simplesamlphp_auth_existing_user from running. This was happening because the else that triggered it was moved inside of this if which checked for an existing user:

    if (!$account) {
      $existing_user = $this->entityTypeManager->getStorage('user')->loadByProperties(['name' => $authname]);
      $existing_user = $existing_user ? reset($existing_user) : FALSE;
      if ($existing_user) {

which would not generally be true when you're implementing a custom hook_simplesamlphp_auth_existing_user() to look up the user by an attribute other than their name. I moved the else outside of this check like the original module and adjusted the linkUser method accordingly.

Patch is attached. I also opened an MR with the patch and my new change.

While the patch seems to be working for me now, I think there may still be some room for improvement. I don't know the full rationale for the drastic changes between patch 8 and patch 11, but I feel as though the patch may be doing more than it needs to to fix the issue, which may have other side-effects.

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

I've opened an MR and attached a corresponding patch.

dylan donkersgoed → created an issue.

Attaching a version of the patch built for the current release as well.

This patch uses a {% spaceless %} tag which is no longer supported and can cause fatal errors. I've opened an MR with the patch applied and that fixed. Attached is a patch file. I've attached no interdiff as that's handled in the MR.

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

The clean-code-deprecated-dynamic-property-3170043-5.patch patch no longer applies to the latest dev/2.5-rc1 versions because the moduleHandler property name has been fixed. Attaching a new patch which includes only the setValue fix.

That last patch doesn't work with composer, uploading another in diff format.

I've fixed the conflicts in the MR. Also attaching a patch for the 3.0-rc2 version.

Uploading a patch rerolled for the 4.x version. I pushed the changes to the 2471481-integrate-typed-data-4.x branch as well. It's not letting me open it as an MR right now, but I may at some point in the future. In the meantime here's the patch file.

dylan donkersgoed → changed the visibility of the branch 2471481-integrate-typed-data to hidden.

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

I've added a patch for the actual Date filter plugin. I tried to transplant the code tobiberlin provided, but it resulted in fatal errors. The value is a string instead of an array unless it has multiple values, could be related to their custom plugin? The solution I've provided is pretty similar though.

This patch should probably have automated tests to go with it, so I'm marking it as Needs Work. But the patch seems to be working for me locally at least.

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

I merged in the latest 8.x-3.x branch. Rerolled patch for 8.x-3.0-beta4 attached as well.

I opened a new MR with a patch for the 2.x version. Patch is attached as well.

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

I've opened an MR and attached a patch.

Dylan Donkersgoed → created an issue.

@fruizalejos

We encountered further issues with this where sometimes access checks would check the default revision and the default revision wouldn't be referenced in any translation. I'm not sure how/why this happens, but it's possible you've run into the same issue. I worked around it by further expanding the check in a custom module to include the revision ID of the default revision when evaluating access. I didn't add it to the patch because I don't think we addressed the root cause and I'm not sure it's secure in all situations. It technically means someone might be granted edit access to a block revision that they otherwise wouldn't be able to edit.

I've opened an MR. Also attaching a patch file.

Dylan Donkersgoed → created an issue.

I've noticed an issue related to the one described in https://www.drupal.org/project/video_embed_field/issues/3311063#comment-... ✨ Add support for Ckeditor 5 Needs review where videos will render raw JSON. This is also related to the regex, but in this case seems to happen when there is an earlier { character before the preview_thumbnail property. In my case this was due to data-entity-embed-display-settings="{ in a <drupal-entity> tag but it probably could happen in other situations as well. Could be the cause of the issues a couple other users reported.

To fix this I pushed up a small change to the regex to make the opening

tag required. From what I can tell in the code for both the CKE4 and CKE5 plugins videos should always be wrapped in a paragraph so I believe this should be safe, but it would be nice to have some additional review/confirmation.

Updated patch is attached.

This patch worked for me, so moving to RTBC.

I needed to clear the storage for my site using the "Clear site data" button under the "Application" tab in Chrome (may differ depending on your browser) to get this to start working.

The latest version of the group patch this module depends on ( https://www.drupal.org/project/group/issues/2797793#comment-15236297 ✨ Entities identified by strings as group content Closed: won't fix ) uses a dedicated property, entity_id_str, for the ID instead. That results in an issue with this patch never picking up any menus.

I've pushed a change to this MR to accomodate that. It should still work for the old field as well. Patch file is attached.

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

Looking at the code it does appear to be intended that the autoplay setting from the WYSIWYG settings should be applied in the modal. It's trying to load default settings from there but they always turn out blank because it has the wrong plugin key ("video_embed", probably corresponding to the CKE4 plugin, instead of "video_embed_wysiwyg_video_embed"):

        $plugin_settings = NestedArray::getValue($editor_settings, [
          'plugins',
          // This should be video_embed_wysiwyg_video_embed for CKE5.
          'video_embed',
          'defaults',
          'children',
        ]);
      $settings = $plugin_settings ? $plugin_settings : [];

I've updated VideoEmbedDialog.php to check for both in the MR and attached the corresponding patch.

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

Patch attached.

Dylan Donkersgoed → created an issue.

I've merged in mkimmet's fix and it seems to be working for me. It discovers anchor links with the name attribute set and converts them to id anchor links, but never saves links with the deprecated attribute. Please let me know if that's not suitable for your use case/if it's not working for you. It's on the 3.0.x branch currently, but drupal.org hasn't generated the composer release yet.

This does not seem to require the patch from @DamienMcKenna, I think because it's not actually saving the attribute.

Seems sensible. Merged into the 3.0.x branch, thanks. I'll test it and tag it a bit later.

DamienMcKenna → credited Dylan Donkersgoed → .

Try running composer require 'drupal/anchor_link:^3.0@alpha' again? I've noticed that it does not always seem to pull in the library on the first go. I think it's because the composer.libraries.json doesn't exist until after the module is updated. If you don't have the library under [YOUR WEB ROOT]/libraries/ckeditor5-anchor-drupal the module will not work.

It appears I didn't actually have the module enabled on the site where I noticed this. So unable to reproduce the issue so far. If you could provide steps to reproduce with a clean Drupal site I may be able to help.

I tried installing on a clean D10 site and I can see the anchor link button when populating the toolbar in the text format.

However, earlier today I did see a similar issue on another site. I haven't been able to pin down the cause though and may not for a few days (it's on my work computer). I did have some JS errors in the console unrelated to anchor_link which may have been the cause, though other toolbar icons worked fine.

I don't think the issue is related to the external library after all. It shouldn't even be loaded on the admin page.

As for why you had to run composer twice I suspect the issue was that before anchor_link was updated the composer.libraries.json file didn't exist or didn't have information about fetching the library, so the composer merge plugin didn't import those dependencies at first. I encountered the same issue when installing from the instructions on the module home page. I'll update them.

I'm going to try to run through the steps as documented with a clean Drupal site when I get the chance (probably won't be until this evening). I wrote them from memory/samples in my project and may have missed something. If you're in a hurry you can probably just grab the library directly from https://registry.npmjs.org/@northernco/ckeditor5-anchor-drupal/-/ckedito... and decompress it in the relevant folder, it just won't auto-update until you set it up the composer way.

@mkimmet Can you verify whether the JS library exists in your libraries directory (e.g. docroot/libraries/ckeditor5-anchor-drupal)?

I've merged in a few fixes that were awaiting review including that one and tagged an alpha release. Thanks all for your testing and improvements.

Thanks for the PR mstrelan and vipul-tulse for reviewing. The latest dev version requires a tag with this PR merged in.

As for bringing this into Drupal I'm not sure bringing it into the module is the way to go. Despite the name of the repo, I don't think it's actually Drupal specific. It works inside the non-Drupal CKE5 sample environment at least.

When I first started work on this I was attempting to adapt the existing plugin to work the way I'd seen other Drupal CKE5 plugins work. I did not realize what I was actually doing was adapting it to be a DLL plugin, which is a recognized method of building a CKE5 plugin that works even outside of Drupal. So I called it ckeditor5-anchor-drupal.

I merged this in a while ago but didn't mark the issue done. Thanks for the PR ok-steve, the latest dev version requires a new tag with your PR merged in.

Thanks for the PR Brian-C. That appears to be working and I've merged it into the dev branch.

A GitHub PR was opened for this at https://github.com/northernco/ckeditor5-anchor-drupal/pull/3. However, I'm not sure it's working. See the PR for details.

I'm working on another D10 upgrade today for a site that uses this module so I plan to take a look at this today. I agree that this is ready for an alpha version, we already have it in use on some sites, but there's a PR for https://www.drupal.org/project/anchor_link/issues/3391107 🐛 Empty anchor links are not editable Active which I'd like to test and probably merge in first.

Rajab Natshah → credited Dylan Donkersgoed → .

That last patch does not work (at least against the 2.0.0 version) better one attached.

Changes are in the MR.

I'm also attaching a patch which includes both this change and the change from https://www.drupal.org/project/ckeditor_bootstrap_tabs/issues/3397894 🐛 Error when list elements are on the page RTBC .

Dylan Donkersgoed → created an issue.

Ran into the same issue. The patch above was in the right place but doesn't quite work, here's an updated one.

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

@matthiasm Are you using the latest version of the module and library? A PR from ok-steve that fixed this issue was merged in at the start of this month.

Some users have reported the node package not being updated on composer update in some situations. Please double check you have the 0.3.0 version of the JS package (or newer, I'll likely tag another version soon).

@vakulrai Are you certain this functionality came from anchor link? The last site I was working on that had it enabled does not have this on the production site still using CKE4. It also doesn't really look relevant for anchor links - they don't have URLs.

Same issue here, also on a multilingual site. I've opened a branch with the fix. It doesn't seem to want to let me open an MR right this second, but I'm attaching a patch with the fix.

This fix basically just switches the generated URL to be relative. I'm not sure what the reason for the original change was so not sure this is an acceptable solution or if an absolute URL is actually needed. But it does solve the issue.

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

Also, attaching a new patch file from the latest MR.

I pushed up a few more fixes for issues we ran into:

• When adding a new tab set it adds paragraphs by default. Without it it's quite tricky to select and edit text inside the tab after first adding tabs. Also, with just raw text CKE5 overwrites this with a paragraph when saving anyway (or at least something about Drupal's full html format does), so this keeps things more consistent.
• I fixed an issue where changing the number of tabs would sometimes add too many new tabs.
• I fixed an issue where the heading plugin (and maybe others, though that was the only one I could find with the issue) was not usable inside tabs. I don't fully understand why, but splitting the tab content and tab pane into multiple elements seems to have fixed it.

@RobLoach I also added a rudimentary composer.json. Please feel free to push to the MR if you need to,

Apparently I forgot to push so that was just the same patch as before. Here's the rebuilt patch for real.

Forgot to actually build the plugin after the last change, new patch.

Few more fixes. Notably there was an error when trying to edit source with my last few changes and there was a conflict with the core list plugin.

See issues for editing anchor links:

https://www.drupal.org/project/anchor_link/issues/3391107 🐛 Empty anchor links are not editable Active

and the bug where mailto links can be converted to anchor links:

https://www.drupal.org/project/anchor_link/issues/3391107 🐛 Empty anchor links are not editable Active

Dylan Donkersgoed → created an issue.

Dylan Donkersgoed → created an issue.

This library seems to be working for several people now. I've brought in the fixes from ok-steve and randy Tang. I'm going to merge this into the 3.0.x branch, double check that the install from 3.0.x is working, then close this issue and open a new issue for the problem with empty anchors not being editable. Thanks all for your testing and patches.

MR seems to be in a fully functional state now, attaching a patch.

There's probably room for some code cleanup, I'm not sure some of the separate upcast/downcast methods are still necessary, though they work fine.

I've expanded this patch to handle upcasting/downcasting bootstrap tabs. You can now load an existing page with tabs on it, edit the tabs, and save it and have everything work (at least from some cursory testing).

There's still a bit of work to do (e.g. the button isn't working correctly to add/expand tables at the moment and you can't change the active tab) but I hope to have that done within the next couple days.

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

Patch attached.

Dylan Donkersgoed → created an issue.

Patch attached.

Dylan Donkersgoed → created an issue.

Patch attached.

Dylan Donkersgoed → created an issue.

This might be a noob question. When I create a feed type using the LDAP Query fetcher and select my query. None of the LDAP attributes are listed as a source when I'm trying to map to my node fields. Only Feed entity items appear in the source list eg. Feed: Authored by

The fetcher doesn't automatically discover LDAP attributes, you need to manually enter the LDAP attribute name. In the same dropdown with "Feed: Authored by" there should be an option for a custom source. I don't recall off the top of my head exactly what it's called but I believe it's separated from the others toward the bottom of the dropdown.

As for why your feed is locking up it's hard to say without more details. I might be able to take a look if you can attach some configuration (the LDAP server, query, and feeds configuration or just an entire test database) that replicates the issue. You can use a public test LDAP server like the one from https://www.forumsys.com/2022/05/10/online-ldap-test-server/.

mcdruid → credited Dylan Donkersgoed → .

@everhee Nevermind, I see the issue and have pushed up a fix to the MR. New patch attached as well.

@everhee That should be working already. The new CKE5 plugin does store them in a different format, but it's supposed to load the child content of old tags into the value attribute of new ones. Is that not working for you and if so could you provide some sample markup with the issue?

The old footnotes widget supported entering the footnote text in a text area, but it was a single line input in the new one. This made editing long footnotes much harder. I've pushed up another change to switch the footnote text input back to a text area and enlarge it. Patch is attached.

@loopy1492 Please feel free to bring that into my branch. I was originally thinking we could keep it to retain CKE4 support but it probably does make more sense to just have a 3.0.x branch that supports only CKE5 and doesn't bring in the CKE4 dependencies.

As for the other issue with empty anchor links not being editable on click I fixed something similar for Footnotes, leaving it here in case someone else wants to apply a similar fix or for if/when I get around to it: https://git.drupalcode.org/project/footnotes/-/merge_requests/4/diffs?co...

The old version also allowed editing footnotes by clicking them, I've added that to the new version as well. Updated patch attached and it's in the MR.

I've pushed up an MR and attached a patch with a simple fix which uses the raw textContent for script and style tags. There's probably room for improvement (e.g. there might be other tags affected, or maybe a less naive check than just checking the parent tag name should be used and possibly there should be a test), but this fixes the issue for me.