🇨🇦Canada @TrevorBradley

Account created on 5 April 2010, over 14 years ago
#

Recent comments

🇨🇦Canada TrevorBradley

Nope, user error here. webform_icheck_webform_libraries_info() is just fine (especially when uninstalled).

Folks, a note it's important to run a composer update --lock to make sure composer.lock correctly links to the right library.

🇨🇦Canada TrevorBradley

6.2.8 doesn't include https://www.drupal.org/files/issues/2024-11-26/webform-icheck-library-is... .

I'm still getting deploy fails with the call in webform_icheck.module:

In CurlDownloader.php line 641:
                                                                               
  The "https://codeload.github.com/drgullin/icheck/zip/1.0.2/tags/1.0.2" file  
   could not be downloaded (HTTP/2 404 )                                       
                                             
function webform_icheck_webform_libraries_info() {
  $libraries = [];
  $libraries['jquery.icheck'] = [
    'title' => t('jQuery: iCheck'),
    'description' => t('Highly customizable checkboxes and radio buttons.'),
    'notes' => t('iCheck is used to optionally enhance checkboxes and radio buttons.'),
    'homepage_url' => Url::fromUri('http://icheck.fronteed.com/'),
    'download_url' => Url::fromUri('https://github.com/dargullin/icheck/archive/refs/tags/1.0.2.zip'),
    'version' => '1.0.2 ',
    'optional' => FALSE,
    'deprecated' => t('The iCheck library is not being maintained. It has been <a href=":href">deprecated</a> and will be removed in Webform 7.0.', [':href' => 'https://www.drupal.org/project/webform/issues/2931154']),
    'license' => 'MIT',
  ];
  return $libraries;
}

Trying to patch webform in addition to the 6.2.8 update to see if that fixes the issue...

🇨🇦Canada TrevorBradley

For example, this trivial patch would allow additional context to be passed down to the XmlEncoder, and now a simple call to:

$response = new ResourceResponse($response, $response_code, [], ['xml_root_node_name' => 'myroot']);

allows me to put in my own arbitrary root node.

I'm not proposing this as a solution, just trying to understand the problem. Why is the definition of the $serialization_context in ResourceResponseSubscriber so rigid?

🇨🇦Canada TrevorBradley

Adding a request to also look at this issue for the new 6.x branch.

Would love to have a little more control over the >3,000 daily simple_oauth errors in the logs on our high volume site.

🇨🇦Canada TrevorBradley

Wrote up a hopefully polite documentation complaint on the D11 documentation queue here: https://www.drupal.org/project/drupal/issues/3460690 🐛 web.config functionality removed without deprecating FileSecurity::writeWebConfig() Active

🇨🇦Canada TrevorBradley

....and if you're using lando like me: lando rebuild lando's APC caches the contents of the vendor directory in a weird way as referenced here: https://www.drupal.org/forum/support/post-installation/2018-01-13/fatal-...

🇨🇦Canada TrevorBradley

Yup, the problem with a drupal_git install to get around patching is that it ignores ALL requirements... Not just drupal/consumers but also everything in simple_oauth's compiser.json:

    "require": {
        "league/oauth2-server": "^8.5 || ^9.0",
        "steverhoades/oauth2-openid-connect-server": "^2.6",
        "drupal/consumers": "^1.17",
        "php": ">=8.1"
    },

Simple short term fix:
composer require league/oauth2-server:^8.5 steverhoades/oauth2-openid-connect-server:^2.6

and all is well.

Learning moment for me, hopefully useful to some poor person doing a google search later.

🇨🇦Canada TrevorBradley

It's definitely not a circular reference - in Container:get() the authentication_subscriber service never moves out of the loading state.

It's an error during loading the loading the container: Interface "League\OAuth2\Server\Repositories\AccessTokenRepositoryInterface" not found

Yup, that library is missing from the vendor directory... This is almost certainly related to broken requirements because of my weird drupal_git install.

Let me figure out how to install simple_oauth correctly on Drupal 11, document it here, then close this ticket.

🇨🇦Canada TrevorBradley

Note: This error was so bad on 2.19 (Complete CSS collapse) as we updated to 10.2.6, and the fix with 2.20 so complete, it's my opinion only but I'd strongly recommend flagging 2.19 as no longer supported.

We didn't catch in in our local and our dev environments because JS aggregation was only enabled on prod.

🇨🇦Canada TrevorBradley

Verified that requiring chi-teck/drupal-code-generator:4.x-dev will get around the issue - just be sure to clean up when Drupal 11 and Drush 13 go live. 

🇨🇦Canada TrevorBradley

Sorry for taking so long to get back to this.

I've recreated my scenario 4 before and verified it's still failing to behave as expected.

I upgraded to config_ignore 8.x-3.x dev (hash 96094ab987) which includes the recent fix to 🐛 Splits: Ensure config_ignore works for everyone - set default to functional value Needs review and it does work! (but only after a cache clear). If a setting is both in config_split and config_ignore it will not import in the split specific directory.

Now, drush cex still exports and overwrites the split+ignored value when it shouldn't, but if I recall that's a different, known issue.

Verified that composer require drupal/config_ignore:3.x-dev#96094ab987 solves this issue. I'll watch for that config_ignore next release.

Let me close this.

🇨🇦Canada TrevorBradley

@bircher - Acknowledge receipt of message.

This is in my queue - which unfortunately has blown up. I'll see if I can research this next week.

🇨🇦Canada TrevorBradley

And how do you expect this to work when you export the config?

Presumably with the new config_ignore, with a simple config split it should not export or overwrite anything.

Thanks for the other issue link. I'll check this out Monday and see if it's a duplicate.

🇨🇦Canada TrevorBradley

Config Ignore's config is ultra simple. I'm wondering if it's something in the preamble of config split

langcode: en
status: false
dependencies: {  }
id: local
label: Local
description: ''
weight: 2
stackable: false
no_patching: false
storage: folder
folder: ../config/local
🇨🇦Canada TrevorBradley

Again, this patch is not ideal - open to ideas as to how this should be solved.

🇨🇦Canada TrevorBradley

Testing looks good on my systems here. Layout Builder styles seem to apply well, no weirdness, errors on editing, creating styles, etc.
+1 to switching to bootstrap_styles 2.x branch. :D

🇨🇦Canada TrevorBradley

Just a heads up for anyone having issues deploying this.

I previously had bootstrap_layout_builder 2.1.2, which required bootstrap_styles 1.1.5, which required media_library_theme_reset 1.5.0

The strategy of the patch is to require bootstrap_styles:^2.0 - a little dicey as bootstrap_styles 2.x doesn't have a release branch, but let's run with it.

It's insufficient to simply patch composer.json, as the composer dependencies don't care for patches. To get around this, you need to (temporarily) install bootstrap_layout_builder from a different source, to avoid composer dependencies. In the repositories section of composer.json:

        {
            "type": "package",
            "package": {
                "name": "drupal_git/bootstrap_layout_builder",
                "version": "2.1.2",
                "type": "drupal-module",
                "source": {
                    "url": "https://git.drupalcode.org/project/bootstrap_layout_builder.git",
                    "type": "git",
                    "reference": "2.1.2"
                }
            }
        }

Then in the patches section of composer.json:

wget https://git.drupalcode.org/project/bootstrap_layout_builder/-/merge_requests/22.diff
mv 22.diff patches/3325151-bootstrap_layout_builder-media_library_theme_reset_not_supported-20231215.diff
            "drupal_git/bootstrap_layout_builder": {
              "3325151: Media Library Theme Reset not supported": "patches/3325151-bootstrap_layout_builder-media_library_theme_reset_not_supported-20231215.diff"
            }

(don't put merge_requests directly into your composer.json, kids)

And in the require section of composer.json:

"drupal_git/bootstrap_layout_builder": "^2.1.2",

(Usually I reserve this drupal_git repository trick for patched dev branches, but it'll work here for now. This whole drupal_git business )

Run composer install to get bootstrap_layout_builder installed.

For some reason, this didn't upgrade bootstrap_styles for me. I had to do the following steps:

1) composer require drupal/bootstrap_styles:^2.0
2) I had to do a drush cex (to capture changes to bootstrap_styles.settings.yml), remove media_library_theme_reset: 0 from my core.extension.yml and do a drush cim

I think this leaves me with a system upgraded to bootstrap_styles-2.x dev branch, no dependency on media_library_theme_reset (imposed by bootstrap_styles 1.1.5) and media_library_theme_reset removed from my system.

Initial testing suggests this is working fine. My Bootstrap Layout Builder admin menu is working as expected, my odd custom bootstrap layout sections appear to be working. Going to deploy this to my dev server and see if it survives further testing...

🇨🇦Canada TrevorBradley

Just wandered in here from Media Library Theme Reset. It looks like the transition is finally here. The module is no longer "deprecated" but has now moved into "unsupported" state. See: https://www.drupal.org/project/media_library_theme_reset

🇨🇦Canada TrevorBradley

Just wandered in here from the Bootstrap Media Theme Reset module. It appears to have finally moved into its unsupported state. See: https://www.drupal.org/project/media_library_theme_reset

🇨🇦Canada TrevorBradley

@arafen. Will do. If this ticket is closed by then I'll create a new ticket (But I'll reference it from here).

It's something to do with the theme not being defined in the request. I thought it might be bootstrap related, but when I went to set Claro as the default theme the problem persisted. It's... tricky. Might not be able to get to it for few days but I'll crack it.

🇨🇦Canada TrevorBradley

@catch - I was on Drupal 10.1.5. I just upgraded to 10.1.6 - No change - I still see BadRequestHttpException in the watchdog as a php log of type "Error". when attempting to access /web/sites/default/css/foo.

I tried this on a plain D10.1.6 site, and you're right, only a simple page not found error. This is something specific to my site I need to go investigate. Looks like something odd going on in my theme layer that just happens to throw the same error.

Carry on and ignore me for now. :D Thanks for the info.

🇨🇦Canada TrevorBradley

@catch - to clarify, yes this isn't throwing a PHP error causing a WSOD. It is however adding an entry to watchdog, of type "php", with severity "Error".

🇨🇦Canada TrevorBradley

Just a tiny bit more context - this also fires on any Drupal 10.1 site with any 404 in the /sites/default/files/css/ directory. e.g. /sites/default/files/css/foo

I noticed this while trying to use week old web logs to do load testing on a new server. The css files had become out of date and renamed on the new server. Every time I loaded an old missing css file, this php error would fire.

The argument here is that what is effectively a file not found warning should not throw a BadRequestHttpException and pollute the logs.

🇨🇦Canada TrevorBradley

Argh, my git logs reference the wrong issue. I'm going to see if I can fix this, but feel free to trash my commits and reroll them.

🇨🇦Canada TrevorBradley

I'm similarly seeing

Deprecated function: Creation of dynamic property Drupal\views_natural_sort\IndexRecord::$type is deprecated in Drupal\views_natural_sort\IndexRecord->generateType() (line 213 of /app/web/modules/contrib/views_natural_sort/src/IndexRecord.php)

It's still deprecated, not an error, so give me a few days to come back around to this.

🇨🇦Canada TrevorBradley

Let's try this patch out - not suggesting this is the solution, but it hopefully will help someone else out.

🇨🇦Canada TrevorBradley

This is an old one, but I similarly hit a page not found on /modules/contrib/we_megamenu/assets/css/we_megamenu_backend.css.map

This patch does not apply against 1.16 without error.

🇨🇦Canada TrevorBradley

@davemybes, there's a third option:

You can add the git repo for a contrib module into the repositories section of your composer.json:

        {
            "type": "package",
            "package": {
                "name": "drupal_git/encryption",
                "version": "2.0.0",
                "type": "drupal-module",
                "source": {
                    "url": "https://git.drupalcode.org/project/encryption.git",
                    "type": "git",
                    "reference": "8.x-1.x"
                }
            }
        },

You can then composer require drupal_git/encryption, rather than drupal/encryption (in your require section), and then patch encryption in your extras => patches section, and it does work. This gets around the info.yml version requirement.

Example of this here: https://www.computerminds.co.uk/articles/apply-drupal-9-compatibility-pa...

🇨🇦Canada TrevorBradley

Just some context - after a bit more investigation my client was trying to enter in a digital rights window of 99 years, apparently quite common in legal contexts.

https://en.wikipedia.org/wiki/99-year_lease

In history, 99 years was initially a placeholder for "forever" but apparently as the century passed, the law got kind of prickly about being literally 99 years and now it's commonplace.

This is in a field where rights windows are quite frequently just a few weeks, but they want to be able to use the field the same way for both instances.

🇨🇦Canada TrevorBradley

I hit an issue configuring the settings.php variant on my AWS server.   tx_isolation didn't exist.

Instead I used:

   'init_commands' => [
      'isolation_level' => 'SET SESSION transaction_isolation=\'READ-COMMITTED\'',
    ],

... and this worked for me.

The documentation here is inconsistent about tx_isolation vs transaction_isolation.

🇨🇦Canada TrevorBradley

I see WSOD, I open a ticket. No judgement on how fast it gets fixed.

Either 1 ticket or two works for me. I see the justification for both, but if they're two separate commits, they'll definitely need to be sequenced.

🇨🇦Canada TrevorBradley

Something's not quite right about that math. I swear the duration data is in minutes, not seconds. Signed mediumint is 2^23 = 8.38M minutes = 15.9 years, and the system fails on 16 years but works on 14 years.

So a signed integer (2^31 minutes) would be 4085 years.

(If duration isn't minutes but seconds, something else odd is going on)

Honestly, I think it's more important to put in validation on the date range field to prevent a WSOD.

🇨🇦Canada TrevorBradley

> Why do you need to store events with such large durations?

In this case, it's genuinely an 18 year date range, just negotiated up from a 14 year date range. The client wants to store the date range for which rights have been granted for a particular media object, and wants to store that date range authoritatively (not fake it and say the rights started today).

I dunno, copyright licensing is weird.

Regardless of the duration, the module should throw an error on validation rather than WSOD on an invalid SQL insertion, regardless of the data being entered into the UI.

🇨🇦Canada TrevorBradley

@mandclu. Yes. That updated the date fields.

This is explicitly about the duration field, which I believe maxes out at signed mediumint.

Here's the DB table post 4.1.0-rc2 update:

mysql> describe node__field_rights_window;
+---------------------------------+--------------+------+-----+---------+-------+
| Field                           | Type         | Null | Key | Default | Extra |
+---------------------------------+--------------+------+-----+---------+-------+
| bundle                          | varchar(128) | NO   | MUL |         |       |
| deleted                         | tinyint      | NO   | PRI | 0       |       |
| entity_id                       | int unsigned | NO   | PRI | NULL    |       |
| revision_id                     | int unsigned | NO   | MUL | NULL    |       |
| langcode                        | varchar(32)  | NO   | PRI |         |       |
| delta                           | int unsigned | NO   | PRI | NULL    |       |
| field_rights_window_value       | bigint       | YES  | MUL | NULL    |       |
| field_rights_window_end_value   | bigint       | YES  | MUL | NULL    |       |
| field_rights_window_duration    | mediumint    | YES  |     | NULL    |       |
| field_rights_window_rrule       | int          | YES  | MUL | NULL    |       |
| field_rights_window_rrule_index | int          | YES  | MUL | NULL    |       |
| field_rights_window_timezone    | varchar(32)  | YES  |     | NULL    |       |
+---------------------------------+--------------+------+-----+---------+-------+
12 rows in set (0.00 sec)

Note field_rights_window_duration is still a mediumint.

🇨🇦Canada TrevorBradley

Sorry for all the spam folks.

I'm a bit stumped as to why #12 isn't applying but #18 is. Someone better check this one last time.

I moved this out of "Reviewed and Tested" back to "Needs Review". Something's not quite right with #12.

🇨🇦Canada TrevorBradley

Sorry folks, my brain is broken on Friday. Let's try this one last time.

🇨🇦Canada TrevorBradley

Darn it, uploaded the wrong file...

🇨🇦Canada TrevorBradley

I have no idea why, but patch #12 does not deploy against the dev branch. Here's my revised version:

🇨🇦Canada TrevorBradley

I don't care about commit credit at all. Just desperately needed this fixed for my site.

Close this ticket as a duplicate and steal the code for the main ticket, genuinely dont mind at all.

🇨🇦Canada TrevorBradley

Quick and simple patch to properly check for null values.

🇨🇦Canada TrevorBradley

Looks like the fault was introduced with 8.x-1.14. 8.x-1.13 allows me to place blocks.

The fault is triggered in web/core/lib/Drupal/Core/Plugin/CategorizingPluginManagerTrait.php.

strnatcasecmp(): Argument #1 ($string1) must be of type string, Drupal\\system\\Entity\\Menu given in strnatcasecmp() (line 97 of /app/web/core/lib/Drupal/Core/Plugin/CategorizingPluginManagerTrait.php)

The issue appears to be that the Place Block button expects the block array definition of 'admin_label' to be a string, but instead it's a Drupal\system\Entity\Menu.

Testing various git branches, it looks like the fault was introduced in 1f9da / 📌 Drupal 10 readiness Fixed .

Got it. There's a tiny fault in web/modules/composer/we_megamenu/src/Plugin/Derivative/WeMegaMenuBlock::getDerivativeDefinitions.

Patch incoming.

🇨🇦Canada TrevorBradley

I hit this exact problem. This was previously resolved as part of 🐛 Deprecated error /PHP. 8.0 Drupal 9 Fixed .

It looks like 1.15 is a broken release - removing previously fixed bugs.

You can revert back to 1.14. Or you can go back and reapply the patches from 🐛 Deprecated error /PHP. 8.0 Drupal 9 Fixed . They apply cleanly.

🇨🇦Canada TrevorBradley

Verified. 1.15 just reverted the fixes as made part of 🐛 Deprecated error /PHP. 8.0 Drupal 9 Fixed

Commit ce2a7d4 was wiped out with 1.15:
https://git.drupalcode.org/project/we_megamenu/-/commit/ce2a7d4760c100a4...

Reverting back to 1.14 resolves this issue.

I think 1.15 is broken.

🇨🇦Canada TrevorBradley

Looks like this was broken as part of

commit dba69e001b93d96e546b8b6627b1e691ec03d1d5 (tag: 8.x-1.14-rc1)
Author: Bao Tran
Date: Mon Aug 14 16:19:55 2023 +0700

Various revert & fixes

ce2a7d47 got reverted as part of this update 3 days ago.

🇨🇦Canada TrevorBradley

Neither patch 11 or 12 is applying against drupal/simple_pass_reset:dev-1.x#4041f81.

Looks like a standard core/core_version_requirement fix - let me roll my own...

Ahh, it's identical to patch #2, which applies just fine against dev-1.x#4041f81

@carsonw, @Gold, I wonder if you're patching the dev version or the release version?

🇨🇦Canada TrevorBradley

@generalredneck - testing now...

On my local:

Remove my -2 patch from composer.json

Switch back and forth between my local and prod config split environments to replicate the problem...

Verified, the problem is still there. Going from local to production:

  SQLSTATE[22007]: Invalid datetime format: 1292 Truncated incorrect DOUBLE value: 'devel': DELETE FROM "views_natural_sort"
  WHERE ("eid" = :db_condition_placeholder_0) AND ("entity_type" = :db_condition_placeholder_1); Array
  (
      [:db_condition_placeholder_0] => devel
      [:db_condition_placeholder_1] => menu
  )

Adding -4 patch.

Switch back and forth between my local and prod config split environments to replicate the problem...

Works great! I can verify the #4 patch is a good replacement for my #2 patch.

🇨🇦Canada TrevorBradley

@generalredneck - this is on my radar, but I won't be able to look at it until Monday at the earliest.

Thank you!

🇨🇦Canada TrevorBradley

I get that this solution does work... but why does it work? I'm a little cautious about a solution where we don't understand the root problem.

I see that this change adds the following to autoload_psr4.php and autoload_static.php:

$ diff site_fixed/vendor/composer/autoload_psr4.php site_old/vendor/composer/autoload_psr4.php 
68d67
<     'Drupal\\bootstrap\\' => array($baseDir . '/web/themes/contrib/bootstrap/src'),
$ diff site_fixed/vendor/composer/autoload_static.php site_old/vendor/composer/autoload_static.php 
119d118
<             'Drupal\\bootstrap\\' => 17,
384,387d382
<         ),
<         'Drupal\\bootstrap\\' => 
<         array (
<             0 => __DIR__ . '/../..' . '/web/themes/contrib/bootstrap/src',

But other themes like claro don't even have a composer.json file.

I would have guessed this has something to do with bootstrap's slight of hand tricks with autoload, but I'm still really unclear why this is working fine without the autoload fix on my lando local but not working on my production sites.

I really don't want to delay a working fix though.

🇨🇦Canada TrevorBradley

Possibly related to: https://github.com/drupalprojects/bootstrap/blob/8.x-3.x/autoload-fix.php ?

Why does bootstrap need an "autoload-fix.php" in the first place? Anyone know the history of this? Could this fix finally have broken?

There's a function _find_autoloader() in scripts/bootstrap.php that makes reference to Drupal 8 - I'm wondering if this mechanism needs a tuneup.

🇨🇦Canada TrevorBradley

I tried tackling this again this evening, without much headway. I did determine a few things:

1) This isn't a bootstrap subtheme issue. The same error 500 occurs on both subthemes and bootstrap.

2) Full stack trace for anyone who can decipher what might be happening:

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

Error: Class "Drupal\bootstrap\Bootstrap" not found in include_once() (line 31 of themes/contrib/bootstrap/bootstrap.theme).
Drupal\Core\Extension\Extension->load() (Line: 137)
Drupal\Core\Theme\ThemeInitialization->loadActiveTheme() (Line: 74)
Drupal\Core\Theme\ThemeInitialization->initTheme() (Line: 150)
Drupal\system\Controller\AssetControllerBase->deliver()
call_user_func_array() (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 583)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext() (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 166)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 74)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 58)
Drupal\Core\StackMiddleware\Session->handle() (Line: 48)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 704)
Drupal\Core\DrupalKernel->handle() (Line: 19)

I'm going to bump this up to Major - obviously something is bootstrap specific and clashing with the 10.1 update. Anyone who thinks that's overkill can come help us solve this nasty issue.

🇨🇦Canada TrevorBradley

@KlemenDEV: Two pieces of evidence that contradict the nginx/PHP-FPM theory:

1) This is happening to me on apache.
2) This isn't happening on other themes like Claro.

🇨🇦Canada TrevorBradley

There are entries in the D10.1.0 changelog that imply D10.1 handle building css and js aggregation differently. See: https://www.drupal.org/node/2888767

Basically the theme file is failing to load the library with "use Drupal\bootstrap\Bootstrap;". The use command is there but it can't find it. But my contrib *.module files similarly use "use" statements with no issues.

What bugs me is the fact that this is working on my local (after I turned on caching and re-enabled css/js aggregation) but failing on my deployed dev server. There are small mismatches in apache, php, mysql versions and some libraries, but nothing that quite adds up.

"drush cr" kind of kicks it in the pants, but not enough. It's the oddest behaviour.

I'll think about this and see what I come up with tomorrow.

🇨🇦Canada TrevorBradley

@pick_d - heads up that bootstrap 3.28 works great on Drupal 10.0.9 if you want to get started on the D10 major upgrade. Fix the version: "composer require drupal/core-recommended:10.0.9".

I would caution against a D10.1.0 upgrade until we figure this out.

Annoyingly, I can't replicate this on my lando local - it works great with aggregated CSS. Investigating.

🇨🇦Canada TrevorBradley

Derp. My patch still has that errant code in it. One last time...

On the positive side - I just tested this on my D10 site, by moving the patched code into modules/custom - works great in D10.

🇨🇦Canada TrevorBradley

Additionally - upgrade_status did a reactor test against the code - apart from the core/drush updates, there's no deprecated code. This should be a fairly straightforward upgrade.

🇨🇦Canada TrevorBradley

Derp - I've got multiple patches in my patch. Once more with feeling...

🇨🇦Canada TrevorBradley

Just stumbled across this. I just created this same patch independently. This solution is correct - core_version_requirement, drupal/core, AND drush/drush need to be upgraded.

@scambler, you've got a tiny error in your patch: "drush/drush": "^10.0" || ^11.0 isn't valid. Let's try this new patch.

🇨🇦Canada TrevorBradley

OK, an update.

I've mostly gotten Patch #4 to work for D10, with modifications. There are flaws, one of them I can't fix without context.

Again, patch #4 applies only against 8.x-2.6, not the current dev branch.

Current fixes to get things mostly working:

Apply patch #4 to 8.x-2.6

Change private_taxonomy.info.yml to permit version ^10

Modify line 115 of src/Plugin/Field/FieldWidget/PrivateTaxonomyAutocompleteWidget.php:
'#autocomplete_route_name' => $this->getSetting('autocomplete_route_name'),
(the old function was deprecated)

I thought everything looked great, but then I pointed PHPStan against it. I found a few faults in src/Type/PrivateTaxonomyTermReferenceItem.php:

It makes reference to several classes without referring to them in a use clause - I added them in manually:

use Drupal\Core\Entity\EntityTypeInterface;
use Drupal\Core\Entity\FieldableEntityInterface;
use Drupal\Core\Entity\TypedData\EntityDataDefinition;
use Drupal\Core\Field\FieldItemBase;
use Drupal\Core\Field\FieldStorageDefinitionInterface;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\Core\TypedData\DataReferenceDefinition;
use Drupal\Core\TypedData\DataReferenceTargetDefinition;

Also, one bug I can't fix. propertyDefinitions() makes reference to $target_type_info twice, but doesn't define it.

Anyways, TLDR, Patch #4 is pretty close, would be good to get it to apply against dev to get things moving forward.

🇨🇦Canada TrevorBradley

Verified - Once I figured out the dance of the authorization code flow - a grant_type:authorization_code correctly returns a refresh_token, but a simple client_credentials does not. Presumably because "Client Credentials" are about the credentials of an app, rather than being the credentials of a user. Simple clients don't need refresh tokens.

🇨🇦Canada TrevorBradley

I'm pretty sure a client_credentials request doesn't generate refresh tokens. vendor/league/oauth2-server/src/Grant/ClientCredentialsGrant.php::respondToAccessTokenRequest() only generates Access Tokens. By comparison, AuthCodeGrant seems to generate both AccessToken and Refresh Tokens.

Let me ditch Client Credentials and try to get Authorization Codes working.

🇨🇦Canada TrevorBradley

I see where Refresh Token can be enabled on both the Consumer entity and the Scope. However, I thought this was about *accepting* the refresh token, rather than *generating* one as part of the Client Credentials authorization. I'm still only getting access_tokens from /oauth/token, no matter how I seem to configure the consumer or scope.

Do I need to parameterize the request to /oauth/token in some way to get it to generate a refresh_token alongside my access_token? Right now I'm just passing in client_id, grant_type (set to client_credentials for now), and scope (although that's optional, it will use the default if I don't)

Obviously using grant_type = refresh_token is incorrect here - I need the refresh token before I can use it.

🇨🇦Canada TrevorBradley

As best I can tell, my local testing suggests the patch in #4 works against 8.x-2.6 (so log as I change info.yml to permit Drupal 10), but when I attempted to create a fork against 8.x-2.x-dev, the patch fails to apply.

I'm going to assume there have been changes since 2.6 for a reason. But one vote up for this automated patch still working in D10.

Production build 0.71.5 2024