Account created on 4 August 2016, almost 8 years ago
#

Merge Requests

More

Recent comments

🇧🇪Belgium gorkagr

Indeed that path is broken in 10.3.0

I have just noticed line 318 (the one that causes the error) is

    $form = $this->formBuilder()->getForm($this);

comparing the DraggableListBuilder class, in 10.2 there is available:


  /**
   * Returns the form builder.
   *
   * @return \Drupal\Core\Form\FormBuilderInterface
   *   The form builder.
   */
  protected function formBuilder() {
    if (!$this->formBuilder) {
      $this->formBuilder = \Drupal::formBuilder();
    }
    return $this->formBuilder;
  }

while in 10.3 that function it is not available anymore but it seems using $this->formBuilder is ok
So maybe having a code such as to maintain BC ??

    if (\version_compare(\Drupal::VERSION, '10.2.9999', '<')) {
      $form = $this->formBuilder()->getForm($this);
    }
    else {
      $form = $this->formBuilder->getForm($this);
    }

but i cannot check this right now in any 10.2.x or lower drupal site

🇧🇪Belgium gorkagr

Hi!
You have to configure the domains on admin/config/multi_domain_login

Then, once you do login in one domain, a session will be open in the rest of the domains configured.

🇧🇪Belgium gorkagr

Hi!

Not sure if it is the same issue or not, as i did not see anything related with the "Send all affiliates" field.
I have opened smth similar with a patch provided there ( https://www.drupal.org/project/domain_path/issues/3452676#comment-15628074 🐛 Assuming 'field_domain_all_affiliates' exists create duplicate aliases Needs review )

🇧🇪Belgium gorkagr

hi!

after https://www.drupal.org/project/http_client_manager/issues/3451503 🐛 Revert #3446808 and rename branch 3.x Fixed i believe this should be now against branch 10.x instead to 9.3.x.

Best

🇧🇪Belgium gorkagr

yeah, all seems good now.

9.3 should be back to be compatible with old drupals (sorry one more time) anow the newer version appears to be installed

🇧🇪Belgium gorkagr

@achton, i opened https://www.drupal.org/project/http_client_manager/issues/3451503 🐛 Revert #3446808 and rename branch 3.x Fixed as a follow-up to revert this change (so u can still use 9.x without pinning the version), but also to propose a renaming of 3.x so it uses a proper versioning :)

Apologies

🇧🇪Belgium gorkagr

I have pushed the code to revert 3446808.
Then the only tasks remaining are:
- to merge and do a new release on 9.x so people using the module are not affected
- maybe flag as unsecure 9.3.11 (apologies)
- and to rename 3.x branch to 10.x so the versioning is higher, making a 10.x release

🇧🇪Belgium gorkagr

hi @achon

After upating to 9.3.11 (therefore this pacth), all is good.
But it seems your symfony is a bit outdated, as

Symfony\Component\Config\Loader\LoaderInterface::load($resource, $type = null)

seems to be a bit old code

Which version of Drupal / PHP are you?

Just to see the version that I got installed of symfony:

$ ddev composer info symfony/config
name     : symfony/config
descrip. : Helps you find, load, combine, autofill and validate configuration values of any kind
keywords : 
versions : * v7.0.7
released : 2024-04-18, 1 month ago
...
🇧🇪Belgium gorkagr

All good now, update_status can analise the module and skip the @internal errors :)

🇧🇪Belgium gorkagr

Hi!

Before applying the patch, i get the following error in one module:

Class Drupal\ieg_core\Form\BaseCreateNodeEventForm extends @internal class Drupal\node\NodeForm.

after applying the patch, i get the following error:

PHPStan command failed:

/usr/bin/php /var/www/html/vendor/bin/phpstan analyse --memory-limit=1500M --error-format=json --configuration=/tmp/upgrade_status/deprecation_testing.neon /var/www/html/web/modules/custom/ieg_core
Command output:

Empty.
Command error:

In NeonAdapter.php line 44: Error while loading /tmp/upgrade_status/deprecation_testing.neon: Duplicated key 'drupal_root' on line 6, column 3. analyse [-c|--configuration CONFIGURATION] [-l|--level LEVEL] [--no-progress] [--debug] [-a|--autoload-file AUTOLOAD-FILE] [--error-format ERROR-FORMAT] [-b|--generate-baseline [GENERATE-BASELINE]] [--allow-empty-baseline] [--memory-limit MEMORY-LIMIT] [--xdebug] [--fix] [--watch] [--pro] [--fail-without-result-cache] [--] [...]

best

🇧🇪Belgium gorkagr

Hi!

As per (https://github.com/mglaman/phpstan-drupal/pull/754/commits/e3f40b68f35d4...), released in mglaman/phpstan-drupal V1.2.11 on May 10th, if you have that version in your local the error is accepted.

There is a second issue opened in upgrade_status module ( https://www.drupal.org/project/upgrade_status/issues/3445307 Disable ClassExtendsInternalClassRule Needs work ) so until that one is merged, the error is only fixed in the CLI analysis.

Nevertheless I have update the phpstan.neon of the module to include the rule that triggers the error for then upgrade_status is updated :)

🇧🇪Belgium gorkagr

As this is blocking the option of removing the users (and somehow the patch is not applied fully in the composer.lock when I am doing composer update drupal/http_client_manager -W), could we also have a 9.3.11 release when this is merged, please?

🇧🇪Belgium gorkagr

I think the issue only appears when doing PHPStan analysis to the code, but I havent run drupal-status to the module nor PHPstan command to this module.

Best

🇧🇪Belgium gorkagr

Hi!

I have also noted in 3.3.x the issue is still active.

When doing a basic code such as:

  $new_location = \Drupal::service('entity_type.manager')->getStorage('node')->create([my_fields]);
  $new_location->save();
  $new_event->addRelationship($new_location, 'plugin');

if the node entity contains any postSave() function, such as:

  /**
   * {@inheritdoc}
   */
  public function postSave(EntityStorageInterface $storage, $update = TRUE): void {

    // PostSave as expected.
    parent::postSave($storage, $update);

    // Do this only in an update; the insert should be done via hook too.
    if ($update) {
      // Let's do here some redirection here for example
    }
  }

}

that one is triggered one more time during the addRelationship() function as the following function is triggered (in GroupRelationship.php):

  /**
   * {@inheritdoc}
   */
  public function postSave(EntityStorageInterface $storage, $update = TRUE) {
    parent::postSave($storage, $update);

    // For memberships, we generally need to rebuild the group role cache for
    // the member's user account in the target group.
    $rebuild_group_role_cache = $this->getPluginId() == 'group_membership';

    if ($update === FALSE) {
      // We want to make sure that the entity we just added to the group behaves
      // as a grouped entity. This means we may need to update access records,
      // flush some caches containing the entity or perform other operations we
      // cannot possibly know about. Lucky for us, all of that behavior usually
      // happens when saving an entity so let's re-save the added entity.
      $this->getEntity()->save();
    }
  ........
🇧🇪Belgium gorkagr

Hi @amandeep_lnwebworks

Patch in #8 seems incomplete and code missing, and the install() seems to not work as the info of the module is incomplete.
My patch only works with the latest code from the dev branch, not if you have the latest release install (for that you need the rest of the patches in between this MR and the last release); If i see the image of the error, it looks like you have missing code as line 139 does not match with the changes i did.

I will hide for the moment your patch just in case and will wait for a maintainer to take a look at both.

🇧🇪Belgium gorkagr

HI

I have reviewed the comments. In https://git.drupalcode.org/project/consumers/-/merge_requests/12/diffs?c... i c&p from the class to the installer and i did not notice the @label, but in the first commit was good, so i set directly consumer inside the t().

Best

🇧🇪Belgium gorkagr

I forgot that tests could fail as the patch is using code that does not exist in the consumers module. So back to "needs work" until consumers has a new release with the proposed MR

thnks

🇧🇪Belgium gorkagr

Hi!

I think that is the code needed to create a new status field and set a value to existing consumers.
I have also added a new column on the listBuilder as well.

Best

🇧🇪Belgium gorkagr

Hi!

I believe that is the place to check if a consumer is active/inactive in combination with the Consumers' MR mentioned in the issue.
Best

🇧🇪Belgium gorkagr

Hi!

if using this module (v5.2.5) for login in a gitlab instance from a D10, I get the following error with the respective MR:

Could not authenticate you from OpenIDConnect because "Request uri must have schema. possibly add 'http://' to the request uri?".

Without the patch, i have the error in #1.

in gitlab I have 2 login providers: oauth2_generic (works fine) and openid_connect (error)

Config details:

gitlab_rails['omniauth_providers'] = [
  {
    name: "openid_connect",
    label: "OIC",
    args: {
      name: 'openid_connect',
      scope: ['openid', 'oauth2_access_to_profile_information'],
      response_type: 'code',
      issuer: 'https://oauth.ddev.site',
      discovery: false,
      uid_field: 'preferred_username',
      client_auth_method: 'basic',
      client_options: {
        identifier: 'gitlab',
        secret: 'gitlab',
        redirect_uri: 'https://gitlab.ddev.site/users/auth/openid_connect/callback',
        userinfo_endpoint: "https://oauth.ddev.site/oauth/userinfo",
        authorization_endpoint: "https://oauth.ddev.site/oauth/authorize",
        token_endpoint: "https://oauth.ddev.site/oauth/token"
      }
    }
  },
 {
   name: "oauth2_generic",
   label: "OAUTH",
   app_id: "git",
   app_secret: "git",
   args: {
     client_options: {
       site: "https://oauth.ddev.site",
       user_info_url: "/oauth/v1/userinfo",
       authorize_url: "/oauth/authorize",
       token_url: "/oauth/token"
     },
     user_response_structure: {
       root_path: [],
       id_path: ["sub"],
       attributes: {
         email: "email",
         name: "name"
       }
     },
     authorize_params: {
       scope: "oauth2_access_to_profile_information"
     },
     strategy_class: "OmniAuth::Strategies::OAuth2Generic"
   }
 }
]
🇧🇪Belgium gorkagr

Agreed on the comment, it is always the hardest part :D :D , but i liked your last sentence so i have used it ;)

@cat, you mean in core/modules/system/tests/modules/performance_test/src/Cache/CacheTagsChecksumDecorator.php::isValid(), correct?

if we do as well there the

    if (empty($tags)) {
      return TRUE;
    }

should we also do a $this->logCacheTagOperation([], 0, 0, CacheTagOperation::isValid); (or smth like that before the return? Or there is no need to log the operation?

🇧🇪Belgium gorkagr

Hi!
I hope is fine, i have added the snippet of @wim Leers in https://www.drupal.org/project/drupal/issues/3421881#comment-15456992 📌 Track cache tag queries separately in performance tests Active in here :)

🇧🇪Belgium gorkagr

If that is the approach, then i assume the same view should be created in a hook_update for those with the module already enabled and the view not created.

The other option i was considering was to alter the GroupViewBuilder and overwrite isViewModeCacheable(), but might be better the view approach :)

🇧🇪Belgium gorkagr

Implemented @nod_ suggestion and also updated the test as node__1 there uses a template on the theme and not in core/modules/node/templates, so that's why the test was failing. All should be good, unless we need a new test to cover the case where no template is overridden (maybe with the node_edit_form template?)

🇧🇪Belgium gorkagr

ok, with my last commit, the test fails.
But if understand correctly the test flow, the active theme ($variables['directory']) is 'core/modules/system/tests/themes/test_theme/', correct?
and because of that, the template of check is provided by the theme too (not the node module itself), so the assert ($this->assertStringContainsString("BEGIN OUTPUT from '$template_filename'", $output, 'Full path to current template file found.');) should be changed too?

🇧🇪Belgium gorkagr

Changed to see if the template file that is being checked is from the active directory, so in that case it should be an overridden template, shouldnt it?

Best

🇧🇪Belgium gorkagr

I have removed the return type, but as the function is protected, i think it was ok to leave it there (i did query the function within git and i did not find a match except for his function itself)

🇧🇪Belgium gorkagr

If it is missing to anonymous only it could be some missing permissions (either general ones or maybe inside group if the module is being used)

🇧🇪Belgium gorkagr

Implemented the suggestion if I understood it correctly

🇧🇪Belgium gorkagr

A template could also be placed in a module under the /custom folder, so the last commit I'd rather say "CUSTOM TEMPLATE" rather than custom theme as it might point to the wrong location

🇧🇪Belgium gorkagr

Just updated DDEV to latest version and it comes with php8.2.15 (and it seems with 8.3.2 too based on the changelog)

The error on the CLI is gone, and i assume any other error.

I am moving this to "needs review" but I'd say that we can change it to "Fixed" and close the issue if more ppl have the msg gone

🇧🇪Belgium gorkagr

@larowlan

I opened an issue about that

https://www.drupal.org/project/drupal/issues/3414368#comment-15422422 🐛 Incorrect types in variables used in Drupal\Core\Field\Annotation\FieldType Needs review

🇧🇪Belgium gorkagr

Sorry, i forgot to remove the draft the last time.
Replied to the comment with one example where an array is used in core :)

🇧🇪Belgium gorkagr

IF this is implemented, should the file be added as well to the .gitignore file?

🇧🇪Belgium gorkagr

Hi!

I have adapted & implemented the changes done here (for 11.x) locally to a 10.2 (as FieldStorageAddForm is quite different) and fields with a 1-line description now appears properly (eg: reference fields or the ones of the address module with the patch in https://www.drupal.org/project/address/issues/3413017 🐛 Using a translatable string as a category for field type is deprecated in drupal:10.2 Needs work ), as you can see in both images uploaded.

🇧🇪Belgium gorkagr

Can be the test fails as i left @ingroup plugin_translatable but the var now is set as \Drupal\Core\Annotation\Translation|array? or it is not related and failed another thing?

🇧🇪Belgium gorkagr

Although fixed, another example of a module (comment module of core) affected if a role (authenticated in that case) is deleted.

🇧🇪Belgium gorkagr

Taking a look at the code lines of your error:

      $this->authenticatedCanPostComments = $this->entityTypeManager
        ->getStorage('user_role')
        ->load(RoleInterface::AUTHENTICATED_ID)
        ->hasPermission('post comments');

the code is trying to load the 'authenticated' user. If you are missing it from your system, then it will be null and hasPermission will fail obviously.

So try to restore the user as I have mentioned before :)

🇧🇪Belgium gorkagr

Hi!

For restoring your missing role, you can do either:
- create it via the UI and set the machine name "authenticated"
or
- if you have a second drupal site, get the file from config/sync/user.role.authenticated.yml, copy it into the faulty site, open it with the editor to double check first you dont have any permission you should not, and then run with drush a config import (drush cim) so it is restored.

Most likely, if you dont have that role, the problems might come from there

🇧🇪Belgium gorkagr

8.2.14 landed a few days ago :)

I dont know if DDEV updates php.automatically or a new release is needed, but smth for next week :)

Best

🇧🇪Belgium gorkagr

gorkagr changed the visibility of the branch 3409663-rebased-datetime-duplication to active.

🇧🇪Belgium gorkagr

gorkagr changed the visibility of the branch 3409663-rebased-datetime-duplication to hidden.

🇧🇪Belgium gorkagr

Hi @kristiaanvandeneynde

New patch works fine at least in 3.2.2 :)
thnks

🇧🇪Belgium gorkagr

That is more a question for a core maintainer

🇧🇪Belgium gorkagr

Hi, sorry for all the comment and edits

I have also noticed the css it is used for rendering the icon field-icon-date_time, not even field-icon-daterange, as it takes the category for the rendering due to the new panel.

🇧🇪Belgium gorkagr

Hi!
You just changed the description but not the actual array value.

🇧🇪Belgium gorkagr

Hi @lauriii

Applied the last patch. It works with and without the @traslation in the description and also with plugins without the description

Thnks for the patch :)

🇧🇪Belgium gorkagr

Something such as


  /**
   * {@inheritdoc}
   */
  public function __construct(array $configuration, string $plugin_id, array $plugin_definition) {
    $plugin_id = $configuration['unique_identifier'];
    $plugin_definition = [
      'label' => $configuration['label'],
      'description' => (isset($configuration['description'])
        ? ($configuration['description'] instanceof TranslatableMarkup ? $configuration['description'] : new TranslatableMarkup($configuration['description']))
        : new TranslatableMarkup('')),
      'weight' => $configuration['weight'] ?? 0,
    ] + $plugin_definition;
    parent::__construct($configuration, $plugin_id, $plugin_definition);
  }

can cover the empty case, the string only and the use of @translation in the annotation.

🇧🇪Belgium gorkagr

Hi @longwave

Having only description = "text" in the annotation causes:

TypeError: Drupal\Core\Field\FieldTypeCategory::getDescription(): Return value must be of type Drupal\Core\StringTranslation\TranslatableMarkup, string returned in Drupal\Core\Field\FieldTypeCategory->getDescription() (line 26 of core/lib/Drupal/Core/Field/FieldTypeCategory.php).

so yes, there is an issue with that part too

🇧🇪Belgium gorkagr

Hi

Also on the UI the following error (I also have the drush one too)

Warning: Narrowing occurred during type inference of ZEND_FETCH_DIM_W. Please file a bug report on https://github.com/php/php-src/issues in include() (line 576 of /var/www/html/vendor/composer/ClassLoader.php).

include('/var/www/html/web/core/includes/bootstrap.inc') (Line: 576)
Composer\Autoload\{closure}('/var/www/html/web/modules/contrib/upgrade_status/src/ProjectCollector.php') (Line: 427)
Composer\Autoload\ClassLoader->loadClass('Drupal\upgrade_status\ProjectCollector') (Line: 259)
Drupal\Component\DependencyInjection\Container->createService(Array, 'upgrade_status.project_collector') (Line: 177)
Drupal\Component\DependencyInjection\Container->get('upgrade_status.project_collector') (Line: 127)
Drupal\upgrade_status\Form\UpgradeStatusForm::create(Object) (Line: 28)
Drupal\Core\DependencyInjection\ClassResolver->getInstanceFromDefinition('\Drupal\upgrade_status\Form\UpgradeStatusForm') (Line: 48)
Drupal\Core\Controller\HtmlFormController->getFormObject(Object, '\Drupal\upgrade_status\Form\UpgradeStatusForm') (Line: 58)
Drupal\Core\Controller\FormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 627)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 121)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 181)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 76)
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: 36)
Drupal\Core\StackMiddleware\AjaxPageState->handle(Object, 1, 1) (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle(Object, 1, 1) (Line: 704)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Same versions of everything as the OP

Production build 0.69.0 2024