Promotions that apply automatically shouldn't allow the user to create promo codes

[aldesantis]

When I create a promotion that is automatically applied to all orders, I shouldn't be able to manage the promo codes for that promotion, since creating a code will result in the promotion getting into a broken state and failing validation.

Solidus Version:
2.9, but I guess it affects all versions

To Reproduce

1. Create a promotion that applies automatically
2. Click on View codes list
3. Click on Create promotion code
4. Create a new code

Current behavior
The promotion displays:

All orders will attempt to use this promotion

This promotion uses the promotion code: c

Furthermore, the promotion cannot be updated in any way, since it fails validation with the following error: "Apply Automatically Disallowed for promotions with a code".

Expected behavior
I shouldn't even be allowed to create the promo code.

Screenshots

Desktop (please complete the following information):
N/A

Smartphone (please complete the following information):
N/A

Additional context
N/A

[kennyadsl]

What about promotions that did already have promo codes and are set to apply automatically afterwards? If we add a validation, admin users will need to remove the promo codes before changing that.

I think we should try to change the logic so that this kind of promotion doesn't try to use the promo code at all and be clear in the Admin UI that if you check apply automatically promo codes will be ignored.

Promotion code is very complex, though. If the above optimal (IMO) solution is not possible, what you proposed is a good trade-off.

[MassimilianoLattanzio]

I think the best way is disallow the creation of promo code.

If the promo codes is ignored when "Apply to all orders" is checked, the operation of creation is useless so why not disable it?

But is also important notify the user that selecting "Apply to all orders", promo codes will be disabled. This can be done in Admin UI while creating a new promotion and also in editing.

Only for backward compatibility, we can change the logic so this kind of promotion ignore the promo code. Otherwise we can try a simpler solution such as create a command/task to delete the promo codes if some promotions applies automatically.

[aldesantis]

@MassimilianoLattanzio I think that's a good course of action. Let's disable the creation of promo codes for promotions that apply to all orders AND make sure that we properly handle any existing promotions.

I was thinking that some stores may actually be using those promo codes in some custom logic, so perhaps we should keep them in place and simply hide the UI rather than deleting them in a migration. An alternative would be to run the migration but let the user decide what they want to do (delete or keep).

@kennyadsl do you have any ideas?

[kennyadsl]

I agree the best option is making both checks and avoid removing data.

Removing existing promo codes when admin users change the value of the checkbox would be a good option but doesn't take into account already existing promotions with "Apply Automatically" and some promo codes in place. These promotions would make the database non-consistent after the upgrade and the only way to rely on that would be adding a migration, as @aldesantis was suggesting. I think we should avoid destroying records in data migrations, it's too dangerous since we don't know how users are customizing their stores.

I'd go with:

• do not add a model validation, since it would be useless, even just only on creation since promotions creation is a two-step process and promo codes are added in the second step.
• disable adding new codes in the UI if "Apply Automatically" is set to true
• when applying a promotion code, exclude promotions with "Apply Automatically" set to true