Thanks for the patch, @amanbtr72. Did you verify if your patch still works with Drupal 10.2?
vaish → created an issue.
Merge request !11 created from the patch in #17. I've been using this patch in production for very long time.
@Blush I didn't include your changes from #19 because I didn't experience any issues with anonymous users not being able to use address autocomplete. If you still think there is a problem, please test the new release of the module which will be created soon and create new issue with steps how to reproduce it. Thanks.
@geoffreyr I decided to merge patch from #2 because I've been using it for a along time in production while I'm not familiar with the seemingly unrelated changes you added to MR !6. Could you please open up new issue for your changes where you would explain which problem you are trying to solve.
@Sourav_Paul, I'm already working on this.
vaish → changed the visibility of the branch 3445097-warning-undefined-array to hidden.
Commit a1dedd2c was a mistake. It introduced a schema error and needs to be reverted.
This is expected behavior. Default value on the field edit page expects both country and phone number to be populated. If you only want to set default country, you should use form display setting.
vaish → made their first commit to this issue’s fork.
Thanks @chrisolof. Everything works great. I just made few minor tweaks.
Darell, there are two steps you need to complete before being able to use ASN-level rate limiting. You did download the ASN database already. What's left is to install PHP package geoip2/geoip2. That's what error message you got is about. You can install this package via composer, as usual.
composer require geoip2/geoip2
Please, let me know if you run into any other issues with this feature. I'm about to merge this MR but feel free to open a follow up issue if you find any bugs.
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.
Thank you.
I resolved the merge conflict and addressed the nitpicks. Moving to needs review.
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 .
Thanks @chrisolof.
Forgot to update credits.
Thank you, @zaryab_drupal.
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.
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
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".
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.
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:
- patch by chrisolof from #10
- commit 78502842 from merge request !7: Fix results not being returned in the correct language
- updated dependencies in libraries.yml to bring in compatibility with Drupal 10: Use
core/once
instead ofcore/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
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:
- Drupal core commit - see line 2234 in file
core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
- 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:
- Drupal 9 version 9.5.4 or higher
- Search API module configured to index content immediately upon save
- Search API module configured to index rendered entity
- Apache Solr configured as a search backend
- Content type with one of the following
- Regular page display with EVA (Entity Views Attachment) field
- 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
Patch attached.