🇺🇸United States @Greg Boggs

I don't see code in the current changes that assume a database. So, perhaps the comment in #18 is about surrounding code outside of these changes. I am going to mark this needs review again until it's more clear what work needs to be done on this MR.

This is a major issue. So far, none of the revision delete modules I have tested have support for Paragraphs revisions. Has anyone found a solve for this?

are you using the latest dev release or the tagged release?

Thanks for all your work on this feature. It works perfectly for us. However, the views_natural sort table is one of the largest tables in our database:

views_natural_sort ~66,998 InnoDB utf8mb4_general_ci 5.5 MiB

For context, our node revision table has 15,236 rows. So, 67k rows is quite large for our size of website. Does the module really need a table this large to function? I'm asking because the size of our database is making our website slow and I'm looking for ways to slim down our database.

After many attempts, I was able to get the uninstall through and now I have this error:

Drupal\Component\Plugin\Exception\PluginNotFoundException: The "paragraphs_library_item" entity type does not exist. in Drupal\Core\Entity\EntityTypeManager->getDefinition() (line 139 of core/lib/Drupal/Core/Entity/EntityTypeManager.php).

I cannot uninstall Paragraphs Library at all and it's creating a very large database table.

The website encountered an unexpected error. Please try again later.

TypeError: Drupal\Core\Entity\ContentEntityStorageBase::onFieldDefinitionUpdate(): Argument #1 ($field_definition) must be of type Drupal\Core\Field\FieldDefinitionInterface, null given, called in /var/www/html/web/core/lib/Drupal/Core/Field/Entity/BaseFieldOverride.php on line 234 in Drupal\Core\Entity\ContentEntityStorageBase->onFieldDefinitionUpdate() (line 546 of core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).
Drupal\Core\Field\Entity\BaseFieldOverride::postDelete(Object, Array) (Line: 460)
Drupal\Core\Entity\EntityStorageBase->delete(Array) (Line: 347)
Drupal\Core\Entity\EntityBase->delete() (Line: 212)
Drupal\Core\Config\ConfigManager->uninstall('module', 'paragraphs_library') (Line: 470)

That sounds perfect. Please do if you have the time and energy!

Greg Boggs → made their first commit to this issue’s fork.

Ok. I think I did that right.

For the tests, lets try again to fix what we can, for annoyingly difficult tests on old update hooks and such, lets just remove the problem tests. The important bit with the tests is that the tests pass, not that we have great coverage.

Awesome!

Greg Boggs → made their first commit to this issue’s fork.

Thanks for working on the tests Spuky. Looks like they are still failing:

1) Drupal\Tests\easy_breadcrumb\Functional\EasyBreadcrumbConfigureTest::test8006DefaultConfigurationUpdate
InvalidArgumentException: The configuration property capitalizator_ignored_words.0.0 doesn't exist.

smustgrave → credited Greg Boggs → .

Looks good, Thanks!

Greg Boggs → made their first commit to this issue’s fork.

woot woot

Greg Boggs → made their first commit to this issue’s fork.

Greg Boggs → created an issue.

Yes, that will work. Sorry I missed this question, I had not subwcribed to issues yet.

If anyone else finds this issue trying to figure out why they can't set their site hash in config, here's the answer on how to set and get your site hash now that it's in state:

https://acquia.my.site.com/s/article/360045953834-The-importance-of-uniq...

drush sget search_api_solr.site_hash

Awesome. Thanks for the update!

Looks good! Thanks for the work on this one team.

Unfortunately, they dev release is not ready yet. This issue still needs an MR and tests passing:

https://www.drupal.org/project/easy_breadcrumb/issues/3440889 🐛 Broken findMatchingRedirect() call Fixed

Here's the patch for that issue: It needs a Merge Request created still before I can add it to the dev release.

https://www.drupal.org/project/easy_breadcrumb/issues/3440889 🐛 Broken findMatchingRedirect() call Fixed

Hrm, what error did you get on the white screen?

This has been marked major for 8 years. So, maybe not major?

I fixed this is a couple of minutes by adding a filter to the author for the current interface language. I get no duplicates. I am not really sure about the joins and filters mentioned in 15, but adding this filter works for me.

looks good, can we get a MR for Git lab CI testing?

Hi Abhikeh Gupta,

Lets keep using findMatchingRedirect rather than switching to a different method while fixing this issue since findMatchingRedirect was working previously and was tested.

To improve workflow, we will need Merge Requests and not patches.

I'll get it taken care of this week.

ok got it! Sounds like we can include this as is.

Thanks!

Greg Boggs → made their first commit to this issue’s fork.

The code looks perfect! I'd merge it, but it looks like the patch file is also in the MR, and we don't want the patch file to end up in the final module.

Thanks for the fix. Generally, we use array_key_exists to check to see if the key exists.

Also, please open merge requests and not patches for code you want to include.

Lets go Ruslan! Thank you so much for getting things committed. Can you create a tagged release for the module so we have a clear release to test and a change log? I'll find some time to re-run my D7 to D10 migration and test the release.

https://www.drupal.org/docs/develop/git/git-for-drupal-project-maintaine... →

MR looks good.

I posted this as a combined patch with the other features I posted because this patch depends on the Likert patch that hasn't been looked at yet.

Greg Boggs → created an issue.

looks good. Feel encouraged to open an MR.

Needs an MR opened.

Have you tried updating Facet API module to the dev release to fix the bug instead of re-rolling this patch? Updating the Facet module should fix the issue for almost everyone (yes the Facet module needs a new tagged release because the current tagged release only works in Drupal 9 despite being marked as Drupal 10.

It looks like it needs a rebase against the latest 2.0 code so that it's mergeable. The dev release has a buuunch of coding standards fixes that have probably impacted this merge.

Heck Yea!

Here's a patch of the latest work in the MR

+1 for this feature. The page titles and metatags on all my contact forms are poor.

Awesome, thanks for spending time looking at Easy Breadcrumb, let me know if I can help.

~G

Awesome work, Thanks for helping! Feel encouraged to fix more things and get more credits, if you're interested, I saw some strings inside t() that were $this->t("String"); and they should be $this->t('string'); And, our tests are also not passing. So those could use some love too.

Please open merge requests and not patches for code you want to include: https://www.gregboggs.com/drupal-merge-requests/

Ha! Thanks. You write correct code. I should merge it. That's the social contract. I figure if I do my part, folks are more likely to contrib because they know their work will be in a public release in short order.

Perfect solve. Thank you!

Greg Boggs → made their first commit to this issue’s fork.

Awesome. Can we get that as a merge request so it can get included into the module's releases?

wow, super smart, didn't think of that.

This patch only works with the following conditions:

1. You set your content language the same as your Interface language. So, you have to translate both the admin interface and the content. If your users have a different language for the admin interface, this patch doesn't work for them.

2. If a content type does not have translation enabled, this patch does not work and it should. Even if a node is not translatable, it still has a valid path in each language and that path should be used to prevent an untranslated node from switching a user's chosen language.

Awesome. Thanks @odai Jbr. Can you update the merge request so I can get this into the release?

I got you now. I think it has value. In fact, I was thinking tonight of making the routes a configuration item to make them customizable after someone made a new builder to customize crumbs on a single route:

https://www.drupal.org/project/easy_breadcrumb/issues/3424369 💬 Node with multiple taxonomies - can I pass value as URL param Active

Interesting approach to run a new builder restricted only for a certain path to selectively apply options. If you had used the taxonomy option in Easy Breadcrumb, it would apply to every route on the site instead of selectively to your commerce products. Super cool.

All of the improvements look great. Super straight-forward obviously better.

I can add it to the documentation.

Awesome! If you can, it would be awesome if you could post your alter hook as an example for future folks before we close this.

~G

Wow, code looks great! Adding extra hooks doesn't hurt, gives programmers the options to write custom code if they want to.

I think once we test everything, we're almost ready to merge.

There's some phpunit test fails we need to update first:

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for easy_breadcrumb.settings with the following errors: easy_breadcrumb.settings:entity_types missing schema

You can implement hook_breadcrumb_alter to modify with custom code here's an example from a contrib module:

https://git.drupalcode.org/project/dynamic_breadcrumb/-/blob/master/dyna...

You can also try the substitute feature. In general, the breadcrumb matches the path. There is a setting to display taxonomy added to crumbs, I just don't know how it chooses which one to use when there are multiple.

Sounds like a plan. I will work on it as well when I can.

I've added you both with full permissions on Easy Breadcrumb.

Awesome. Since Dynamic crumbs uses breadcrumb alter and Easy Breadcrumb doesn't, we should be able to just copy and paste the code into EB and then just test to make sure everything works together nicely.

Thanks Admirlju!

I believe the Gitlab fail is coding standards which is pretty small, and we have another ticket for getting standards to pass, I'm going to merge.

Needs a rebase against latest dev

Thanks!

Greg Boggs → made their first commit to this issue’s fork.

I think it may be fixed by https://www.drupal.org/project/easy_breadcrumb/issues/3421745 🐛 Home page segment not rendered as a link Fixed can you try the latest dev checkout and let me know?

Thanks

Thanks!

Looks good. Feel encouraged to open an MR.

Can you switch to the default theme for testing and see what happens?

The most likely cause of a broken pager is that you need to upgrade facets to the dev release. The latest stable release of Facets does not support the new pager AJAX.

MR needs a rebase from the current code.

Greg Boggs → created an issue.

Greg Boggs → created an issue.

This needs a merge request.

Looks good. This needs a Merge Request.

Needs a MR created.

Current patch does not fix crumbs in forum, only skips them.

I think this is the same issue:

https://www.drupal.org/project/easy_breadcrumb/issues/3196198 🐛 "Paths to be excluded while generating segments" option says regex is supported, but it is not Needs work

This needs a rebase against current release.