🌌
Account created on 26 May 2009, about 15 years ago
#

Merge Requests

More

Recent comments

🇪🇸Spain manuel.adan 🌌

Another approach made to this in branch 1255092-front_page_outbound_url_path_processor, based as well in URL processor but outside of URL alias as suggested by Berdir at #66 🐛 url() should return / when asked for the URL of the frontpage Needs work . I added outbound URL processing to the existing front page path processor.

It may fail in case of a front page path with query parameters were set, but it is also not well managed by other core parts like PathMatcher, see 🐛 Front page path with query parameters makes front path condition to fail Active . Added as related issue since support for query parameters should be added.

🇪🇸Spain manuel.adan 🌌

Inbound path processor replaces response composition by sub-request the content entity canonical path when accessing the holder on its URL.

Tested aliasing both the holder path and the content entity.

🇪🇸Spain manuel.adan 🌌

Branch 3438332-automated-drupal-11 tested on current D11-dev. Session seems to be required in subrequest by newer synfony version used in D11.

🇪🇸Spain manuel.adan 🌌

manuel.adan changed the visibility of the branch 3438103-request-path-condition to hidden.

🇪🇸Spain manuel.adan 🌌

Created 3422897-coding_standards issue branch from the static patch file 3422897-2.patch.

Several reviews on top on that, ready to merge.

🇪🇸Spain manuel.adan 🌌

manuel.adan changed the visibility of the branch 3422897-fix-the-issues to hidden.

🇪🇸Spain manuel.adan 🌌

manuel.adan made their first commit to this issue’s fork.

🇪🇸Spain manuel.adan 🌌

Extended the issue to the pre query method as well.

I found that current implementation calls pre query method after building the request params, so query alterations have no effects. It was fixed.

Now the active issue branch is 3435998-pre_post_query_events

🇪🇸Spain manuel.adan 🌌

manuel.adan changed the visibility of the branch 3435998-post_query_event to hidden.

🇪🇸Spain manuel.adan 🌌

Moving to fixed since it is already committed and published into the last alpha4 release.

🇪🇸Spain manuel.adan 🌌

Patch file with static plain diff of issue 3435998-post_query_event and 8.x-7.x branches. Contains the event definition and trigger. Also deprecates the postQuery method.

🇪🇸Spain manuel.adan 🌌

Patch file attached containing plain diff of current state of branches issue/3102388-nodespark_des_connector_data_source and origin/8.x-7.x for convenience only.

🇪🇸Spain manuel.adan 🌌

Created new branch 3102388-nodespark_des_connector_data_source in the issue fork for works on the 8.x-7.x (nodespark/des_connector based) branch only with the implementations from #3102388-8 updated to the latest base code changes.

Patch from #20 Add Search API data source implementation Needs work , which contains several changes from the lastest comments, brokes my site currently running previous patch version, probably because of the plugin ID change. We have no easy way to review changes in patches from #15 Add Search API data source implementation Needs work to #20 Add Search API data source implementation Needs work since there is no interdiff and also some patches contain more than one change. Changes from latest patch files should be commited on top of the new branch, as 1 commit per change, in order to be able to follow and review them.

🇪🇸Spain manuel.adan 🌌

manuel.adan changed the visibility of the branch 8.x-7.x to hidden.

🇪🇸Spain manuel.adan 🌌

Comparto la presentación del evento. Gracias a todas/os por vuestra asistencia!

🇪🇸Spain manuel.adan 🌌

Thank you for the review! Committed as suggested, so now we have basic test coverage to be extended.

🇪🇸Spain manuel.adan 🌌

Going ahead with last changes to move forward. Thanks!

🇪🇸Spain manuel.adan 🌌

I have tested the solution and it is working fine, thank you all!

🇪🇸Spain manuel.adan 🌌

The label field is required, so after the patch, editing existing block instances forces to provide now a title. I think that it could be optional, since create new instances without a label is a admitted situation.

In addition to that, there is another "Title" field when the "Override title" options is enabled, which might cause confusion. Renaming the instance label as "Administrative title" could be a work around for that.

🇪🇸Spain manuel.adan 🌌

Title was replaced by the filter ID in cases that the empty PHP function evaluates as true, eg. for '0'.

It would be a more complete solution to unconditionally add the filter ID to the labels. By that way, we also cover cases like having different exposed filters with the same title.

🇪🇸Spain manuel.adan 🌌

First browser test covering the module's main feature.

🇪🇸Spain manuel.adan 🌌

Already working in a full schema in the main issue 🐛 Make configuration translatable RTBC .

🇪🇸Spain manuel.adan 🌌

I added the full schema on top of the existing changes, keeping the idea of extending core views block schema.

🇪🇸Spain manuel.adan 🌌

manuel.adan made their first commit to this issue’s fork.

🇪🇸Spain manuel.adan 🌌

There was no feedback from @ahmad-smhan. I've reviewed it and committed. Thanks!

🇪🇸Spain manuel.adan 🌌

Clear bug. Manually tested on D8, D9 and D10. Thanks!

🇪🇸Spain manuel.adan 🌌

There was no feedback from @ahmad-smhan, so I have finally been testing the changes manually on a D8, D9 and D10 installation. It seems to be working correctly but, as commented, we need automatic tests to perform this task correctly.

🇪🇸Spain manuel.adan 🌌

@ahmad-smhan, could you do some final test on this as a double-check, please?

🇪🇸Spain manuel.adan 🌌

It seems OK as well for me. Just found PHP 8.2 deprecation due to set _serviceId property that is no longer required . We no longer ensure compatibility with D8 as well.

🇪🇸Spain manuel.adan 🌌

From the project form about the ecosystem field:

Ecosystem
Use this field to indicate that this project's primary purpose is to provide enhancement for one or more other projects, and thus forms part of an ecosystem around it.

This module can be used outside of layout builder, actually it was born when layout builder was not yet in core, so moving it into its ecosystem may bring the false idea that the module cannot be used independently.

In any case, I have added a mention about the convenient use in combination with layout builder on the module page:

"It is particular useful in combination with Layout Builder to create simple markup blocks within any configured display."

🇪🇸Spain manuel.adan 🌌

In a standard installation with the language module enabled, only the first part of the form is shown:

Language selection

[ ] English
[ ] German

Select languages to enforce. If none are selected, all languages will be allowed.

The language selector seems to be added by some third-party extension. Any way, it is not related to the custom markup block itself.

🇪🇸Spain manuel.adan 🌌

After 2+ years with no activity or any other report, it seems that it happened due to some third-party reason. Feel free to reopen if it happens again.

🇪🇸Spain manuel.adan 🌌

I really thank you for bringing help on maintaining the Email Confirmer project.

For me it's OK that @ahmad-smhan was being listed as maintainer. However, as far I can see, there is no issues marked as followed by @ahmad-smhan (open or not) or submitted by the same user account ever .

It suggests me that there has been no previous participation on the module development or maintenance. The lack of experience is not a problem, but it will require some follow-up in order to ensure the project code quality and security. So, let me make a final review before launching any new release. I will also follow @ahmad-smhan maintenance activity in the upcoming months.

🇪🇸Spain manuel.adan 🌌

We need a HAL < 2.x compatible normalizer class. Initial approach.

🇪🇸Spain manuel.adan 🌌

Committed, thank you all! Sorry for the delay, pretty busy at work these weeks.

🇪🇸Spain manuel.adan 🌌

This issue is being addressed in the earlier 🐛 Unable to reset user password on non-ldap accounts Postponed: needs info along with other type-casting related ones.

🇪🇸Spain manuel.adan 🌌

This issue is being addressed in the earlier #3251947 along with other type-casting related ones.

🇪🇸Spain manuel.adan 🌌

I tested changes both in D10.0 and D10.1. I also tested in an upgraded system from 10.0 to 10.1. Tests passed OK, it seems functional and it's working as expected. Great work!

I just found some CS related issues fixed in the attached update.

Many URL have been changed, so adding redirections from the old to the new ones could be useful. Affected URLs are not public anyway, so might not have many broken links after the upgrade. Shortcuts, aliases or direct link in administration guides would be the only affected. It could be addressed now or in a new issue after this one becomes fixed.

🇪🇸Spain manuel.adan 🌌

Successfully tested on D9.5.9 and D10.1

🇪🇸Spain manuel.adan 🌌

I really appreciate Nic's colaboration on MimeDetect, Drupal needs more makers and you are one of them :)

I've been reviewing the module issue queue and found your participation in some of them. Unfortunately, I found there were still some points to review, despite being in RTBC status.

I've listed you as co-maintainer, but by now, let me do a final review before committing.

I think that our main goal at this time should be to have a D9 and D10 compatible release (#3358503).

Finally, consider that, as many of us, I maintain MimeDetect exclusively in my free time, so I might take some time in my responses.

🇪🇸Spain manuel.adan 🌌
  1. +++ b/src/MimeDetectService.php
    @@ -87,10 +87,21 @@ class MimeDetectService implements MimeDetectServiceInterface {
    +      if (\Drupal::moduleHandler()->moduleExists('stage_file_proxy')) {
    +        $server = \Drupal::config('stage_file_proxy.settings')->get('origin');
    +        $fetch_manager = \Drupal::service('stage_file_proxy.fetch_manager');
    

    Dependency injection should be used here to make use of the module handler, config manager and Stage File Proxy fetch manager.

  2. +++ b/src/MimeDetectService.php
    @@ -87,10 +87,21 @@ class MimeDetectService implements MimeDetectServiceInterface {
    +        $file_path = implode('/', [$server, $file_dir, $relative_path]);
    

    It seems that path to file is composed using the remote server name, which ends in a non local file path (eg.: http://www.example.com/sites/default/files/image.jpg). The following mime detection method (file UNIX command) is unable to manage remote URLs, it only works with local files, so the file itself should be retrieved.

I think that there is no point of checking remote files since they should be already checked on their environment. A better approach here would be just to skip non-local files.

🇪🇸Spain manuel.adan 🌌
+++ b/src/MimeDetectService.php
@@ -22,7 +22,7 @@ class MimeDetectService implements MimeDetectServiceInterface {
+   * @var \Symfony\Component\Mime\MimeTypesInterface

You advert that the MIME type guesser class in symfony >= 4.3 has changed, but MimeTypesInterface is a different interface.

It was moved to \Symfony\Component\Mime\MimeTypeGuesserInterface and removed from its original location in Symfony 5.0 and their methods renamed.

Patch reviewed accordingly.

Production build 0.69.0 2024