Brooklyn, NY
Account created on 26 March 2010, over 14 years ago
#

Merge Requests

Recent comments

🇺🇸United States JonMcL Brooklyn, NY

Not sure this is working. I set a field value to be =SUM(1,3) and the CSV file had "=SUM(1,3)" for that column.

But, most importantly, this patch will only work when the output is a CSV file. There are other options with this module, such as XLSX, and this patch will actually alter the data in such a way that it will no longer be a valid XLSX file.

🇺🇸United States JonMcL Brooklyn, NY

If I am following along with this change, am I correct that this

public function getFieldDefinitionSettings(): array {
  return [
    'target_type' => 'media',
    'handler' => 'default:media',
    'handler_settings' => [
      'target_bundles' => [
        'image',
      ],
    ],
  ];
}

needs to be changed to


public function getStorageDefinitionSettings(): array {
  return [
    'target_type' => 'media',
  ];
}

public function getFieldDefinitionSettings(): array {
  return [
    'handler' => 'default:media',
    'handler_settings' => [
      'target_bundles' => [
        'image',
      ],
    ],
  ];
}

Am I understanding this change correctly?

🇺🇸United States JonMcL Brooklyn, NY

I am also noticing a memory issue introduced by admin_toolbar/admin_toolbar_tools/src/Plugin/Derivative/ExtraLinks::getDerivativeDefinitions()

I end up with over 400 items in $links. If I hack the code to exclude some entity types, and get the total count of $links down to around 300, all seems to work well. It doesn't appear to matter which entity types I exclude.

Strangely, we are most likely to trigger the issue one some node bundles' Manage Field page, but the issue doesn't trigger on other entity types' bundles.

Sorry, this is probably not very useful information.

Disabling the module, or returning an empty array from ::getDerivativeDefinitions() does seem to not cause the issue to happen.

🇺🇸United States JonMcL Brooklyn, NY

The Facets patch at 🐛 Facets breaks all AJAX views that uses pagers even without facets Active (#25) fixes the issue for us. The fix is not yet in an official release version.

Interestingly, the patch at #44 of this issue ALSO fixes the issue.

🇺🇸United States JonMcL Brooklyn, NY

This fixed multiple issues for use. Our theme CSS is at the end of (as it should be) and then a module's library CSS (select2) ends up also added to the end of when the library is included with an AJAX dialog. With our theme CSS no longer at the end of we would need to review all CSS selectors to potentially add more specificity.

This patch works well for us (10.2.6).

🇺🇸United States JonMcL Brooklyn, NY

JonMcL changed the visibility of the branch 3347343-10-2-x--do-not-merge to hidden.

🇺🇸United States JonMcL Brooklyn, NY

JonMcL changed the visibility of the branch 3347343-10-2-x--do-not-merge to active.

🇺🇸United States JonMcL Brooklyn, NY

Sorry in advance for the "noise" but can someone give a a summary of where things stand with this REGRESSION? I feel like it is not getting the proper attention from core maintainers that it should? Or am I wrong in that impression.

I see two MRs. One is marked for Drupal 10 and the other, I assume, is for Drupal 11? I suppose work has stopped completely on 9.5.x even though this regression was introduced in 9.5.9?

Also, I think there is still problem with menu items that reference a view that has a contextual filter. I recall others discussion that, but I don't know if it was in this issue or another.

🇺🇸United States JonMcL Brooklyn, NY

I am curious if anyone has tried to tackle a token handler that might work like this:

$message->addContext('salutation', 'Greetings ' . $name);

And then in the message template could be a token like: [message:contexts:salutation]

🇺🇸United States JonMcL Brooklyn, NY

Setting back to "Needs review"

🇺🇸United States JonMcL Brooklyn, NY

Potentially major problem with the else statement in this block of code:

    // Special treatment for Core's user entity.
    if ($entity_type_id == 'user') {
      $widget = &$form['account']['name'];
    }
    else {
      $widget = &$form[$label]['widget'][0]['value'];
    }

Not all entity form displays are going to have a label field in the form. Assigning a non-existent form element to $widget by reference actually created the element with a value of null;

Problem is that ContentEntityForm::copyFormValuesToEntity then gets called and because the label widget was added to the form (with a value of null), ::copyFormValuesToEntity then sets the label to be null.

This is probably a problem only when auto_entitylabel is disabled for a particular entity type and when the label widget is not present in the form currently being submitted.

🇺🇸United States JonMcL Brooklyn, NY

JonMcL created an issue.

🇺🇸United States JonMcL Brooklyn, NY

Very frustrating to find out that using the "theme" category on a module's library, with a group value that is higher than CSS_AGGREGATE_THEME (100), does not allow the module's CSS to come after the theme's CSS in the DOM. Impossible to do simple CSS refinements when the module wants to do them.

I am glad I am not alone in this :)

For what it may be worth, here is my implementation of hook_css_alter that re-applies the group property from the library data:

function modulename_css_alter(&$css, \Drupal\Core\Asset\AttachedAssetsInterface $assets) {

  foreach($assets->getLibraries() as $name) {
    [$module, $id] = explode('/', $name);
    if ($module && $id) {
      $library = \Drupal::service('library.discovery')->getLibraryByName($module, $id);
      foreach($library['css'] ?? [] as $definition) {
        if (isset($definition['group']) && $path = $definition['data']) {
          if (isset($css[$path])) {
            $css[$path]['group'] = $definition['group'];
          }
        }
      }
    }
  }
}

It is probably not very efficient, but should only run when needed and then the alterations are cached.

🇺🇸United States JonMcL Brooklyn, NY

I was thinking of simplifying my question:
Where does the IEF element set the new values, taken from $form_state, into the form elements?

If I can find the code, I can maybe determine what else we need to do to our multistep framework to make it compatible with the IEF element.

🇺🇸United States JonMcL Brooklyn, NY

Sorry for the delay. I haven't had a chance to do any testing in a long while, so hopefully everything internal is still working well enough for D10. I know it's difficult to upgrade without module's having new core version requirements, so I wanted to get this out there.

🇺🇸United States JonMcL Brooklyn, NY

I know it should probably be another issue, but if this issue ever manages to make it into core, it would be great if one small addition could be added:

          class: $originalButton.attr('class'),
          disabled: $originalButton.attr('disabled'),

allowing the dialog buttons to inherit the disabled attribute from the original button.

🇺🇸United States JonMcL Brooklyn, NY

Previous behavior was to prepend the CSS:
$('head').prepend(response.data);

I think the new code is appending to which can cause order of precedence issues. A module might have CSS component assets (weight of 0) which are added into after our theme's CSS_THEME assets (weight of 200).

This might not be an issue for most situation, but I just noticed it with some quick D10 testing in advance of an upgrade. Maybe it was just a coincidence that we were able to have our theme CSS override the module's component CSS?

Any known workarounds for this (potential) issue?

🇺🇸United States JonMcL Brooklyn, NY

This MR also allows for:

  1. Use TNS string to specify connection
  2. Add init commands to be executed before each connection
🇺🇸United States JonMcL Brooklyn, NY

JonMcL created an issue.

🇺🇸United States JonMcL Brooklyn, NY

@bircher My mistake and please accept my apologies if my question seemed like a complaint.

I searched for the this patch's config property and I thought I saw it in the 3.0 module's schema file, and then no references to the property in the code. However, I was incorrect and the enable_export_filtering property is not used or needed.

The 3.0 version of the module is working great, for both import and export, in simple mode. Thank you!!

🇺🇸United States JonMcL Brooklyn, NY

Never mind. I definitely don't know what I am doing. :)
reflection/validator won't get installed by composer if the git.drupalcode.org drupal/form_alter_service repo comes before https://packages.drupal.org/8 in the repositories list. The git.drupalcode.org repos need to come after.

🇺🇸United States JonMcL Brooklyn, NY

Either I did something wrong with the issue fork, or there is a problem. After installing the patched version from the issue fork, I am getting:
PHP Fatal error: Uncaught Error: Class "Reflection\Validator\Annotation\ReflectionValidatorAnnotationReader" not found in /var/www/html/web/modules/contrib/form_alter_service/src/FormAlterCompilerPass.php:95

🇺🇸United States JonMcL Brooklyn, NY

I went ahead and created an issue fork and merge request. I don't know if I will ever get this new process done right, but hopefully it is?

Adding this to my composer.json repositories section worked for me:
(it must come before https://packages.drupal.org/8 in the repositories section)

{
    "type": "package",
    "package": {
        "name": "drupal/form_alter_service",
        "version": "3.x-dev",
        "type": "drupal-module",
        "source": {
            "url": "https://git.drupalcode.org/issue/form_alter_service-3297260.git",
            "type": "git",
            "reference": "3297260-automated-drupal-10"
        }
    }
}

Then run composer update drupal/form_alter_service

🇺🇸United States JonMcL Brooklyn, NY

Yes please. Without this fix, this module causes a fatal error with the entity_ui module because that module attempts to get this module's label in the EntityTabListBuilder class.

Completely correct to add provider to tabs created by a derivative plugin.

🇺🇸United States JonMcL Brooklyn, NY

Patch does not apply to the 8.x-3.0 branch. The enable_export_filtering setting is now part of the module's schema, but it is set to false by default. I also can't find anywhere it is used and no way to change it on the settings form.

Any chance some of the maintainers could chime in? Maybe the setting was added in anticipation of a new release?

🇺🇸United States JonMcL Brooklyn, NY

I don't think this notice has anything to do with the message logged in trace on doTrustedCallback.

I think the notice is because \Drupal\Core\Entity\Element\EntityAutocomplete::processEntityAutocomplete is expecting to get a form object from $form_state with this snippet from line 180:

    // Put entity into settings.
    $form_object = $form_state->getFormObject();

Right below it is a check to see if $form_object has an expected value, and if not, it proceeds on.

I don't know if this is a correct solution, but this seems to get us past the notice:

  protected static function setAutocompleteRouteParameters(array &$element): array {
    $complete_form = [];
    $form_state = new FormState();
    $form_state->addBuildInfo('callback_object', NULL);
    $element = EntityAutocomplete::processEntityAutocomplete($element, $form_state, $complete_form);
    $element['#autocomplete_route_name'] = 'select2.entity_autocomplete';
    return $element;
  }

Patch is attached.

🇺🇸United States JonMcL Brooklyn, NY

@amateescu: Thank you for your work on this.

Unfortunately it no longer applies to 9.5.x or 10.x. Any chance you have rerolled this patch recently?

🇺🇸United States JonMcL Brooklyn, NY

JonMcL created an issue.

🇺🇸United States JonMcL Brooklyn, NY

I think @Berdir is correctly pointing out that there is unlikely a good/proper way to have this module enabled along with either the context or menu_position modules. All 3 modules are attempting to take over the menu.active_trail.

The decorator approach does allow both context and this module to be enabled successfully, and this module's functionality does seem to work correctly in my limited tests. However, it appears that the context's ContextMenuActiveTrail class will no longer get called unless "Drupal Core Behavior" is chosen. This is probably to be expected.

However, I think I might accept this for our use cases. If this module works well enough for us, we might not need to use the context module's "Menu" reaction. Maybe a warning on the status page (requirements) that this module is disabling the active trail functionality within the context module?

Configuring a menu's Menu Trail Source to use Drupal Core Behavior will also again allow the context module to set the active menu trail (since it takes over core).

🇺🇸United States JonMcL Brooklyn, NY

I hope you all don't mind me creating merge request !5. It is for Drupal 10 compatibility and config_filter ^2.4 (necessary to Drupal 10 compatibility). !5 was made off of !4.

🇺🇸United States JonMcL Brooklyn, NY

JonMcL made their first commit to this issue’s fork.

🇺🇸United States JonMcL Brooklyn, NY

Patch in #8 works with with the 8.x-1.3 branch, even though this issue is tagged with the 8.x-2.x-dev branch.

🇺🇸United States JonMcL Brooklyn, NY

I think creating an issue fork would help make upgrading to D10 with this patch a little easier. (composer can fetch modules from issue forks on drupalcode). I was going to create it, but then I realized I might be attributed as the contributor.

🇺🇸United States JonMcL Brooklyn, NY

Sorry, my mistake. I thought I would create an issue fork so that the patched code can be pulled down by composer.

I didn't realize you already have Drupal ^9 || ^10 in the current dev branch. I can pull from that.

So please ignore the unnecessary issue fork I created.

🇺🇸United States JonMcL Brooklyn, NY

JonMcL made their first commit to this issue’s fork.

🇺🇸United States JonMcL Brooklyn, NY

Obviously an extremely simple MR :) I have no idea yet if there are incompatibilities between context_breadcrumb 2.0.1 and context 5.0.0-rc1

To be able to upgrade both modules, I added the the following changes to my composer.json:

    "repositories": [
        {
            "type": "composer",
            "url": "https://packages.drupal.org/8",
            "exclude": [
                "drupal/context_breadcrumb"
            ]
        },
        {
            "type": "package",
            "package": {
                "name": "drupal/context_breadcrumb",
                "version": "2.0.1@dev",
                "type": "drupal-module",
                "source": {
                    "url": "https://git.drupalcode.org/issue/context_breadcrumb-3369056.git",
                    "type": "git",
                    "reference": "2.0.x"
                }
            }
        }
    ],

Then I ran:
composer require 'drupal/context:^5.0@RC' 'drupal/context_breadcrumb:^2.0@beta'

🇺🇸United States JonMcL Brooklyn, NY

Per my update in #55, I think something like this is needed in EntityReference::validateExtraOptionsForm. This will be to handle entities with no bundles and possibly for selecting no bundles on the subform.


    // If no checkboxes were checked for 'target_bundles', store NULL ("all
    // bundles are referenceable") rather than empty array ("no bundle is
    // referenceable" - typically happens when all referenceable bundles have
    // been deleted or when an entity type has no bundles).
    if ($subform_options['target_bundles'] === []) {
      $subform_options['target_bundles'] = NULL;
    }

    // Store the sub handler options in sub_handler_settings.
    $form_state->setValue(['options', 'sub_handler_settings'], $subform_options);

🇺🇸United States JonMcL Brooklyn, NY

@DieterHolvoet: I just happened to bump up against a similar situation. In my case it's a custom entity type that has no bundles.

I think the issue is actually in DefaultSelection, which I do not think is being touched by this patch. I suspect that only users of this patch would be trying to use DefaultSelection with a bundle-less entity, so maybe that is why it hasn't been an issue before? I'm not entirely sure.

In DefaultSelection::buildEntityQuery, there is this snippet:

    // If 'target_bundles' is NULL, all bundles are referenceable, no further
    // conditions are needed.
    if (is_array($configuration['target_bundles'])) {
      // If 'target_bundles' is an empty array, no bundle is referenceable,
      // force the query to never return anything and bail out early.
      if ($configuration['target_bundles'] === []) {
        $query->condition($entity_type->getKey('id'), NULL, '=');
        return $query;
      }
      else {
        $query->condition($entity_type->getKey('bundle'), $configuration['target_bundles'], 'IN');
      }
    }

And in DefaultSelection::buildConfigurationForm, there is this condition:

    if ($entity_type->hasKey('bundle')) {
      // ..code removed for comment..
    }
    else {
      $form['target_bundles'] = [
        '#type' => 'value',
        '#value' => [],
      ];
    }

So I think that '#value' => []needs to become '#value' => null, but I am not entirely sure that is the best solution or what other impact that change would have elsewhere. Alternatively, EntityReference::buildExtraOptionsForm could be modified to set target_bundles to null if the entity type does have have bundles.

As a workaround, I ended up writing a custom EntityReferenceSelection plugin that extends DefaultSelection and changes target_bundles to null, when the entity has no bundles, inside ::buildEntityQuery. A little messy, but gets the job done (for now).

🇺🇸United States JonMcL Brooklyn, NY

No problem, other Jon :)

I didn't do much testing, primarily because flysystem_s3 need updates to work with the flysystem 3.0.x branch and my primary use case is S3. Please open another issue if you find problems. I'd be happy to investigate.

🇺🇸United States JonMcL Brooklyn, NY

Working well for us. We are using this to set a specific route parameter based on external information:

      /** @var \Drupal\Core\Url $url */
      $url = &$local_actions['local_action.id']['#link']['url'];
      $url->setRouteParameter('node', $node->id());

A word to the wise: don't be like me and remember that you may likely need to have a method of invalidating the cache for this rendered item.

🇺🇸United States JonMcL Brooklyn, NY

Yes, please!

Works well for us to chain through two entity reference fields.

🇺🇸United States JonMcL Brooklyn, NY

Adding my use case for this ..

We are in active development of a new Drupal application. Migration source is an Oracle database (non Drupal). We do not have any migrations created by Drupal itself. All of our migrations are being created like foo_migrate/migrations/*.yml. These migration configs are very rapidly changing and will go through many rounds of refinement before they can be considered ready for config/install or config/sync directories.

Our issue is that all our testing and staging environments are in Kubernetes clusters and ssh access is not readily available. So no easy drush execution.

It would be great to have a UI that can run theses migrations. I fully understand it is not a simple problem to solve. The current lists are based on displaying migrate_plus.migration configs while migrations that exist inside of a migrations/ folder are actually plugins. So maybe a separate UI that only lists plugins might be needed.

🇺🇸United States JonMcL Brooklyn, NY

So if I'm reading things correctly, this cannot be fixed in D7 until it is fixed in D8? (since it is a core patch and there is the backport policy)

So where does D8 patch at #85 stand? Is this still an issue in D8?

It would be great if this can be put into D7 for a future release. Some of us forget to re-apply core patches after we install security upgrades to core :)

Any suggestions for getting to to move forward with D7 or do we have to wait for the D8 fix no matter what?

Production build 0.71.5 2024