Account created on 11 March 2009, over 16 years ago
  • Senior Developer at Phase2 
#

Merge Requests

More

Recent comments

🇺🇸United States recrit

@scott_euser thanks! That looks promising

🇺🇸United States recrit

@prudloff Not a hard bug, but maybe a warning / caveat for others. You can no longer run ` drush p:invalidate tag node:1`, it will not work. Anyone using "Simplify cache tags" should evaluate there pages to confirm the expected cache tags are present.

For my use case, I was able to use `hook_purge_cache_tags_simplify_dictionary_alter()` to ensure that the current page's entity context had tags for the node, group content / relation, and group. So that clearing any of those specific tags, clears those pages.

🇺🇸United States recrit

After doing some more testing with "Simplify cache tags" enabled, there is a fundamental issue with this feature.

The cache tag header is used by a Varnish to tag the cached response that it receives from Drupal. When cache tag simplification used, the "{ENTITY_TYPE}:{ENTITY_ID}" tag (e.g. "node:1234") is removed from the header value when the node is saved.
For example:
- Drupal cache tags for the headers: "node:1234", "node_list" . The list tag is added by `EntityBase::invalidateTagsOnSave()`.
- Simplified tags: "node_list"
- RESULT = BUG: Varnish does not tag the node page with it's own tag "node:1234". So if the Drupal cache tag "node:1234" is invalidated by itself, then the Varnish page is not invalidated. You have to invalidate it with "node_list" which is wrong.

Tag simplification removes the ability to invalidate a single node tag, e.g. "node:1234".

🇺🇸United States recrit

I create an MR with the changes for the Zero config purger to support minifying the tokens in Minify the cache tags sent in the header Needs work . This is slightly different than the original 2020 patch so that token validation works and the tag is only processed one time.

Attached is a static patch of the MR for any composer build.

🇺🇸United States recrit

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

🇺🇸United States recrit

Uploading a static patch for the latest MR updates that supports simplifying {ENTITY TYPE}_list:{ENTITY BUNDLE}. Example: "node_list:article".

🇺🇸United States recrit

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

🇺🇸United States recrit

there is a similar fix added in this older issue 🐛 Handle Missing $data['invalidations'][0] in Invalidation Check Active

🇺🇸United States recrit

Attaching a static patch of the MR6 to use in any composer build.

Alternatively, if the module did not define the property $reverseProxies as private, then implementers could extend the class and set the $reverseProxies more specific for their use case.

🇺🇸United States recrit

@scott_euser thanks for the video. That would be great if we had access to the updated entity object in the AJAX call.

🇺🇸United States recrit

@scott_euser From what I can tell all of AI module's Content Suggesters and field actions for taxonomy still only use a text field to get the text used for the suggestion. The main advantage of the ai_auto_reference module is that it allows a rendered view mode to be used to get the text used for the suggestion. This is useful on sites that do not have a simple "body" field.

🇺🇸United States recrit

Thank you. The changes look good to me.

🇺🇸United States recrit

@shayaanmiyy I also noticed that the "ai" dependency should be "ai:ai", not "drupal:ai" since "ai" is not a submodule of Drupal itself.

🇺🇸United States recrit

@shayaanmiyy thanks. Ignore the "3545171-auto-apply-does" branch. I accidentally pushed that to this issue fork repository.

🇺🇸United States recrit

recrit changed the visibility of the branch 3545171-auto-apply-does to hidden.

🇺🇸United States recrit

Changed the issue title back to the original which reflects all of the changes in this issue.

I updated the MR with the change in patch #67 since that patch no longer applied to 2.0.x. The only changes that I saw was this - https://git.drupalcode.org/project/facets/-/merge_requests/291/diffs?com...


      if (reload) {
        href = addExposedFiltersToFacetsUrl(href, options.extraData.view_name, options.extraData.view_display_id);
-        options.url = addFacetsToExposedFilterRequest(options.url, href);
+        const url = new URL(href);
+        const relativeUrl = url.pathname + url.search + url.hash;
+        options.url = addFacetsToExposedFilterRequest(options.url, relativeUrl);
        updateFacetsBlocks(href);
      }

Attached is a static patch of the MR for any composer builds.
Please contribute any further updates to the MR.

🇺🇸United States recrit

@cyberwolf thanks for the review. I'll make the updates when I get a few minutes.
For extending the core class - Yes, it was needed. Other modules such as https://www.drupal.org/project/gcontent_moderation do the same by extending core's class.
For reference, I have gcontent_moderation installed along with content_moderation_permissions and they are working well together.

🇺🇸United States recrit

MR4 created with the following changes:

  • Updated the service content_moderation_permissions.state_transition_validation to decorate core's service content_moderation.state_transition_validation so that the duplicated code from the content_moderation module is not needed.
  • Removed the Field widget "moderation_state_permissions_default". An update is provided to set the widget configuration back to the "moderation_state_default".
  • Removed the field constraint / Validation plugin: "ModerationStatePermissions"
🇺🇸United States recrit

@cyberwolf I agree. That is what the service's "decorates" property was meant to do. The "decorates" let's you extend the parent service - content_moderation.state_transition_validation in this case - without overriding the entire service.
I am working on a PR to do this.
We will be stuck with keeping the "Moderation state permissions" field widget to support existing sites using it. We could mark as deprecated since it is no longer needed. Alternatively, we could create an update to set them all back to Drupal core's field widget.

🇺🇸United States recrit

If anyone got to this issue and wondered why your views data export display is still showing in the front end theme instead of the admin theme, it is most likely because your view path does not start with "/admin". This is because the change for this issue only targeted those paths - see https://git.drupalcode.org/project/views_data_export/-/commit/fcafcfeac3....

This can be fixed in your build by implementing a route subscriber. See https://www.drupal.org/docs/drupal-apis/routing-system/altering-existing... for details on how to create one.

Example: The following will set all "data_export" views displays to use the admin theme.


namespace Drupal\your_custom_module\Routing;

use Drupal\Core\Routing\RouteSubscriberBase;
use Symfony\Component\Routing\RouteCollection;

/**
 * Listens to the dynamic route events.
 */
class RouteSubscriber extends RouteSubscriberBase {

  /**
   * {@inheritdoc}
   */
  protected function alterRoutes(RouteCollection $collection) {
    foreach ($collection->all() as $name => $route) {
      // Set all views_data_export diplays to use the admin theme.
      if (str_starts_with($name, 'view.') &&
          ($display_plugin_id = $route->getOption('_view_display_plugin_id')) &&
          $display_plugin_id === 'data_export') {
        $route->setOption('_admin_route', TRUE);
      }
    }
  }

}
🇺🇸United States recrit

attached a static patch of MR31 for composer builds.

🇺🇸United States recrit

I created a new branch 3400166-use-purge-api (MR 10) that extends the original updates in MR 6 and refactors to use the purge plugins and API.

  • Implements its own purge plugins for a queuer and processor
  • There is only a selection for the new "Workflow", eliminating the selection for a queuer and processor since that is no longer needed.
  • Added warnings on the settings page and status report regarding supported URL purgers.
  • Updated the module code to support the new workflows.
  • Added an update function to convert the old configuration to the new configuration

Attached is a static patch of MR10 for composer builds.

🇺🇸United States recrit

@jonhattan Regarding "Purge API is not being used the right way by the module and thus, by this MR.", that is your opinion.
The settings form for the purge_file module follows what would be expected by a Drupal contrib module that implements a plugin system - select a plugin that affects the system. However, the purge module's plugins actually do not do anything other than provide labels for log messages. For example - In \Drupal\purge\Plugin\Purge\Queue\QueueService::add(QueuerInterface $queuer, array $invalidations)>code>, the <code>$queuer argument is meaningless and is only used for the log message.

public function add(QueuerInterface $queuer, array $invalidations) {
    foreach ($invalidations as $invalidation) {
      if (!$this->buffer->has($invalidation)) {
        $this->buffer->set($invalidation, TxBuffer::ADDING);
      }
    }
    $this->logger->debug("@queuer: added @no items.", [
      '@queuer' => $queuer->getPluginId(),
      '@no' => count($invalidations),
    ]);
  }

Another example is the processor plugin. The purge module's own submodule purge_processor_cron implements its own hook_cron and then just passes the "cron" plugin to \Drupal\purge\Plugin\Purge\Purger\PurgersService::invalidate(ProcessorInterface $processor, array $invalidations)<code> which the <code>$processor argument is never used, any code validation would detect it as "unused".

Any plugin for the purge module's system does not do anything without additional custom code.
So one might ask the alternate question - why does the purge module even have plugins?

🇺🇸United States recrit

I created the MR 5 with the new hook. Attached is a static patch that can be used for composer builds. Any further development should be done in the issue branch.

🇺🇸United States recrit

For Drupal 10.4.x+ support along with jQuery UI updated to 1.14.1 in the jquery_ui module, test the MR 20 on 📌 Update to jQuery UI 1.14.0 Active , see comment #15 📌 Update to jQuery UI 1.14.0 Active for details. The MR 20 📌 Update to jQuery UI 1.14.0 Active includes the changes from 📌 Update to jQuery UI 1.14.0 Active and this issue 🐛 Remove duplicate modules in JavaScript bundles Active .

🇺🇸United States recrit

@flyke and @nelo_drup please test the MR 20 see comment #15 📌 Update to jQuery UI 1.14.0 Active for details.

🇺🇸United States recrit

I opened a new MR 20 that is MR 19 with the changes from 🐛 Remove duplicate modules in JavaScript bundles Active . The result is that jquery_ui is updated to 1.14.1 similar to Drupal core after 10.4.x and gives jquery_ui full control over the files with the changes from 🐛 Remove duplicate modules in JavaScript bundles Active .

Attached is a static patch of MR 20 for any composer builds.

🇺🇸United States recrit

recrit changed the visibility of the branch 3420890-remove-duplicates--after-3483054-jqueryui-1.14.1 to hidden.

🇺🇸United States recrit

@flyke - with still having errors, then you are most likely running into 🐛 Remove duplicate modules in JavaScript bundles Active where some of the core files are still loaded. The patch in 3420890 gives the jquery_ui module full control over all the JS files.

I am working on updating the patch to work with the updates in this issue for 1.14.1 since Drupal 10.4.x includes that version too.

🇺🇸United States recrit

@v.dovhaliuk - you may want to try the fix on 🐛 Re-order + remove broken with the Entity Reference (and File) widget Needs review instead since it has some cardinality fixes as well.

🇺🇸United States recrit

Attached is a static patch of the latest MR that can be used for composer builds.

🇺🇸United States recrit

Attached is a static patch of the latest MR that can be used for composer builds.

🇺🇸United States recrit

MR 22 created with the latest 1.0.x-dev and using Drupal's FileExist enums. I also added some error catching since the file move operation can throw a few errors, most notably when the file does not exist which commonly occurs when using the stage_file_proxy module.

🇺🇸United States recrit

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

🇺🇸United States recrit

attached is a static patch for any composer builds

🇺🇸United States recrit

created MR with a cleaner unset based on the array key instead of the loop variable.
attached is a static patch that can be used for composer builds.

🇺🇸United States recrit

Attached is a static patch of the MR that applies to tmgmt 8.x-1.17

🇺🇸United States recrit

@fskreuz your original MR was ignoring the already created list of fields $embedded_fields = ContentEntitySource::getEmbeddableFields($entity); which is already patched for computed fields in this issue's changes.

🇺🇸United States recrit

@nelo_drup This patch does not affect jQuery UI libraries, for example jquery_ui_accordion/accordion. It only replaces JavaScript files in the Drupal Core libraries with the equivalent files in the jQuery UI module's assets directory.
For that reason, it would not be causing the issues that you are seeing with jquery_ui_accordion/accordion.

I would suggest doing some debugging:

  • Are there any JavaScript errors on the page in the developer console? Use the browser's developer tools to inspect the page.
  • Check the Drupal watchdog logs for any errors
  • Verify that "jquery_ui_accordion" is installed
  • Do you have any custom code - JavaScript or Drupal - that could be interfering with it
  • If you have access to Drush on the site, then post the output of the following commands in order to see the library definitions:
    • drush ev "echo print_r(\Drupal::service('library.discovery')->getLibraryByName('jquery_ui_accordion', 'accordion'), TRUE);"
    • drush ev "echo print_r(\Drupal::service('library.discovery')->getLibraryByName('faqfield', 'faqfield.accordion'), TRUE);"
🇺🇸United States recrit

@nelo_drup The faqfield module does have the correct dependency for jquery_ui_accordion/accordion. The change in this ticket should not affect that dependency.

What are the errors that you are seeing?

What version of faqfield are you using?

🇺🇸United States recrit

The patch and MR need updated to address the issue described in #7 🐛 modal dialog close button doesn't reset buttons that trigger modal Needs work

Regarding the comment in #9 🐛 modal dialog close button doesn't reset buttons that trigger modal Needs work : You can have more than one dialog open at a time. If have nested entities using the entity browser to edit or select the reference fields on them.

🇺🇸United States recrit

marking as needs review. the MR tests are failing for unrelated PHP unit tests.

🇺🇸United States recrit

This is not a duplicate of 🐛 modal dialog close button doesn't reset buttons that trigger modal Needs work . This is issue is specific to the "Replace" option.

🇺🇸United States recrit

I recently ran into this issue. When Drupal.detachBehaviors() is called with an empty context, it will numerous issues. Any JS implementing a detach will remove everything that it did in attach. For example, any autcomplete UX added by core/misc/autocomplete.js.

I created the MR with the 1 line fix.
The attached static patch can be used for composer builds and applies to both 11.x and 10.3.x+.

🇺🇸United States recrit

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

🇺🇸United States recrit

MR 39: Updates the existing bynder_metadata to output a single metadata attribute from the JSON field value.

Pending automated tests.

🇺🇸United States recrit

Created MR 38 that adds the loading attribute support.

🇺🇸United States recrit

let's try adding that 10.3.x patch again

🇺🇸United States recrit

MR passed. Attached is an equivalent 10.3.x for any builds still using that.

🇺🇸United States recrit

@peterwcm thank you. Using the node language worked for me and is a lot cleaner. I will update the PR.

🇺🇸United States recrit

@drunken monkey - MR looks good to me and works as expected. There is conflict with src/Utility/AutocompleteHelper.php but other than that looks good. Thanks!

🇺🇸United States recrit

@drunken monkey - I am using the better_exposed_filters . It appears that it builds the element with its element info process callbacks before any form alter would be called, for example "search_api_autocomplete_form_views_exposed_form_alter".

MR Updates:
I have added tests to simulate the better_exposed_filters exposed form plugin.
I manually ran the "test-only" tests on the MR and it failed as expected for "There should not be duplicate element #process callbacks."

better_exposed_filters: 6.0.6 or 7.0.2: \Drupal\better_exposed_filters\Plugin\views\exposed_form\BetterExposedFilters::exposedFormAlter().

    // Ensure default process/pre_render callbacks are included when a BEF
    // widget has added their own.
    foreach (Element::children($form) as $key) {
      $element = &$form[$key];
      $this->addDefaultElementInfo($element);
    }

Drupal: 10.3.10
better_exposed_filters: 6.0.6
"#process" IN:

['Drupal\Core\Render\Element\Textfield',' processAutocomplete']
['Drupal\Core\Render\Element\Textfield',' processAjaxForm']
['Drupal\Core\Render\Element\Textfield',' processPattern']
['Drupal\Core\Render\Element\Textfield',' processGroup']

WITH BUG:
"#process" OUT:

['Drupal\Core\Render\Element\Textfield',' processAutocomplete']
['Drupal\Core\Render\Element\Textfield',' processAjaxForm']
['Drupal\Core\Render\Element\Textfield',' processPattern']
['Drupal\Core\Render\Element\Textfield',' processGroup']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processSearchApiAutocomplete']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processAutocomplete']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processAjaxForm']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processPattern']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processGroup']

WITH THE FIX IN THIS ISSUE:

['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processSearchApiAutocomplete']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processAutocomplete']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processAjaxForm']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processPattern']
['Drupal\search_api_autocomplete\Element\SearchApiAutocomplete',' processGroup']
🇺🇸United States recrit

Pushed some fixes for better error handling. This is now working for me when Vimeo does not have a high resolution option.
Thinking about this more, I feel like it could be better handled in \Drupal\media\Plugin\media\Source\OEmbed::getLocalThumbnailUri() so that the high resolution image is only requested once and then the local file is created. Currently, the fixes in this issue would causes the high resolution images to be request every time the resource cache is rebuilt. This seems a bit excessive to me.

🇺🇸United States recrit

There are a few issues with the patch in #19:

  1. In the new method doVimeoRequest, the TransferException is not defined and needs to be imported.
  2. the import "use GuzzleHttp\Client" is never used.
  3. The fallback for Youtube should be uncommented:
          catch (RequestException $e) {
            // Use default thumbnail.
            //$data['thumbnail_url'] = str_replace('maxresdefault', 'hqdefault', $data['thumbnail_url']);
          }
    
🇺🇸United States recrit

The MR build is failing for phpstan errors unrelated to the 1 line change in the MR.
Error from phpstan:

 ------ ------------------------------------------------------------------------------------------------------ 
  Line   src/Controller/NodeRevisionController.php                                                             
 ------ ------------------------------------------------------------------------------------------------------ 
         Ignored error pattern #^Method                                                                        
         Drupal\\Core\\Form\\FormBuilderInterface\:\:getForm\(\) invoked with                                  
         2 parameters, 1 required\.$# in path                                                                  
         /builds/issue/diff-3488283/web/modules/custom/diff-3488283/src/Controller/NodeRevisionController.php  
         was not matched in reported errors.                                                                   
 ------ ------------------------------------------------------------------------------------------------------
🇺🇸United States recrit

MR is ready for review. Attached is a static patch for composer builds.

🇺🇸United States recrit

code cleanup complete.
Attached is a static patch of the MR to use for composer builds.

🇺🇸United States recrit

Use the MR 109 for the latest changes applied to 1.8+. Attached is a static patch of the MR to use for composer builds.

🇺🇸United States recrit

attached is a backport patch for 6.0.x for anyone that has not updated to 7.0.x yet.

🇺🇸United States recrit

MR 105 created with the following:
- Maintains the same intended solution as the original - find the element with "data-bef-auto-submit-delay" and attach listeners to it.
- Finds the correct parent with "[data-bef-auto-submit-full-form]" even when there are intermediate elements in between "[data-bef-auto-submit]" and "[data-bef-auto-submit-full-form]".
- Changed the "$form" variable to "$needsAutoSubmit" since not all elements found with the selectors are forms.

Production build 0.71.5 2024