🇺🇸United States @dww

dww → created an issue.

Probably duplicate.

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!

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

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

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.

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

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

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

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.

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

Thanks!
-Derek

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

Removing duplicate related issue

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

I believe you meant to post that at 📌 [PP-1] Determine how to implement Form Alters with attributes. Active , right?

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

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.

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

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

Yay, thanks! I published the CR

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

Huzzah, thanks!

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

📌 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

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

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

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

Okay, cool, that's working now.

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

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

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?

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

Tagging as a 11.2.0 release priority. See 📌 Deprecate/remove the ability to update a module from a URL and authorize.php Active and friends.

smustgrave → credited dww → .

Rebased. Pipeline is green.

@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

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

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.

Groovy, thanks!

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?

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

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.

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.

Yay, thanks!

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

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

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

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

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

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

Saving credit

Done, thanks!

Tagged https://www.drupal.org/project/condition_query/releases/8.x-1.5 → so I don't forget. 😂

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

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

Let's see if that solves it... 🤞

Probably https://www.drupal.org/node/3337193 → is the culprit

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

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

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.

💬 TATA Consultancy Services is bulk posting requests to gain maintainer access to modules Active

dww → created an issue.

@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

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

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

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 Needs review , 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

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.

quietone → credited 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

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.

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.

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

dww → created an issue.

I added this deprecation to core. Turns out that due to extreme weirdness with Views, it doesn't actually matter what plugin ID you use in your views.yml files until 🐛 Views handler loading should respect configuration Needs review is released (11.2.0 at the earliest). 🙃 So it's actually safe to change this now. In D9 and <10.2, the plugin won't exist, but it'll be ignored. In >=10.3.0, the new plugin will exist, and be used, regardless of what your views config says. In >=11.2.0 the Views config will actually start to matter. 😅 See #3451750-18: Fix: The update to convert "numeric" arguments to "entity_target_id" is deprecated → as an example.

I just updated the CR to add a Note on Backwards compatibility → section to explain this weirdness, and the recommended approach (which is to update immediately, even for modules that are compatible with pre-10.3.x core):
https://www.drupal.org/node/3441945/revisions/view/13586313/13871846 →

Therefore, the changes in the MR are proper, we don't have to touch core_version_requirements, and everything should work fine on older versions of core. Arguably the summary here could be fleshed out, but I'm going to go ahead and RTBC this.

Thanks!
-Derek

Still need this. 😅 There were merge conflicts, so I rebased the MR. There's a phpcs and phpstan (max PHP version) failure in profile.module that's untouched by this MR. Not sure what's going on with that, but seems out of scope to fix it here.

phpcs

--------------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
--------------------------------------------------------------------------------
 71 | ERROR | [x] Parameter $items has null default value, but is not marked as
    |       |     nullable.
    |       |     (SlevomatCodingStandard.TypeHints.NullableTypeForNullDefaultValue.NullabilityTypeMissing)

phpstan (max PHP version)

 ------ ----------------------------------------------------------------------- 
  Line   profile.module                                                         
 ------ ----------------------------------------------------------------------- 
  71     Deprecated in PHP 8.4: Parameter #4 $items                             
         (Drupal\Core\Field\FieldItemListInterface) is implicitly nullable via  
         default value null.                                                    
 ------ ----------------------------------------------------------------------- 
 [ERROR] Found 1 error                                                          

Trivial fix, but seems out of scope. Not sure why this pipeline is complaining about it, since I didn't touch that and it's identical in the 8.x-1.x branch (which seems to be passing).

hestenet → credited dww → .

+1 to allowing optional comments.
+1 to allowing those comments to be formatted as a formal docblock if needed for something.
+ 1 to allowing optional new lines where appropriate for legibility.
-1 to requiring comments.
-1 to requiring comments to be formal "docblock" style. Inline should be fine in the majority of cases.
-1 to requiring newlines.

So yeah, we'd need to update this: https://www.drupal.org/docs/develop/standards/php/api-documentation-and-... →

To deal with this:

object (NOT "stdClass")

Thanks for all your efforts and support! We'll definitely miss you.

Okay, the official answer is we should be more cautious and do this properly. So I'm converting this into a meta/plan issue, and opened 3 child issues to handle the phases of this effort as I understand them:

1. 📌 Remove UI and routes for the ability to update modules and themes via update.module and authorize.php Active
2. 📌 Deprecate authorize.php and the FileTransfer system Active
3. 📌 [12.x] Remove all legacy code related to authorize.php and FileTransfer Postponed

The 1st two are 11.2.0 release priorities (and could happen in parallel, although probably easier for review + scope to do them separately), and each will have parts of the MR changes from here. The last one is a 12.0.0 task to finally remove everything.

Does that all sound reasonable? If so, I'll proceed to split up the changes in this MR into each of those as appropriate.

dww → created an issue.

dww → created an issue.

dww → created an issue.

I also did some minor edits (mostly just formatting) on the CR, and agree that looks really good.

Tried to resolve as many of the MR threads as I could.

There's still some valid feedback that hasn't been addressed.

Re: #60: Any future refactoring to consolidate these traits and methods will end up being more strictly typed. As new code, I believe our policy is to add always strict types, including return values. No code will be simultaneously using both traits -- a test is either Functional or Kernel but not both. I don't understand any benefit of leaving off the return types. There's no BC to preserve, since this is all brand new.

Thanks!
-Derek

p.s. Initial pass at saving credit:
@joachim, @pooja_sharma, @samit.310, @mstrelan, @alexpott, @mxr576 for code.
@oily, @nikolay shapovalov and myself for reviews.

I have no idea. 😅 Drupal.org is planning to move issues from project_issue to GitLab. So I don't know of anyone planning to invest effort into porting project_issue. However, as I understand it, projects and releases will still live on Drupal.org. That said, I don't know if the plan is to migrate to entirely custom content types and code, or to port at least project and project_release to modern Drupal.

To avoid conflicts and trouble, this should wait until 📌 Convert hook requirements that do not interact with install time Active lands.

Opened the follow-up: 📌 Improve wording of Navigation requirements regarding Toolbar Active . @oily: Please continue the discussion there.

Thanks!
-Derek

dww → created an issue.

Oh yeah, tagging as a 11.2 priority.

Thanks for fixing that up. I see nothing else to improve. Pipeline is green. Code is clean and good. Mostly just moving things around, with a few slight improvements (direct links to evergreen docs, better comments). Title and summary are clear. No need for a CR or release note. RTBC!

Don't love the loadInclude() hack, but as a transitional solution, I suppose it's fine if we improve the comments about it.

Almost there!

Thanks,
-Derek