🇪🇸Spain @rodrigoaguilera

rodrigoaguilera → created an issue.

Fixed all phpcs, phpstan and cspell issues.

Thanks!

Totally agree. I think this code was a copy of what devel was doing back then for inspecting entities

Don't know why the Merge button in gitlab is not working :(

Thanks for the contribution :)

Not sure 100% of use cases would want the date unchanged but I do believe is the correct default behaviour. I'm open to changes that would make this configurable.

Thanks for the contribution!

Indeed, I agree the tab title is too long.

Thanks for the contribution

rachel_norfolk → credited rodrigoaguilera → .

rodrigoaguilera → created an issue.

Applied the patch and the import worked. I believe that patch is the same as the MR. Setting it to RTBC.

Being one additional argument I guess it keeps compatibility with Drupal 10

rodrigoaguilera → created an issue.

I also fixed a bug with the emails to admins not being sent due to this line

    if($this->apply_for_role_config->get('send_user_approval_email')){

The idea would be to only use tokens in the future instead of the custom replacements logic but I think this is a small step in the right direction.

Please review

rodrigoaguilera → created an issue.

I ran the upgrade status scan on D10 and it reported the issues that I include in the MR.
I will perform more manual testing now with the module installed on Drupal 11.1.6 but usually upgrade_status catches all the issues so setting it to NR

rodrigoaguilera → created an issue.

Tests are failing because of the new default of FALSE for hide_untranslated_menu_links in the menu block.

I think the upgrade path is now ready so removing that tag

I won't be working on this in the near future

Merged 11.x into the branch.

I agree with @catch so I applied the idea of the

OnlyShowTranslatedMenuLinksInterface

instead of checking for a specific class.

If the pipelines return green then I will make $context optional and default to null when not passed. I see no reason for making it mandatory

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

Since the underlying data is an image field I think what is happening here is that the "Remove" is not triggered when clearing the canvas.

Not sure if that would be a viable solution or the button for removing the image should be exposed as some way of starting a fresh canvas deleting the previous image. Saving at this "clean" state will delete the image.

Let's discuss what should be the correct behavior

Sure

Thanks for the contribution

borisson_ → credited rodrigoaguilera → .

I tested with Drupal 11 with Drush 13 and Drupal 10 with Drush 12 both with the 3.x branch and the generators are not detected but the error is gone.
I think some PHP attributes are needed for recent version of the generators
https://github.com/Chi-teck/drupal-code-generator/blob/4.x/src/Command/P...

Opened MR with the proposal

rodrigoaguilera → created an issue.

rodrigoaguilera → created an issue.

I adjusted the code for 2.x. Added the description text copied from Drupal core and tests.

I don't understand point 2 from comment #14. Can you explain more what is needed?

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

I understand but for that purpose you have other modules like
https://www.drupal.org/project/view_unpublished →

Or core issues that try to include that into Drupal
https://www.drupal.org/project/drupal/issues/3160748 🐛 Support "View any unpublished [entity_type]" and "Administer [entity_type]" permissions Postponed

With that information I think I can roll a new release

Sure, don't forget to make the 3.x the default branch to signify better the new direction of the module

Hello Nick

Thank you for your offer to maintain but I would like to see some initiative in the form of MRs or ideas on how to improve the module from you.
I admit the module doesn't receive the timely updates it deserves but it is just because I don't use in any of my active projects and I haven't receive any request for updates. I'll try to make it compatible with D11 this weekend.

Please don't feel discouraged to continue contributing, if the aspects I mention above change I am more than happy to share the maintainer role just like the person before me did for me.

poker10 → credited rodrigoaguilera → .

For people arriving here from a search: the key is how Drupal core change session ID generation
https://www.drupal.org/node/3006306 →

rodrigoaguilera → created an issue.

mradcliffe → credited rodrigoaguilera → .

Forgot to close. Please open a new issue for the 2.0 version of the module if you want me to explain better how to use different form modes per-menu

There is not enough interest in providing compatibility.
Please open a new issues with what is expected from this module

It seems there is not enough interest

Thanks for the rebase. I added some $this->t() to other string to be consistent.

Plase review again

I was able to reproduce and opened a MR to provide a workaround.

It gets rid of the "base_hook" in the theme definition since according to the docs it is only used for suggestions.
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Render%21...

This allowed to have suggestions but a when #theme like "menu__field_content__MENU_NAME" was provided and template existed for it Drupal core treats it as a "menu" theme definition and it won't detect the variables for the "menu__field_content" theme definition.

This basically demonstrates how weak was the decision to use a hook definition with double underscores, that confuses the Drupal theme system since is treated as a suggestion of "menu".

Please review the change and confirm it can be a valid solution/workaround.

I will start thinking about a 2.0 version of the module that gets rid of this ugly hack.

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

I opened a MR that illustrates how to use the hook for the case I described.

attached is the the patch for composer-convenience.

rodrigoaguilera → created an issue.

I ran into an error where the config:import happened before the update. There fore the entities are created by the import and trying the update later fails with messages like:

'content_translation_redirect' entity with ID 'node__blog' already exists.

Please review

rodrigoaguilera → changed the visibility of the branch 3185224-fix-update to hidden.

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

This morning I re-saved and it happened the same. I tried again some hours later and now is alright

You can see all the links in the source file

https://git.drupalcode.org/project/signature_pad/-/blob/1.x/README.md?re...

Rather strange. Took it to the drupalorg slack channel. Thanks for reporting

Thanks. I applied the suggestions from the MR

Looks good, thanks for the contribution

rodrigoaguilera → created an issue.

mradcliffe → credited rodrigoaguilera → .

The phpstan issues were solved in other commits so no need to modify the baseline anymore. Back to RTBC

Thanks for the contribution

Thank you for confirming is D10 compatible

mradcliffe → credited rodrigoaguilera → .

mradcliffe → credited rodrigoaguilera → .

mradcliffe → credited rodrigoaguilera → .

mradcliffe → credited rodrigoaguilera → .

Commited directly. Thanks for the contribution

I don't understand this issue. It is just for testing two issues at the same time?

Can you reroll the patch as an MR? It doesn't apply anymore to the latest 8.x-1.x branch

Looks good. Thanks for the contribution

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

I tested on the site that was giving me problems to update

Opened an MR

Usually modules changing injected services add an empty hook_post_update_NAME() to force a cache clear. Read more here:
https://www.drupal.org/node/2960601 →

To solve a single_content_sync.post_update.php file can be created with this hook.

I agree with keeping the scope of this issue small. Fixing the tests that are affected and it should be ready to go.

For me a logical followup would be to be able to customize that character instead of being hardcoded to a colon

Thanks but I'm wondering if there is any reason in particular to drop D10 compatibility on this commit
https://git.drupalcode.org/project/menu_manipulator/-/commit/b8d5335e384...

A Drupal 11 from Drupal 10 upgrade is much easier when you get all modules to be compatible with both versions and then just upgrade the core. Can you at least declare compatibility for Drupal ^10.3 ?

Additionally it is a bit confusing to have 3.1.x release on the 3.0.x branch. Maybe is time for a new branch.

If you don't want to be bothered it also fine I just want to note how other module maintainers manage their branches and releases.

I can confirm there is no other change needed for the module other than the MR2 to work on Drupal 11.

Please roll a new release with the change included

Please re-add the D11-compatibility as this module is still useful in Drupal 11

I installed the MR on a Drupal 11.1.0-rc1 install and it works as expected so I'm confident to mark it RTBC. It has many code standards changes but it doesn't hurt to include. Please declare the D11 compatibility and roll a release.

Have you contacted Rakesh?
https://www.drupal.org/user/1177822/contact →

Please read the steps for taking over a module
https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or... →

I tested manually the module on a Drupal 11.1.0-rc1 install with the changes applied and it works as expected. Upgrade status doesn't report any other problem with the module.

Please commit this and roll a new release. MR12 includes extra changes to comply with coding standards so it won't hurt to include those, so my RTBC go for that one.

I installed the 2.x branch on a Drupal 11.1.0-rc1 site and applied the compatibility patch.

I can confirm the module works as expected on Drupal 11 and that scans from the upgrade_status module don't return any other issue.

Please commit and roll a new release

ivnish is now an active maintainer so I don't think this is need anymore. Thanks

I got confused because the most updated branch was project-update-bot-only and the MR was oldest. I hid the outdated branch (opposite of what I did before).

The branch now has no deprecations running phpunit. There still one risky test but I think that is out of scope of this issue.

Glad to see one of the maintainers around :)

The MR is ready to review again

rodrigoaguilera → changed the visibility of the branch 3434446-automated-drupal-11 to hidden.

rodrigoaguilera → changed the visibility of the branch project-update-bot-only to active.

I fixed the deprecations but now there is some tests failures. Need a deeper look. I can't right now

Good catch.
Let me have a look

rodrigoaguilera → changed the visibility of the branch project-update-bot-only to hidden.

I might be wrong but it seems like bootswatch themes are compiled sith SCSS variables and not CSS variables the also include some CSS for each theme.

In my UI Suite Bootstrap subtheme I was able to apply one of the bootswatch themes with

libraries-override:
  ui_suite_bootstrap/bootstrap:
    css:
      theme:
        /libraries/bootstrap/dist/css/bootstrap.min.css: /libraries/bootswatch/dist/sketchy/bootstrap.min.css

and composer req npm-asset/bootswatch:^5

Basically ConfigFormBase now takes two arguments for the constructor

https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Form%21Co...

The warning in the phpstan (next major) was already warning about it

https://git.drupalcode.org/project/user_register_notify/-/commit/efb9d23...

Please roll a release after this to be able to use the module.

rodrigoaguilera → created an issue.

Makes sense

I found this googling for a solution. A better alternative for me has been to use https://www.drupal.org/project/views_rss →

The module takes care of outputting the raw description for RSS

Moving to the ownership queue

Sure. Added tests for the formatters as well

Seems like it was an issue with not declaring the schema for the new third party setting field.
I also squashed the experiments that I did while I didn't have a proper dev enviroment.

In the last two commits I add just the test, failing in the correct place and the the simple fix.

The test only checks widgets, not formatters but since it was the same fix for both I didn't find it very useful to duplicate testing there.