🇪🇸Spain @rodrigoaguilera

rodrigoaguilera → changed the visibility of the branch 3523070-move-nodeinterfacepublished-and to hidden.

Hello @marcoliver

Are you working on this in Vienna as part of the mentored contribution?

As co-maintainer of the module that doesn't use it actively anymore I 100% agree. I see that you maintain other modules successfully.

Marking as RTBC to signify my approval. I don't have permission to change maintainers so I hope the original maintainer can read this and give you access like it happened for me.
If that doesn't happen I think you can go through the process of claiming that the module is "abandoned" although is not 100% true but without the ability to add other co-maintainers it will eventually die.

Thanks for the review

I agree that "Published" should not have a description.

Next step is to take patch in #4, open a merge request with it and then look at the inconsistencies left between media types and content types (listed on the proposed resolution)

I agree with joachim that we don't need an enum for the 1 and 0 possible values.
Next steps is to read carefully how to deprecate the constants
https://www.drupal.org/about/core/policies/core-change-policies/how-to-d... →

Then change the current proposed code to deprecate according to that document.

Triaging this I think this still a good issue for novices. For example you can look at following issue for inspiration
✨ Make the `field_config_base` structure fully validatable Active

As @penyaskito mentioned is not about form validation but adding constraints. Feel free to open a new branch

Triaging this for Drupacon Vienna 2025 mentored contribution I don't think the next actions here are clear enough for a novice to continue working on this.

Also hiding the latest branch since it doesn't start with the latest patch and doesn't explain what was done nor queuing for review.

Updating the title with the current direction of the issue

rodrigoaguilera → changed the visibility of the branch 3017214-drupalsystementityactionexecute-is-missing to hidden.

Use better style for the note

Link to the merge request guidelines and note the patches workflow has been abandoned

Remove reference to re-rolling patches

rodrigoaguilera → created an issue.

rodrigoaguilera → created an issue.

Add link to the tasks section in the contributor guide

Thanks for creating the merge request. I edited it to include parts of the original proposal like the features.

The idea is for the readme to become the project page so I also removed links to it

Added to the readme in https://www.drupal.org/project/publishcontent/issues/3480936 📌 Proposed Documentation Active

Both MRs were the same so I hid the one with the lowest id. Rebased and now the tests are green, back to needs review. I think is still a good issue for a novice to review.

rodrigoaguilera → changed the visibility of the branch 3422895-configurablelanguagemanagerinterfacegetlanguageconfigoverride-and-languageconfigfactoryoverrideinterfacegetoverride to hidden.

@astonvictor Can you explain more about the change?

Seems like it is not enough to disable the widget since the widget has a default value of "1".

I came across a similar problem but for entity references so I created a contrib module for that.
https://www.drupal.org/project/entity_link_formatter →

I wonder if this change could allow for some extensibility to allow `link_rel` to be the name of a valid link template for other modules to be able to use the formatter without extending with a new class and adding a new schema.

rodrigoaguilera → created an issue.

Now that the rule is changed to 80 now is about to fix all the instances.

Have a look at the "fix" command for eslint
https://eslint.org/docs/latest/use/command-line-interface#options

I don't think the remaining tasks in this issues are for novice folks

rodrigoaguilera → changed the visibility of the branch 3251879-unclear-docs-for to hidden.

Title seems fine now. Back to RTBC

I think this a difficult issue to figure out for a novice

I did a quick verification and it seems the radios have proper focus styling now in claro

Updated the IS without verifying if the bug still exist.

I think it is still a good novice issue so I will keep it on the list for the mentoring team for Vienna.

Updating the IS as that last feedback contradicts it.

We need to look through all the implementations of schema()
https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Field%21F...

Collect all exception that those methods throw aside from \Drupal\Core\Field\FieldException
and update the MR with multiple @throws lines as needed.

Exactly, the tests are broken due to this. Please merge.

The scheduled tests already run for the next minor so I guess is up to the maintainers if they want to run them for each MR.
Not sure why DRUPAL_CORE: "$CORE_STABLE" is needed.

I only found this issue now. That is the proper fix
🐛 __clone method called on non-object in contact_storage_module_preinstall() Active

Not sure why between Drupal 11.1 and 11.2 the definitions can't be retrieved as before but I think this workaround does the trick.

rodrigoaguilera → created an issue.

Thanks for reviewing.

I added a description to the group type selection to hopefully make it more clear.
Added more info to the comment of the getPluginIdWithType() method and addressed the rest of the feedback

Merged :)

Thanks for the contribution

Can you try the MR from 🐛 Issue upgrading from 8.x-1.4 to 8.x-1.10 Active
It includes fixes for the warning reported here also

Thanks for reporting. I added checks around the code that is giving warning including the ones from the other issue linked.
Can you try the MR?

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

Please review

With the change from 🐛 Move hook_entity_delete to hook_entity_predete Active and the MR I proposed the error is gone for me.

Please review the redirect issues first as it would be a requirement to for the fix here.

rodrigoaguilera → created an issue.

I'm facing the error from the summary with the latest version of this module and redirect 8.x-1.11 so I think it is still happening.
The relationship gets deleted and then the redirect module can't build a canonical URL for deleting redirects.

I think it makes sense for this module to perform its action on the predelete and I think redirect should do too.
I'm going to propose that change

Please review

rodrigoaguilera → created an issue.

Please review

rodrigoaguilera → created an issue.

I don't maintain any projects that require this anymore so it can be closed from my point of view

Please review

I have the same issue but on Drupal 10 and I can't upgrade yet.
I attach a patch for Drupal 10.4.8 in case it can help anyone

rodrigoaguilera → created an issue.

This would be a change in one of the files of the repository, reame.md, so anyone can propose that change with a merge request.

https://www.drupal.org/community/contributor-guide/task/create-a-merge-r... →

Once the merge request is merged I can edit the description of the module page and use that readme markdown file to transform it into HTML.

I faced similar issues with the new route so I can confirm this is the proper fix

I gave it a read and it improves on the current description.
Made a small edit removing the version.

Next step would be to convert this into the readme.md for the module
I would keep the: "Recommended modules" and "other modules" section.

Thanks for the effort

Perfect. Thanks for the improvement!

Makes sense. Thanks for the contribution

I merged https://git.drupalcode.org/project/simple_entity_merge/-/merge_requests/10 first so now this one has a conflict

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

Great cleanup :)

Thanks for the contribution.
I will roll a release to avoid the warning.

Indeed it makes sense to remove it. I think it comes from a time were the src folder of themes was not autoloaded.

Do you see why the pipeline is failing?

Sure

Oops, thanks for the reminder.

All layouts have a schema now + some schema fixes revealed by the functional test.
For the test to pass the Drupal 11 compatibility needs to be merged first
https://www.drupal.org/project/rocketship_core/issues/3536132 🐛 Compatibility with Drupal 11.2.2 Active

Locally the tests pass with dozens of deprecation warnings but we can work on those later and having them listed will ease the work.

Please review

rodrigoaguilera → created an issue.

I committed the suggested change to the 1.x branch from my phone.
Can you confirm it works?

Thanks for the detailed bug report and solution.

Sure. 1.0.1 released

Merged, thanks!

Hi
The code looks good except for the mix usage of [] and array. Can you use the short version in the new code? I wouldn't mind if you also convert some arrays around the code changed like $defaults.
Thanks for the contribution!

rodrigoaguilera → created an issue.

So when you uninstall this module your are able to see the unpublished content again?

I updated the patch for 6.0.x

As said before, there must be a better way to store the nonce

Added just the schema for rs_one_col in the MR.

rodrigoaguilera → created an issue.

I think the schema is correct now

I attended

Yes, totally a duplicate. Adding it as related issue.

I think RTBC might get more attention from a core maintainer to assign credit

rodrigoaguilera → created an issue.

#87 is an incomplete patch, here is the right version

Reroll attached of #85 applied agains Drupal 11.2.2 (with #356 from here 📌 Allow synced Layout override Translations: translating labels and inline blocks Needs work also applied)

Rebased the branch and moved the tests. Not sure why tests are failing.

Attached also you can fin the patch for Drupal 11.2.2 for composer

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

Just rebased the #22 patch and created an MR so it applies to Drupal 11.2.2
IS still needs updating.

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

In the context of OpenID Connect when using RS256 a "kid" is required on the JWKS endpoint and the header of the token.

If you try to pass the playground test with simple_oauth and this patch https://git.drupalcode.org/project/simple_oauth/-/merge_requests/171
It will complain about the "kid" being missing
https://openidconnect.net/

So the same kid needs to be in the JWKS endpoint and in the OpenIdConnectIdTokenResponse. I added one line there

    // Add required id_token claims.
     $builder = $this->getBuilder($accessToken, $userEntity);

     $builder = $builder->withHeader('kid', 'singlekey');
  

And I was able to go through all the steps in the playground

I have no issue using proper relative paths that start with ".."
Only absolute paths should start with /

I only tried on 6.0.0 not 5.2.x but seems that part hasn't changed much.
Closing because in case is not posible on the 5.2.x branch I doubt it will be added as a feature when is already present in 6.0.x.

Please reopen if you think the help text needs better guidance

Tested manually and it works great. Fixed some more phpcs issues

Thanks for the contribution!

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

I tested the latest MR and one thing I found weird is that the in the header array the indices for the rows can't be numeric.
Also when using multilevel the issues of having attributes in the headers cells is also solved, hence adding it as a related issue

rodrigoaguilera → created an issue.

+1

Ok, then I guess that config is needed if it is in the template and I imagine that since JS sniffing is deprecated in phpcs it won't be needed to exclude JS explictly as those files will be ignored by default by phpcs.

Thanks!

An example phpcs run showing the errors
https://git.drupalcode.org/project/font_awesome/-/jobs/5453102