Account created on 20 August 2008, almost 16 years ago
#

Merge Requests

More

Recent comments

🇬🇧United Kingdom welly

Just to note I'm creating these patches so we can apply them as standard in our composer workflow :) Let me know how we can get this issue over the line and merged. This is a pretty important bit of work for a project! Cheers!

🇬🇧United Kingdom welly

$options['update'] should be an int not a bool.

🇬🇧United Kingdom welly

Previous patch didn't apply for some reason. Let's try this one

🇬🇧United Kingdom welly

Having tested this recently, the changes made in the merge request work well and I'm able to expose additional config in the migrate form (I'm currently also overriding the MigrateSourceUi form class).

I've created a backport patch for RC1 (as that's the version I'm currently using) but happy to help out moving this ticket along. Is there anything specific that needs doing?

🇬🇧United Kingdom welly

Running tests..

🇬🇧United Kingdom welly

welly made their first commit to this issue’s fork.

🇬🇧United Kingdom welly

Thanks for the contribution. Have merged that and will look at your other tickets and hopefully create a new release for them as soon as possible.

🇬🇧United Kingdom welly

Yeah, this is a great start! There were a couple of QA issues that needed cleaning up (coding standards, spelling) which I've done but happy to merge this.

Thanks @Mingsong!

🇬🇧United Kingdom welly

welly made their first commit to this issue’s fork.

🇬🇧United Kingdom welly

I don't think this is necessary any longer looking at the changes made from the D10 compatibility fixes. Let's close this.

🇬🇧United Kingdom welly

> Curious why you didn't use trim((string) $var) instead?

Yeah, looking at that post, this seems like a tidier approach. I'll update the merge request and include the changes in the patch, will test it against Drupal 10.

🇬🇧United Kingdom welly

Have been experiencing redirect loops on a site we're using the patch with so back to "needs work" and I'll try and have a look at this myself.

🇬🇧United Kingdom welly

Right, I think this is ready to be looked at. There were some additional changes that needed making. Would be keen on any feedback on the changes made (this included enabling gitlab-ci for the module and then making updates to pass phpstan tests)

🇬🇧United Kingdom welly

welly made their first commit to this issue’s fork.

🇬🇧United Kingdom welly

@Steven Jones, that was the patch in #4 I tested and have been using. Cheers!

🇬🇧United Kingdom welly

Trying again to change status - it didn't change before!

🇬🇧United Kingdom welly

I'm going to put this into RTBC because I'm not experiencing any redirect loops and we've fairly thoroughly tested the patch in a staging environment with no experience of redirect loops. And in fact, trying to create a redirect to itself the module flags this and gives a error message.

🇬🇧United Kingdom welly

I think this should be working pretty well now. Will need some testing.

🇬🇧United Kingdom welly

I've created a rough patch to sort the language array which is related to this bit of work. I'll try and integrate your work and get this new behaviour finished. Thanks for making a start on it!

🇬🇧United Kingdom welly

This patch doesn't output any languages on the front end. Backend works fairly nicely though.

🇬🇧United Kingdom welly

Thank you very much! Appreciate your approval!

🇬🇧United Kingdom welly

I've been experiencing similar issues and have tried the patch. So far, so good. But will need more testing. I'm not sure how Redirect currently handles redirects pointing to itself but a redirect loop sounds like it would be the expected behaviour to me. Before applying the patch, what happened in that example? Did it resolve itself or something else?

🇬🇧United Kingdom welly

So I've spent a bit of time trying to resolve this TimestampFormatterSettingsUpdateTest error without success. When that test runs, the runUpdate method call in testPostUpdateTimestampFormatter fails with the following assertion failures

There should be no errors in configuration 'core.entity_view_display.node.page.default'. Errors:

Schema key core.entity_view_display.node.page.default:third_party_settings.layout_builder.sections.0.components.b961467b-96b6-47da-baab-fec95554f963.configuration.label_display failed with: variable type is string but applied schema class is Drupal\Core\TypedData\Plugin\DataType\BooleanData
Schema key core.entity_view_display.node.page.default:third_party_settings.layout_builder.sections.0.components.89467df8-8ca7-48a6-9f92-8292f125dd36.configuration.label_display failed with: variable type is string but applied schema class is Drupal\Core\TypedData\Plugin\DataType\BooleanData

Failed asserting that Array &0 (
    'core.entity_view_display.node.page.default:third_party_settings.layout_builder.sections.0.components.b961467b-96b6-47da-baab-fec95554f963.configuration.label_display' => 'variable type is string but applied schema class is Drupal\Core\TypedData\Plugin\DataType\BooleanData'
    'core.entity_view_display.node.page.default:third_party_settings.layout_builder.sections.0.components.89467df8-8ca7-48a6-9f92-8292f125dd36.configuration.label_display' => 'variable type is string but applied schema class is Drupal\Core\TypedData\Plugin\DataType\BooleanData'
) is true.
 

I've tried to debug where the config change is occurring that is inserting those two components but I've had absolutely no success. I'd like to get this issue over the line but this one has me stumped and I've hit a wall.

Would appreciate any help with this one!

Thank you

🇬🇧United Kingdom welly

Removed reference to Drupal Console as that project is neither still in development nor recommended nor covered by the Drupal security advisory policy. I don't think we should be recommending anyone use that project any longer.

🇬🇧United Kingdom welly

Link to desktop modeller was incorrect.

🇬🇧United Kingdom welly

I'll fix it for the sake of a clean phpcs report! 🙂

🇬🇧United Kingdom welly

Forgot to update this

🇬🇧United Kingdom welly

I've found a few more and pushed a commit fixing those.

Now, I'll try and find a "proper" issue to work on! :)

🇬🇧United Kingdom welly

That makes sense, thanks very much @jurgenhaas! Appreciate your work on this module, it's a brilliant endeavour!

🇬🇧United Kingdom welly

Just to clarify this a bit further, I'm new to ECA and also trying to understand successors but possibly in the context of the classic modeller.

If I understand it correctly, the event, conditions and actions are not a "this first (event), then this (condition) and then this (action)"? but the conditions and actions fieldsets provide conditions and actions for the events created in the events fieldset?

I originally understood it as you create an event (user logs in) and then you create a condition (user role is admin) and then you create an action (show a message) - but actually the condition and action you actually assign to the event?

🇬🇧United Kingdom welly

Initial tests written. Will revisit this issue in the future when there is more behaviour to test.

🇬🇧United Kingdom welly

welly created an issue.

🇬🇧United Kingdom welly

@apaderno Thanks for your comments and quick review! I've updated this issue to another project I've been working on recently which will hopefully be more suitable for review! This one includes a custom config entity, plus use of state management, services etc.

🇬🇧United Kingdom welly

welly created an issue.

🇬🇧United Kingdom welly

I think this can probably be done with Cookie Condition ( https://www.drupal.org/project/cookie_condition ) in combination with Feature Toggle i some cases - for example, blocks, but would agree it'd be a useful feature to enable conditions including cookies.

Switches ( https://www.drupal.org/project/switches ) allows this behaviour but it looks like that module is not longer being maintained.

🇬🇧United Kingdom welly

Moving this to Drupal.org project ownership as project maintainer/owner hasn't responded in over 2 weeks.

Would also like to offer to co-maintain the project.

https://www.drupal.org/project/cookie_condition

🇬🇧United Kingdom welly

I've updated the documentation using the description on the module home page, however I'm aware some of the behaviour isn't available - such as hooks in the D7 version. I'll update the documentation to reflect this.

🇬🇧United Kingdom welly

Looks good to me for some initial documentation.

🇬🇧United Kingdom welly

welly made their first commit to this issue’s fork.

🇬🇧United Kingdom welly

@heddn, good shout. I've created a merge request now.

🇬🇧United Kingdom welly

Going to look at the modified fixtures and see if they need repairing....

🇬🇧United Kingdom welly

Have hit a bit of a wall with these tests.

Failed tests I'm coming up against are the following:

1. https://git.drupalcode.org/issue/drupal-2544708/-/jobs/381558#L1695

I'm struggling to see where the string type variable is coming from. When I put breakpoints into the code, I can see that the config value for core.entity_view_display.node.page.default:third_party_settings.layout_builder.sections.0.components.aeb0f8ae-7f1a-4d65-aaeb-c26ce56fa27d.configuration.label_display is set to "0". I just don't know where that is coming from.

It doesn't seem to be here: https://git.drupalcode.org/issue/drupal-2544708/-/blob/2544708-display-t...

2. https://git.drupalcode.org/issue/drupal-2544708/-/jobs/381558#L818

This seems to be unrelated to my PR so again, struggling to see how this is failing. If I check

core/tests/fixtures/config_install/multilingual.tar.gz there doesn't seem to be any deleted views configs so again, struggling to understand how this is failing.

3. https://git.drupalcode.org/issue/drupal-2544708/-/jobs/381557

All the tests are passing so not sure what the fail is here.

ERROR: Job failed: pod "runner-tdd8nzs6-project-119803-concurrent-1-w67bvbwo" status is "Failed"

And there are some failed tests in the Functional Javascript tests which I'll come back to as I've not looked at them too closely yet.

So any advice would be much appreciated!

🇬🇧United Kingdom welly

reverted changes. this works

🇬🇧United Kingdom welly

Updated ddev instructions

🇬🇧United Kingdom welly

In the DDEV instructions, composer project wasn't created so ddev composer install fails with the following error:

Composer could not find a composer.json file in /var/www/html
To initialize a project, please create a composer.json file. See https://getcomposer.org/basic-usage
Composer [install] failed, composer command failed: exit status 1. stderr=

🇬🇧United Kingdom welly

There is already an existing issue for this. Please can we collaborate on this ticket 📌 Automated Drupal 10 compatibility fixes RTBC

🇬🇧United Kingdom welly

The changes you made to MessageViewBuilder.php and LayoutBuilderEntityViewDisplay.php I'm pretty certain are not correct although the other two ()help.post_update.php and MigrateBlockTest.php) look to have been missed. The failing tests are update tests which need a correct update hook - the one I added isn't quite correct. Will be looking at this on Friday and hopefully will be able to get this passing all tests! :-)

🇬🇧United Kingdom welly

Another +1 for DDEV. I've used Lando in the past, although not recently, and it's a capable and solid system but ddev is incredibly simple to get going and support from @rfay is second to none as well as the #ddev on drupal slack being a little more noticeably active than Lando.

🇬🇧United Kingdom welly

Sorry about the mess above! I'm still learning this new system.

I've got most of this done now, but having a problem with some tests passing, which you can see here - https://git.drupalcode.org/issue/drupal-2544708/-/jobs/202425

Would welcome some advice on how to get those tests passing. I believe it will be because the block config in the config tarball will have the old schema but not 100% sure how to update that.

I'll put this into "needs review" so we can get some eyes on it but appreciate it's not quite ready!

🇬🇧United Kingdom welly

Looks pretty good to me. I would say it'd probably be preferable to inject the AccountProxy service rather than using Drupal::currentUser but can see the module was previously using that method to get the user. The class is well set up for dependency injection so I think it might be worth doing that.

That's the only thing I would suggest, otherwise looks good.

I'll put this in "Needs work" for now - if you feel like using dependency injection for the current user/accountproxy service then please do otherwise perhaps someone else can take a look and move it to RTBC?

🇬🇧United Kingdom welly

Also cleaned up the CSS as per the stylelint feedback from Gitlab. Obviously this will need testing to make sure it all still works/looks correct!

🇬🇧United Kingdom welly

Looks good to me - I've also committed the gitlab-ci template so we can take advantage of running automated tests (mainly phpcs at this point).

🇬🇧United Kingdom welly

welly made their first commit to this issue’s fork.

🇬🇧United Kingdom welly

welly created an issue.

🇬🇧United Kingdom welly

I've found that this issue seems to occur when I use drush to run a migration - drush migrate:import users_migration, which has a required dependency on users_roles_migration, results in this error:

Migration users_migration did not meet the requirements. Missing migrations users_roles_migration. requirements: users_roles_migration.

However, if I run the migration through the Migrate Tools UI it executes correctly and runs the required dependencies first. I've got $settings['cache']['default'] = 'cache.backend.null'; commented out.

🇬🇧United Kingdom welly

Ok, closing this (again) :) It looks like it's something in MenuTreeStorage which is causing this issue and I think I've found a work around.

🇬🇧United Kingdom welly

Reopening this as I'm running into a new similar/related issue. When programmatically creating menu links that will live in a different menu entirely, I keep finding that menu_tree gets updated with group_menu_link_content-1 rather than the menu name that I've specified.

I've been creating my menu link as follows. The node is a node that belongs to a group so wonder if something is post processing the menu link creation to assign the link to a different menu? Oddly, if I look at menu_link_content, I can see the correct group has been stored in menu_name, it's just in menu_tree where it differs.

      $menu_link = \Drupal::entityTypeManager()
        ->getStorage('menu_link_content')
        ->create([
          'title' => $definition['title'],
          'description' => $definition['description'],
          'link' => [
            'uri' => 'entity:node/' . $definition['route_parameters']['node'],
          ],
          'menu_name' => $menu_name,
          'weight' => $definition['weight'],
          'enabled' => boolval($definition['enabled']) ? TRUE : FALSE,
          'expanded' => boolval($definition['expanded']) ? TRUE : FALSE,
          'parent' => $definition['id'] !== $parent_menu_link['id'] ? $parent_menu_link['id'] : NULL,
        ]);
      $menu_link->save();
🇬🇧United Kingdom welly

Just as an update, I also uninstalled pathauto, ran drush cr and I didn't get the error when deleting a menu link. So I guess it's all related somewhere. Will do some further investigation.

🇬🇧United Kingdom welly

I've uninstalled redirect module and I still get the following error:

Drupal\Core\Entity\EntityStorageException: Some mandatory parameters are missing ("group") to generate a URL for route "entity.group_content_menu.canonical". in Drupal\Core\Entity\Sql\SqlContentEntityStorage->delete() (line 761 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).
Drupal\Core\Routing\UrlGenerator->getInternalPathFromRoute('entity.group_content_menu.canonical', Object, Array) (Line: 132)
Drupal\Core\Routing\UrlGenerator->getPathFromRoute('entity.group_content_menu.canonical', Array) (Line: 68)
Drupal\Core\Render\MetadataBubblingUrlGenerator->getPathFromRoute('entity.group_content_menu.canonical', Array) (Line: 802)
Drupal\Core\Url->getInternalPath() (Line: 32)
Drupal\path\Plugin\Field\FieldType\PathFieldItemList->computeValue() (Line: 33)
Drupal\pathauto\PathautoFieldItemList->computeValue() (Line: 34)
Drupal\path\Plugin\Field\FieldType\PathFieldItemList->ensureComputedValue() (Line: 15)
Drupal\pathauto\PathautoFieldItemList->delegateMethod('preSave') (Line: 191)
Drupal\Core\Field\FieldItemList->preSave() (Line: 938)
Drupal\Core\Entity\ContentEntityStorageBase->invokeFieldMethod('preSave', Object) (Line: 888)
Drupal\Core\Entity\ContentEntityStorageBase->invokeHook('presave', Object) (Line: 563)
Drupal\Core\Entity\EntityStorageBase->doPreSave(Object) (Line: 753)
Drupal\Core\Entity\ContentEntityStorageBase->doPreSave(Object) (Line: 517)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 804)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 339)
Drupal\Core\Entity\EntityBase->save() (Line: 294)
Drupal\group\Entity\GroupRelationship::postDelete(Object, Array) (Line: 494)
Drupal\Core\Entity\EntityStorageBase->delete(Array) (Line: 751)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->delete(Array) (Line: 347)
Drupal\Core\Entity\EntityBase->delete() (Line: 101)
Drupal\group_content_menu\Entity\GroupContentMenu::preDelete(Object, Array) (Line: 484)
Drupal\Core\Entity\EntityStorageBase->delete(Array) (Line: 751)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->delete(Array) (Line: 347)
Drupal\Core\Entity\EntityBase->delete() (Line: 71)
Drupal\Core\Entity\ContentEntityDeleteForm->submitForm(Array, Object)
call_user_func_array(Array, Array) (Line: 114)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 52)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 597)
Drupal\Core\Form\FormBuilder->processForm('group_content_menu_subsite_menu_delete_form', Array, Object) (Line: 325)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->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: 169)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 81)
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: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 718)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

So I don't think this is related to redirect.

🇬🇧United Kingdom welly

It's just occurred to me why this isn't working. So I'll close this issue.

🇬🇧United Kingdom welly

Not working for me unfortunately. I'm getting the same error with or without the patch.

Production build 0.69.0 2024