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

Merge Requests

More

Recent comments

πŸ‡ͺπŸ‡Έ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 β†’ made their first commit to this issue’s fork.

πŸ‡ͺπŸ‡Έ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 🌌

New one updated with the last fixings.

πŸ‡ͺπŸ‡Έ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 🌌

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

πŸ‡ͺπŸ‡Έ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 🌌

Sure, it's on the way!

πŸ‡ͺπŸ‡Έ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 🌌

@ahmad-smhan, could you please take look on this? thanks

πŸ‡ͺπŸ‡Έ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.

πŸ‡ͺπŸ‡ΈSpain manuel.adan 🌌

1.2 release, D10 compatible, already declares dependency on drupal/hal

πŸ‡ͺπŸ‡ΈSpain manuel.adan 🌌

Format review. Contents updated according to latest changes on code.

πŸ‡ͺπŸ‡ΈSpain manuel.adan 🌌

MR merged, thank you everybody!

Production build https://api.contrib.social 0.62.1