🇨🇦Canada @joelpittet

Made sure the IDs where unique by keying on them.

joelpittet → created an issue.

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

We’ve returned to this issue as per 📌 Move the work on drupal.org. Enable testing Fixed

The functionality discussed in https://github.com/Gizra/og/issues/254 was originally addressed in https://github.com/Gizra/og/pull/253, which was later closed in favor of https://github.com/Gizra/og/pull/565. However, the current implementation has some issues:

1. The code fails when no role_ids are passed in, as $this->ogMembership->getUserGroupIdsByRoleIds() requires role_ids to function.
2. There’s a minor bug in the form where type="text" should be type="textfield" for the two relevant options.

To provide a working solution, I’ll repost the code from https://github.com/Gizra/og/pull/253.

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

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

Hmm this isn't correct, the issue summary says prefixed with drupal: but these were prefixed with the module name as if they were contrib modules and not part of core.

Updated CR as per #273 @quietone's suggestions to remove D8 and a bit more.

Just an FYI, we ran into this and it wasn't the data being wrong, it was a views aggregated field that needed timezone added to the "Group Columns" or it didn't have access to the value.
Hope that helps anybody struggling with this one and pulling their hair out as we were.

Yes, we won’t focus on this as it would take a long time to review and wouldn’t fix any issues it’s more likely to cause rerolls on issues that do.

I do however appreciate the effort it takes to do these kinds of clean up as it’s no small feat after the initial phpcbf run, thank you for trying to improve the quality of the code base!

Targetting against the dev branch instead of the specific version and changing to needs review.

Seems like a regression by accident presumably in 🐛 Controlled-by fields inside a Paragraph don't work Needs work
https://git.drupalcode.org/project/conditional_fields/-/commit/962c0a834...

Unfortunately the previous code that worked didn't document why it was needed, nor does the new code document why it's not, so kinda hard to write a test, no?

Moving priority due to the PHP error that is triggered
https://www.drupal.org/docs/develop/issues/fields-and-other-parts-of-an-... →

Trigger a PHP error through the user interface, but only under rare circumstances or affecting only a small percentage of all users, even if there is a workaround.

Thanks @ramil g. The test looks great and it's red/green on test-only/with fix. 🚀 it!

We have various screenshots on this issue, so removing the PHP related issue tag and needs screenshots.

@akram khan Thanks for the patch, could you explain the changes you made so we are all clear as your reroll added more removals than @ramil g did.

Yes totally, dev version doesn’t have crush command from what I saw.

Do we have access to config inside a custom MigrateBoostServiceProvider class?

The above code works in #2 BTW, just need to remember to uninstall the migration module after the migration, hence a better place would be here...

Oh yeah, I was thinking that too, though is that hooks are events now?

This came from #2409901-14: Implement Migration Paths for Flag 7.x →

Experimenting

<?php

declare(strict_types=1);

namespace Drupal\custom_migration;

use Drupal\Core\DependencyInjection\ContainerBuilder;
use Drupal\Core\DependencyInjection\ServiceProviderBase;

/**
 * Defines a service provider for the custom_migration module.
 *
 * @see https://www.drupal.org/node/2026959
 */
final class CustomMigrationServiceProvider extends ServiceProviderBase {

  /**
   * {@inheritdoc}
   */
  public function alter(ContainerBuilder $container): void {
    if ($container->hasDefinition('flag.count')) {
      $container->removeDefinition('flag.twig.count');
      $container->removeDefinition('flag.count');
    }
  }

}

joelpittet → created an issue.

joelpittet → created an issue.

We created a custom destination plugin to lookup the value in the getEntity() method, the only suspicious part is I hardcoded english because we aren't migrating the langcode:

<?php

declare(strict_types=1);

namespace Drupal\custom_migration\Plugin\migrate\destination;

use Drupal\migrate\Attribute\MigrateDestination;
use Drupal\migrate\Plugin\migrate\destination\EntityContentBase;
use Drupal\migrate\Row;

/**
 * The 'registration_settings' destination plugin.
 */
#[MigrateDestination(
  id: 'registration_settings'
)]
final class RegistrationSettings extends EntityContentBase {

  /**
   * The entity type plugin id for RegistrationSettings entity.
   */
  protected static function getEntityTypeId($plugin_id): string {
    return 'registration_settings';
  }

  /**
   * {@inheritdoc}
   */
  protected function getEntity(Row $row, array $old_destination_id_values) {
    // Extract the keys to determine if the entity already exists.
    $entity_type_id = $row->getSourceProperty('entity_type');
    $entity_id = $row->getSourceProperty('entity_id');
    // Hardcode en if not available.
    $langcode = $row->getSourceProperty('langcode') ?? 'en';

    // Load the existing entity if it exists.
    $existing_entities = $this->storage->loadByProperties([
      'entity_type_id' => $entity_type_id,
      'entity_id' => $entity_id,
      'langcode' => $langcode,
    ]);

    // Get the first match, if any.
    $entity = reset($existing_entities);

    if ($entity) {
      // Allow updateEntity() to change the entity.
      $entity = $this->updateEntity($entity, $row) ?: $entity;
    }
    else {
      // Attempt to ensure we always have a bundle.
      if ($bundle = $this->getBundle($row)) {
        $row->setDestinationProperty($this->getKey('bundle'), $bundle);
      }

      // Stubs might need some required fields filled in.
      if ($row->isStub()) {
        $this->processStubRow($row);
      }
      $entity = $this->storage->create($row->getDestination());
      $entity->enforceIsNew();
    }

    return $entity;
  }
}

joelpittet → created an issue.

The Views handler doesn’t require a sort option, which is the root cause of the issue.

I’m hoping this doesn’t require any tests? It’s just a quick DX fix to help people get over their migrations. I'm RTBC'ing because it gets me (and others) over the hump.

Here's my bad data:
a:8:{s:12:"entity_types";a:0:{}s:17:"field_permissions";a:1:{s:4:"type";i:0;}s:7:"indexes";a:1:{s:9:"target_id";a:1:{i:0;s:9:"target_id";}}s:8:"settings";a:4:{s:7:"handler";s:5:"views";s:16:"handler_settings";a:2:{s:9:"behaviors";a:1:{s:17:"views-select-list";a:1:{s:6:"status";i:0;}}s:4:"view";a:3:{s:4:"args";a:2:{i:0;s:56:"[field_collection_item:host:node:field-research-area:id]";i:1;s:47:"[field_collection_item:host:node:field-year:id]";}s:12:"display_name";s:17:"entityreference_1";s:9:"view_name";s:24:"utility_er_courses_upper";}}s:16:"profile2_private";b:0;s:11:"target_type";s:8:"academic";}s:12:"translatable";i:0;s:7:"storage";a:5:{s:4:"type";s:17:"field_sql_storage";s:8:"settings";a:0:{}s:6:"module";s:17:"field_sql_storage";s:6:"active";s:1:"1";s:7:"details";a:1:{s:3:"sql";a:2:{s:18:"FIELD_LOAD_CURRENT";a:1:{s:37:"field_data_field_draft_course_section";a:1:{s:9:"target_id";s:36:"field_draft_course_section_target_id";}}s:19:"FIELD_LOAD_REVISION";a:1:{s:41:"field_revision_field_draft_course_section";a:1:{s:9:"target_id";s:36:"field_draft_course_section_target_id";}}}}}s:12:"foreign keys";a:1:{s:12:"eck_academic";a:2:{s:5:"table";s:12:"eck_academic";s:7:"columns";a:1:{s:9:"target_id";s:2:"id";}}}s:2:"id";s:2:"70";}

And here's how I tracked it down:
SELECT * FROM `field_config` WHERE (`module` = 'entityreference') AND (`data` NOT LIKE '%sort%')

Thanks @mortona2k I have merged that in, I have a couple other things to add before I make another release.

I committed and released this in 1.8

What is involved in supporting backdrop too?

I agree @dieterholvoet, making a release plan for 1.9 and releasing 1.8

joelpittet → created an issue.

Thanks for catching that @rformato

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

Just an FYI this is all related to this core issue BTW which is the last one that makes the calendar_datetime sub module or the core patch needed.
🐛 UI fatal caused by views argument handlers no longer can provide their own default argument handling Needs work

@mortona2k good catch, I didn't mean to put that there. Corrected to Ymd for FullDate

I see where I overshot there a bit. I am trying to find a way to get this working like D7 with the hyphens in the args.
LMK if the change there which was missing the class comparison for date_fulldate

The default Y-m-d is still in \Drupal\views\Plugin\views\argument\Date

But Ymd is used in \Drupal\datetime\Plugin\views\argument\FullDate and \Drupal\views\Plugin\views\argument\FullDate

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

Oh sorry, I was just trying to reproduce a bug but ran into this, if they are identical then yeah it doesn’t matter, sorry to bother. My bug was in environment_indicator in the end.

Thanks @harkonn for testing it out!

I guess I changed the code so Needs Review is appropriate, I deleted those CSS lines that were marked for removal

This still fixes an issue with gin compatibility (even without the JS) but I will re-roll.

Here is what it looks like before the patch:

Just bumping this fix as I see somethings getting committed

joelpittet → changed the visibility of the branch 4.x to hidden.

@jurgenhass thanks for fixing this I thought I needed to. I created this dupe 📌 Allow gin_toolbar 2.0 to install Active

Could you do a patch release when you get a chance? Thanks for updating this!

Sorry actually this is a duplicate 📌 Clean-up composer constraint for gin_toolbar Active

Sorry I miss read composer output, my bad...

joelpittet → created an issue.

We used MR 52 to explicitly fix this error in preview (usually I don't enable preview on nodes...)

This seems to fix the problem for us.

joelpittet → created an issue.

@Nick Dickinson-Wilde had the permission, gave me the permission, who in turn gave you the permission @smustgrave.

Welcome to the team.

I am hoping the composer and info.yml changes will deal with the new module dependency.

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

I looked and I don't have the permission to add new co-maintainers, maybe reach out to @shelane as they added me this year 📌 Offering to co-maintain quicktabs Fixed ?

Adding tag back from #245 as it I think it was accidentally removed.
We tested MR7501 and it will solve our immediate requirement.

@milos.kroulik we tried conditional_rendering and it doesn't seem to solve what this issue is trying to solve. It also only works for block_content so not other block types, or simple_block for example.

Thanks @vensires re #9, I came here for that. And thanks @berdir for diving into the deep-end on this problem.

Thank you @ramil g again. That fixes a bug where the class Date was reading from itself!

joelpittet → changed the visibility of the branch 3163197-allow-hiding-configured to hidden.

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

Thanks again @lavanyatalwar I have merged the MR.

Yes, “reorganizing the entire admin menu” is definitely out of scope! 😄 The issue largely stems from contrib modules adding their own admin pages under existing ones or creating nested subpages (but core too as screenshoted in #44 🐛 Second level menu items can't be reached if they have children Active ). Addressing this by fixing contrib would likely require more effort and education than working with the existing patterns commonly used.

The article raises valid concerns about split buttons in navigation, it might overstate their universal unsuitability. Instead of relying solely on these arguments, we should test those concerns ourselves, perhaps against @scott_euser propposal in #6 🐛 Second level menu items can't be reached if they have children Active , to see how they hold up in practice. A balanced approach that evaluates context, user needs, and new design tricks might reveal a more nuanced view of this pattern. Safe enough to try?

@lavanyatalwar That is great, yeah leave those, is there a way in style lint to explicitly ignore like phpcs:ignore? I'd also be happy to have them moved to a comment, as they are only there to document what is at your disposal

Thanks @megachriz, I was going to post there (I linked it as a parent issue) but I wasn't quite sure if it's related. The feeds_clean_list seems to trigger this issue in my case because it always starts fine. I unlock the feed, it seems to work for a bit, then starts with that error, then later this tempnam() keeps happening.

I feel like I am missing something in this and started a new issue as to not conflate the issues (yet) until I know for sure.

joelpittet → changed the visibility of the branch 8.x-1.x to hidden.

More context: this is stored in our private directory which is mounted NFS, outside the webroot and this is only happening on 1 or 2 feeds at the moment. The other feeds seem to be fine.

And it seems to happen on cron every hour after this error

Drupal\Core\Database\IntegrityConstraintViolationException: SQLSTATE[23000]: Integrity constraint violation: 1062 Duplicate entry '9-141838' for key 'feeds_clean_list.PRIMARY': INSERT INTO "feeds_clean_list" ("feed_id", "entity_id") VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1), (:db_insert_placeholder_2, :db_insert_placeholder_3); Array
(
    [:db_insert_placeholder_0] => 9
    [:db_insert_placeholder_1] => 141838
    [:db_insert_placeholder_2] => 9
    [:db_insert_placeholder_3] => 141914
)

The feeds are triggered every minute, though maybe the solution is to stop that...

joelpittet → created an issue.

Thanks for your help on this @lavanyatalwar

This looks great everyone, nice work!

Thanks for clarifying @wheelercreek

I’m exploring a potential solution by parsing the Styles plugin to dynamically add or update span elements. This would allow editors to define and manage styles more flexibly without creating a new plugin or allowing source editing workaround.

Would there be interest in pursuing this direction? Any thoughts or concerns?

The split button proposed by @scott_euser in #6 🐛 Second level menu items can't be reached if they have children Active seems like the most intuitive direction here. I also share the concerns raised about the additional ‘Overview’ injected links.

@ckina and @lauriii could you give this a second look?

This affects a number of modules I have co-maintainership of such as:

• Menu Position →
• Calendar → (by way of dependency Views Templates → )
• Feeds →

And simple_block as mentioned previously and core's Display modes all under Structure I might add. Overview seems to make sense for Configuration at a glance but not Structure. Though I believe the Split button approach is more versatile.

Thanks @scott_euser that is where I should have commented.

I am seeing what's been reported in #12 by @anruether and echo'd by @scott_euser in #19.

Boils down to second level parents are not possible to to navigate to because the entire item is intended to toggle the children.

The primary menu item parents are possible to navigate to, though you need to click twice as they open those items. One to open the children and on the header of the children.

Proposal: split button for the arrow? I was going to give you an example of a site I did, but just saw that I broke the JS on it while I was getting the link, lol... there goes my day...

@lavanyatalwar leave that commented code, don't change the indents or delete it, it needs to be evaluated to see if it's needed (beyond scope of this issue)

Back to RTBC, thanks for removing the test as well ivnish

joelpittet → created an issue.

@lavanyatalwar Thanks for taking this on. There is a lot of commented code, so don't worry if you don't get them all, any progress on this is great.

A tip: When working on Drupal issues, avoid assigning them to yourself unless you’re actively fixing them. Assigned issues can give the impression they’re in progress, even if they’ve been set aside. Instead, leave a comment (as you did) with your thoughts or progress, and make small commits to contribute towards a solution. This keeps the issue visible and encourages collaboration.

joelpittet → changed the visibility of the branch 3489746-phpcs to active.

joelpittet → changed the visibility of the branch 3489746-phpcs to hidden.

joelpittet → changed the visibility of the branch 8.x-1.x to hidden.

Thanks @ramil g, one less blocker!

I removed the authors as well because they aren't working on 1.x branch

joelpittet → created an issue.

Got most of the cspell issues resolved and everything else was added to the project words file to ignore.

Is this also for 2.x or 1.x? There is this reported for 🐛 Duplicate Events showing up when there are multiple event dates Active that sounds similar