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

Merge Requests

More

Recent comments

🇺🇸United States dww

Hi! Thanks for your request and interest in this module. Really appreciate all the contributions you’ve made.

I won’t undercut Jaypan and directly make you a co-maintainer. But I will say I’ve finally got some time for this module again, and will be working the issue queue next week for a client gig. So I can at least move a bunch of your MRs forward (and hopefully cut a new release) in a few days.

Thanks again!
-Derek

🇺🇸United States dww

D7 is EOL. (Finally) doing some cleanup on the update.module component... 😅

🇺🇸United States dww

This seems duplicate with 🐛 Code Review for Update module Closed: outdated . Regardless, both are "outdated" now, and none of these suggestions are still relevant.

🇺🇸United States dww

Most of this code no longer exists. Indeed, we're not using t() in tests. All of the code style stuff has long been fixed as this was ported to D8 and beyond.

🇺🇸United States dww

Indeed, this is no longer relevant. The "Update Manager" UI provided by update.module is now gone from core.

🇺🇸United States dww

Indeed, this was actually a problem with the XML release history feeds provided by update.drupal.org, which was fixed on January 22nd of this year. Closing this as outdated.

🇺🇸United States dww

I know it's confusing, but update.php and the database update UI is a separate component from update.module, which is only the "Update status" module that provides the "Available updates" report at /admin/reports/updates (and in earlier versions of core, the "Update Manager" UI that tried to install available contrib updates on your webserver).

🇺🇸United States dww

phpunit (next minor) is failing, but that seems to be about new 11.2.x deprecations, so I don't think that's anything I need to worry about here. Everything else is now green.

🇺🇸United States dww

Wasn't really sure what to call it, but here's a start. Working fine in production on the site where I needed this.

🇺🇸United States dww

Re: #143/#144: Or don't start chromedriver locally and try to run any FunctionalJavascript test.

🇺🇸United States dww

Thanks for working on this. However, since it's entirely dead code at this point, and there's no explanation for what the actual problem is, I'm going to mark this "won't fix". I hate to see anyone spending more time on a dying / dead system.

🇺🇸United States dww

Re: #5: hah, fair enough. 😅 Apologies for my less-than-tactful wording, and thanks for not taking it personally!

🇺🇸United States dww

Thanks for doing a trial run like this! Very helpful and encouraging that the parent issue is on the right track.

If a branch off an issue branch confuses the tooling so much, probably not the end of the world to have an initial commit of Add drupalGet() to KernelTestBase Active in here, no? The changes tab will be bloated, but it should be fairly simple to review changes via each proceeding commit...

Anyway, thanks again!

🇺🇸United States dww

Bot is happy and green. I guess this is a new feature. 😅

I'm probably forgetting something about how this could break things for existing sites with existing views config. Especially while 🐛 Views handler loading should respect configuration Active is unresolved. But it's here for folks that want to try it out. Maybe I'm over-thinking it, and this is harmless for legacy sites.

🇺🇸United States dww

Thanks! Pipeline is green. I’ve read the diff many times now. Nothing left to improve to my eyes, other than the follow-ups. 😉 let’s get this in.

Thanks for your patience and persistence!

🇺🇸United States dww

cspell failed on the latest. Added another MR suggestion to fix it. So very close! 😅

🇺🇸United States dww

Just marked 🐛 Users can edit/delete content but not unpublish it Active duplicate with this, which had 35 followers.
I imagine there are other duplicates lurking in the issue queue for this.
This is a very common need. Further evidence core should provide this out of the box.

Thanks!
-Derek

🇺🇸United States dww

IMHO, yes, this is something core should do. Reading the comments, we've got "yes this is a good idea" from all sorts of folks who are either past or current release managers, product managers, etc. It's fairly absurd that we have granular permissions for delete, but not publish/unpublish. Yes, content moderation provides more advanced publishing workflows. But for something so fundamental to a content management system as publish status, I think core needs to solve this.

🇺🇸United States dww

Thanks for opening those followups. Resolved my open threads. I'll let @nicxvan apply the phpcs suggestion, then it's RTBC to me.

Thanks!
-Derek

🇺🇸United States dww

Thanks for opening this. I'm not even really sure *where* to document this. 😅 I guess that's one of the remaining tasks...

🇺🇸United States dww

Thanks for opening this issue based on my feedback at #3489141.

Sad panda. I wish we were consistent with this sort of thing. It's too bad we released Hook with this slightly weird DX. It's definitely minor, no real harm done the way it is, other than the mild WTF.

If this is "[PP-2]", it should be postponed, and the summary should say what it's blocked on.

Thanks,
-Derek

🇺🇸United States dww

Looks like we need a few follow-ups here:

  • Something to document how to put form submit callbacks into Hook classes
  • Fixing Hook to use NULL for both of its optional parameters, instead of '' for $method vs NULL for $module

Otherwise, I think this is RTBC.

🇺🇸United States dww

Humans are smarter than bots. 😂 I still have questions and concerns. But we're really close now.

Thanks!
-Derek

🇺🇸United States dww

Late to the game, but this isn't a bug. Nothing was broken. It's just removing dead code. Hence, task.

🇺🇸United States dww

Nice, didn’t know we had that. 😅 TIL. Thanks!

🇺🇸United States dww

Great work here! The change is exactly the right fix for random fails in JS tests: if we're trying to use an element, we need to wait for the AJAX request to complete before we proceed. As @catch's initial attempt shows, we can't just wait on the field itself -- it's already there. 😅 We need to wait on the AJAX request that clicking that field triggers.

The 2 repeat MRs show that the fix is indeed solving the problem.

Nothing left to improve. RTBC.

Thanks!
-Derek

🇺🇸United States dww

Thanks! Overall, this looks really good and clean. But (as usual) I've got some concerns and nits on the MR. 😅

Initial pass at saving credits.

🇺🇸United States dww

Yes, indeed. And there are some open questions at 📌 Rename update module back to Update Status Active that I would appreciate some input on.

Thanks!
-Derek

🇺🇸United States dww

Thanks for adding the CR and committing this. Hopefully this helps more than it confuses. 🤞

🇺🇸United States dww

Hi folks, thanks for working on this. Apologies this issue fell through the cracks. There's a ton of existing work towards implementing this here:

#2766491: Update status should indicate whether installed contributed projects receive security coverage

Let's continue over there if anyone's interested in moving this along.

Thanks!
-Derek

🇺🇸United States dww

Re:

except probably exceptions need constructor to throw deprecations

From @catch via Slack:

https://drupal.slack.com/archives/C079NQPQUEN/p1741811869453559?thread_t...

catch: I don't think we have anything explicit for deprecating exceptions, but given these aren't used anywhere outside the system we're deprecating I think just @trigger_error() is fine.
dww: But then I have to add __construct() methods. 😅 Can I just do the annotation, instead?
catch: Sorry that's what I meant...

🇺🇸United States dww

📌 Deprecate authorize.php and the FileTransfer system Active is now ready for review.

Closed the MR here since it was for the 'Remove all the things' approach, before we decided to split this up into a plan with multiple phases.

Thanks,
-Derek

🇺🇸United States dww

Added another API change point for:

  • Every method in core/modules/update/update.manager.inc

Everything is now done. Pipeline is green. Simplified remaining tasks.

This is now ready for review (both the MR and the CR). Unassigning.

Thanks!
-Derek

🇺🇸United States dww

Added an item to API changes for:

  • Every method in core/modules/update/update.authorize.inc

Updating remaining tasks to cross off what's done. Almost there...

🇺🇸United States dww

@andypost: Argh, what are you doing? I assigned this to myself and am actively working on it.

🇺🇸United States dww

Okay, cool, that's working now.

Adding a TODO list under remaining tasks, and crossing out what's already been pushed...

🇺🇸United States dww

@andypost, see:
https://www.drupal.org/about/core/policies/core-change-policies/how-to-d...

I’m assigning myself, since I’ve got more of this done locally. Stay tuned.

🇺🇸United States dww

I asked in #core-development in Slack, but x-posting here:

core/modules/system/tests/src/Functional/System/SystemAuthorizeTest.php is now failing since it's triggering deprecations. Can I completely remove that test as part of this issue, or do I need to convert it to expect deprecations, etc?

🇺🇸United States dww

Opened a CR for this (to get a valid NID for deprecation messages) and started deprecating everything in the IS. Pushed commits for authorize.php and system_authorized_*(). Opened an MR. Still incomplete, so NW, but wanted to at least get this started and to see if the bot explodes in any unexpected ways. 😅

🇺🇸United States dww

@andypost: Not sure what you're requesting in #3. That MR is for a set of changes to completely remove all this code. Instead, it'll probably be more simple to start over from scratch here, since there's now a different scope and approach needed for this issue.

But if I'm misunderstanding, please let me know.

Thanks!
-Derek

🇺🇸United States dww

Blocker is in. Just need trivial rebase and those should be green pipelines

🇺🇸United States dww

Great, thanks!

Dare we mention any of the motivations in either the CR or release note? That composer is the only supported way to safely manage dependencies? And to pave the way for “automatic updates”? Although the wording / branding is going to be confusing with that, since the new thing is now being called “Update Manager” 😂

Also, I’m not sure we should say “drush or manual updates” in the CR. I thought we want to discourage “manual updates”, and I don’t think drush has a solution for this, either.

🇺🇸United States dww

UpdateManagerUpdateTest is failing now, since it's assuming the old default.

However, that test is completely removed @ 📌 Remove UI and routes for the ability to update modules and themes via update.module and authorize.php Active .

Should we bother fixing the test here and needing to re-roll one of these MRs whenever the other lands, or should we postpone this on committing #3502973 first?

🇺🇸United States dww

LGTM, but I haven't directly contributed to Drupal CMS config / recipes yet, so I don't feel super qualified to RTBC this.

Only question is if this will still work if the default in core changes, too: 📌 Consider flipping the default for update.settings:check.disabled_extensions Active

🇺🇸United States dww

The UI for when you turn this on isn't terrible. 😅 The uninstalled stuff appears in a whole separate table at the bottom of the report. So hopefully won't be too much confusing noise in the regular case.

In general, +1 to the change.

🇺🇸United States dww

I won't merge this until 📌 Enable GitLab CI and cleanup build pipelines Active is done and we have passing green pipelines on all relevant versions.

🇺🇸United States dww

Thanks for working on this! I should have mentioned all these changes could have been selectively merged into this fork from all the commits I pushed to the parent issue. Hope you didn't completely duplicate the effort. 😅

But thrilled to see progress here without me. 😉 I've been way too slammed on way too many fronts to work on this.

Thanks again!
-Derek

🇺🇸United States dww

TODO: See comment #4, points 1 and 4 (at least).

🇺🇸United States dww

Thanks for working to smash this bug!

Latest pipeline failed in phpstan. We need to see green testbot results before this is ready for review. In this case, hopefully a failing test-only run, too.

Thanks,
-Derek

🇺🇸United States dww

Quick driveby review. A few pedantic nits for now. Haven’t finished, but this is all I could do from my phone while eating lunch with the other hand. 😂

🇺🇸United States dww

I'm not so sure. 😅 For 📌 Remove adding an extension via a URL Fixed we removed the routes entirely, without deprecation. True, we didn't have a mechanism at the time, but from a release manager and framework manager standpoint, this stuff is so old, problematic, and presumably unused, that it's better to hide it from end users entirely than to keep the functionality visible (even if there are warnings).

🇺🇸United States dww

Coming here from https://drupal.slack.com/archives/CGKLP028K/p1739920511290749

I don't see this mentioned anywhere at https://project.pages.drupalcode.org/gitlab_templates/jobs/upgrade-status/ or otherwise. Google search is failing me. I've got a project I'm getting pinged about for a D11 issue. I'd love to run the upgrade status job via the bot. But if I do that, it's defaulting to the current core (11.1.x) and trying to check for D12 compatibility. I started a manual pipeline, and I didn't see any way to control "No, I want you to use D10 to run upgrade status to confirm D11 is cool".

  1. How do you do this?
  2. Shouldn't this be the default config for Upgrade status jobs? :sweat_smile:

The answer is when creating a manual pipeline, although there's no obvious choice in the initial list of variables to configure, if you manually set DRUPAL_CORE at last row of the form to ^10 or 10.4.2 or whatever you want, and then tweak the visible RUN_JOB_UPGRADE_STATUS variable to 1, you'll get what you need (a test run against previous major that then checks upgrade_status against the current major you're porting to)...

🇺🇸United States dww

Pushed some more commits. Test seems to be working now on all versions.

🇺🇸United States dww

Nope, still failing. I'm way out of time for this. If anyone wants to get the Kernel test passing on D11, knock yourself out. 😉

Thanks/sorry,
-Derek

🇺🇸United States dww

Not sure why, and don't have time to further investigate, but the tests are still failing on D11, even with compatibility declared.

🇺🇸United States dww

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

🇺🇸United States dww

Fixed cspell and phpcs.
phpunit is failing on D11, since 📌 Automated Drupal 11 compatibility fixes for condition_query Needs review isn't merged.
Everything else seems okay, so I merged this to 8.x-1.x.

🇺🇸United States dww

dww created an issue.

🇺🇸United States dww

@mvonfrie, re #45: Feel free to open another postponed child issue about that, much like Add $entity->toUrl() and $entity->toLink() methods to allowed methods list in Twig sandbox policy Needs work . Once this issue is merged and fixed, it'll be much easier and safer to expand the allowed methods and target specific interfaces where we want to allow other methods.

Thanks,
-Derek

🇺🇸United States dww

Thanks for working on this! Working on an MR review. About to open a bunch of threads. But moving out of RTBC queue for now so a core committer doesn't spend time looking just yet...

🇺🇸United States dww

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

🇺🇸United States dww

This is still causing troubles in contriblandia. See 🐛 Fix Deprecation warning for profiles view for Drupal 10.3 Active . We're trying to fix a deprecation that requires changing default views. Due to 🐛 Views handler loading should respect configuration Active , it's "safe" to update default views provided by contrib modules to use the new handler IDs, since the actual handler used is determined by the Views Data API, not the views config (until 11.2.0 when #3458099 will be released). However, if you change the config, and run tests against 10.2.x core, the plugins don't exist, and the whole test run explodes due to strict config schema errors. See https://git.drupalcode.org/issue/profile-3465501/-/jobs/2317666

I'd love it if there was an easy way to globally set $strictConfigSchema = FALSE for an entire pipeline. Or whatever this issue proposes to allow contrib to ignore these errors that are only triggered during automated testing and have no impact on runtime behavior.

Haven't closely read every comment, but this seems like the right issue to raise this concern. 😅 Please let me know if I should be barking up another tree, instead.

Thanks!
-Derek

🇺🇸United States dww

Oh yeah, apologies about that. I forgot we ran into that at 📌 Fix: The update to convert "numeric" arguments to "entity_target_id" is deprecated Needs review , too. The only problem is the $strictConfigSchema being set to TRUE in this case. Everything still works, but the strict config schema checks will not in 10.2.x and earlier.

We can either disable those checks entirely, or conditionally for older test runs. See https://git.drupalcode.org/project/scheduler/-/merge_requests/159/diffs for one approach.

p.s. The failure is for "Previous minor", not next, which makes sense.

🇺🇸United States dww

Wow, blast from the past! Indeed, in D8+ ("modern Drupal"), when you're defining a block (Plugin), you can specify the #cache key and you get all sorts of control.

https://www.drupal.org/docs/drupal-apis/cache-api/cache-max-age is still sorta broken. See the Limitations of max-age section for details. But that's not what this issue was trying to solve.

Now that D7 is EOL and we're closing all the old issues, this one can die, too.

Thanks!
-Derek

🇺🇸United States dww

In principle, this all sounds good.

Initial cleanup of MR metadata and resolving threads that have already been addressed.

Need to do a more thorough review, but no time for that right now.

🇺🇸United States dww

Note: I see the D12 compatibility was added so that the upgrade_status job would "work" when run against 11.1.x core. This is the wrong approach. It's pointless to run the upgrade_status job on every MR pipeline until you're actually getting ready to port to D12.

🇺🇸United States dww

Note: probably not wise to declare D12 compatibility until D12's API is finalized. 😅 See 🐛 Premature declaration of D12 compatibility Active

Production build 0.71.5 2024