🇪🇸Spain @rodrigoaguilera

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.

Seems like trying to fail the test on purpose, changing assertNotEmpty for assertEmpty, also passes the test so I'm clueless about what is happening with that test :(

Weird. The tests are not failing where they are supposed to.

The solution to fixing the test should be in core/modules/field_ui/src/Form/EntityFormDisplayEditForm.php protected function thirdPartySettingsForm().

Adding:
$settings_form[$module] = ($settings_form[$module] ?? []) + $hook(

Filed 🐛 Support multiple implementations for hook_field_widget_third_party_settings_form Active

rodrigoaguilera → created an issue.

Apologies for the noise. Indeed multiple implementations are supported but there is special cases like hook_field_widget_third_party_settings_form that won't work the multiple implementations. Will file an issue for that.

I changed the CR to reflect that
https://www.drupal.org/node/3442349 →

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

Thanks for the link.
The project I migrated is https://git.drupalcode.org/project/change_labels/-/tree/1.x?ref_type=heads
but is already frankensteined with traits to workaround this issue that I'm talking about.

To showcase the problem I created a patch for automated_cron with similar hook_field_widget_third_party_settings_form implementations.
Only the "second" implementation end up displaying its form in the widget settings.

Maybe this is a special case since the calling module is important to later determine where to save the third party setting.

Moving one of the classes to another module (and changing the namespace) makes both form items to appear.

Can you link to the issue? Just want to investigate if there is a possibility to be added back.

I updated the CR to clarify the point about the multiple implementations
https://www.drupal.org/node/3442349 →

While migrating a module to OOP hooks I noticed that the restriction of "one implementation per module" is still there. Now is rather silent as there is no error about it. For example having two classes in the \Drupal\my_module\Hook namespace that have the same hook annotations.

I would be really convenient to have multiple implementations per module to organize related features into the same class. E.g. a module that has two field widget third party settings and each one has its hook_field_widget_third_party_settings_form with a corresponding hook_field_widget_complete_form_alter. One implementation for each of the third party setting separated in different classes.

Is this something that can be tackled in this issue or should I open a new one?

We need to add that warning somehow. First step would be the readme file

Sound like a great feature
Can you convert it to a MR?

I don't follow how you use the "target" setting as field_name. Can you explain more?

I think instead of
$entity->get($field_name)->getValue();
You can do
$entity->get($field_name)->getValue();

merged

I attempted to fix the tests by adding the tour module that became a contrib module but there is more failures.
I guess the Drupal 9 fixture is trying to upgrade directly to Drupal 11 and that creates situations like the help_topics module to be uninstalled (current test failure).

So probably adding the tour module was not a step in the right direction.
I feel the proper solution is to create a fixture that starts from Drupal 10, therefore dropping support for the upgrade path from Drupal 9.
People coming from rabbit hole v1 on Drupal 9 need to upgrade first to Drupal 10.

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

Released

Hello

Thanks for raising the issue. I do not agree on relaxing the restriction that much.
Major versions can change the structure of third party settings so those need to be checked before raising the requirements.

In this case it seems like they are compatible with drulma_companion
https://git.drupalcode.org/project/block_class/-/blob/4.0.x/config/schem...

Please review and I will merge it with a new release afterwards.

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

Added test for the deprecation so back to NR

Isn't a bit too much adding deprecation tests for a protected method?

I'm curious about how to mark them so they will be removed along the deprecated code.

I tried to look for examples but couldn't find much. I guess it would be about setting up an artificial code path with a class that extends from EntityViewsData that calls the deprecated method.

rodrigoaguilera → changed the visibility of the branch 2930736-10.1-entity-views-data-assumptions to hidden.

DeprecatedServicePropertyTrait is used for when there is a \Drupal::service() replacement call
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Dependenc...

So deprecation with no replacement I think is the appropriate thing to do here.

I rebased against 11.x and created a new MR.
Hopefully this can be merged for Drupal 11.2.0

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

rodrigoaguilera → created an issue.

I think this is valid for configuration that remains active (but not enabled) and yet invisible in the UI.
I believe the steps that I provided are still valid to reproduce this.

I don not agree that once disabled the config for the view mode should remain but if for some reason that is the intended way we can improve the UI following already existing patterns like in views where disabled display modes are still visible and they are clearly marked as disabled.

Changing the issue title to better reflect this.