Account created on 19 January 2006, over 18 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States dww

Mostly looks great, thanks! This is very close to
ready. However, I opened an MR thread about one of the new tests that I think should be addressed before this is RTBC.

🇺🇸United States dww

In case it was missed in the middle of #29:

Should we backport the deprecations in core/lib/Drupal/Core/Updater/*, too? And change them to being deprecated in 10.4.0? Or leave all that alone?

🇺🇸United States dww

p.p.s. Added a link to the CR into the draft release note. Hope that's cool. Feel free to edit / remove if needed.

p.p.p.s. Crediting @andypost and @catch for reviews.

🇺🇸United States dww

Thanks, y'all! Pushed commits to address the review of the backport MR in #27:

  1. Added deprecation to core/modules/update/src/Form/UpdateManagerInstall.php (both a @deprecated in the class comment itself, and a trigger_error() in the constructor.
  2. Cherry picked 2 commits from the 11.x MR into the 11.0.x MR to address point 2 about keeping docs in sync.
  3. Added deprecation to update_authorize_run_install() (and then fixed the syntax error I introduced 😅)

Should we backport the deprecations in core/lib/Drupal/Core/Updater/*, too? And change them to being deprecated in 10.4.0? Or leave all that alone?

Re: docs edits in #28: those might be better handled via the parent meta, no? There's already a lot of talk of docs that need updating in there. I guess we could open a specific issue to handle the docs only around this install UI as a child of this one. I defer to the release managers...

Anything else before this is ready?

Thanks again,
-Derek

🇺🇸United States dww

Whoops. I never commented here when I linked this issue as one of the points for 🌱 [meta] Deprecate tarballs, because they are incompatible with Composer-managed dependencies, Automatic Updates, Project Browser, and release managers' health Active . What’s the status of this? Is it worth immediately resurrecting for 11.0.0 release? Possibly 10.4.x? Can we in fact convert this manual thing to a composer-based solution without needing all of auto updates workings and stable?

Thanks!
-Derek

🇺🇸United States dww

P.s. I just pinged the parent meta about progress here. Probably worth reconsidering the backports here in light of that discussion.

Also, how would we deprecate a form and routes, if we did want to do that? 😅

🇺🇸United States dww

P.s. as Drumm has pointed out multiple times, parts of the summary aren’t true.

🇺🇸United States dww

We’re plowing ahead with at least removing the UI to install new stuff. 📌 Remove adding an extension via a URL Postponed now has 2 MRs:

I’m not interested in moving all this now broken stuff to a contrib module and deprecating it. It served a need for a long time, and I’m glad I wrote it when I did, but it should go, even without project browser in core.

I’m a little more torn about the updater parts. I think we really need a working replacement before we remove it all. But perhaps we could at least formally deprecate all that (not exactly sure how we deprecate a form and route, honestly).

🇺🇸United States dww

You mean for the backport MR? No, @catch explicitly said no new deprecations for the backport.

If you mean the primary MR, I don’t think it makes sense to deprecate. We should just remove all this crap per @xjm.

But thanks for reviewing!
-Derek

🇺🇸United States dww

My ride is late, so I had a moment to deal with this while still at the airport. 😅

https://git.drupalcode.org/project/drupal/-/merge_requests/8810 is now open for 11.0.x and 10.*.x branches.

@catch, is that what you had in mind? Happy to make any further changes as needed.

Thanks!
-Derek

🇺🇸United States dww

I worked on a new branch with the minimal changes described by @catch while on a flight. Just landed, but it’s gonna be a little while before I can get my laptop online and open a backport MR. assigning to myself so no one else bothers to duplicate the effort. Stay tuned. Thanks!

🇺🇸United States dww
  1. Cleaned out all the dead code in core/lib/Drupal/Core/Updater and added the deprecations discussed in the MR thread.
  2. Did some more grepping of core and removed a few lingering stale docs references to installing new extensions.
  3. Did a fresh rebase to the latest 11.x branch code (commit 45bcde63).

Latest pipeline (226079) is all green once more. I think this is RTBC again, but that's for someone else to say. 😅

Thanks,
-Derek

🇺🇸United States dww

Added a release note to the summary.
Draft CR at https://www.drupal.org/node/3461934

Back to 'Needs review', but I suspect we're going to want to make more changes in core/lib/Drupal/Core/Updater/* before this is RTBC again. Awaiting direction from core committers before I push any commits. See https://git.drupalcode.org/project/drupal/-/merge_requests/8781#note_342201

Thanks!
-Derek

🇺🇸United States dww

Crediting @longwave for the careful review, thanks!

Addressed all your MR threads, and the note in comment #15. However, that comment got me looking closely at core/lib/Drupal/Core/Updater and made me open a new MR thread for feedback. See above. Thoughts?

🇺🇸United States dww

P.s. if you want “immediate” email notifications of security updates, subscribe to the security announcements newsletter and/or consume the feed via RSS.

🇺🇸United States dww

I still think #34 makes more sense and would be vastly easier to implement than #39 or #40

🇺🇸United States dww

xjm said in slack it’d have to be 11.1.x, but I don’t really understand why it couldn’t be 11.0.0. I guess it’s too big of a change to happen now that we’re already in rc. Plus all our semi weird rules about a new major version must be identical to the last minor release of the previous major with only deprecated code removed. I still don’t fully understand or support core’s interpretation of semantic versioning, but I don’t want to die on that hill here. 😅

🇺🇸United States dww

Oh so very satisfying -- I just marked about a dozen issues closed (outdated) given this is now moving forward. 🎉 Most are now child issues of this, although some already had parents and point here as related.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed so closing this as outdated.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed so closing this as outdated.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed so closing this as outdated.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed so closing this as outdated.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed . I don't think we need any followups anymore, and we should call this fixed.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed so closing this as won't fix.

🇺🇸United States dww

The entire UI is being removed from update.module. I'm tempted to close this is outdated. There's no need to have a helper function to share code, project browser is the only thing that will need it.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed so closing this as outdated.

🇺🇸United States dww

The UI to install new stuff is being removed at 📌 Remove adding an extension via a URL Postponed .
Not sure it's worth fixing the other parts since that UI will also be removed once Auto Updates is stable.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed so closing this as outdated.

🇺🇸United States dww

This whole UI is being removed at 📌 Remove adding an extension via a URL Postponed so closing this as outdated.

🇺🇸United States dww

Sorry y'all spent so much effort trying to fix this. But the UI to install new themes (and modules) is being removed entirely at 📌 Remove adding an extension via a URL Postponed . Closing this as outdated.

🇺🇸United States dww

The default GitLab CI has a test-only job. You don't have to worry about setting up a separate branch / MR for that.

🇺🇸United States dww

Preliminary MR now available. I might have missed some spots, but this is a start. Curious what the bot thinks. 😅

🇺🇸United States dww

Per @xjm, we can proceed to remove this UI entirely. Closing this in favor of 📌 Remove adding an extension via a URL Postponed which is a more complete solution.

Thanks,
-Derek

🇺🇸United States dww

Updating proposed resolution accordingly.

🇺🇸United States dww

I believe this is duplicate with 🐛 Cron tries to send update notification email while no email is set Needs work . Let's concentrate our efforts over there.

Thanks!
-Derek

🇺🇸United States dww

Replacing the "update manager" pages entirely via autoupdates and project browser would help. Then we'd be back to a single "Available updates" report. I'm not keen on spending any energy improving the UIs that should be deprecated and removed.

And yeah, there are a bunch of issues to improve the UI of just the available updates report itself...

Tempted to call this "outdated" at this point, although neither project browser nor autoupdates are actually ready to add to core, so it's a murky area.

🇺🇸United States dww

I'm also confused about what's being proposed here. Definitely need an issue summary update and a better title.

Looking at the MR, we almost certainly do not want any changes to core/modules/update/src/Form/UpdateManagerUpdate.php. That's the form for actually updating your installed contrib code. I believe this is trying to change something about the email notifications and the available updates report itself?

🇺🇸United States dww

"If the XML endpoint was compromised this could trigger an XSS attack"

If the XML endpoint (updates.drupal.org) was compromised, XSS seems like the last of our worries. 😅 But yes, it's a good idea to harden this for defense in depth.

🇺🇸United States dww

update.module is for the available updates report at admin/reports/updates. This is a bug report for the "database update system" and update.php.

🇺🇸United States dww

This page is going to be deprecated and removed once project browser exists and is stable. So I'm not sure it's worth removing it. But yes, it's the identical form, available from 3 different locations.

🇺🇸United States dww

Indeed, there's no explicit telemetry as part of update.module. d.o "just" notices that a site has requested available update data for a specific project, which is how we "know" that project is in use.

I'd be in favor of changing this, but it's not really a feature request for update.module and more a "build a whole new telemetry feature". Moving to base system for now, since there's not yet a better component for this request.

🇺🇸United States dww

Adding screenshot from my local 9.5.11 site showing the bug.

🇺🇸United States dww

🌱 Add an event listing to Umami (to show off core's datetime handling) Active is related. I’ll try to share more concrete thoughts on this when I can. Definitely important, and I have a lot of experience and opinions about Drupal’s date and event handling. 😅

Thanks!
-Derek

🇺🇸United States dww
  1. I made a very minor edit to the CR. Agreed it’s worth having.
  2. Good catch on the test comment. Apologies I missed that. That’s what I get for quickly reviewing on my phone. 😅
  3. Changes look good. Feedback addressed. Pipeline is green.

Back to RTBC.

Thanks,
-Derek

🇺🇸United States dww
  1. Changes look reasonable to me, and IMHO are all in scope.
  2. Pipeline is green.
  3. I took a stab at a release note snippet (which we'll definitely need).
  4. I'm not sure if this needs a CR for distribution maintainers, too.
  5. Bumping to RTBC so the Product managers will see it and make a final call both on the change, and the need for a CR or not.

Thanks!
-Derek

🇺🇸United States dww

+1. I have to do this on every site I set up. Admin-only should be the default until folks opt-in to something more permissive.

🇺🇸United States dww

I’m okay to keep discussing, but 10.3.0-rc1 is out, so we have very little time to get this done if it’s going to matter at all. 😅

To me, it boils down to the costs of leaving it in (hardship for maintainers that like to “chase HEAD”) vs the costs of removing it (none, other than the time to discuss it). 😉

The primary argument from Berdir against this is basically “chasing HEAD is a fruitless, pointless effort that only leads to more work and suffering”. I pretty much agree with that, and don’t do it myself. 😂 But some maintainers like to do it, anyway, and by one perspective, they’re trying to do the right thing by staying on top of core development that impacts their code.

As I wrote in the summary, the perspective I do hold is that, when feasible (or in this case, trivial) to make it easier for contrib to put out releases that are as widely compatible as possible, we should.

Not sure who else needs to chime in before a decision is made. I only hope it happens soon. 🙏

Thanks,
-Derek

🇺🇸United States dww

I sort of mentioned at the parent, but I’ll say it again here: if this were for discussing actual supply chain attacks we knew to have occurred, we’d need to be doing it privately on the security.drupal.org issue queue. The only reason we’re talking about any of this publicly is precisely because it’s all “hypothetical” and we’re trying to mitigate vectors proactively before they’re exploited, not scramble to deal with the fallout of a real attack.

That said, I’m -1 to this proposal, too. 😅 I’d rather make it more clear when owners have changed if a (set of) maintainer(s) has/have gone missing and someone else has taken over than to encourage forking. In my biased opinion, one of the great strengths of Drupal is that we have always enabled and encouraged collaboration over forking as the fundamental model of contribution.

I, too, don’t at all agree with this:

If there is no ability to "adopt" modules the majority of the supply chain attack risk disappears and the ecosystem is protected.

I don’t believe project adoption is a (hypothetical) vector for the “majority” of supply chain attacks. Doesn’t mean we should not change anything about the process and the tools related to it, but completely preventing adoption would do much more harm to the ecosystem than it would potentially “protect”.

🇺🇸United States dww

It has been an ongoing regret that when I very first implemented the project level permissions and maintainers tab on d.o, that I didn’t build this into it from the beginning. 😢 Completely aside from the supply chain aspects and whether end users would care to see it, simply for the benefit of d.o site admins to figure out WTF is going on in various conflicts and disagreements that have come up, it would have been incredibly handy.

Also note that while issues are planned to migrate to GitLab, AFAIK, the long term plan is to keep both project nodes and releases on d.o. So the fact GitLab has a feed for some useful things doesn’t solve that the historical view of changes to the “maintainership” is lacking and would be useful, even outside the scope of the parent meta.

There’s probably an issue somewhere in the GitLab migration family of issues about how to keep project-level permissions in sync between the two worlds, but I don’t have a handy link. And yeah, the two have different models and not all perms cleanly map back and forth. It’s an unfortunate situation. If only 17 years ago me knew what I know now. 😂

🇺🇸United States dww

P.s. I’m all in favor of other meta issues to track other aspects of supply chain security that don’t directly involve the project adoption process. I tend to agree that there are plenty of possible threats that don’t require project adoption. No one should assume this is the only set of vectors worthy of exploration and mitigation. But we need small scope if we ever want to get anything done.😅

🇺🇸United States dww

@gisle: please do not take this meta or any of the child issues as a personal attack or condemnation of the work you and others have been doing with the project adoption system. Y’all do a ton of mostly thankless but extremely important work, and I’m incredibly glad you do.

I believe this was opened as an honest and sincere attempt at “defense in depth”, and mitigating whatever vectors we can dream up before they are exploited and real harm is done.

I don’t believe it is wise to only focus on what has been done (and how) from previous exploits. Malice can come in many forms. People who set their eyes on Drupal will find ways to exploit our community and processes, not necessarily replicate other attacks on other “supply chains”.

I personally oppose some (many?) of the proposed solutions in child issues. 😂 But I’m grateful for the discussions, and think there will be some agreeable and actionable changes that come out of it.

We’re all interested in the same things: preventing exploits and keeping our community and its user base safe. Let’s keep an open mind, brainstorm freely, and assume good faith and goodwill.

🇺🇸United States dww

@berdir: Re #212: That's an extremely compelling case for why trying to "chase HEAD" is a headache. Quoting my advice to @jonathan1055 at #3451750-9: Fix: The update to convert "numeric" arguments to "entity_target_id" is deprecated :

So my recommendation for right now is to leave your views using numeric, tell GitLab CI to ignore the resulting depreciation notices until you’re ready to ship a release that requires at least 10.3.x, then you can convert your shipped views and reconfigure your pipelines to fail on depreciations (if that’s what you actually want). 😅

However, I'm somewhat torn. You can't conditionally include different .yml in your views config based on versions. There really is no work-around if you want to try to keep up. There's 0 harm in silencing this deprecation in 10.3.x -- folks have years to learn about it and switch. I'm a big fan of making it as easy as possible to be as compatible with as many versions as possible, since ultimately, that's best for end users.

When I pinged @catch in Slack about this (before your comment), he replied:

So leave everything in 10.3 so contrib can actually use the new thing when we get to 10.4?
Silencing or just removing the deprecation in 10.3 seems ok yeah.

So I opened 📌 Remove deprecation about Views argument config conversion from numeric to entity_target_id from 10.3.x Closed: duplicate with a green MR so we can make things easier before the 10.3.0 final release.

Let's continue to discuss there, if necessary, and let this giant issue finally go silent for the 90 followers that only wanted their entity reference arguments to work. 😂

Thanks!
-Derek

🇺🇸United States dww

Also, re end of #66, I think this is definitely not just about services.

for example, this is a common parameter for which this documentation would be redundant / pointless:

* @param EntityInterface $entity
*   The Entity to operate on.

Or UserInterface $user, etc.

🇺🇸United States dww

We could have a rule that says, more of less:

If any @param statements are required, all must be documented but the descriptions are optional for self-documenting cases.

Eg

/**
 * Does the important and weird thing.
 *
 * @param EntityTypeManager $entity_type_manager
 * @param bool $is_special
 *   TRUE if the special recursively cached access checking 
 *   on even numbered entities is necessary, otherwise FALSE. 
protected function doImportantWeirdness(…): void {
   …
}

Forgive typos / formatting, I’m typing this comment on my phone while boarding a flight. But hopefully it’s “clear”. 🤓😆

🇺🇸United States dww

Re: #10 - indeed, that's definitely how it works with a state_machine field. If we use content_moderation, same story: the “Published” flag is still there as a single bit, but the “moderation state” is a separate, more granular field with (potentially lots) more options. While configuring states, one of the most significant options (in all such cases) is / would be: “Is this state published/active or unpublished/blocked?” (Not literally that text in the UI, but the spirit of it).

Since this issue is back at the top of my tracker, I’ll report that the project using state_machine on user entities I was talking about in #8 has launched and this aspect has been working flawlessly. So I highly endorse that for folks looking for an immediate solution to this in contrib…

🇺🇸United States dww

That tag was added in #9 as a request to keep the scope limited to class properties. Life moved on. The vast majority of class properties will be auto-documented via constructor property promotion. The whole scope of this issue now is for method params and any non-promoted class members. No follow up needed. Removing tag and restoring status.

Scratch that, moving to RTBC so our committee can review this at our next meeting. I don’t believe anything else is needed for that to be the next step.

🇺🇸United States dww

In principle, +1 to being able to validate that people are people. But huge -1 to this specific proposal. It would mean only people with the privilege and resources to travel for in person events could be vetted. That seems like a non-starter to me.

🇺🇸United States dww

Re #56: see #41. Someone needs to update the summary to use some of those as the examples…

Totally support this, now that it’s focused on something we can automate.

🇺🇸United States dww

Mentioned at the core issue, but maybe we want a quick follow up to silence this deprecation in 10.3.x so it only starts getting hit in 10.4.x when 10.2.x is no longer supported?

🇺🇸United States dww

I also replied there. But for here, TL;DR: the problem isn’t really about strict schema validation. It’s that the plug-ins do not exist in 10.2.x. So until folks can drop 10.2.x support, they’ll have to live with the deprecation warnings in 10.3.x and up that the auto-conversion will be dropped in 12.0.x.

Should we do a quick follow up to silence this deprecation in 10.3+ so folks don’t have to worry about it until they’re on 11.x?

🇺🇸United States dww

When I get a moment when I’m not on my phone, I’ll try to update the CR for some additional clarity. But a few points here now that I’ve slept:

  1. The change to fix the bug was to add new argument handler plugins that properly deal with entity references instead of using entity IDs as numbers. These made it into 10.3.x and up, but not 10.2.x and down.
  2. If you ship a view still using numeric it’ll keep “working” as well as it always has.
  3. You can’t change your default view to use the new plugin, strict config schema or otherwise, in 10.2.x since the new handlers do not exist there.
  4. But what you can (and probably should) do is to ignore the deprecation warning in 10.3.x about it until you can safely drop support for 10.2.x and require at least 10.3.x.
  5. This deprecation is about the fact that the automatic conversion to the new/shiny handlers will be removed in 12.0.x. So you’ve got plenty of time and things will be auto-converted for you.

So my recommendation for right now is to leave your views using numeric, tell GitLab CI to ignore the resulting depreciation notices until you’re ready to ship a release that requires at least 10.3.x, then you can convert your shipped views and reconfigure your pipelines to fail on depreciations (if that’s what you actually want). 😅

Thanks / apologies,
-Derek

🇺🇸United States dww

Oh, I misread. Yeah, if you’re running against 10.3.x, you need to update any default views to use the new handler. Maybe we need another CR to clarify?

🇺🇸United States dww

Quite confused. Nothing was backported to 10.2.X. I don’t see how this could possibly be causing any test failures. But it’s late and I’m not of my right mind, so I’ll try to look again when I’m sober and have had some sleep. 😂

🇺🇸United States dww

My comment is linked in the summary, but copying here for visibility:

However, I had to completely rewrite ViewsConfigUpdater::processEntityArgumentUpdate() to operate on entire View entities, instead of using all the per-handler plumbing we've got. To my great dismay, if you copy the existing patterns, ViewsConfigUpdater process functions make a bunch of changes to handler config, then try to save the view, but that instantiates another ViewsConfigUpdater object (this time, with deprecations enabled) to do all the work again. 🤯 So apparently, The Right Way™️ to update a view with all this plumbing is that your process function has to directly update the $view (which isn't even normally passed in to your process function if you think you're just dealing with handlers), not just fix the $handler. 🤦‍♂️

It's probably out of scope to actually fix that as part of an issue with the title "Document ...", but maybe we want to expand the scope here?

🇺🇸United States dww

Ahhh, gotcha. Thanks for explaining.

ed1e913d2bb2fb6c3a469a09a26d1ae575746e99 looks fine -- not even a "typo" really, only adding a , to the warning message to improve legibility.

Definitely still RTBC. Let's get this in.

Thanks again!
-Derek

🇺🇸United States dww

Added Unit test coverage that shows the bug and proves the fix.

We probably want to extend some Functional coverage for this, too, since there might be other spots that blow up in this case, and it'd be nice to have an end-to-end test to verify it.

🇺🇸United States dww

Here's a quick start. Needs tests, but wanted to get an MR open ASAP.

Elevating to critical since this is potentially very bad. 😬 While there are guardrails for the version strings themselves when creating releases on d.o, there's nothing to prevent people from having typos in the core_version_requirement they put into tagged releases. Update manager needs to be resilient to those sorts of mistakes, not hard crash with uncaught exceptions. Especially since we generally prevent anyone from fixing a broken release, so once there's a problem, we've got it "forever".

Fixing the summary, both formatting, and some technical details.

🇺🇸United States dww

p.s. Doesn't really matter, but I'm restoring this to a bug. 😅

🇺🇸United States dww

FYI: Both 🐛 Fix label token replacement for views entity reference arguments RTBC and 🐛 [PP-1] Use labels in Views argument summaries for entity references Active are committed and will be available in 10.3.0 and up. 🎉 At last, these bugs are fixed! We copied over credit for the effort here to #3442227, too.

Enjoy,
-Derek

🇺🇸United States dww

Oh, the irony! 🤣

https://git.drupalcode.org/issue/drupal-3422537/-/jobs/1747113

Issues found:
./.gitlab-ci.yml:404:202 - Unknown word (unshallow)
CSpell: Files checked: 1, Issues found: 1 in 1 file.

Anyway https://git.drupalcode.org/project/drupal/-/merge_requests/8258/diffs looks excellent. That's more or less exactly what I had in mind.

I'm not totally following the results in https://git.drupalcode.org/project/drupal/-/merge_requests/8259/pipelines to see that this definitely fixes the problem. But on first principles, the merge-able MR (!8258) looks right, and based on the history of these failures and what we understand about their cause, it should solve it.

Super minor cost to run git diff twice, and that avoids always doing a full clone (which would be much more costly).

Ship it! 😉

Thanks,
-Derek

🇺🇸United States dww

Woo hoo! 🎉 Thanks, @catch!!

I got the follow-up ready if anyone following here is interested in reviewing (and hopefully RTBC'ing) that one:

🐛 [PP-1] Use labels in Views argument summaries for entity references Active

Thanks again! Very happy to see this fixed.

🇺🇸United States dww

So long as the threads are addressed and the tests are actually testing this, I'm happy. So signing off as subsystem maintainer.

🇺🇸United States dww

@smustgrave (bless his soul!) traded me a review here for his reviews of 🐛 Fix label token replacement for views entity reference arguments RTBC (which just landed for 10.3.0 🎉). Finally circling back to uphold my end of the bargain. 😂

In principle, +1 to the general approach of marking the recommended update as "Recommended security update" if it's a security update from the same branch as you're currently running. We want folks to get off known-insecure versions ASAP, and a security release from the same branch is the least disruptive thing to do, so that's what we should "Recommend".

However, I left a detailed review on the MR and opened 9 threads, some of them fairly easy suggestions to review, but a couple of important ones.

I've only looked at the code, haven't tried any manual testing.

One of my key review points is that I don't think the test changes are testing anything about the new functionality, so we need to fix that before we can have confidence from the GitLab pipeline. Probably would be good to do some manual testing, too.

Thanks for keeping this alive and moving forward, and apologies for the long lapses in my availability to review this...
-Derek

p.s. Crediting @smustgrave for all the efforts here, both reviewing and tracking subsystem maintainers, and myself for the detailed review.

🇺🇸United States dww
  1. Rebased so the issue fork branch is clean and back to only the test and the fix.
  2. Updated the MR description now that that's done.
  3. Updated the summary here with a more common use case (nodes referencing taxonomy terms) and updated remaining tasks.
  4. Main pipeline is all green.
  5. Test-only failed exactly as intended:
There was 1 failure:
1) Drupal\Tests\views\Kernel\Handler\ArgumentSummaryTest::testArgumentSummary
Failed asserting that '1 (4) 2 (2)' [ASCII](length: 11) contains "rejeaplf (4)" [ASCII](length: 12).
/builds/issue/drupal-3442227/core/modules/views/tests/src/Kernel/Handler/ArgumentSummaryTest.php:146
FAILURES!
Tests: 1, Assertions: 14, Failures: 1.

Ready for review (and hopefully quick RTBC). Would be lovely to also get this in before 10.3.0.

Thanks!
-Derek

🇺🇸United States dww

Pretty sure this is duplicate with 🐛 Fix label token replacement for views entity reference arguments RTBC which just landed and will be included in 10.3.0

🇺🇸United States dww

Woo hoo, the blocker is in. This needs a rebase, but then should be ready. I’ll do it later tonight when I’m back at a computer, unless someone else is inspired to do it before then.

🇺🇸United States dww

- Shortened the keys
- Added the missing config schema

Y'all can write tests if you need them. I'm way out of time on the project where I need this functionality, and it's working fine for me.

Cheers,
-Derek

🇺🇸United States dww

Implemented the separate 'Matching strategy' radios and corresponding logic changes in evaluate(). Also replaced the deprecated user_role_names() call with the recommended alternative. Light local testing is working well. Probably wants automated tests, but I don't have time for that now (about to board a flight).

🇺🇸United States dww

Addressed most of #6, but not the fancy roles operations stuff at the end. Converted to MR, hiding patches.

🇺🇸United States dww

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

🇺🇸United States dww

The core Update Manager cannot correctly update modules like address that have external dependencies. You need to use composer to manage your updates if you use this module.

Production build 0.69.0 2024