Access for node is removed when an unpublished translation is added

Coming back to this. We noticed that the patch works indeed for that case, but doesn't cover this:

• node with specific permissions set is created and published
• unpublished translation is created

in this case the patch only sets the default access, thus opening the node's permissions. Working on a fix.

Reworked the patch and confirmed tests are now passing. I plan on adding a couple tests for the case that we noticed failing, it will come next week.

Here are some more tests. I really think @peter-majmesku some effort should be put into making the module reliable with multilanguage installs.
I had a bit of troubles understanding how the permission_mode should work, but i hope i got it right.

thanks

What would be needed to test the correct branch? There's only 8.x-1.x available here.

permissions_by_term-3308318-18.patch and permissions_by_term-3308318-19.patch introduced new bug, permissions_by_term-3308318-10.patch seems to work. I'm new to to this so I thought I'd first start with fixing tests for the permissions_by_term-3308318-10.patch. Does that make sense? If that's ok, then maybe keep it for what was meant to be fixed in permissions_by_term-3308318-19.patch

Reroll the patch to ensure compatibility with the most recent version.

Status check. So there's merge request and patches in this issue. The new defaultGrants() method in MR and in permissions_by_term-3308318-10.patch is exactly the same. Patch permissions_by_term-3308318-18.patch already has more logic in it. Latest patches from @joey-santiago were
permissions_by_term-3308318-18.patch and permissions_by_term-3308318-19.patch
Interdiff between them is:

diff -u b/tests/src/Kernel/NodeAccessRecordsTest.php b/tests/src/Kernel/NodeAccessRecordsTest.php
--- b/tests/src/Kernel/NodeAccessRecordsTest.php
+++ b/tests/src/Kernel/NodeAccessRecordsTest.php
@@ -98,25 +98,14 @@
     $node->save();
     self::assertTrue($this->isNodeAccessRecordCreatedInPBTRealm($node->id()));
 
-    \Drupal::configFactory()->getEditable('permissions_by_term.settings')->set('permission_mode', TRUE)->save(TRUE);
-    $term = Term::create([
-      'name' => 'term2',
-      'vid'  => 'test',
-    ]);
-    $term->save();
-
-    $node = Node::create([
-      'type' => 'page',
-      'title' => 'test_title',
-      'field_tags' => [
-        [
-          'target_id' => $term->id()
-        ]
-      ],
-      'language' => 'en',
-    ]);
-    $node->save();
+    $translation = $node->addTranslation('fr');
+    $translation->title->value = $this->randomString();
+    $translation->status->value = 0;
+    $translation->save();
     self::assertTrue($this->isNodeAccessRecordCreatedInPBTRealm($node->id()));
+    self::assertFalse($this->isNodeAccessRecordCreatedInDefaultRealm($node->id(), 'en'));
+    self::assertFalse($this->isNodeAccessRecordCreatedInDefaultRealm($node->id(), 'fr'));
+
   }
 
   public function testCreateIfTermHasNoPermissionButPermissionModeIsOn(): void {
@@ -139,6 +128,15 @@
     ]);
     $node->save();
     self::assertTrue($this->isNodeAccessRecordCreatedInPBTRealm($node->id()));
+    self::assertFalse($this->isNodeAccessRecordCreatedInDefaultRealm($node->id(), 'en'));
+
+    $translation = $node->addTranslation('fr');
+    $translation->title->value = $this->randomString();
+    $translation->status->value = 0;
+    $translation->save();
+    self::assertTrue($this->isNodeAccessRecordCreatedInPBTRealm($node->id()));
+    self::assertFalse($this->isNodeAccessRecordCreatedInDefaultRealm($node->id(), 'en'));
+    self::assertFalse($this->isNodeAccessRecordCreatedInDefaultRealm($node->id(), 'fr'));
   }
 
   private function isNodeAccessRecordCreatedInPBTRealm(string $nid): bool {

So more tests were added as was said in https://www.drupal.org/project/permissions_by_term/issues/3308318#commen... 🐛 Access for node is removed when an unpublished translation is added Needs review

I'm a afraid my comment and patch in #21 might have been kind of incorrect as this is also related to https://www.drupal.org/project/drupal/issues/962664 ✨ Taxonomy Index for unpublished entities Needs work
I would say permissions_by_term-3308318-19.patch is the last correct patch for PbT 3.1.19 and Drupal core 9.

Problem is that the patch still fails. I would assume it was working in some env but what am I missing...

Ok, I think I get it now. The patch was only tested with the test that was updated, but not whole permissions_by_term test set. tests/src/Kernel/NodeAccessRecordsTest.php works just fine.

Got tests working on Permissions by Term version 3.1.19, Drupal core 9.3.22 (could have been probably 9.5.x). Next up is to update againast latest version of PbT and maybe Drupal 10.2

I could not create interdiff for some reason, but it the new patch should be correct. I've tested this on permissions_by_term 3.1.33 and core 10.2.7

This is also issue for testing:
→