Account created on 30 July 2007, over 17 years ago
#

Merge Requests

Recent comments

πŸ‡ΊπŸ‡ΈUnited States majorrobot

#49 works for me. Passes Axe Core automated testing. I also tested with keyboard and screenreader, and it all worked.

I tested with version 2.2 of the module.

I didn't see the issue in #50 πŸ“Œ Provide an accessible variant Needs review . Going to set this to Needs Review.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

@emptyvoid -- curious what the difference between this issue and patch and https://www.drupal.org/project/ckeditor_accordion/issues/3124167 πŸ“Œ Provide an accessible variant Needs review ?

πŸ‡ΊπŸ‡ΈUnited States majorrobot

This seems like a good feature to replace the Contact Form Permissions module (which appears abandoned and doesn't have a supported release anymore). So I'm excited to see it in progress!

I've created a fork/MR from @naveenvalecha's patch in #18 and updated core version requirements.

I've also attempted to address @larowlan's requests in #22.

+++ b/modules/contact_permissions/src/ContactPermissions.php
@@ -0,0 +1,55 @@
+class ContactPermissions {
This needs to use \Drupal\Core\Entity\BundlePermissionHandlerTrait now so that we get the correct dependencies

I've added this, as I understand it -- but this is my first time handling permissions in Drupal 8+, so I may have missed something.

+++ b/modules/contact_permissions/contact_permissions.module
@@ -0,0 +1,61 @@
+ $entity_types['contact_form']->setAccessClass('\Drupal\contact_permissions\ContactFormAccessControlHandler');
we also need to switch the permissions route provider from EntityPermissionsRouteProviderWithCheck to EntityPermissionsRouteProvide

I'm actually not following this one -- I don't see where EntityPermissionsRouteProviderWithCheck is used. Nor have I been able to figure out the intent of the note with a little research. Happy to make changes if someone could clarify, though.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

majorrobot β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

#14 worked for my case. We have a multi-value field that allows a maximum of 2 values. Previously, you could add as many values as you want -- the values would not save, nor would an error be thrown.

After applying a patch from #14, user gets an error when >2 values are chosen.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

Can confirm this is also broken in 4.0.2.

However, the most recent fix (3309324-single-day) does the trick (at least for the dev version of the module). Would be great to get this merged in and released so we don't have to depend on the dev version!

πŸ‡ΊπŸ‡ΈUnited States majorrobot

Hey @jminarick, this makes sense. Better to let the theme / individual site owner decide how to deal with that header. Looks good.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

I can verify this works. I removed the title setting and it kindly no longer appeared.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

majorrobot β†’ created an issue.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

@jminarick, this change makes much more sense. I'd say this is good to go.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

Good call, @swirt.

@jminarick, I've reviewed this small change and it looks good.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

@jminarick -- this looks good to me. Tried it out, and it fixes the issue.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

I took a peek at the failing tests -- looks like there's one main PHPUnit failure. I rebased on most recent 8.x-1.x-dev and cleaned up a few of phpcs's complaints. I don't have a deep familiarity with the module, so am still digging into the unit test issue.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

If I am wrong, reopen this issue by setting the status to Active and explain what is not working, what error message you are getting and anything you think is relevant.

Ack, I hate to reopen this issue, but I think it is warranted. If I should not, let me know! I'm having the same experience as OP, as well as @Shiraz Dindar above.

I'm running Drupal 10.2.5 and I'm migrating from another site on 10.2.5. Though I'm using the migrate_drupal_d8 contrib module, it still calls the core Migration Lookup plugin. In my case, I have an image field migrating to an identical image field. Both sites have Content Translation enabled for the content type. It does not seem to matter, but I have the image field configured not to be translatable.

#2912348: Handle entity_references related to Drupal 6 and 7 node translations with different IDs has some similarities, but I think it is actually a different issue.

I understand that issue to solve a problem where entity reference migrations aren't looking up the correct original target_ids. The issue here is that entity references end up with 2 destination ids: the target_id and the langcode. The target_id is correct. However, EntityReference->setValue() only accepts a single scalar value.

Thus, I'm still getting
"Value is not a valid entity." line 106 on core/lib/Drupal/Core/Entity/Plugin/DataType/EntityReference.php

The patch in #13 πŸ› Migrate translated entity reference from D7 to D8 Needs review solves the issue for me, too. The solution in the patch seems a bit heavy-handed, but I don't know other use cases well enough to say whether or not the patch would break other migrations. It seems a bit dangerous, but I can't say for sure:

     if ($destination_ids) {
-      if (count($destination_ids) == 1) {
+      if (count($destination_ids) >= 1) {
         return reset($destination_ids);
       }
       else {
πŸ‡ΊπŸ‡ΈUnited States majorrobot

@benjifisher and @joachim was there an issue filed with core that I can follow?

I see https://www.drupal.org/project/drupal/issues/2871217 πŸ› Avoid error when $options is NULL in buildUrl() Needs work , has tagged this issue, but what they're addressing there is actually a bit different. (Here we're dealing with serialized data that has actually been stored incorrectly -- the error we're seeing is valid; #2871217 πŸ› Avoid error when $options is NULL in buildUrl() Needs work is dealing with cases where $options is NULL.)

πŸ‡ΊπŸ‡ΈUnited States majorrobot

@chaitanyadessai's patch seems to be a reroll of !54, so I applied that locally.

I found that the code did not load multiple gtag script tags correctly. Multiple tags loaded, but each tag repeated the config of the first tag (except for that the tagId). This gave a 404 for those tags.

To resolve this, I took the pattern from google_tag_page_top() and refactored google_tag_page_attachments() accordingly.

gtm.js also needed to account for multiple tag configs, so I also refactored parts of that file.

Since MR !54 was out of date, I took the MR's branch and rebased on 2.0.x., then added my changes and pushed the new branch. I created MR !83. I'm not completely clear if this handles every case, but it should get us closer.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

majorrobot β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

Same here. Had the same error and can confirm updating to Field Permissions 1.3.0 resolved the problem.

I think it's safe to close this one?

πŸ‡ΊπŸ‡ΈUnited States majorrobot

majorrobot β†’ made their first commit to this issue’s fork.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

FYI @luca.vocella there are already (at least) 2 issues open around migrating translating content:

You might want to take a look at those issues. The first one πŸ› Support translated content Needs review seems more mature, and I've tested it, with success.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

Thanks for all your work on this @spiderman and @DieterHolvoet. I was able to get this to work with nodes with translations.

For those attempting this kind of migration, there were two things that were not obvious to me, but completely necessary. (It may be like this in non-drupal8 translation migrations, too -- I just don't know/)

  1. Create a migration for the first language and then a migration (which will be very similar) for any other language you want to import. Make sure you have translations: true in the source and destination portions of the non-original-language migrations.
  2. When migrating the non-original language(s), you'll need to use a migration_lookup in the process section to set the nid of the translated nodes. (The original and all translations of a node share the same nid.)

An example, migrating a "resource" content type with translation to a second language:

id: my_migration_en
label: my_migration_en
migration_tags:
  - Drupal 8
  - resources
source:
  plugin: d8_entity
  key: migrate
  entity_type: node
  bundle: resource
process:
  title: title
  body: body
  status: status
  langcode: langcode
  bundle: 'resource'
destination:
  plugin: entity:node
  default_bundle: resource
id: my_migration_es
label: my_migration_es
migration_tags:
  - Drupal 8
  - resources
source:
  plugin: d8_entity
  key: migrate
  entity_type: node
  bundle: resource
  translations: true
process:
  title: title
  body: body
  status: status
  bundle: 'resource'
  langcode: langcode
  nid:
    plugin: migration_lookup
    migration:  my_migration_en
    source: nid
destination:
  plugin: entity:node
  default_bundle: resource
  translations: true
  nid: nid

I've set this issue to Needs Review to see if the fix will move along.

πŸ‡ΊπŸ‡ΈUnited States majorrobot

I've tested the MR on my local (which has Drupal in a subdirectory) and can confirm I can export and import correctly. Thank you, @swirt!

πŸ‡ΊπŸ‡ΈUnited States majorrobot

@JayDarnell, I've created a MR. Basically, instead of using the group_subscriptions_groups.groupname column, I'm grabbing the group's actual label in the database call starting on line 146. There are other alternatives to this β€” like loading each group from the query results and getting the label that way β€” but they seem to be much messier and would involve a good amount of refactoring.

As I mentioned in the issue description, it would be best to not have group_subscriptions_groups.groupname at all, but that is a more ambitious project.

Production build 0.71.5 2024