🇪🇸Spain @omarlopesino

Sorry I have uploaded just 10 minutes ago the MR with the solution. However, let's see the feedback from the maintainers. Thanks for the help.

omarlopesino → created an issue.

This problem should have been fixed at 1.1.0 with this issue: https://www.drupal.org/project/purge_file/issues/3308665 ✨ Include Query String Variants in Purge Request Fixed . May you enable the wildcard option at the purge file configuration, and check if that solves the issue?

If that solves the problem, we could improve the documentation to explain how to solve problems with image style derivatives.

It looks good to me. Merged and released at 1.1.2

I've updated the patch to not use access contexts. In my tests, the module behaves as usual. Pending to review with the tests that will be added at https://www.drupal.org/project/node_authlink/issues/3450411 📌 Add automated tests Active

I would like to propose adding tests to the module. This issue affects the node access and having tests may prevent regressions or future security issues.

I've just created a MR using URL cache contexts. I've tested manually and node Authlink works as usual:

- The Authlink URL only works on the proper URL.
- When the Authlink is removed, if I try to enter the Authlink URL on a browser without session, I get an access denied.

Please review, thanks!

omarlopesino → created an issue.

The latest releases of commerce payment synchronization doesn't require this library. PLease add it manually in your project during the next update. Thanks for the contribution and sorry for the inconvenience.

I've tested this in several projects and it works fine, moving to RTBC. The code is also fine.

Fixed the MR to only invalidate caches if the submission does not have source entity. Now tests should pass.

I've noticed:

1. The session cache context is already added https://git.drupalcode.org/project/webform/-/blob/6.2.x/src/WebformSubmi... . So the current patch needs work as it is only removing the cache invalidation.
2. The test passes when the webform is added with source entity, but not when it is added without source entity.

Still checking for solutions.

Webform paragraphs drafts are failing in the last pipeline:

1) Drupal\Tests\webform\Functional\Paragraphs\WebformParagraphsDraftsTest::testParagraphsDrafts
Behat\Mink\Exception\ExpectationException: The string "A partially-completed form was found. Please complete the remaining portions." was not found anywhere in the HTML response of the current page.

Checking why it fails.

I would like to help with this topic as I need to allow caching in a website with high traffic. As it has been suggested, we should find a way to do the validation through AJAX. The problem with the server-side validation is that the captcha session ID gets lost.

Does it make sense to generate the captcha session ID on the JS side through an AJAX request, and then send the captcha session ID as a form value in the submit? It would also require some validation in client side to check it is created by the system and not maliciously.

Please let me know your thoughts about this, I will investigate further and in the case I find a stable solution I would come up with a merge request.

Please review the merge request I've just created, which use session cache context.

Let me know what do you think, thank you!

omarlopesino → created an issue.

I agree with @frouco. I didn't mention that because it is also coupled with the embed integration which doesn't need to be present in every Drupal site. But the Oembed part can be decoupled in a different issue.

To do that I suggest dispatching an event here: https://git.drupalcode.org/project/youtube_cookies/-/merge_requests/31/d... , so that the submodule can hook into the event and show the popup when the cookies are not accepted.

It looks good to me. I've added a question to the MR suggesting an optimization that could be done. Waiting for a reply before deciding merging or moving to needs work. Thanks!

That argument is completely different from what was said in the description.
However, that is your point of view and I do not share it, sorry. Also, I don't find it useful to argue more.

The issue is still closed, sorry.

And please, do not talk to me like it is obvious a feature is needed, it is really uncomfortable. Thanks!

The commerce sermepa module does a redirection when the checkout step of the order changes during the onReturn step: https://git.drupalcode.org/project/commerce_sermepa/-/blob/8.x-2.x/src/P...

omarlopesino → created an issue.

Merge request done. Please review, thanks!

omarlopesino → created an issue.

Merged!

It looks good to me. Thanks, Drupal bot!

The changes looks good to me. THis is ready to be merged, thanks!

Thanks for the contribution! However, that looks a custom solution for a very specific problem. There are better ways to check logs do not appear. For example, doing a manual test reproducing the error and check the error does not happen again. Plus, if the problem is solved, the logs will eventually disappear from db log as db log messages are periodically cleared.

Apologies for not accepting the feature request. I'm open to changing my mind in the case that is strictly necessary in the project.

I will check it ASAP. There may be a bug or regression when the module is being used in content without translation enabled, which may have not been tested enough. I'll come back to this issue with a resolution this week.

Thanks! Merged and released at 1.0.0-alpha5

It looks good to me and as I have mentioned in the merge request , I would like some changes that will allow adding more meta values using the same method.

I added blob support after finding a regression in a filter that was working before the patch. The specific filter is the layout section field. The field was already added by reading field storage definitions, and now is being added in both field storage definitions and bundle field definitions.

omarlopesino → created an issue.

I've had the same problem as #66. I've added some commits to the latest MR, to allow handlers to detect bundle fields. I've hidden the rest to prevent confusion to know which MR is canonical.

The only problem is that the changes are not compatible with Drupal 11.x because on the changes added at this issue 📌 Allow adding computed bundle fields in Views Fixed , related with computed bundle field and views. So it is needed to update the MR to be compatible with 11.x

omarlopesino → changed the visibility of the branch 2898635-95x-PP1-bundleFieldDefinitions-are-not-added-in-EntityViewsData to hidden.

omarlopesino → changed the visibility of the branch 2898635-views-data-bundle-field-definitions to hidden.

omarlopesino → changed the visibility of the branch 5.0.x to hidden.

Created MR fixing the problem. Please review, thanks!

omarlopesino → created an issue.

Thank you @dineshkumarbollu . Indeed I've already made that solution but forgot to create the MR. Now the MR is created. Please review, thanks!

omarlopesino → created an issue. See original summary → .

Thanks for the feedback.

MR has been updated and now is up to date with 5.0.

I've found this error after doing the fixes: Trying to access array offset on value of type null Cron.php:264

I've fixed it in the same MR because it would have conflicts with this issue, so it will be easier to review in this way.

Please review the MR, thanks!

omarlopesino → created an issue.

Created MR. Please review, thanks!

omarlopesino → created an issue.

I've added 2 commits with corrections that already were present in thepatch. Now the MR is canonical, so the patch will be hidden to prevent confussions.

Now it is ready to review through the MR.

The MR 16 works only when the page is in the language is needed to translate, but it is possible to add a translation in another language.

Example: mysite.local/node/23/translation/add/en/fr. In this case, the edit URL will appear in the current language, instead of french it will appear the current language.

Please review MR 18 which takes in account target translatiion route parameter, which should work fine.

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

It works okay for me and the code is fine.

I have some feedback that you can do now or later:

1. When I add a new tab, I can't select this tab as default until I save the section configuration, and configure it again. THe AJAX should also refresh the default tab selector to select the new tab as default. It would me more friendly.
2. After I add content to a specific tab, the first tab is chosen. This makes me go back to the tab I was. It would be nice to preserve the selected tab when an element is added to the section.

However, is RTBC ihmo.

It works fine for me and the code looks okay! Ready to merge

I've tested it and it works fine! The code also looks nice. May you consider merge it?

Released in 1.1.7

Ready to review

@tunic about #3 comment, I agree with you that a general approach must be done and must be provided from core. In the meanwhile, we must look for a temporary solution that let us enable CSS /JS aggregation back in sites using vlsuite modal.

Also, I have been struggling trying to process separately additional CSS libraries added by Vlsuite modal so the admin theme is used, but it looked to be a non-ending workaround.

After thinking about it along with @crzdev, I've just made a MR with the following solution: disable aggregation for layout builder administrative pages. In this way, the aggregation will work for the entire site except on page building, where is less priority as it is only used for admins, mostly in desktop.

I feel this is a workaround for a contributed module as CSS /JS aggregation should work at every page. I would like to know also your opinion about whether this is good to have in the module or if a different solution should be done.

In other side, I think having an already working solution is good for sites with css aggregation disabled, so it can be enabled again with guarantees.

Please review, thanks1

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

Pleas review the MR with the solution proposed, thanks!

omarlopesino → created an issue.

omarlopesino → created an issue.

MR created. Please review, thanks!

omarlopesino → created an issue.

MR ready. Please review, thanks!

omarlopesino → created an issue.

Merged, it will be released in 1.1.5

Tests have been validated, so they have been merged into 1.x. Closing

omarlopesino → created an issue.

Tests are completed and are pending to review. Core functionality

During the tests development some bugfixes have been done and released to 1.1.3 and 1.1.4.

Please review, thanks!

There are tests that check the entity translation assignment works. The next step is to do the tests to reassign the content.

Fixes are done, please review the MR.

In the gitlab CI build, the test-only step does not fail because this is a correction in one of the modules used for testing, So I've attached a screenshot of what happens when fixes are not one.

omarlopesino → created an issue.

Created tests to access the translation assignment page. WIP.

omarlopesino → created an issue.

Sure! The MR is canonical so I've just hidden the patch.

Merged into 1.0.x.Soon I will release 1.1.0

Merged on 1.0.x. I will create today a 1.1.0 releasenwith changes from https://www.drupal.org/project/entity_translation_sync/issues/3400859 ✨ Users should be able to view the fields they are not able to sync Active

omarlopesino → created an issue.

Now there are tests that check the configuration page works well, knowing that it has specific form states to show/hide the fields, plus validations.

Ready to review.

Added tests that checks the main functionality, with three use cases:

Sunny day: Synchronization works with one language and one field.
Variant: Synchronization works with multiple languages and different fields in each language
Entity has been synchronized successfully to: English, Italian

The only test pending is the one which asserts configuration can be properly configured from UI,

1. Added gitlab ci file so tests are run by gitlab ci.
2. Corrected all reported static analysis errors.
3. Added tests to ensure module installation is safe, plus only allowed users can access to the sync page.

Pending:

1. Add tests to check the full synchronization process.

omarlopesino → created an issue.

Feedback from #14 has been solved:

1. Added video showing the problem in 'Steps to reproduce'
2. MR 872 is closed, and 5262 has been created

Please review the latest contributions to the issue, thanks!