If the site is on an insecure version of an old minor and there is a secure version of that old minor available, the update status report should link that release

I tried to make some progress here. I tested the patch on 10.0.4 and used those results for the before and after screenshots. It is probably best just to look at those too see the changes.

Fix errors caught by PHPStan

Sigh, and again.

The last submitted patch, 50: 2990476-51.patch, failed testing. View results →

The failure is about themes which I have not worked on yet. This restores drupalci so all tests runs. No interdiff.

The last submitted patch, 52: 2990476-52.patch, failed testing. View results →

This looks like a good change -- list the immediate and simple patch version update first, before the more offputting minor version update.

I notice from the screenshots we've no longer got one of them in bold.

Is there a spec somewhere for what a bold means on the update status report?

A few more fixes:

• The failing test is fixed by finding the element with new label, "Recommended security update".
• I don't know of any spec for the use of bold. It looks like it is simply used to highlight the recommended version.
• This also restore some accidental formatting changes
• Change watchdog_exception to Error::logException(
• New after screenshot

Wondering if this could be answered in the Issue summary

Decide whether to continue linking the latest security release if it is not the latest release overall and the user is on a different minor branch.

If there is a 10.0.9 would that be the recommended security update version?

If there is a 10.0.9 would that be the recommended security update version?

Yes. The screenshot is for a site on Drupal 10.0.4 and there are two security releases → after that. It is the latest one that is displayed.

Still to do #56.2. I am setting back to NR because the patch doesn't need work right now. We need to make sure this change fits with the overall plan for what is show on the page.

@quietone who's needed to make that call?

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

Convert to MR and hide patches.
And here are some more after images

Same question as before who can make the call. This just sat there before.

Lets see if a submaintainer can chime in.

This screenshot → shows that this is still not quite right. For 10.0, the latest security release should be displayed and it is not.

Testing with a Drupal version of 10.1.0 shows the changes made here.

Postponing on 📌 Convert update_calculate_project_update_status() into a class Needs work

This should be done before #2990476: If the site is on an insecure version of an old minor and there is a secure version of that old minor available, the update status report should link that release to avoid making update_calculate_project_update_status() even more complicated

That issue will take much longer to resolve. I think it is more important to fix this Critical bug than wait for an architectural change.

Then can the other issue summary be updated

As usual, this fell off my radar. Tagging to be smashed. I can't do a full subsys maintainer review anytime soon, but trying to remind myself to get back to this ASAP.

@smustgrave (bless his soul!) traded me a review here for his reviews of 🐛 Fix label token replacement for views entity reference arguments Fixed (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.

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

@dww and @snustgrave, thanks for working together so this issue got a review!

I have updated the MR and left comments in the MR.

Looks like this needs a reroll

Rerolled and there is a valid test failure. I don't see why yet and debugging these update tests is working as I would like.