Closed JDGrimes closed 7 years ago
I guess this also relates to #8 and #9, although setting the minimum will not currently kick in when the arg value is empty or the rounding method is invalid. But it is another angle to all that which has to be taken into account there.
I guess though partly it is separate too, because as far as when the value isn't set, we could have the minimum kick in if we wanted to. That sounds like what we ought to do, actually. When the rounding method is invalid though, that is a different story, because then we can't actually calculate the value properly. (Though really, that is just an extra sanity check, the rounding method can never be invalid at run time because we run validation just before firing.)
On the other hand, it is possible that the user intends that the minimum only kick in when the arg is actually set. I think that cor consistency with the other settings, we need to not have it do that, and instead ad a fallback at some point if that is a feature users want. So for now, the minimum will only apply when the arg has a value to start with.
Right now though, we are bailing even if the arg is zero, which is different. We should only bail if the arg is null. Or an empty string, I guess. Or false.
There is a difference between it being empty and unset, and also differences between different kinds of being unset (different parts of the hierarchy that are where the break in the chain is). To truly give the user control over when the minimum is awarded and when it isn't, Conditions need to be used instead. They are better suited to this, as otherwise we are turning these from just reaction settings to conditions, and conditions that won't truly behave as such (in regard to rates and limits and things like that). So I guess that means that we really have to always respect the minimum, so that we don't have to add condition-like settings to this extension "reactor".
I wonder if we should also display this information in the HTGP shortcode table? (And the same for the multiply by setting.) I had thought about this when initially working on our integration with HTGP, and kind of intended to do it when adding these settings, but then I forgot.
I guess really there isn't much of a reason not to display that info in the shortcode, since we can. Unless we feel like it will take up too much room. But I guess we should just give in a try and see how it looks.
Here is what the minimum could look like in the HTGP shortocde (at the bottom):
Really, the arg titles can be pretty long already, so I don't think this is really going to be that significant there. One thing that we could do though is have a line break between each part, which might make the table easier to scan quickly.
We can't use a line break because it is stripped by the wp_kses()
. So I guess we'll just leave it as is for now. If we want to change it it the future, we could modify WordPoints to allow that.
On another note, I don't think that minimum exactly expresses what we want here. Maximum will make sense to most users I think, but the minimum might not. But I guess that if necessary the admin can explain that in the description or in the page where they are using the shortcode.
See https://github.com/WordPoints/wordpoints/issues/469#issuecomment-263280416:
And https://github.com/WordPoints/wordpoints/issues/469#issuecomment-272296530: