Automated Drupal 10 compatibility fixes for Permissions by Term

Created on 16 June 2022, over 2 years ago
Updated 19 April 2023, over 1 year ago

Problem/Motivation

Hello project maintainers,

This is an automated issue to help make this module compatible with Drupal 10.

To read more about this effort by the Drupal Association, please read: The project update bot is being refreshed to support Drupal 10 readiness of contributed projects โ†’

Patches will periodically be added to this issue that remove Drupal 10 deprecated API uses. To stop further patches from being posted, change the status to anything other than Active, Needs review, Needs work or Reviewed and tested by the community. Alternatively, you can remove the "ProjectUpdateBotD10" tag from the issue to stop the bot from posting updates.

The patches will be posted by the Project Update Bot โ†’ official user account. This account will not receive any issue credit contributions for itself or any company.

Proposed resolution

You have a few options for how to use this issue:

  1. Accept automated patches until this issue is closed

    If this issue is left open (status of Active, Needs review, Needs work or Reviewed and tested by the community) and the "ProjectUpdateBotD10" tag is left on this issue, new patches will be posted periodically if new deprecation fixes are needed.

    As the Drupal Rector project improves and is able to fix more deprecated API uses, the patches posted here will cover more of the deprecated API uses in the module.

    Patches and/or merge requests posted by others are ignored by the bot, and general human interactions in the issue do not stop the bot from posting updates, so feel free to use this issue to refine bot patches. The bot will still post new patches then if there is a change in the new generated patch compared to the patch that the bot posted last. Those changes are then up to humans to integrate.

  2. Leave open but stop new automated patches.

    If you want to use this issue as a starting point to remove deprecated API uses but then don't want new automated patches, remove the "ProjectUpdateBotD10" tag from the issue and use it like any other issue (the status does not matter then). If you want to receive automated patches again, add back the "ProjectUpdateBotD10" tag.

  3. Close it and don't use it

    If the maintainers of this project don't find this issue useful, they can close this issue (any status besides Active, Needs review, Needs work and Reviewed and tested by the community) and no more automated patches will be posted here.

    If the issue is reopened, then new automated patches will be posted.

    If you are using another issue(s) to work on Drupal 10 compatibility it would be very useful to other contributors to add those issues as "Related issues" when closing this issue.

Remaining tasks

Using the patches

  1. Apply the latest patch in the comments by Project Update Bot โ†’ or human contributors that made it better.
  2. Thoroughly test the patch. These patches are automatically generated so they haven't been tested manually or automatically.
  3. Provide feedback about how the testing went. If you can improve the patch, post an updated patch here.

Providing feedback

If there are problems with one of the patches posted by the Project Update Bot โ†’ , such as it does not correctly replace a deprecation, you can file an issue in the Drupal Rector issue queue โ†’ . For other issues with the bot, for instance if the issue summary created by the bot is unclear, use the Project analysis issue queue โ†’ .

๐Ÿ“Œ Task
Status

Fixed

Version

3.1

Component

Code

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine niko-
  • Status changed to Needs review almost 2 years ago
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Anybody Porta Westfalica
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Anybody Porta Westfalica

    @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.

  • ๐Ÿ‡ฉ๐Ÿ‡ฐDenmark ressa Copenhagen

    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 abandoned contributed projects for Drupal 10 readiness โ†’ , so maybe someone will volunteer?

  • heddn Nicaragua

    @ressa added to the spreadsheet.

  • ๐Ÿ‡ฉ๐Ÿ‡ฐDenmark ressa Copenhagen

    Thanks @heddn.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Amber Himes Matz Portland, OR USA

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

  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States MegaKeegMan
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States MegaKeegMan

    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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States MegaKeegMan

    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

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine niko-

    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

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Anybody Porta Westfalica

    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.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Peter Majmesku ๐Ÿ‡ฉ๐Ÿ‡ชDรผsseldorf

    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 ๐Ÿ’ฌ Searching for co-maintainers Needs review

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine niko-

    @Peter Majmesku I can help with module

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Peter Majmesku ๐Ÿ‡ฉ๐Ÿ‡ชDรผsseldorf

    @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.

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Peter Majmesku ๐Ÿ‡ฉ๐Ÿ‡ชDรผsseldorf

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

  • ๐Ÿ‡บ๐Ÿ‡ฆUkraine niko-

    @Peter, yep tnx

  • ๐Ÿ‡ฉ๐Ÿ‡ฐDenmark ressa Copenhagen

    Adding project name in title.

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sudesh.solaskar mumbai

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

  • Status changed to Needs review over 1 year ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sudesh.solaskar mumbai
  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Peter Majmesku ๐Ÿ‡ฉ๐Ÿ‡ชDรผsseldorf

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

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sudesh.solaskar mumbai

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

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Peter Majmesku ๐Ÿ‡ฉ๐Ÿ‡ชDรผsseldorf

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

  • Test for 3289053-30.patch

    patch
    Checking patch composer.json...
    Checking patch modules/permissions_by_entity/tests/src/Kernel/EntityAccessCheckTest.php...
    Checking patch modules/permissions_by_entity/tests/src/Kernel/EntityPublicationTest.php...
    Checking patch permissions_by_term.info.yml...
    error: while searching for:
      - 'drupal:taxonomy'
      - 'drupal:path_alias'
    type: module
    core_version_requirement: '^9'
    version: '3.1.9'
    package: 'Permissions by Term'
    configure: permissions_by_term.settings
    
    error: patch failed: permissions_by_term.info.yml:6
    error: permissions_by_term.info.yml: patch does not apply
    Checking patch permissions_by_term.module...
    Checking patch src/Event/PermissionsByTermDeniedEvent.php...
    Checking patch src/Listener/KernelEventListener.php...
    Checking patch src/Service/AccessCheck.php...
    Checking patch src/Service/TermHandler.php...
    Checking patch tests/src/Kernel/AccessStorageTest.php...
    Checking patch tests/src/Kernel/MultilingualTest.php...
    Checking patch tests/src/Kernel/NodeEntityBundleInfoTest.php...
    Checking patch tests/src/Kernel/PBTKernelTestBase.php...
    Checking patch tests/src/Kernel/TermHandlerTest.php...
    
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia sudesh.solaskar mumbai

    @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

  • @sudesh
    PbT Test version: 3.1.21
    Drupal version : 10.0.7

    Test PbT module info.yml

    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
    
  • This patch looks to have trouble applying because the packing script alters the info file. Likely this issue : https://www.drupal.org/project/drupal/issues/3036459 ๐Ÿ› Packaging info from .info.yml often creates conflicts when patching Active

    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 ๐Ÿ› Packaging info from .info.yml often creates conflicts when patching Active
    https://getcomposer.org/doc/06-config.md#preferred-install

  • ๐Ÿ‡ฉ๐Ÿ‡ชGermany Peter Majmesku ๐Ÿ‡ฉ๐Ÿ‡ชDรผsseldorf

    @_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... ๐Ÿ“Œ Automated Drupal 10 compatibility fixes for Permissions by Term Fixed

    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... ๐Ÿ“Œ Automated Drupal 10 compatibility fixes for Permissions by Term Fixed

    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 this issue for a better understanding https://www.drupal.org/project/drupal/issues/3036459 ๐Ÿ› Packaging info from .info.yml often creates conflicts when patching Active

  • 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.

  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia larowlan ๐Ÿ‡ฆ๐Ÿ‡บ๐Ÿ.au GMT+10

    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

  • Status changed to Needs work over 1 year ago
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Kristen Pol Santa Cruz, CA, USA

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

  • fathima.asmat London, UK

    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.

  • Status changed to Needs review over 1 year ago
  • Status changed to RTBC over 1 year ago
  • ๐Ÿ‡ฆ๐Ÿ‡บAustralia larowlan ๐Ÿ‡ฆ๐Ÿ‡บ๐Ÿ.au GMT+10

    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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Kristen Pol Santa Cruz, CA, USA

    Note that @fathima.asmat and others have been given co-maintainership per ๐Ÿ’ฌ Searching for co-maintainers Needs review so hopefully they can help get this merged in with our support.

  • fathima.asmat London, UK

    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....

  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia amitvin

    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.

  • fathima.asmat London, UK

    @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.

    • 3397a02e committed on 3.1.x-dev
      Issue #3289053 by Project Update Bot, niko-, _KASH_, fathima.asmat,...
  • fathima.asmat London, UK

    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.

  • Status changed to Fixed over 1 year ago
  • fathima.asmat London, UK

    A new issue is created to create a project release for the new tag, https://www.drupal.org/project/permissions_by_term/issues/3353813 ๐Ÿ“Œ Create 3.1.22 release for Permissions by Term Fixed

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States wheelercreek

    The new release (3.1.22) has not fixed the issue for me. I'm still seeing 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)

    Drupal 9.5.7

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States Kristen Pol Santa Cruz, CA, USA

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

  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024