SUSE / doc-susemanager

SUSE Manager documentation - Deprecated - docs have migrated to the Uyuni-project
https://documentation.suse.com/suma/
21 stars 34 forks source link

Salt Formulas: $ifEmpty and $optional are not documented #281

Closed mvidner closed 5 years ago

mvidner commented 5 years ago

Best Practices does not document the $ifEmpty and $optional attributes which has been shipped in these formulas:

In particular I am confused what the precise relationship between $default, $optional, and $ifEmpty is. $ifEmpty was implemented in https://github.com/SUSE/spacewalk/pull/4261 .

For context, I need this because YaST is adding support for Salt Formulas: FATE#32722

keichwa commented 5 years ago

@aaannz or @hustodemon , I need info about $default, $optional, and $ifEmpty as @mvidner asked above.

hustodemon commented 5 years ago

Let's illustrate these attributes on an examples:

$optional

displayName:
    $type: string
    $optional: True

$optional - boolean attribute. If it's true and the field displayName is empty in the form, then this field will not be generated in the formula data (the generated dictionary will simply miss the displayName key). If this was false, the formula data would contain displayName: null entry.

$ifEmpty

displayName:
    $type: string
    $ifEmpty: DP2

$ifEmpty - value to be used if the field is empty (user didn't input any value). Can only be used when $optional is false or not defined! If $optional == true, then $ifEmpty is ignored. In the example above, the DP2 string would be used if user leaves the field empty.

$default

$default - the pre-populated values of the form (i.e. if user edits the form for the 1st time, they'll see these values in the UI)

Some more sauce here: https://w3.suse.de/~fkobzik/supporters-training/2018/formulas/presentation.html#1

HTH, ping me otherwise.

mvidner commented 5 years ago

@hustodemon Thank you! The presentation is helpful.

OK, but as I understand it, I can replace $ifEmpty: foo with $default: foo and the resulting pillar will be same (in the simple case when the user does not explicitly delete the default value in the form). IMHO an explicit default is better than an implicit one. Or did you think it would be visual overload and wanted the default to be implicit?

aaannz commented 5 years ago

@mvidner $ifEmpty is some form of form validation. There are some pillar data that must be present and cannot be empty. So even if user deletes data from the form, formula system tries to produce working pillar. In case of $default, when user deletes the data in form, none is passed to the pillar. $ifEmpty is useful also in case when empty value is ok, but must be in correct type - e.g. empty string instead of none

joesusecom commented 5 years ago

Maybe I'm missing a corner case, but isn't setting both $optional to False and providing a $default value going to give you exactly the same thing as $ifEmpty?

And while we're at it: In HTML 5 the approach is to label a field required. I know that this is the same as setting $optional to False, but why not stick to the more familiar approach form HTML?

aaannz commented 5 years ago

@joesusecom Consider this: 1) $optional is False, $default is set. User deletes the value in the form, then the pillar will have for that key value none 2) $optional is False, $ifEmpty is set. User deletes the value in the form, then the pillar will have for that key value set in $ifEmpty.

$default is applicable when the entry does not exists at all. Not when it is deliberately set to empty string.

joesusecom commented 5 years ago

Honestly, this doesn't sound very intuitive. The very definition of a required field is that it can't be empty. So your first option would be a bug in the forms handler. Or is the idea to allow enforcing a none value? In that case, how should a user know that this will happen if the field is empty? Wouldn't the more logical result be that the field is an empty string?

aaannz commented 5 years ago

I think the best would be to have some form validation in forms system. But we do not have that. When user makes a configuration error in forms, then they will discover it only when they apply the formula. I don't know what the original motivation was for this feature, but I guess that they wanted to do same sane checks. Also I remember discussions about what the return value should be e.g. none vs empty string, which is solved by $ifEmpty. When $ifEmpty is "", then deleted string results in "". If $ifEmpty is missing, then the result is none.

joesusecom commented 5 years ago

https://github.com/SUSE/spacewalk/issues/2666 has the details where this is coming from. I still think that if we have the chance to re-work the Formulas syntax one more time (probably only if we introduce syntax versions), we would want to reverse the logic (required vs optional, and allowing a "None" default that's picked up if the field is empty or so).

keichwa commented 5 years ago

Prepared here: https://github.com/SUSE/doc-susemanager/tree/ke-git-6512-forms-features

keichwa commented 5 years ago

done