Account created on 4 August 2016, over 7 years ago
#

Merge Requests

More

Recent comments

πŸ‡§πŸ‡ͺ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

gorkagr β†’ made their first commit to this issue’s fork.

πŸ‡§πŸ‡ͺ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

gorkagr β†’ made their first commit to this issue’s fork.

πŸ‡§πŸ‡ͺ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

As it seems to be fixed, we can set the issue as fixed I believe :)

πŸ‡§πŸ‡ͺ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

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi!

I like the idea of #55 as using a class called LocalActionWithRedirect is more intuitive than MenuLinkAdd.

On the other hand, in the documentation (a bit outdated), it is written in both menu.links β†’ and local.tasks β†’ examples of how to use the 'options' within the yml, and:

  options:
    query:
      destination: '/path/to/route'

can be defined if we need a destination different than the same route (which is added by default when using the class).

But nothing is documented in the links.actions page

Best

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi!

Thanks for the patch. I can confirm it works as well on the 3.2.0 version too.
Best,

πŸ‡§πŸ‡ͺBelgium gorkagr

Sorry, i have realized the proper link should be https://www.drupal.org/node/3383363 β†’

Right now the `@see ` points to https://www.drupal.org/project/group/issues/3383363 β†’ instead (causing 404)

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi

I can confirm the patch works and I can run rector in a module or via CLI after the patch is applied.
Thnks!

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi!
the demo button goes to a second line as well in your image.
Which resolution do you have in your device?

It might be that it does not have enough space to render everything in one line and causes this issue

πŸ‡§πŸ‡ͺBelgium gorkagr

gorkagr β†’ created an issue.

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi!

Sorry, project run out of funding and we did not have allocated time nor resources to work on this.
Anyway, pushed the patches and now is D10 ready.

Will try to fix the coding standard issues raised in the tests at some point in the incoming month.

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi

In my case i did as @genebobmiller said (to downgrade and lock on 6.7.0) but alsi I locked league/uri-interfaces to 2.3.0
Now it works for me

Thnks

πŸ‡§πŸ‡ͺBelgium gorkagr

The patch; the error with drupal-check is gone as the function is defined in the interface.

Best

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi!

+1 to the patch; i did not know about the error but the patch fixes it.

Best

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi

After needed a token for subgroups as well and doing my own version , i have just found the patch (cause i did not think about it before)
Patch seems to work fine in subgroup 3.x :D :D
Tomorrow will do a bit more tests.

Thnks

πŸ‡§πŸ‡ͺBelgium gorkagr

Thanks dineshkumarbollu

You were faster than me; i was replying in https://www.drupal.org/project/drupal/issues/2980806 πŸ› Fields with unlimited cardinality show 1 extra input field Needs review before i start with this one :D

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi!

Thanks joaopauloc.dev for the patch. Unfortunately i could not apply it for 9.5.x so here it goes a path for that version.

I have noticed a duplicated declaration of $context that you have removed in your patch (I think the reason it does not work with 9.5.x), but i have opened an issue ( https://www.drupal.org/project/drupal/issues/3378657 πŸ› Duplicate declaration of $context in WidgetBase Fixed ) to spot that duplication and remove it, so i have omitted it from my patch.

PD: I have not tested it within fields inside paragraphs, but once i have a bit of time, i will check this too

Best

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi

I dont use the module, but when i was testing it, yes, the issue appeared when enabling it (in my case via drush)

best

πŸ‡§πŸ‡ͺBelgium gorkagr

Sorry, not familiar with tests (I hope now is fixed once for all)

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi!
Fixing a small error in the test, that was causing a fatal error.

πŸ‡§πŸ‡ͺBelgium gorkagr

Hi!

Thanks for the original patch!
I has facing a similar issue, as i have in a custom module a hook_node_update that was being called couple of times, just to notice that group was triggering the function during some relationship updates. Unfortunately, the original patch only works with v1.x and now v2.x (and 3.x) are released, making the patch incompatible.

Here is (what i think it should be fully compatible) the patch for the v2.x of the module (sorry, i have not tested the test).

I am not sure if i should change the version of the metadata to v2.x or to any other version, so I leave in 1.x for now

Production build https://api.contrib.social 0.61.6-2-g546bc20