Security Advisory coverage status is hardcoded on Recipes

I think the answer here might be to allow the use of NULL in the isMaintained column, meaning 'unknown' and we would not show the shield for those.

I think that it should just take the same status as the project that those recipes come from. We are looking for recipes in core and contrib modules in here: https://git.drupalcode.org/project/project_browser/-/blob/2.0.x/src/Plug...

Recipes from core can be safely put as "Covered", and the ones from contrib should be able to change depending on the module.

When navigating through the above recipes (gathered from multiple sources), we already have an "if/else" block for core vs not-core in here: https://git.drupalcode.org/project/project_browser/-/blob/2.0.x/src/Plug...

So I suggest doing the calculations there and then adapting the code below with the calculated value.

I think it might be better to just remove the security coverage status from recipes. As far as I know, there is no official policy around this yet anyway. A recipe might contain a bunch of contrib modules, some of which are covered and some not. There are a few other nuances that make it pretty unclear how security coverage would even work for recipes.

It seems important to show the coverage before the modules are installed on a site. The status is pretty prominent on the project page to help people make informed choices.

But it's not showing the module status, it's showing the recipe status which is hard coded to say it's covered. It's certainly not checking the status of the modules required by the recipe. If that's what we all agree should happen that's fine, but it's pretty far from what's happening now.

This isn't much of an issue right now because the only recipes that are available are either from Drupal core or Drupal CMS, so they are vetted at least. And until we figure it out, I think it's better to remove the inaccurate info.

Yeah I agree that a good first step would be to show nothing instead of misleading information.

What happens now in project browser if a module is opted into security coverage, but it depends on a module that is not opted into security coverage? Does project browser expand the dependency tree before installing dependencies or not? If it doesn't then this is probably applies to modules and themes too.

And is the badge purely related to the opt in status or does it also check for stable releases? Projects can and do opt into security coverage when they only have alphas, just means the coverage actually starts at stable. So if we need to conditionally show the badge this comes back to the other related issues here about which version will get installed.

@pameeela We can hide it, but it overlaps with other issues - for example the Forms recipe in Drupal CMS still contains Webform 6.3.0-alpha3, which means that you will end up with a not SA covered module(s) in specific cases, which is not what users expect from a stable product (Drupal CMS 1.0.0). But if we solve this one 🐛 Missing information about the version of the project going to be installed Active , we can probably also list the all the modules which will be installed by a recipe?

which is not what users expect from a stable product

Does this mean that the >381,000 sites that are using webform should consider D11 unstable, because they can only use an alpha version of webform with it? I'm genuinely asking because although I completely understand the concerns, this feels quite far from the reality for most people building sites. If you were starting a new project today that had to use webform, would you really use D10 so that you had security coverage?

Webform 6.3.x (alpha) is used by 2,159 sites now and that is their decision (because it needs to be installed manually). For example it is not something, which is allowed in most company/goverment enviroments. But Drupal CMS/Forms recipe/PB will make the decision (without acknowledgment) instead of users, which is the difference.

is the badge purely related to the opt in status

Yes.

I agree that if the concept of security status does not exist in recipes, we should just not show it, regardless of where the recipe comes from.

We can differentiate between true/false and null to get over the technical hurdle in the code. This way, we won't need to refactor too much. Just a check for null, and ignore in that case.

for example the Forms recipe in Drupal CMS still contains Webform 6.3.0-alpha3, which means that you will end up with a not SA covered module(s) in specific cases

I think this might be a general issue with project browser and dependencies of projects that are going to be installed, rather than just recipes.

e.g. if I install https://www.drupal.org/project/webform_booking → that has stable 11.x support via project browser, this project depends on webform which is alpha.

Unless project browser recursively calculates the dependencies, and shows me all the things that are going to be installed and which version, then installing a stable, security supported project doesn't guarantee this across its dependencies.

I've replied to @pameela and @poker10's other points on #3447882-14: Determine some heuristics for module inclusion to avoid conflicts with core functionality, set expectations for security coverage etc. → because we're at risk of going off-topic here.

Maybe the best short-term answer here is not to show anything.

In the long run it seems great if we could do the recursive calculating you mention, catch, and then have icons/text to explain "all modules are covered with stable releases" or "It's a mix of statuses and stable releases, get details."

Yeah I think showing nothing for recipes is better than hard-coding, and then we can try to figure out what it actually should show.

Updating the IS based on the consensus to remove the status for now.

Updating IS to show what really needs to happen here.

I think this is all we'll need -- if coverage and maintenance are unknown (NULL), then the badges shouldn't show up, since null is a negative status.

Well this ended up turning into a big ole thing. @chrisfromredfin and I have discussed this at length, but for those new to the party: Project Browser's code base is in a bit of a sad state because it was not originally designed for the product requirements being demanded of it. This means that asking for reasonable-seeming changes often end up necessitating complicated, inscrutable workarounds, or very aggressive refactoring. This is one of those times.

Recipes opting out of setting maintenance or security coverage flags means that the hard-coded maintenance status and security coverage filters no longer apply to it. Unfortunately these filters are baked into the Svelte app, so ripping them out in favor of whatever filters the source plugin does (or doesn't) define is necessary to get this done.

The good news is, the necessary work is already partially done; me and some other DAT engineers built new value objects for the filters to be defined by the backend, but never got around to fully integrating them into the frontend. This MR takes it almost the entire rest of the way, in service of allowing recipes to not present these (temporarily useless) filters.

one small issue from manual testing -

on 2.0.x
modal - icon & number appear
card view - just number appears
list view - icon & number appear

in this MR
modal - icon & number (good)
card view - just number appears (good)
list view - just icon appears (bad)

I don't think this needs a test in this MR. Ideally there would be one since we obviously caused a regression that wasn't caught, but we have other open issues about the icons appearance, etc - and I will do them there.

This is a great net-negative, and more secure. And hits up on some work we needed to do with the DA. Huge win. 💪

Automatically closed - issue fixed for 2 weeks with no activity.