- Status changed to Needs review
almost 2 years ago 7:29am 7 February 2023 - ๐ฉ๐ช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?
- ๐บ๐ธUnited States Amber Himes Matz Portland, OR USA
- Status changed to Needs work
over 1 year ago 3:22am 9 March 2023 - ๐บ๐ธ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
- ๐ฉ๐ช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?
- ๐ฎ๐ณ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 6:25am 25 March 2023 - ๐ฉ๐ช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.7Test 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
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 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 2:53am 5 April 2023 - ๐บ๐ธ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 modulepermissions_by_entity
.The patch applies cleanly on D9.5 and D10 as well,
upgrade_status
seems happy just with a flag for SymfonyHttpKernelInterface::MASTER_REQUEST
which can't be replaced to useHttpKernelInterface::MAIN_REQUEST
yet as the module has to support D9.5 as well. - Status changed to Needs review
over 1 year ago 3:17pm 10 April 2023 - Status changed to RTBC
over 1 year ago 12:27am 11 April 2023 - ๐ฆ๐บ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 forHttpKernelInterface::MASTER_REQUEST
asMAIN_REQUEST
constant is only supported by Drupal 10 but not D9. To keep the module compatible for both D9 and D10, we should retainHttpKernelInterface::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,...
- 3397a02e committed on 3.1.x-dev
- 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 4:39pm 12 April 2023 - 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
- fathima.asmat London, UK
There is a new release for D10 compatibility - https://www.drupal.org/project/permissions_by_term/releases/3.1.22 โ .
- ๐บ๐ธ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.