FE: Update failed validation error messages to be more specific
Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.
Why are we doing this work
- when a policy fails validation, a user wants a more descriptive explanation on what the problem is and how to fix it
From &9696 (comment 1507774380)
To create a policy, you must include valid conditions, actions, or settings.
I like the update! I think we can make the error message more specific so users know what to fix:
- Scenario 1: When there is no valid condition, an error message shows
To create a policy, you must include valid required conditions which are: {required-condition-name-1}, {required-condition-name-1}, {required-condition-name-1}- Scenario 2: When there are no settings or approval actions (approval num == 0), an error message shows,
To create a policy, you must include valid action or overwritten settings.- Scenario 3: When there is no valid condition and no settings, and no approval actions (approval num == 0), an error message shows,
To create a policy, you must include valid required conditions which are: {required-condition-name-1}, {required-condition-name-1}, {required-condition-name-1} </br> Also at least one valid action or overridden settings are requiredNote: for scenario 1 -3, the button
configure with a merge requestshould be disabledI'm debating on if we should yet include a warning like this. I could see a future possibility where we implement an Audit Mode, where we might more clearly delineate between having approvers or only having a way to observe violations without enforcement. Maybe we can consider this 2nd design as an iteration based on feedback? If @cam.x feels strongly about it, however, that's okay with me to include it.
So this is Scenario 4; we show a warning message, but the user can still create the policy:
You didn’t choose any approvers. When the condition is met in a merge request, only the override project approval settings will take effect and there will be no required approvers.I like the Audit Mode direction. My concern is that what if the user makes a mistake and forgets to include the approver instead of purposely doing that? A warning doesn't block them from doing anything, just a reminder. If we have it, the worst case is that the user ignores it. If we don't have it, the worst is that the user makes a mistake and needs to spend some effort to triage the policy. So I think it is better to include this for now.
If I try to submit an empty policy today, we end up with a bunch of errors, so I suppose the intent is to make these errors nicer.
Should this change? Should it be possible to submit a policy without any approvers if there are some project overrides selected?
I think so; this is the scenario 4 If I am not mistaken?
If we agree with these, I can make a list and let @rdickenson go over it.
Relevant links
Non-functional requirements
-
Documentation: -
Feature flag: -
Performance: -
Testing:
Implementation plan
-
update handle_error to handle different scenarios and output a more descriptive error message