Automated Drupal 10 compatibility fixes for Permissions by Term

Currently blocked since there is an issue related to running PHPUnit tests with Drupal 10: https://www.drupal.org/project/drupal/issues/3315351

My temp. changes are in this Git feature branch: https://git.drupalcode.org/project/permissions_by_term/-/tree/3289053-dr...

Setting it to Postponed sabotages the bot. It currently doesn't consider the module D10 compatible, addition fixes are to be expected. Sounds like you reviewed the proposed refactoring, suggest to allow for further fixes by setting to Active.

Drupal 10 is out.

The maintainer currently has not enough time to work on this amazing module: #3316032: Searching for co-maintainers.

@Peter Majmesku #3315351: How to run PHPUnit tests with Drupal 10? is fixed now in case you didn't see yet. Thanks.

@mmjvb, @ressa, @Kristen Pol would someone of you like to Co-Maintain the project perhaps?
I already have too many projects maintainerships, so I can't add this large one. ~5.000 sites use this module, so it's sad to see there's no D10 compatibility.

Thank you @Anybody, for all the great work you and your colleagues do maintaining modules, such as CAPTCHA and Riddler. It's greatly appreciated, and I totally understand if you don't have the ressources for another module. Sadly, my coding skills are too limited to maintain the module, but I have added it on #3342443: META: Adopt contributed projects for Drupal 10 readiness, so maybe someone will volunteer?

@ressa added to the spreadsheet.

Thanks @heddn.

Here's an interdiff between the patches in #4 and #12 as they have the same name and no interdiff was posted in #12.

I applied patch #12 to a fresh D10 install and upon enabling the module encountered an unexpected error:
WARNING: [pool www] child 2054 said into stderr: "NOTICE: PHP message: Uncaught PHP Exception Error: "Call to undefined method Symfony\Component\HttpKernel\Event\RequestEvent::isMasterRequest()" at /var/www/html/web/modules/contrib/permissions_by_term/src/Listener/KernelEventListener.php line 81"

Additionally the submodule, permissions by entity, has not had any automatically generated updates for D10 and will also require some work.

Fixed a small deprecated function call, configured the module, and then went to add a new article, and encountered the following unexpected error:
Too few arguments to function Twig\Environment::loadTemplate(), 1 passed in /var/www/html/web/modules/contrib/permissions_by_term/src/Service/NodeEntityBundleInfo.php on line 89 and at least 2 expected

patch #21 against 3.1.21.

Message Too few arguments to function Twig\Environment::loadTemplate(), 1 passed in /var/www/html/web/modules/contrib/permissions_by_term/src/Service/NodeEntityBundleInfo.php on line 89 and at least 2 expected not related to drupal 10. loadTemplate is internal method

Due to the maintainer inactivity and blocking D10 upgrade I'm unsure about the future of this module and the other open issues.
So I decided to switch over to entity_access_by_role_field which is Drupal 10 compatible and has a more direct logic. Due to this different logic, this might not be the right replacement for everyone here, but perhaps in some cases it's a good or better choice, if you were using permissions_by_term more as a workaround, like in my case.

Still, I think Drupal 10 compatibility would be super important here due to the very many users.

I am the maintainer of the Permissions by Term module. I do not have an ongoing project, which gathers value from this module. I was working on this module for a long time and did enough. If you like to have Drupal 10 compatibility, feel free to contribute: https://www.drupal.org/project/permissions_by_term/issues/3316032

@Peter Majmesku I can help with module

@niko: Nice! Feel free to work on the existing patches. You can share your updated patch here and if it's tested, we can create an new release.

@niko: Do you have everything to solve this issue?

@Peter, yep tnx

Adding project name in title.

@peter I have updated few fixes that where shown by the upgrade status module.

@sudesh.solaskar: Thanks. Do you think that the D10 upgrade patch is ready or do you see issues?

@peter I think based on the reports of upgrade status all of the D10 compatibility issues are fixed.

Great, sudesh.solaskar. Could anybody with an ongoing project please test the recent D10 patch?

@erdm, can you confirm on which version of this module you are trying to apply the patch. As 3.1.21 already has core version requirement set to ^9 || ^10

3.1.21 does looks like it only has ^9 not ^10

https://git.drupalcode.org/project/permissions_by_term/-/blob/3.1.21/per...

This patch looks to have trouble applying because the packaging script alters the info file. Likely this issue : https://www.drupal.org/project/drupal/issues/3036459

I was able to work around it and get the patch to apply cleanly with this

    "config": {
        "preferred-install": {
            "drupal/permissions_by_term": "source",
            "*": "auto"
        },

https://www.drupal.org/project/drupal/issues/3036459#comment-13851631
https://getcomposer.org/doc/06-config.md#preferred-install

@_KASH_: That looks weird. If a patch can be applied via Git, it should be applicable by Composer without any extra config. Which error do you get without this Composer extra-config? And why this extra-config is necessary?

I get this error https://www.drupal.org/project/permissions_by_term/issues/3289053#commen...

Notice that the code in the git repo is NOT that code that ends up in the project prior to the patch being applied.

This is the git repo code https://git.drupalcode.org/project/permissions_by_term/-/blob/3.1.21/per...

name: 'Permissions by Term'
description: 'Restricts access to nodes and taxonomy terms via user to term relations.'
dependencies:
  - 'drupal:system'
  - 'drupal:field'
  - 'drupal:taxonomy'
  - 'drupal:path_alias'
type: module
core_version_requirement: '^9'
version: '3.1.9'
package: 'Permissions by Term'
configure: permissions_by_term.settings

This is what the patch is trying to apply against https://www.drupal.org/project/permissions_by_term/issues/3289053#commen...

name: 'Permissions by Term'
description: 'Restricts access to nodes and taxonomy terms via user to term relations.'
dependencies:
  - 'drupal:system'
  - 'drupal:field'
  - 'drupal:taxonomy'
  - 'drupal:path_alias'
type: module
core_version_requirement: '^9'
# version: '3.1.9'
package: 'Permissions by Term'
configure: permissions_by_term.settings

# Information added by Drupal.org packaging script on 2022-11-09
version: '3.1.21'
project: 'permissions_by_term'
datestamp: 1668009537

Read the links in my prior post for a better understanding.

After applying the patch I am seeing this error on the node edit page:

The website encountered an unexpected error. Please try again later.

ArgumentCountError: Too few arguments to function Twig\Environment::loadTemplate(), 1 passed in /opt/drupal/web/web/modules/contrib/permissions_by_term/src/Service/NodeEntityBundleInfo.php on line 89 and at least 2 expected in Twig\Environment->loadTemplate() (line 330 of /opt/drupal/web/vendor/twig/twig/src/Environment.php).

Drupal\permissions_by_term\Service\NodeEntityBundleInfo->renderNodeDetails('modules/contrib/permissions_by_term/src/View/node-details.html.twig', 'en', '899') (Line: 353)
permissions_by_term_form_alter(Array, Object, 'node_result_edit_form') (Line: 545)
Drupal\Core\Extension\ModuleHandler->alter('form', Array, Object, 'node_result_edit_form') (Line: 838)
Drupal\Core\Form\FormBuilder->prepareForm('node_result_edit_form', Array, Object) (Line: 282)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object) (Line: 39)
Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 580)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 163)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 74)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 58)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 686)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Running 10.0.7

Here is an updated patch that fixes the error on the node edit page. Seems like the function loadTemplate() NodeEntityBundleInfo.php line 89 should be removed/replaced as it is marked @internal. However this change seems to work.

All the changes here make sense, there's just a minor whitespace issue that will likely impact coding standards fails

+++ b/src/Service/TermHandler.php
@@ -122,6 +122,7 @@ class TermHandler {
+	  ->accessCheck(FALSE)
       ->condition('name', $sTermName)
       ->execute();
 
@@ -139,6 +140,7 @@ class TermHandler {

@@ -139,6 +140,7 @@ class TermHandler {
    */
   public function getTermNameById($term_id) {
     $term_name = $this->termStorage->getQuery()
+	  ->accessCheck(FALSE)

there's some whitespace issues here

Thanks, moving back to needs work for the minor formatting fix.

Here is an updated patch for the formatting issue and the info.yml compatibility for the sub module permissions_by_entity.

The patch applies cleanly on D9.5 and D10 as well, upgrade_status seems happy just with a flag for Symfony HttpKernelInterface::MASTER_REQUEST which can't be replaced to use HttpKernelInterface::MAIN_REQUEST yet as the module has to support D9.5 as well.

This looks good to commit in my book, I can't see anything there that will cause regressions for sites on 9.x, so this should be good to commit and queue tests

Note that @fathima.asmat and others have been given co-maintainership per #3316032: Searching for co-maintainers so hopefully they can help get this merged in with our support.

Thanks Larowlan and Kristen +1. I'm happy to commit this to Git and will need help creating a release for it as I don't have permissions for it.

I just tested my git push access to the repo and it seems like it's working, https://git.drupalcode.org/project/permissions_by_term/-/tree/d10-compat....

I am not able to apply patch #46 with 9.5.7, may be because I have multiple patch applied for this module. I tried to improve on patch #22 and #43. Tested it without applying other patch, it works.

@amitvin, what issues were you having while applying the patch? Could you add the interdiff for the patch you created so that it would be easy to view the changes that you have made with the patch, there is some guidance here about interdiff in case it's useful, https://www.drupal.org/node/1488712.

The patch #51 though enforces HttpKernelInterface::MAIN_REQUEST that can't still be replaced for HttpKernelInterface::MASTER_REQUEST as MAIN_REQUEST constant is only supported by Drupal 10 but not D9. To keep the module compatible for both D9 and D10, we should retain HttpKernelInterface::MASTER_REQUEST. This would not be a problem for D10 still as the constant will not be dropped by Symfony until version 7.

So as majority suggested, I have merged the patch to dev branch and committed a tag 3.1.22. I will speak to Kristen Pol and get a module release out as I don't have enough permissions to do that. Thanks to everyone who contributed with this +1.

A new issue is created to create a project release for the new tag, https://www.drupal.org/project/permissions_by_term/issues/3353813

There is a new release for D10 compatibility - https://www.drupal.org/project/permissions_by_term/releases/3.1.22.

On Drupal 9.5.7 with the new release (3.1.22) I'm getting this error when I attempt to create or edit content:

Twig\Error\LoaderError: Template "__TwigTemplate_1204b7436b150da256126149d5547692" is not defined. in Twig\Loader\ChainLoader->getCacheKey() (line 98 of /code/vendor/twig/twig/src/Loader/ChainLoader.php)

I had to roll back to 3.1.21 and that seems to be working.

This issue is also reported here:
https://www.drupal.org/project/permissions_by_term/issues/3354478

Thanks for the bug report. Would you please open a new issue and cross link them?