Brussels
Account created on 22 March 2018, almost 7 years ago
  • Backend Developer at Minsky 
#

Merge Requests

More

Recent comments

🇧🇪Belgium dieterholvoet Brussels

For me the reason to do this in a queue is mostly about error handling, having failed requests left in the queue instead of just disappearing without being able to debug/retry later. I don't agree with removing the queue handling, I think it's a good approach. We could make it configurable though, for projects that don't really use queues.

🇧🇪Belgium dieterholvoet Brussels

@josh.stewart I added the changes from your patch to the MR. What do you mean by 'Might not be 100% perfect', anything specific to look out for?

🇧🇪Belgium dieterholvoet Brussels

It is still required, changes are required in both Gin and Gin Toolbar and neither issues have been fixed yet, so I'm re-opening.

🇧🇪Belgium dieterholvoet Brussels

You have marked this as Fixed, but you haven't merged the MR or committed the fix manually. Am I missing something?

🇧🇪Belgium dieterholvoet Brussels

Here's a simpler version of the patch posted in #11, for sites with only 1 language.

🇧🇪Belgium dieterholvoet Brussels

That works! Thanks.

🇧🇪Belgium dieterholvoet Brussels

The non-mandatory field automatically selects the first item from the entity reference field.

For the record, this only happens when the field accepts 1 value, not multiple values. I tested your changes, but it doesn't work as it should yet. When loading the form, the first result is not selected, but when submitting the form the following validation error is displayed:

This value should be of the correct primitive type.

This happens on a single cardinality, optional entity reference field with the Tagify Select widget.

🇧🇪Belgium dieterholvoet Brussels

This is a feature request, not a bug report, so there aren't any steps to reproduce here. If you're looking for an example: you don't need views for that, just creating a query in code is enough. Something like this:

$index = Index::load('nodes');
$query = $index->query();
$query->addCondition('type', ['article', 'page'], 'IN');
$results = $query->execute();
🇧🇪Belgium dieterholvoet Brussels

Closing this one as a duplicate of 🐛 Views "IN" filters not working Needs review since that one has a more complete solution.

🇧🇪Belgium dieterholvoet Brussels

Hitting the same issue. It's happening because Drupal\views_aggregator\Plugin\views\style\Table::preRender() creates a duplicate and executes it without saving. Down the line, in Drupal\views_data_export\Plugin\views\display\DataExport::attachTo(), $view->getUrl() is called and fails because the attached view doesn't have an ID yet, because it is new and unsaved.

I'm able to fix this in Views data export by calling isNew() before trying to generate an url. I believe that's the way to go, so I'm moving this issue to that project.

🇧🇪Belgium dieterholvoet Brussels

The maintainer doesn't have their contact tab enabled, so I can't contact them.

🇧🇪Belgium dieterholvoet Brussels

I added a working version in the MR. This includes most of the changes that were done in Add support for the Easy Responsive Images module Active . All that's left to do is add a setting to configure how long images that can't be associated with a file entity should be cached.

🇧🇪Belgium dieterholvoet Brussels

I added a working version in the MR. This includes most of the changes that were done in Add support for the Easy Responsive Images module Active . All that's left to do is add a setting to configure how long images that can't be associated with a file entity should be cached.

🇧🇪Belgium dieterholvoet Brussels

It also doesn't make sense to mark this module as compatible with Drupal 12, so I removed that. Drupal 12 isn't ready yet, there probably will be more upcoming breaking changes.

🇧🇪Belgium dieterholvoet Brussels

dieterholvoet changed the visibility of the branch 3498620- to hidden.

🇧🇪Belgium dieterholvoet Brussels

There's no need to start using the new base classes yet, the old ones will only be removed in Drupal 12 ( change record ). It's okay to revert back to using the old ones.

🇧🇪Belgium dieterholvoet Brussels

@foxy-vik why did you make the caption a plain text field instead of a formatted text field in #e2e9925? In the current version of the module it's also a formatted text field, so I don't see why you would change that.

🇧🇪Belgium dieterholvoet Brussels

Like @masipila says, that's what config overrides in settings.php are for. There's no need to add more code to this module.

🇧🇪Belgium dieterholvoet Brussels

No need to change the schema, integer seems appropriate for a port number. If the port number in your config is a string instead of an integer, re-saving the config form should fix that.

🇧🇪Belgium dieterholvoet Brussels

This is not necessary.

🇧🇪Belgium dieterholvoet Brussels

I don't see these links anymore on the project page , I'm assuming this is outdated.

🇧🇪Belgium dieterholvoet Brussels

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

🇧🇪Belgium dieterholvoet Brussels

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

🇧🇪Belgium dieterholvoet Brussels

I don't find any references to node_load_multiple() in the latest version of the module or its tests, so I'm assuming this is outdated.

🇧🇪Belgium dieterholvoet Brussels

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

🇧🇪Belgium dieterholvoet Brussels

My maintainership request has been approved, I'll work on getting a new release out in the coming days/week.

🇧🇪Belgium dieterholvoet Brussels

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

🇧🇪Belgium dieterholvoet Brussels

My offer has not been declined and I'm still interested in maintaining this project.

🇧🇪Belgium dieterholvoet Brussels

Can you elaborate on why Ngrok is considered a security risk?

🇧🇪Belgium dieterholvoet Brussels

The issue description is about two separate issues. I'll narrow this down to the DomainSourcePathProcessor issue, since @agentrickard already gave feedback on the (!$this->storage->listAll('domain.config.') check.

🇧🇪Belgium dieterholvoet Brussels

This change could mean a big performance improvement in the Domain module: 📌 Domain Config loading is resource intensive Needs work .

I started a MR with the changes from the patch. I also added route_parameters to the $options array, we need that to extract entity information from the URL.

🇧🇪Belgium dieterholvoet Brussels

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

🇧🇪Belgium dieterholvoet Brussels

Using route information from $options, so without doing any route matching ourselves, I got it down to +-15.9s. These changes are on the 3232343-use-info-from-options branch, but they do depend on a patch from 🐛 Outbound path processors miss the route name Needs work . That seems like the way to go, but we'll have to work on getting that core change in first.

🇧🇪Belgium dieterholvoet Brussels

dieterholvoet changed the visibility of the branch 3232343-performance-problem to hidden.

🇧🇪Belgium dieterholvoet Brussels

dieterholvoet changed the visibility of the branch 2.0.x to hidden.

🇧🇪Belgium dieterholvoet Brussels

By using PathValidatorInterface::getUrlIfValidWithoutAccessCheck() instead of Url::fromUserInput() and $this->aliasManager->getPathByAlias(), I'm able to reduce the time from +-46sec to +-34sec. Still slow, but it's something. I'll start a new MR with this change.

In an ideal world, we wouldn't have to do any route matching at all. $options['route'] already contains the matched route, the problem is that we don't know the route name, which is something that's being proposed in 🐛 Outbound path processors miss the route name Needs work . I'll also create a MR that builds on top of the change in that core issue.

🇧🇪Belgium dieterholvoet Brussels

The problem is that this module is inherently slow and there isn't much we can do about it. For every generated url, we have to check if there's an associated entity and if there is, we have to check its domain access field to know if we need to rewrite the domain. There isn't really a way around that, that's the whole point of the module.

According to my profiler, the most expensive calls are Url::fromUserInput() and $this->aliasManager->getPathByAlias(), they account for 80% of the time spent in the path processor. I'll check if these have cheaper alternatives.

🇧🇪Belgium dieterholvoet Brussels

Weird, I'm getting Issue fork creation failed! when clicking the button. I'll post a patch file.

🇧🇪Belgium dieterholvoet Brussels

Not very different anymore, the 4.x version of DBLog Filter also has the possibility to exclude logs based on the message. Maybe you could consider marking this module as unsupported, there's not much point in having multiple modules with the same functionality, but different names.

🇧🇪Belgium dieterholvoet Brussels

I'm encountering the same problem. Example: loading /admin/structure/menu/manage/admin takes 45.8 (!) seconds. With 3232343-2-domain_source_performance_fix.patch applied, it only takes 15.3 seconds. I understand that certain code is being disabled that shouldn't be, but a performance impact like this is unacceptable. It impacts every page of your website and especially the ones where a lot of urls are being generated. I'll try to look into this more.

🇧🇪Belgium dieterholvoet Brussels

Like the node type, it has a page(/node) that defaults to the homepage.

The /node page is the optional frontpage view that's offered as optional config of the node module. If displays all content that has the promote field set to true. ECK entities don't have a field like that, so what entities would you show on that page?

To be honest, this sounds a bit too niche to be offered automatically by the ECK module. A frontpage view makes sense, but an automatic view per ECK entity type is really only useful on specific kinds of sites.

🇧🇪Belgium dieterholvoet Brussels

I don't think you're supposed to do that in update hooks, since they can also be executed through the UI (update.php). You would need to check if the update hook is executed in a CLI context and you would also need to add a dependency on symfony/console.

🇧🇪Belgium dieterholvoet Brussels

The Drupal 7 release of this module and Drupal 7 itself are unsupported. Feel free to reopen this issue if it is still relevant on the latest release of the module.

🇧🇪Belgium dieterholvoet Brussels

The Drupal 7 release of this module and Drupal 7 itself are unsupported. Feel free to reopen this issue if it is still relevant on the latest release of the module.

🇧🇪Belgium dieterholvoet Brussels

The Drupal 7 release of this module and Drupal 7 itself are unsupported. Feel free to reopen this issue if it is still relevant on the latest release of the module.

🇧🇪Belgium dieterholvoet Brussels

The Drupal 7 release of this module and Drupal 7 itself are unsupported. Feel free to reopen this issue if it is still relevant on the latest release of the module.

🇧🇪Belgium dieterholvoet Brussels

The Drupal 7 release of this module and Drupal 7 itself are unsupported. It seems to have been fixed on the latest release of the module.

Production build 0.71.5 2024