🇷🇸Serbia @vaish

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

vaish → created an issue.

Thanks for reporting the bug. I changed the default value from NULL to 'flag' to make it consistent with the other parts of the module. @nnkavya could you please confirm this MR still resolves your issue.

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

Thank you.

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

I resolved the merge conflict and addressed the nitpicks. Moving to needs review.

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

I run into this issue after the upgrade to 4.2.6. Solution suggested in comment #10 worked for me. VBO field doesn't serve any purpose in the "data export" display.

I would rather try to avoid invoking any hooks in this module. In order for the module to be really useful, it should be able to process each request as early and as fast as possible. Invoking hook adds an overhead which is avoidable. For the same reason I decided not to use Drupal config and create regular configuration page.

Thanks for the feedback and let me know if you happen to decide to work on this.

This would be great improvement. Are you planning on creating a MR? Note that issue ✨ [Patch provided] Allow administrators to specify IP addresses and ranges which should never be rate limited Needs review provides a setting for ignoring IP addresses. We should make sure that the structure of the settings array and naming of the array keys remains consistent and intuitive after both settings have been added. E.g. both setting should use the same verb, either "ignore" or "allow", etc.

Thanks @generalredneck. It's great to have this added to the README.

Thanks for the screenshots. I looked into this and I can reproduce the issue.

If checkbox "My billing information is the same as my shipping information." is checked during the checkout, commerce will populate "Billing information" fields with the values taken from the "Shipping information" section. Value of the Phone field is not copied successfully and Phone field in the "Billing information" remains empty causing the validation error reported here. So, this error refers to the empty field in the "Billing information" section.

Copy of the values is initiated in the Drupal\commerce_shipping\ProfileFieldCopy::alterForm()

      // Copy over the current shipping field values, allowing widgets such as
      // TaxNumberDefaultWidget to rely on them. These values might change
      // during submit, so the profile is populated again in submitForm().
      $billing_profile->populateFromProfile($shipping_profile, $shipping_fields);

Drupal\profile\Entity::populateFromProfile() depends on the TypeData plugins to correctly set values for each of the fields. Both Phone number and Address fields implement Drupal\Core\TypedData\Plugin\DataType\Map plugin. Map::setValue() method is responsible for setting the values.

    // Update any existing property objects.
    foreach ($this->properties as $name => $property) {
      $value = $values[$name] ?? NULL;
      $property->setValue($value, FALSE);
      // Remove the value from $this->values to ensure it does not contain any
      // value for computed properties.
      unset($this->values[$name]);
    }

Problem is that in the case of the Phone Number $this->properties is empty (properties are not set) and therefore no values are set. Phone field defines the following properties: 'value', 'country', 'local_number', and 'extension' but for some reason those properties are not available in the setValues() method.

As a comparison, Address field defines its own properties ('address_line1', 'locality', 'country_code', etc) and those are all available in the setValue() method and values are correctly copied.

Unfortunately, I don't have time to look into this further right now. I hope somebody else can chime in.

Are you getting this error even after you populate the Phone field? Please clarify. Screenshot might be helpful here.

Thanks @orakili. MR looks good to me (both code and functionality) and if you are fine with the changes I made I think this can be moved to RTBC.

I run into this after installing content_entity_clone module on a site where admin_toolbar_tools is enabled and "Enable/Disable local tasks display" setting is turned on. Content entity clone uses hook_menu_local_tasks_alter() to create new local task but it doesn't define #active key. Fix is included in the MR for ✨ Admin content add link in operation Needs review . Unfortunately, I had trouble reproducing the issue. I'm getting it on a project with many contrib and custom modules but not on the clean Drupal install. Judging by the previous comments, issue is extremely rare but it may happen under very specific circumstances.

I was about to create new issue when I noticed that it was already fixed in this MR so I decided to add an additional related fix here instead of creating new issue. These are the issues I'm referring to:

1. Query string is missing when "Clone" local task is displayed in the admin toolbar. This requires that admin_toolbar_tools module is enabled and "Enable/Disable local tasks display" setting is turned on. This has been fixed in this MR by adding the third parameter $options to the Url::fromRoute() call.

  $options = [
    'query' => [
      'content_entity_clone' => $entity->id(),
    ],
  ];
  $url = Url::fromRoute($entity_creation_route_name, $entity_creation_route_parameters, $options);

2. I also run into the issue 🐛 Undefined array key "#active" in Drupal\admin_toolbar_tools\AdminToolbarToolsHelper->buildLocalTasksToolbar() Needs review . As per the docs local task is expected to have "#active" key but in this case key was not defined. Unfortunately, I had trouble reproducing this issue. I'm getting it on a project with many contrib and custom modules but not on the clean Drupal install. I still think it makes sense to define this key. Value will always be FALSE because it's impossible for "Clone" tab to ever be active.

3. I also fixed some minor typos.

Additionally, I noticed that this MR also fixes 🐛 Where is the link to create a clone from node? Closed: duplicate . Marking that other issue as duplicate.

Fix for this issue included in ✨ Admin content add link in operation Needs review .

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

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

Thanks @chrisolof.

Forgot to update credits.

Thank you, @zaryab_drupal.

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

Thanks @mpaler. All these years we were using the patch from #7 which did the job for us. But recently, the module got some updates and the patch couldn't be applied anymore. So instead of re-rolling the patch that will never be merged I worked on the proper solution. Better late than never.

vaish → created an issue.

Thanks for the review, Chris. Composite element 'country-code' removed.

Commit in #15 improves upon the previous version in several ways.

1. Just hiding flag creates ambiguous situation for calling code +1 which is used by multiple countries. It's not possible to know which country is actually selected by looking at the country calling code alone. To solve this issue, the new commit displays two-letter country code when the flag is hidden.

2. Previous version was incomplete, it was covering only webform element but not regular Drupal core field. New commit adds the following missing features:

• implement "Country selection" field widget setting with two options: "Flag" and "Two letter country code".
• add new setting to the field schema
• webform element: move "Country selection" setting into the Phone number settings fieldset for better usability.
• add tests for the new feature

3. This MR also includes fix for the failed phpunit test FunctionalJavascript\SelectCountryTest::testCountrySelect(). Failure was caused by a change introduced in Drupal 10.2. For details see comment #121 in Drupal core issue 📌 Stabilize FunctionalJavascript testing AJAX: add ::assertExpectedAjaxRequest() Fixed . In this particular case usage of assertWaitOnAjaxRequest() was not needed.

4. Since I was already working in this area, I also did some clean up of inefficient JavaScript code in setCountry() function.

• I chained removeClass() and addClass() methods. Previously code was doing two independent tree traversals.
• I removed unnecessary for loop that was going through all the options in Country select list in order to find the selected one. Selected option can be accessed directly without the loop.

Form display

Field widget settings

Webform element configuration

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

But MR showing an "Open Failed Pipeline" message so moving to "needs work"

None of the errors are related to this MR. Moving back to "needs review".

vaish → created an issue.

Here is a patch corresponding to the merge request from #10.

Hello Alex,

I'm really glad to hear you are finding the module useful. Thanks for the patch. I still didn't have time to review it but this feature is something that I find useful addition to the module. Would you mind creating a merge request. It might be easier for me to review it that way.

Best,
Vaish

@czigor You are right, I did overlook backward compatibility. However, Drupal 9 should not be a problem, unless you are on a very old version. Method ::guessMimeType() is available since Drupal 9.1 (but not in Drupal 9.0). So problem exists with Drupal 9.0 and 8.x and I did overlook that this module still claims the compatibility with Drupal 8.9.

Drupal 9: https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21File%21Mi...

This patch adds the missing license URL.

vaish → created an issue.

Although module is compatible with Drupal 10, it's not possible to install stable release via composer because of the outdated version constraint in the composer.json file. This got fixed in the 2.x branch but only after version 2.0.2 was released. Considering 2.0.2 is the only stable D10 compatible release, it's probably time to release new minor version with this fix.

Patch #2 fixes the issue for me. However, I don't think if condition in the patch is necessary. Service file.mime_type.guesser is provided by the class Drupal\Core\File\MimeType\MimeTypeGuesser which implements interface Symfony\Component\Mime\MimeTypeGuesserInterface. I don't see an opportunity to ever get anything else.

This should be sufficient:

-      $image->filemime = \Drupal::service('file.mime_type.guesser')->guess($image_path);
+      $image->filemime = \Drupal::service('file.mime_type.guesser')->guessMimeType($image_path);

Tested patch #12. It's working. Thanks.

Here is a new version of the patch from #2. Tested both on Drupal 9.5 and Drupal 10.1. Main difference is a fix for an error

TypeError: Drupal\Component\Plugin\PluginManagerBase::createInstance(): Argument #2 ($configuration) must be of type array, null given

which occurs when attempting to edit provider settings for the very first time while they are still undefined. In addition to that, I converted variable into a constant and injected Guzzle Http client instead of instantiating it in the constructor.

Note: If you want to use this patch on Drupal 10, you also need to apply #3349849-12: Drupal 10 compatibility → .

This patch incorporates the following:

1. patch by chrisolof from #10
2. commit 78502842 from merge request !7: Fix results not being returned in the correct language
3. updated dependencies in libraries.yml to bring in compatibility with Drupal 10: Use core/once instead of core/jquery.once

Patch is compatible with both Drupal 9.5 and 10.1. If you want to use it with Drupal 10 you will also need #3349849-12: Drupal 10 compatibility → .

I decided not to push my updates to the existing merge request !7 because, I wanted to exclude two commits related to "Filter to full addresses or parts of addresses". In my opinion behavior introduced by those two commits may not match user expectations. When you start typing into the Street address field what you are entering is part of the street name. However, with the commits I excluded, results returned at first include only matching city names, not a single matching street name. Only after you type at least a whole word or more you start seeing street matches. I can image that this can be desired in certain use cases but I don't think it should be default module behavior. Perhaps this could be added as an additional feature in a separate issue.

Here is the patch that corresponds to the current state of Merge Request !7

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

Patch updated to resolve issue with cweagans/composer-patches not being able to apply it to a Drupal project. Previous patch removed the line version: 8.x-1.3.0 from info file which caused conflict because this line gets commented out when module is installed via composer. This line ultimately needs to be removed from the info file but that will have to be done separately.

In addition I also added missing @param to a doc block and removed single quotes from a version constraint in the info file.

This patch brings in full compatibility with Drupal 10. Module should now be fully functional when installed on Drupal 10.

I see. You want a proof that module does what it's supposed to do. Here is a simple way to verify that:

In settings.php configure very low number of requests per interval for "bot_traffic". For example 4 requests within 30 seconds. Then use curl on command line to issue requests to your website. First four requests should return response code 200 while the fifth and subsequent ones should return 429. After 30 seconds, counter resets and you will again be able to get 200 but only for 4 requests.

Here is a curl issuing HTTP requests as a bot: curl --head -A "bot" "https://yourdomain.com/"

You can do similar test in your browser but for this you need to use the settings under "regular_traffic". Then just keep refreshing any page on your website and keep track of the responses you get.

On production you can grep your web server's access.log for entries with response code 429. Those will be the ones that were blocked by crawler rate limit.

Note that crawler rate limit, being a Drupal module, processes only requests that are handled by Drupal. It doesn't do anything to requests that are served directly by the web server, like for example requests for images in the public files folder or requests for CSS and JS assets.

Did you check module's README.md: https://git.drupalcode.org/project/crawler_rate_limit/-/tree/3.x?ref_typ...

Status Report page is mentioned there. See step 3 under "Install Crawler Rate Limit module".

If you have any specific feedback on how to improve README file, please let me know.

You can go to Drupal's Status Report page (/admin/reports/status) and find all the details there. If something is not right you will also see a warning or an error.

Module doesn't use any database tables and doesn't post log messages. That's intentional.

The only place module stores any information is the backend you specified in the module settings. In your case that's Redis. All keys created by this module are prefixed with crawler_rate_limit:.

I started getting this Runtime Exception after upgrading Drupal 9 core and Search API module recently.

It's triggered when Search API attempts to render the field that contains "rendered item".
File: search_api/src/Plugin/search_api/processor/RenderedItem.php
Method: function addFieldValues()
Line 292: $value = (string) $this->getRenderer()->renderPlain($build);

It appears that issue was introduced by a change in Drupal core:

1. Drupal core commit - see line 2234 in file core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
2. Related Drupal core issue: Views pager is using exposed_raw_input instead of exposed_input 🐛 Views pager is using exposed_raw_input instead of exposed_input Fixed

Above commit introduced call to $view->getExposedInput() which in turn calls $session->get('views') (see web/core/modules/views/src/ViewExecutable.php). I confirmed that by commenting out $session->get('views') calls within getExposedInput() method I can get rid of the Runtime Exception.

In order to reproduce the issue you need the following:

1. Drupal 9 version 9.5.4 or higher
2. Search API module configured to index content immediately upon save
3. Search API module configured to index rendered entity
4. Apache Solr configured as a search backend
5. Content type with one of the following
   1. Regular page display with EVA (Entity Views Attachment) field
   2. Layout managed by the Layout Builder that includes views block or EVA field

Most of the times above configuration will cause the Runtime Exception but I did notice few instances where that didn't happen. I'm not sure exactly why. My guess is that views configuration in those cases is such that rendering of the view block or EVA field skips the block of code that is responsible for throwing the exception.

Same change was committed to Drupal 10 so I'm assuming this issue will be present in Drupal 10 as well but I didn't verify that personally.

I'm leaving it to module maintainers to decide whether this issue can be fixed within Search API module or it needs to be moved to Drupal core issue queue.

Patch updated as per #4

vaish → created an issue.

Patch attached.

vaish → created an issue.

vaish → created an issue.

vaish → created an issue.

Version 2.1-beta1 of module introduced a lot of changes and improvements and hopefully this issue is not relevant anymore.

Non-crawler rate limit has been added in the release 2.1.

Support for memcache has been added with the release 2.1 which is currently in beta.

@zcht It's been a long time so I don't know if you are still interested but I finally got a chance to create new version of the module with the support for multiple backends. This version supports Memcached, Redis and APCu. I didn't make the release yet, waiting to get some feedback first but code should be stable and usable. I would appreciate if you could give it a try.

Up to date documentation and installation instructions can be found in the README file: https://git.drupalcode.org/project/crawler_rate_limit/-/blob/2.x/README.md

This version requires PHP 7.4 or higher and Drupal >= 9.4 or Drupal 10.

@bobburns I have new version of the module ready if you would be willing to try it. I didn't make the release yet, waiting to get some feedback first but code should be stable and usable.

New version supports both Redis PECL extensions as well as Predis PHP package. In addition you can also chose to use APCu or Memcached. Up to date documentation can be found in the README file: https://git.drupalcode.org/project/crawler_rate_limit/-/blob/2.x/README.md

You can install this version and all dependencies (Predis package and Redis module) with the following commands:


composer require predis/predis:^1.1.1 drupal/redis drupal/crawler_rate_limit:2.x-dev@dev
drush en redis crawler_rate_limit


I tested this version with Drupal 9 and 10 and PHP 7.4 and 8.1.