[BUU] Variants with "none" tax category have an invisible error

[dacook]

Description

When this option is ticked:

But variants still have "None" selected, they are in an invalid state.
The BUU PRoducts screen shows a summary that they are "modified", yet shows no indication of what's wrong. When attempting save, they are counted as erorrs, but don't show any messages explaining why.

Discovered in:

• [BUU] Dismissing changes is not working #12473

Expected Behavior

I don't think they should show "modified" at all.

When saving, an error is expected for each variant that has "None" selected. There should be a message appearing below the Tax Category dropdown.

Actual Behaviour

Steps to Reproduce

as above

Animated Gif/Screenshot

Workaround

Severity

Your Environment

• Version used:
• Browser name and version:
• Operating System and version (desktop or mobile):

Possible Fix

[dacook]

For reference, this is what happens on the Variant Edit screen: the error message appears at the top, but not at the field. That's probably why we dont' see it on BUU. Hopefully we can fix this at the model level.

[RachL]

@dacook when product require tax category "none" is an option in the dropdown, so I'm not sure I understand why this would give an invalid state? We need to be able to select "none" even if product require tax category.

[dacook]

Hi @RachL , the "None" option is a special blank option which saves as empty (no tax category).

When "products require tax category" is ticked, the blank "None" option is not valid. Why don't we hide it in that case? I had a look but it looked complicated to hide from the dropdown.

So instead I went ahead and fixed the fields to properly show the error messages, see PR.

I also had a look at the default tax category. I think we need to improve how this is handled (e.g. in this case perhaps the system could automatically save it with the default tax category if there is one. I think we discussed this before but I can't remember exactly...

[dacook]

According to @mkllnk, when you select "products require tax category", you must also select a default tax category.

In that case, we should never allow "None" selected, and instead select the default. I think we need to discuss this more..

[dacook]

I think my PR is worth proceeding with for now.
For the future, I've suggested how we can improve the settings screen to avoid bad config:
https://openfoodnetwork.slack.com/archives/C01T75H6G0Z/p1715839142389029

[RachL]

@dacook on staging FR (and in production as well) I'm able to select "none" on variants even though the tax category is required 🤷 I thought it maybe had something to do with enterprise settings, but no.

is there anything else that comes into play?

[RachL]

we should never allow "None" selected

FYi not only do we let people choose "none", but it's also the default option selected when creating new products.

[dacook]

I'm able to select "none" on variants even though the tax category is required

Yes it seems strange, we should change that! The empty "None" option may need to still appear in case it is already selected on the variant. But I think it could be greyed out and disabled, preventing you from saving it.

is there anything else that comes into play?

Looking at the code, it looks like it should select the "default" tax category when creating a new variant. But strangely that's not working in my testing..