🇪🇪Estonia @hkirsman

Regarding mysql57 - setting.inc is some magic file that is included https://git.drupalcode.org/project/mysql57/-/blob/1.0.x/settings.inc?ref... ?

Looking at some other core db modules was the reason I went with patching. I'm hoping for some core expert to say how to properly extend mysql module.

hkirsman → created an issue.

I don't see $transaction being unset in Drupal codebase. Also Claude.ai said it'll automatically unset after try block has finished. Could somebody verify this?

ok, thanks!

This seems to solve issue I had with encdoded backslash https://www.drupal.org/project/drupal/issues/3492437 🐛 Something wrong with symfony/http-foundation ? Active
But did I miss some commit from here because I'm now getting white page with plain text "Invalid URI: A URI cannot contain a backslash." It used to be normal page not found before.

I tried the fix from https://www.drupal.org/project/drupal/issues/3486972 🐛 DefaultExceptionHtmlSubscriber should not clone the request for 401s Active It does get rid of out of memory issue, but then again it shows white page with plain text "Invalid URI: A URI cannot contain a backslash." It used to Drupal normal "page not found" page.

I've created module that breaks the site https://github.com/hkirsman/symfony_http_foundation_debugging

composer require drupal/core-composer-scaffold:10.3.10 drupal/core-recommended:10.3.10 drupal/core-dev:10.3.10 drupal/core:10.3.10  -W 
install -d web/modules/custom
cd web/modules/custom
git clone https://github.com/hkirsman/symfony_http_foundation_debugging.git
drush en symfony_http_foundation_debugging -y
Go to path /%5C

I tested that on minimal install and same issue happened.

Thanks @catch! I'll try that last issue tomorrow. It does sound like it could be it.

We are on 10.3.10 and it seemed like the issue started with that. But then I just downgraded symfony/http-foundation to v6.4.13 and issue went away.

hkirsman → created an issue.

codebymikey → credited hkirsman → .

Here's some context how the colors changed with vanilla Drupal:
→

hkirsman → created an issue. See original summary → .

vladimiraus → credited hkirsman → .

+1

hkirsman → created an issue. See original summary → .

I'm not sure why was this issue marked as fixed?

We have green phpcs checks!

→

I've add another batch of coding standard fixes. For this I've written local tool to run the same tests https://github.com/wunderio/drupal-gitlab-local-pipelines It's using https://github.com/phpro/grumphp for running the scans - you can run them on all code base and it triggers on the files being commited. For now I haven't added this as dependency. I wasn't 100% it really does scan exactly the same as there was too much to check, but now near the end it is clear that it's 1:1 for phpcs. Other checkers can be added later, any maybe in some other tasks.

→

@marcoliver I don't see this file used anywhere so can I remove it https://git.drupalcode.org/issue/permissions_by_term-3455690/-/blob/3.1.... ?

While at this, let's try to fix some coding issues being reported and maybe also include tool like GrumpPHP so we wouldn't see the errors only in Gitlab, but already when committing code?

I ran the code through phpcbf and fixed lot of issues automatically. I've also include comments where needed, but there are still comments missing from in many tests. I'll add more fixes later.

https://git.drupalcode.org/project/permissions_by_term/-/merge_requests/50

Code Quality scans found 3 new findings and 15 fixed findings.
Test summary: 600 failed, 2 errors and 55 fixed test results, 731 total tests

->

Code Quality scans found 4 new findings and 16 fixed findings.
Test summary: 337 failed, 2 errors and 55 fixed test results, 475 total tests

https://git.drupalcode.org/issue/permissions_by_term-3455690/-/jobs/1969500

A TOTAL OF 593 SNIFF VIOLATIONS WERE FOUND IN 48 SOURCES
--------------------------------------------------------------------------------
PHPCBF CAN FIX THE 35 MARKED SOURCES AUTOMATICALLY (288 VIOLATIONS IN TOTAL)

->

A TOTAL OF 328 SNIFF VIOLATIONS WERE FOUND IN 20 SOURCES
-----------------------------------------------------------------------
PHPCBF CAN FIX THE 7 MARKED SOURCES AUTOMATICALLY (51 VIOLATIONS IN TOTAL)

Tests started failing here

https://git.drupalcode.org/project/permissions_by_term/-/commit/433606fd...

There the .gitlab-ci.yml were edited.

Ok, I understand now that there's .gitlab-ci.yml file in this repo and that configures pipelines. There we have enabled both 9.x and 10.x testing, but even if 9.x would have some issues, why is 10.x failing? It works fine in local?

hkirsman → created an issue.

This is also issue for testing:
→

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

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

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.

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

hkirsman → created an issue.

I had the same problem - redirected from landing page to user login and becasue destination parameter was emtpy, you're directed to user page. Here's patch to fix this

hkirsman → created an issue.

Any link to some PR's or commits where that "deprecation protocol" was used?

It was the only way to log if and when block was deleted.

Hm, this then needs also update on Drush side if we're going for injection.

I think we just missed that it would be better to inject it.

I installed the https://github.com/hyrsky/ckeditor_media_embed and CKEditor part seems to work fine.

On the public side though I don't see anything:

What could be the reason?

The fix from last MR works for me and will go to our prod env. Thank you! Hope this gets merged soon.

Thank you @wim-leers and @gaëlg !

I removed all of the $config['symfony_mailer. and everything still works. E-mails are being sent. As far as I know we don't have any contrib e-mail modules in our site.

For this it says unknown configuration setting:

"emptyIndent.removeIndent": true,

hkirsman → created an issue.

I believe the issue is the same as here

https://www.drupal.org/project/search_api/issues/3328702 🐛 Second parameter of request->query->get is not allowed to be an array anymore Closed: duplicate

  public function tokens(Request $request) {
    /** @var MetatagToken $metatag_token */
    $metatag_token = \Drupal::service('metatag.token');
    $token_values = array();
    $tokens       = $request->request->get('tokens');
    $data         = $request->request->get('data');

    if (is_null($data)) {
      $data = array();
    }

->

  public function tokens(Request $request) {
    /** @var MetatagToken $metatag_token */
    $metatag_token = \Drupal::service('metatag.token');
    $token_values = array();
    $tokens       = $request->request->all()['tokens'] ?? [];
    $data         = $request->request->all()['data'];

    if (is_null($data)) {
      $data = array();
    }

No more error locally.

1. What is this talk about PHP 5.3? I did find it was added 13 years ago https://git.drupalcode.org/project/filefield_sources/-/commit/57a0d0a643...
I think this might be replaced by what is in file.module

2. All t() functions return object so doing .= will fail.

3. $file->filename is not string but but object. I think something like $file->getFileName() should be used.

/**
 * Validate a file based on the $element['#upload_validators'] property.
 */
function filefield_sources_element_validate($element, $file, FormStateInterface $form_state) {
  $validators = $element['#upload_validators'];
  $errors = [];

  // Since this frequently is used to reference existing files, check that
  // they exist first in addition to the normal validations.
  if (!file_exists($file->getFileUri())) {
    $errors[] = t('The file does not exist.');
  }
  // Call the validation functions.
  else {
    foreach ($validators as $function => $args) {
      // Add the $file variable to the list of arguments and pass it by
      // reference (required for PHP 5.3 and higher).
      array_unshift($args, NULL);
      $args[0] = &$file;
      $errors = array_merge($errors, call_user_func_array($function, $args));
    }
  }

  // Check for validation errors.
  if (!empty($errors)) {
    $message = t('The selected file %name could not be referenced.', ['%name' => $file->filename]);
    if (count($errors) > 1) {
      $message .= '<ul><li>' . implode('</li><li>', $errors) . '</li></ul>';
    }
    else {
      $message .= ' ' . array_pop($errors);
    }
    $form_state->setError($element, $message);
    return 0;
  }

  return 1;
}

hkirsman → created an issue.

I believe I do. It's the last one, right?
→

Yeah, I saw those.

First is in codebase of 10.1
Second fix did not seem to help.

This seems core issue as with vanilla Drupal height attribute in markup doesn't change either (it's the original images height). Drupal uses CSS height auto to fix it.

#3348603 has been fixed but now at least in my Drupal 10.1 setup the image is resized incorrectly because it's missing the height attribute in the source.

I went with this drupalFile.js for now (the old one in red)

https://git.drupalcode.org/project/editor_file/-/merge_requests/4/diffs?...

Just saw it too. Interestingly if I save the /admin/config/content/formats/manage/basic_html and enable the plugin, then it works. If import from config, the above error happens. The configs just have the new editor_file config in it.

From the code part, I don't think this is correct:

editor_file/js/build/drupalFile.js
this.linkUI.actionsView.once(

There is now $.once() anymore in D10.

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

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

hkirsman → created an issue.

Adding some coding standard fixes.

I've applied the patch and created beta 2.

The module does not have anything else reported by Rector than the patch fixed (info file). Closing this.

hkirsman → created an issue.

Works for us.

Adding patch 2.7

I haven't done the MR request process to the end. Who is supposed to merge it now?

Follow-up at https://www.drupal.org/project/ckeditor_indentblock/issues/3368573 🐛 Can't enable indent block Fixed

Added fix.

hkirsman → created an issue.

I've tested that the MR 25 fixes the last 10 issue.

That helped, tx!

hkirsman → created an issue.

I've opend MR for this. Only one line added.

hkirsman → created an issue.

@dineshkumarbollu Could you maybe change federico prato to Federico Prato? Otherwise good to go.

Seems to be working fine.

Rector finds 0 issues.

hkirsman → created an issue.

Adding patch with some change

1. Added test for EntityInfoGetter class although definitely more would be needed
2. Split up EntityInfoGetter::getInfo() to have less cognitive complexity
3. Fix all issues reported by https://github.com/wunderio/code-quality
4. Make EntityInfoGetter::getInfo() also parse token when it's user field. It was defaulting to what is now generateDefaultString() because entity was "not known" which really meant that there was no $this->entity->getEntityType()->id() === 'user' ? $knownType = 'user'