Portland Oregon
Account created on 12 June 2007, over 17 years ago
#

Merge Requests

Recent comments

🇺🇸United States Greg Boggs Portland Oregon

Drupal 9.5 is no longer supported software. To fix this warning please upgrade to Drupal 11.

🇺🇸United States Greg Boggs Portland Oregon

There is no patch to apply. This has been committed. Just waiting for someone to test the dev release and report that it works in Drupal 11 and we can tag a release.

🇺🇸United States Greg Boggs Portland Oregon

Looks good. I'll give the dev release a test.

🇺🇸United States Greg Boggs Portland Oregon

Code looks good. Needs some more manual testing still. We'll wait for Spuky to take a look.

~G

🇺🇸United States Greg Boggs Portland Oregon

Awesome couldn't tell for certain the Diff, thanks for the extra info!

🇺🇸United States Greg Boggs Portland Oregon

Also! Great freaking work on this merge request. Tests and everything.

🇺🇸United States Greg Boggs Portland Oregon

My only question is on this line:

- $title = Html::decodeEntities(Xss::filter(trim($settings[0])));

Are we still escaping JavaScript if we remove that line?

🇺🇸United States Greg Boggs Portland Oregon

greg boggs made their first commit to this issue’s fork.

🇺🇸United States Greg Boggs Portland Oregon

Thanks for the fix! We have 2 new issues in the current release that we want to fix before the next release. But, do let us know if y'all need a release soon to not be blocked.

🇺🇸United States Greg Boggs Portland Oregon

It's off topic, but I work for our library. Thanks for all the great work y'all do at City of Portland. Feel encouraged to open an MR with what you think the best fix is here.

🇺🇸United States Greg Boggs Portland Oregon

I think it's reasonably safe to assume someone doesn't have a custom path with multiple leading slashes.

🇺🇸United States Greg Boggs Portland Oregon

oph. Thanks for the report. We'll take a look as soon as we can. Clearly we need better test coverage for this feature.

🇺🇸United States Greg Boggs Portland Oregon

To get security coverage, the module must have a stable release, and a maintainer of the module must have permission to opt into coverage. To get permission to opt into coverage, a maintainer has to have created at least one module that was reviewed and approved by the module review team.

If you already have permission to opt projects in, you can click a box. If not, here's the process:

https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

🇺🇸United States Greg Boggs Portland Oregon

A small note on |raw. Raw disables twig's output escaping so users might be able to post JavaScript in the URL if you pass content through Raw. So, you'll want to make sure that content is escaped before it gets to Twig.

🇺🇸United States Greg Boggs Portland Oregon

Saw this exception in my log slowing down my website because bots trigger this one a lot. Looks like there's been significant work done on this merge request since it's last review.

🇺🇸United States Greg Boggs Portland Oregon

Heck yea! Nice.

🇺🇸United States Greg Boggs Portland Oregon

Nice. Thank you!

🇺🇸United States Greg Boggs Portland Oregon

greg boggs made their first commit to this issue’s fork.

🇺🇸United States Greg Boggs Portland Oregon

Logo looks good. Needs a Merge request.

🇺🇸United States Greg Boggs Portland Oregon

Hrm. The site structure tag is already selected.

🇺🇸United States Greg Boggs Portland Oregon

Unless I'm just missing it, this does not appear to be in 3.3.x-dev yet? I ended up here because 3.3.x still installed varation cache and varationcache project page says:

"Please uninstall and remove this module once you are using Drupal 10.2 or higher and have no more code mentioning the Drupal\variationcache namespace."

🇺🇸United States Greg Boggs Portland Oregon

The reason the project name was in the summary was to make the summary clear when it was displayed to users in search engines.

Here's my best attempt to comply with the request while still having a complete sentence.

"This module updates the core Breadcrumb block to include the current page title in the breadcrumbs. It comes with settings that are common features needed in crumbs."

I've also adjusted the full description to better line up with the new summary.

🇺🇸United States Greg Boggs Portland Oregon

Adding a patch of the current MR for testing purposes

🇺🇸United States Greg Boggs Portland Oregon

Ok, apparently I have forgotten how to patch. This one will work, I think.

🇺🇸United States Greg Boggs Portland Oregon

mucked up the patch file last time. trying again.

🇺🇸United States Greg Boggs Portland Oregon

The current patch makes oauth scope a required field. However, some APIs don't use Oauth scope. So this patch makes oauth scope optional. Still needs a reroll for Beta 5

🇺🇸United States Greg Boggs Portland Oregon

The patch needs a re-roll for Beta5

🇺🇸United States Greg Boggs Portland Oregon

Another good place to get support is in Drupal Slack in the #support channel.

https://www.drupal.org/community/contributor-guide/reference-information...

🇺🇸United States Greg Boggs Portland Oregon

Translations have improved some. But, Zip Code label does not translate and there's no UI for adding a translation for this. Any suggestions on next steps to get missing Address labels translated?

🇺🇸United States Greg Boggs Portland Oregon

Yea, the bug is that webform doesn't set a correct page title for the thank you page. I'll open a ticket for webforms for the title bugs and link them here.

🇺🇸United States Greg Boggs Portland Oregon

When viewed in the Gin theme, the crumbs are more readable, but also strange. I think there's some bugs with Easy Breadcrumb and Webforms together

Path: https://drupal-cms-dev.ddev.site/admin/structure/webform/manage/nerd_qui...
Crumbs: Home > Administration > Structure > Webforms > Flex Your Nerdy Opinions > Flex Your Nerdy Opinions

🇺🇸United States Greg Boggs Portland Oregon

Thanks for the review! Happy to get improvements committed if someone wants to open a merge request with fixes, even if they aren't perfect yet.

🇺🇸United States Greg Boggs Portland Oregon

And to confirm the Glossary module is still abandoned with only a release for Drupal 7?

🇺🇸United States Greg Boggs Portland Oregon

I only accept code from Merge Requests on my projects, but it's still helpful if someone adds code in a patch file. It could be nice if Drupal.org notified folks when they upload a patch that the project maintainer prefers a merge request with a link to documentation on how to make one.

🇺🇸United States Greg Boggs Portland Oregon

Thanks, I'll give the patch a try.

🇺🇸United States Greg Boggs Portland Oregon

My site doesn't throw this error on drush cron, but this looks like a good defensive code addition all the same.

🇺🇸United States Greg Boggs Portland Oregon

This issue still exists. The code provided works to solve it. It would be ideal if this were a configuration in the UI instead of requiring custom code to do this. Since this is closed fixed by a maintainer, I won't re-open.

🇺🇸United States Greg Boggs Portland Oregon

that would be awesome. You should contribute the work.

Node revision delete module is a popular solution and has the UI for setting rules to delete revisions of nodes which is a good starting place for this feature.

🇺🇸United States Greg Boggs Portland Oregon

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.

🇺🇸United States Greg Boggs Portland Oregon

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?

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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.

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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)

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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.

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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.

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

Greg Boggs created an issue.

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

Awesome. Thanks for the update!

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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.

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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.

🇺🇸United States Greg Boggs Portland Oregon

I'll get it taken care of this week.

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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

🇺🇸United States Greg Boggs Portland Oregon

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.

Production build 0.71.5 2024